Merge lp:~charlesk/indicator-location/use-properties-cpp into lp:indicator-location/16.04

[Charles Kerr]

Use core::Property<X> fields to notify the indicator when location settings change

A minor cleanup branch of something that's irritated me for awhile in indicator-location, the use of Java-style Listener interface classes for observer/observable to know when CUALC's location settings change.

This MP replaces the Listener interfaces with use of core::Property<X> objects to signal when a value changes.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:161
http://jenkins.qa.ubuntu.com/job/indicator-location-ci/70/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-location-wily-amd64-ci/5
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-location-wily-armhf-ci/5
        deb: http://jenkins.qa.ubuntu.com/job/indicator-location-wily-armhf-ci/5/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-location-ci/70/rebuild

review: Approve (continuous-integration)

[Xavi Garcia (xavi-garcia-mena)]

LGTM, thanks

review: Approve

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2015-04-16 00:44:22 +0000
+++ CMakeLists.txt	2016-01-22 18:23:42 +0000
@@ -35,7 +35,8 @@
                    ubuntu-app-launch-2
                    url-dispatcher-1
                    gio-unix-2.0>=2.36
-                   glib-2.0>=2.36)
+                   glib-2.0>=2.36
+                   properties-cpp>=0.0.1)
 include_directories (SYSTEM ${SERVICE_DEPS_INCLUDE_DIRS})
 
 ##

=== removed file 'src/controller.cc'
--- src/controller.cc	2013-08-25 20:31:26 +0000
+++ src/controller.cc	1970-01-01 00:00:00 +0000
@@ -1,46 +0,0 @@
-/*
- * Copyright 2013 Canonical Ltd.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; version 3.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *
- * Authors:
- *   Charles Kerr <[e-mail]>
- */
-
-#include "controller.h"
-
-void
-Controller :: add_listener (ControllerListener * l)
-{
-  listeners.insert (l);
-}
-
-void
-Controller :: remove_listener (ControllerListener * l)
-{
-  listeners.erase (l);
-}
-
-void
-Controller :: notify_gps_enabled (bool enabled)
-{
-  for (auto it : listeners)
-    it->on_gps_enabled_changed (enabled);
-}
-
-void
-Controller :: notify_location_service_enabled (bool enabled)
-{
-  for (auto it : listeners)
-    it->on_location_service_enabled_changed (enabled);
-}

=== modified file 'src/controller.h'
--- src/controller.h	2015-04-15 13:42:24 +0000
+++ src/controller.h	2016-01-22 18:23:42 +0000
@@ -20,48 +20,22 @@
 #ifndef __INDICATOR_LOCATION_CONTROLLER_H__
 #define __INDICATOR_LOCATION_CONTROLLER_H__
 
-#include <set>
-
 #include <core/property.h>
 
-class ControllerListener
-{
-  public:
-    ControllerListener() {}
-    virtual ~ControllerListener() {}
-
-  public:
-    virtual void on_gps_enabled_changed (bool is_enabled) = 0;
-    virtual void on_location_service_enabled_changed (bool is_enabled) = 0;
-};
-
 class Controller
 {
-  public:
-
-    Controller () {}
-    virtual ~Controller() {}
-
-    void add_listener (ControllerListener *);
-    void remove_listener (ControllerListener *);
+public:
+    Controller();
+    virtual ~Controller();
 
     /// True iff we've gotten status info from the location service
     virtual const core::Property<bool>& is_valid() const =0;
 
-    virtual bool is_gps_enabled () const = 0;
-    virtual bool is_location_service_enabled () const = 0;
-
-    virtual void set_gps_enabled (bool enabled) = 0;
-    virtual void set_location_service_enabled (bool enabled) = 0;
-
-  private:
-
-    std::set<ControllerListener*> listeners;
-
-  protected:
-
-    void notify_gps_enabled (bool);
-    void notify_location_service_enabled (bool);
+    virtual const core::Property<bool>& gps_enabled() const =0;
+    virtual const core::Property<bool>& location_service_enabled() const =0;
+
+    virtual void set_gps_enabled (bool enabled) =0;
+    virtual void set_location_service_enabled (bool enabled) =0;
 };
 
 #endif // __INDICATOR_LOCATION_CONTROLLER_H__

=== modified file 'src/location-service-controller.cc'
--- src/location-service-controller.cc	2015-04-17 16:44:53 +0000
+++ src/location-service-controller.cc	2016-01-22 18:23:42 +0000
@@ -31,21 +31,13 @@
 {
 public:
 
-    Impl(LocationServiceController& owner)
+    Impl()
     {
         m_cancellable.reset(g_cancellable_new(), [](GCancellable* c) {
             g_cancellable_cancel(c);
             g_object_unref(c);
         });
 
-        m_gps_enabled.changed().connect([&owner](bool b){
-            owner.notify_gps_enabled(b);
-        });
-
-        m_loc_enabled.changed().connect([&owner](bool b){
-            owner.notify_location_service_enabled(b);
-        });
-
         g_bus_get(G_BUS_TYPE_SYSTEM,
                   m_cancellable.get(),
                   on_system_bus_ready,
@@ -59,14 +51,14 @@
         return m_is_valid;
     }
 
-    bool is_gps_enabled() const
+    const core::Property<bool>& gps_enabled() const
     {
-        return m_gps_enabled.get();
+        return m_gps_enabled;
     }
 
-    bool is_location_service_enabled() const
+    const core::Property<bool>& location_service_enabled() const
     {
-        return m_loc_enabled.get();
+        return m_loc_enabled;
     }
 
     void set_gps_enabled(bool enabled)
@@ -108,9 +100,9 @@
                                 nullptr);
 
             //  manage the name_tag's lifespan
-            self->m_name_tag.reset(new guint{name_tag}, [](guint* name_tag){
-                g_bus_unwatch_name(*name_tag);
-                delete name_tag;
+            self->m_name_tag.reset(new guint{name_tag}, [](guint* tag){
+                g_bus_unwatch_name(*tag);
+                delete tag;
             });
         }
         else if (error != nullptr)
@@ -122,9 +114,9 @@
     }
 
     static void on_name_appeared(GDBusConnection * system_bus,
-                                 const gchar     * bus_name,
+                                 const gchar     * /*bus_name*/,
                                  const gchar     * name_owner,
-                                 gpointer gself)
+                                 gpointer          gself)
     {
         auto self = static_cast<Impl*>(gself);
 
@@ -275,7 +267,7 @@
     {
         bool success, value;
         std::tie(success, value) = get_bool_reply_from_call(source_object, res);
-        g_debug("service loc reply: success %d value %d", (int)success, (int)value);
+        g_debug("service loc reply: success %d value %d", int(success), int(value));
         if (success)
             static_cast<Impl*>(gself)->m_loc_enabled.set(value);
     }
@@ -286,7 +278,7 @@
     {
         bool success, value;
         std::tie(success, value) = get_bool_reply_from_call(source_object, res);
-        g_debug("service gps reply: success %d value %d", (int)success, (int)value);
+        g_debug("service gps reply: success %d value %d", int(success), int(value));
         if (success)
             static_cast<Impl*>(gself)->m_gps_enabled.set(value);
     }
@@ -319,7 +311,7 @@
 
     static void check_method_call_reply(GObject      *connection,
                                         GAsyncResult *res,
-                                        gpointer      gself)
+                                        gpointer      /*gself*/)
     {
         GError * error;
         GVariant * v;
@@ -368,7 +360,7 @@
 ***/
 
 LocationServiceController::LocationServiceController():
-  impl{new Impl{*this}}
+  impl{new Impl{}}
 {
 }
 
@@ -382,17 +374,18 @@
   return impl->is_valid();
 }
 
-bool
-LocationServiceController::is_gps_enabled() const
-{
-  return impl->is_gps_enabled();
-}
-
-bool
-LocationServiceController::is_location_service_enabled() const
-{
-  return impl->is_location_service_enabled();
-}
+const core::Property<bool>&
+LocationServiceController::gps_enabled() const
+{
+  return impl->gps_enabled();
+}
+
+const core::Property<bool>&
+LocationServiceController::location_service_enabled() const
+{
+  return impl->location_service_enabled();
+}
+
 void
 LocationServiceController::set_gps_enabled(bool enabled)
 {

=== modified file 'src/location-service-controller.h'
--- src/location-service-controller.h	2015-04-17 16:51:19 +0000
+++ src/location-service-controller.h	2016-01-22 18:23:42 +0000
@@ -20,26 +20,26 @@
 #ifndef INDICATOR_LOCATION_CONTROLLER_LOCATION_SERVICE
 #define INDICATOR_LOCATION_CONTROLLER_LOCATION_SERVICE
 
-#include "controller.h"
+#include "controller.h" // parent class
 
 #include <memory> // std::unique_ptr
 
 class LocationServiceController: public Controller
 {
-  public:
+public:
     LocationServiceController();
     virtual ~LocationServiceController();
 
-    virtual const core::Property<bool>& is_valid() const override;
-    bool is_gps_enabled () const override;
-    bool is_location_service_enabled () const override;
+    const core::Property<bool>& is_valid() const override;
+    const core::Property<bool>& gps_enabled() const override;
+    const core::Property<bool>& location_service_enabled() const override;
     void set_gps_enabled (bool enabled) override;
     void set_location_service_enabled (bool enabled) override;
 
     LocationServiceController(const LocationServiceController&) =delete;
     LocationServiceController& operator=(const LocationServiceController&) =delete;
 
-  private:
+private:
     friend class Impl;
     class Impl;
     std::unique_ptr<Impl> impl;

=== modified file 'src/phone.cc'
--- src/phone.cc	2016-01-22 18:23:42 +0000
+++ src/phone.cc	2016-01-22 18:23:42 +0000
@@ -38,7 +38,29 @@
   action_group (action_group_)
 {
   create_menu ();
-  controller->add_listener (this);
+
+  auto on_gps = [this](bool enabled){
+    update_gps_enabled_action();
+    update_header();
+  };
+  controller_connections.push_back(
+    controller->gps_enabled().changed().connect(on_gps)
+  );
+
+  auto on_loc = [this](bool enabled){
+    update_detection_enabled_action();
+    update_header();
+  };
+  controller_connections.push_back(
+    controller->location_service_enabled().changed().connect(on_loc)
+  );
+
+  auto on_valid = [this](bool valid){
+    update_actions_enabled();
+  };
+  controller_connections.push_back(
+    controller->is_valid().changed().connect(on_valid)
+  ); 
 
   /* create the actions & add them to the group */
   std::array<GSimpleAction*, 4> actions = { create_root_action(),
@@ -51,14 +73,11 @@
       g_object_unref (a);
     }
 
-  // the profile should track whether the controller is valid or not
-  controller->is_valid().changed().connect([this](bool){on_is_valid_changed();});
-  on_is_valid_changed();
+  update_actions_enabled();
 }
 
 Phone :: ~Phone ()
 {
-  controller->remove_listener (this);
 }
 
 /***
@@ -73,7 +92,7 @@
 
   // as per "Indicators - RTM Usability Fix" document:
   // visible iff location is enabled
-  return controller->is_location_service_enabled();
+  return controller->location_service_enabled().get();
 }
 
 GVariant *
@@ -123,7 +142,7 @@
 }
 
 void
-Phone :: on_is_valid_changed()
+Phone :: update_actions_enabled()
 {
   const auto map = G_ACTION_MAP(action_group.get());
   const bool is_valid = controller->is_valid().get();
@@ -141,15 +160,7 @@
 GVariant *
 Phone :: action_state_for_location_detection ()
 {
-  return g_variant_new_boolean (controller->is_location_service_enabled());
-}
-
-void
-Phone :: on_location_service_enabled_changed (bool is_enabled G_GNUC_UNUSED)
-{
-  GAction * action = g_action_map_lookup_action (G_ACTION_MAP(action_group.get()), LOCATION_ACTION_KEY);
-  g_simple_action_set_state (G_SIMPLE_ACTION(action), action_state_for_location_detection());
-  update_header();
+  return g_variant_new_boolean (controller->location_service_enabled().get());
 }
 
 void
@@ -177,6 +188,13 @@
   return action;
 }
 
+void
+Phone::update_detection_enabled_action()
+{
+  GAction * action = g_action_map_lookup_action (G_ACTION_MAP(action_group.get()), LOCATION_ACTION_KEY);
+  g_simple_action_set_state (G_SIMPLE_ACTION(action), action_state_for_location_detection());
+}
+
 /***
 ****
 ***/
@@ -184,15 +202,7 @@
 GVariant *
 Phone :: action_state_for_gps_detection ()
 {
-  return g_variant_new_boolean (controller->is_gps_enabled());
-}
-
-void
-Phone :: on_gps_enabled_changed (bool is_enabled G_GNUC_UNUSED)
-{
-  GAction * action = g_action_map_lookup_action (G_ACTION_MAP(action_group.get()), GPS_ACTION_KEY);
-  g_simple_action_set_state (G_SIMPLE_ACTION(action), action_state_for_gps_detection());
-  update_header();
+  return g_variant_new_boolean (controller->gps_enabled().get());
 }
 
 void
@@ -206,7 +216,7 @@
 }
 
 GSimpleAction *
-Phone :: create_gps_enabled_action ()
+Phone :: create_gps_enabled_action()
 {
   GSimpleAction * action;
 
@@ -220,6 +230,13 @@
   return action;
 }
 
+void
+Phone::update_gps_enabled_action()
+{
+  GAction * action = g_action_map_lookup_action (G_ACTION_MAP(action_group.get()), GPS_ACTION_KEY);
+  g_simple_action_set_state (G_SIMPLE_ACTION(action), action_state_for_gps_detection());
+}
+
 /***
 ****
 ***/
@@ -230,7 +247,7 @@
 {
   void
   on_uri_dispatched (const gchar * uri,
-                     gboolean      success     G_GNUC_UNUSED,
+                     gboolean      success,
                      gpointer      user_data   G_GNUC_UNUSED)
   {
     if (!success)

=== modified file 'src/phone.h'
--- src/phone.h	2016-01-22 18:23:42 +0000
+++ src/phone.h	2016-01-22 18:23:42 +0000
@@ -21,13 +21,14 @@
 #define __INDICATOR_LOCATION_PHONE_H__
 
 #include <memory>
+#include <vector>
 
 #include <glib.h>
 #include <gio/gio.h>
 
 #include "controller.h"
 
-class Phone: public ControllerListener
+class Phone
 {
   public:
     Phone (const std::shared_ptr<Controller>& controller,
@@ -37,9 +38,7 @@
 
   protected:
     std::shared_ptr<Controller> controller;
-    virtual void on_is_valid_changed();
-    virtual void on_gps_enabled_changed (bool is_enabled);
-    virtual void on_location_service_enabled_changed (bool is_enabled);
+    std::vector<core::ScopedConnection> controller_connections;
 
   private:
     std::shared_ptr<GMenu> menu;
@@ -55,15 +54,19 @@
     GVariant * action_state_for_root () const;
     GSimpleAction * create_root_action ();
     void update_header();
+    void update_actions_enabled();
 
   private:
     GVariant * action_state_for_location_detection ();
     GSimpleAction * create_detection_enabled_action ();
+    void update_detection_enabled_action();
     static void on_detection_location_activated (GSimpleAction*, GVariant*, gpointer);
 
+
   private:
     GVariant * action_state_for_gps_detection ();
     GSimpleAction * create_gps_enabled_action ();
+    void update_gps_enabled_action();
     static void on_detection_gps_activated (GSimpleAction*, GVariant*, gpointer);
 
   private:

=== modified file 'src/service.cc'
--- src/service.cc	2016-01-22 18:23:42 +0000
+++ src/service.cc	2016-01-22 18:23:42 +0000
@@ -84,44 +84,44 @@
 ***/
 
 void
-Service :: on_name_lost (GDBusConnection * connection,
+Service :: on_name_lost (GDBusConnection * conn,
                          const char      * name,
                          gpointer          gself)
 {
-  g_debug ("%s::%s: %s %p", G_STRLOC, G_STRFUNC, name, connection);
+  g_debug ("%s::%s: %s %p", G_STRLOC, G_STRFUNC, name, conn);
 
-  static_cast<Service*>(gself)->on_name_lost (connection, name);
+  static_cast<Service*>(gself)->on_name_lost (conn, name);
 }
 void
-Service :: on_name_lost (GDBusConnection * connection,
+Service :: on_name_lost (GDBusConnection * conn,
                          const char      * name)
 {
-  g_debug ("%s::%s: %s %p", G_STRLOC, G_STRFUNC, name, connection);
+  g_debug ("%s::%s: %s %p", G_STRLOC, G_STRFUNC, name, conn);
 
   if (name_lost_callback != nullptr)
     (name_lost_callback)(this, name_lost_user_data);
 }
 
 void
-Service :: on_bus_acquired (GDBusConnection * connection,
+Service :: on_bus_acquired (GDBusConnection * conn,
                             const char      * name,
                             gpointer          gself)
 {
-  static_cast<Service*>(gself)->on_bus_acquired (connection, name);
+  static_cast<Service*>(gself)->on_bus_acquired (conn, name);
 }
 void
-Service :: on_bus_acquired (GDBusConnection * connection,
+Service :: on_bus_acquired (GDBusConnection * conn,
                             const char * name)
 {
-  g_debug ("%s::%s: %s %p", G_STRLOC, G_STRFUNC, name, connection);
+  g_debug ("%s::%s: %s %p", G_STRLOC, G_STRFUNC, name, conn);
 
-  this->connection.reset (G_DBUS_CONNECTION (g_object_ref(connection)));
+  this->connection.reset (G_DBUS_CONNECTION (g_object_ref(conn)));
 
   GError * error = nullptr;
 
   /* export the action group */
 
-  unsigned int export_id = g_dbus_connection_export_action_group (connection,
+  unsigned int export_id = g_dbus_connection_export_action_group (conn,
                                                                   INDICATOR_OBJECT_PATH,
                                                                   G_ACTION_GROUP (action_group.get()),
                                                                   &error);
@@ -146,7 +146,7 @@
 
   for (unsigned int i=0, n=G_N_ELEMENTS(menus); i<n; i++)
     { 
-      export_id = g_dbus_connection_export_menu_model (connection,
+      export_id = g_dbus_connection_export_menu_model (conn,
                                                        menus[i].path,
                                                        G_MENU_MODEL (menus[i].menu.get()),
                                                        &error);

=== modified file 'src/service.h'
--- src/service.h	2016-01-22 18:23:42 +0000
+++ src/service.h	2016-01-22 18:23:42 +0000
@@ -30,7 +30,7 @@
 class Service
 {
   public:
-    Service (const std::shared_ptr<Controller>& controller);
+    explicit Service (const std::shared_ptr<Controller>& controller);
     virtual ~Service ();
 
   private:

=== renamed file 'src/controller-mock.h' => 'tests/controller-mock.h'
--- src/controller-mock.h	2015-04-15 14:24:14 +0000
+++ tests/controller-mock.h	2016-01-22 18:23:42 +0000
@@ -17,10 +17,9 @@
  *   Charles Kerr <[e-mail]>
  */
 
-#ifndef __INDICATOR_LOCATION_CONTROLLER_MOCK__H__
-#define __INDICATOR_LOCATION_CONTROLLER_MOCK__H__
+#pragma once
 
-#include "controller.h"
+#include <src/controller.h>
 
 class MockController: public Controller
 {
@@ -31,19 +30,16 @@
 
     core::Property<bool>& is_valid() { return m_is_valid; }
     const core::Property<bool>& is_valid() const override { return m_is_valid; }
-    bool is_gps_enabled () const { return gps; }
-    bool is_location_service_enabled () const { return loc; }
+    const core::Property<bool>& gps_enabled() const override { return m_gps_enabled; }
+    const core::Property<bool>& location_service_enabled() const override { return m_location_service_enabled; }
 
-    void set_gps_enabled (bool enabled) { notify_gps_enabled (gps=enabled); }
-    void set_location_service_enabled (bool enabled) { notify_location_service_enabled (loc=enabled); }
+    void set_gps_enabled (bool enabled) override { m_gps_enabled=enabled; }
+    void set_location_service_enabled (bool enabled) override { m_location_service_enabled=enabled; }
 
   private:
 
     core::Property<bool> m_is_valid {true};
-    bool gps { false };
-    bool loc { false };
+    core::Property<bool> m_gps_enabled {false};
+    core::Property<bool> m_location_service_enabled {false};
 };
 
-#endif // __INDICATOR_LOCATION_CONTROLLER_MOCK__H__
-
-

=== modified file 'tests/gtest-dbus-fixture.h'
--- tests/gtest-dbus-fixture.h	2014-01-27 00:02:51 +0000
+++ tests/gtest-dbus-fixture.h	2016-01-22 18:23:42 +0000
@@ -57,7 +57,7 @@
     static gboolean
     wait_for_signal__timeout (gpointer name)
     {
-      g_error ("%s: timed out waiting for signal '%s'", G_STRLOC, (char*)name);
+      g_error ("%s: timed out waiting for signal '%s'", G_STRLOC, name);
       return G_SOURCE_REMOVE;
     }
 
@@ -113,7 +113,7 @@
   protected:
 
     /* convenience func to loop while waiting for a GObject's signal */
-    void wait_for_signal(gpointer o, const gchar * signal, const int timeout_seconds=5)
+    void wait_for_signal(gpointer o, const gchar * signal, const guint timeout_seconds=5)
     {
       // wait for the signal or for timeout, whichever comes first
       const auto handler_id = g_signal_connect_swapped(o, signal,
@@ -128,7 +128,7 @@
     }
 
     /* convenience func to loop for N msec */
-    void wait_msec(int msec=50)
+    void wait_msec(guint msec=50)
     {
       const auto id = g_timeout_add(msec, wait_msec__timeout, loop);
       g_main_loop_run(loop);

=== modified file 'tests/gtest-dbus-indicator-fixture.h'
--- tests/gtest-dbus-indicator-fixture.h	2015-04-15 14:24:14 +0000
+++ tests/gtest-dbus-indicator-fixture.h	2016-01-22 18:23:42 +0000
@@ -44,9 +44,9 @@
                                   gint          position   G_GNUC_UNUSED,
                                   gint          removed    G_GNUC_UNUSED,
                                   gint          added      G_GNUC_UNUSED,
-                                  gpointer      any_item_changed)
+                                  gpointer      gany_item_changed)
     {
-      *((gboolean*)any_item_changed) = true;
+      *static_cast<gboolean*>(gany_item_changed) = true;
     }
 
   protected:
@@ -83,7 +83,7 @@
 
     void sync_menu (void)
     {
-      g_slist_free_full (menu_references, (GDestroyNotify)g_object_unref);
+      g_slist_free_full (menu_references, GDestroyNotify(g_object_unref));
       menu_references = nullptr;
       activate_subtree (G_MENU_MODEL (menu_model));
     }
@@ -111,7 +111,7 @@
                                                              G_BUS_NAME_WATCHER_FLAGS_NONE,
                                                              on_name_appeared, // quits the loop
                                                              nullptr, this, nullptr);
-      const guint timer_id = g_timeout_add_seconds (TIME_LIMIT_SEC, (GSourceFunc)g_main_loop_quit, loop);
+      const guint timer_id = g_timeout_add_seconds (TIME_LIMIT_SEC, GSourceFunc(g_main_loop_quit), loop);
       g_main_loop_run (loop);
       g_source_remove (timer_id);
       g_bus_unwatch_name (watch_id);
@@ -136,7 +136,7 @@
     {
       g_clear_pointer (&timer, g_timer_destroy);
 
-      g_slist_free_full (menu_references, (GDestroyNotify)g_object_unref);
+      g_slist_free_full (menu_references, GDestroyNotify(g_object_unref));
       menu_references = nullptr;
       g_clear_object (&menu_model);
 

=== modified file 'tests/phone-test.cc'
--- tests/phone-test.cc	2016-01-22 18:23:42 +0000
+++ tests/phone-test.cc	2016-01-22 18:23:42 +0000
@@ -21,12 +21,11 @@
 #define INDICATOR_PROFILE "phone"
 #include "gtest-dbus-indicator-fixture.h"
 
+#include "controller-mock.h"
 #include "src/dbus-shared.h"
-#include "src/controller-mock.h"
 #include "src/service.h"
 
-class PhoneTest: public GTestDBusIndicatorFixture,
-                 public ControllerListener
+class PhoneTest: public GTestDBusIndicatorFixture
 {
   protected:
 
@@ -37,6 +36,7 @@
 
     std::shared_ptr<MockController> myController;
     std::shared_ptr<Service> myService;
+    std::vector<core::ScopedConnection> myConnections;
 
   public:
 
@@ -72,14 +72,27 @@
     virtual void setup_service ()
     {
       myController.reset (new MockController ());
-      myController->add_listener (this);
       myService.reset (new Service (myController));
+
+      myConnections.push_back(
+        myController->gps_enabled().changed().connect([this](bool enabled){
+          gps_enabled_changed = true;
+          gps_enabled = enabled;
+        })
+      );
+
+      myConnections.push_back(
+        myController->location_service_enabled().changed().connect([this](bool enabled){
+          loc_enabled_changed = true;
+          loc_enabled = enabled;
+        })
+      );
     }
 
     virtual void teardown_service ()
     {
       myService.reset ();
-      myController->remove_listener (this);
+      myConnections.clear ();
       myController.reset ();
     }
 };
@@ -123,7 +136,7 @@
 TEST_F (PhoneTest, IsValidVisible)
 {
   // make sure something's enabled so that the indicator should be visible
-  if (!myController->is_location_service_enabled()) {
+  if (!myController->location_service_enabled().get()) {
     myController->set_location_service_enabled(true);
     wait_for_action_state_change("location-detection-enabled");
   }