Merge lp:~unity-api-team/indicator-power/lp-1370791-15.05-adjust-slider-when-brightness-is-changed-by-powerd into lp:indicator-power/15.04

[Charles Kerr]

When in autobrightness mode, keep the brightness slider's value in sync with the system brightness

== Description of the Change

In the brightness code, add a powerd proxy that listens to property changes from the com.canonical.powerd.Brightness property. When this changes, update the indicator's power slider to reflect the change.

== Checklist

* Are there any related MPs required for this MP to build/function as expected? Please list.

Yes, https://code.launchpad.net/~charlesk/powerd/lp-1370791-add-brightness-property is required to publish powerd's brightness changes on the bus.

> * Is your branch in sync with latest trunk? (e.g. bzr pull lp:trunk -> no changes)

Targeted for lp:indicator-power/15.04. bzr pull lp:indicator-power/15.04 -> no changes

> * Did the code build without warnings?

Yes

> * Did the tests run successfully?

Yes

> * Did you perform an exploratory manual test run of your code change and any related functionality?

Yes

> * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

N/A

> * Did your component test plan pass? If on a device, what image number?

Yes mako wily r202

> * Please list which manual tests are germane for the reviewer in this MR.

indicator-power/device-brightness-slider-auto and
indicator-power/device-brightness-slider

> Did you provide a link to this page https://wiki.ubuntu.com/Process/Merges/Checklists/indicator-power

Yes

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:289
http://jenkins.qa.ubuntu.com/job/indicator-power-15.04-ci/2/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-15.04-vivid-amd64-ci/2
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-power-15.04-vivid-armhf-ci/2
        deb: http://jenkins.qa.ubuntu.com/job/indicator-power-15.04-vivid-armhf-ci/2/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-power-15.04-ci/2/rebuild

review: Approve (continuous-integration)

[Alejandro J. Cura (alecu)]

== Reviewer Checklist

* Have you checked that the submitter has accurately filled out the submitter checklist and has taken no shortcuts?

Yes, checked.

* Did you run the manual tests listed by the submitter?

I followed both manual tests, everything seems to work ok.

* Did you do exploratory testing related to the component you own with the MP changeset included?

I did exploratory testing around the new feature, checking the slider with sunlight, flashlight, darkened indoors and covering the sensor. It all seems to work just fine.

* If new features have been added, are the manual tests sufficient to cover them?

The manual tests added to the test plan seem to cover all new features in this MP.

---

Code looks good, and manual testing works as expected.

review: Approve

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2014-10-14 19:09:32 +0000
+++ src/CMakeLists.txt	2015-05-20 19:36:35 +0000
@@ -17,6 +17,10 @@
 # generated sources
 include(GdbusCodegen)
 set(SERVICE_GENERATED_SOURCES)
+add_gdbus_codegen_with_namespace(SERVICE_GENERATED_SOURCES dbus-powerd
+                                 com.canonical
+                                 Dbus
+                                 ${CMAKE_SOURCE_DIR}/src/com.canonical.powerd.xml)
 add_gdbus_codegen_with_namespace(SERVICE_GENERATED_SOURCES dbus-battery
                                  com.canonical.indicator.power
                                  Dbus

=== modified file 'src/brightness.c'
--- src/brightness.c	2014-09-12 16:37:02 +0000
+++ src/brightness.c	2015-05-20 19:36:35 +0000
@@ -18,6 +18,7 @@
  */
 
 #include "brightness.h"
+#include "dbus-powerd.h"
 
 #include <gio/gio.h>
 
@@ -45,7 +46,8 @@
 
   GSettings * settings;
 
-  guint powerd_name_tag;
+  DbusPowerd * powerd_proxy;
+  char * powerd_name_owner;
 
   double percentage;
 
@@ -136,14 +138,15 @@
       g_clear_object(&p->cancellable);
     }
 
-  if (p->powerd_name_tag)
+  if (p->powerd_proxy != NULL)
     {
-      g_bus_unwatch_name(p->powerd_name_tag);
-      p->powerd_name_tag = 0;
+      g_signal_handlers_disconnect_by_data(p->powerd_proxy, o);
+      g_clear_object(&p->powerd_proxy);
     }
 
   g_clear_object(&p->settings);
   g_clear_object(&p->system_bus);
+  g_clear_pointer(&p->powerd_name_owner, g_free);
 
   G_OBJECT_CLASS(indicator_power_brightness_parent_class)->dispose(o);
 }
@@ -202,36 +205,30 @@
  */
 
 static void set_brightness_global(IndicatorPowerBrightness*, int);
+static void set_brightness_local(IndicatorPowerBrightness*, int);
 
 static void
-on_powerd_brightness_params_ready(GObject      * source,
+on_powerd_brightness_params_ready(GObject      * oproxy,
                                   GAsyncResult * res,
                                   gpointer       gself)
 {
   GError * error;
   GVariant * v;
 
+  v = NULL;
   error = NULL;
-  v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &error);
-  if (v == NULL)
-    {
-      if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
-        g_warning("Unable to get system bus: %s", error->message);
-
-      g_error_free(error);
-    }
-  else
+  if (dbus_powerd_call_get_brightness_params_finish(DBUS_POWERD(oproxy), &v, res, &error))
     {
       IndicatorPowerBrightness * self = INDICATOR_POWER_BRIGHTNESS(gself);
       priv_t * p = get_priv(self);
       const gboolean old_ab_supported = p->powerd_ab_supported;
 
       p->have_powerd_params = TRUE;
-      g_variant_get(v, "((iiiib))", &p->powerd_dim,
-                                    &p->powerd_min,
-                                    &p->powerd_max,
-                                    &p->powerd_default_value,
-                                    &p->powerd_ab_supported);
+      g_variant_get(v, "(iiiib)", &p->powerd_dim,
+                                  &p->powerd_min,
+                                  &p->powerd_max,
+                                  &p->powerd_default_value,
+                                  &p->powerd_ab_supported);
       g_debug("powerd brightness settings: dim=%d, min=%d, max=%d, default=%d, ab_supported=%d",
               p->powerd_dim,
               p->powerd_min,
@@ -262,52 +259,83 @@
       /* cleanup */
       g_variant_unref(v);
     }
-}
-
-static void
-call_powerd_get_brightness_params(IndicatorPowerBrightness * self)
-{
-  priv_t * p = get_priv(self);
-
-  g_dbus_connection_call(p->system_bus,
-                         "com.canonical.powerd",
-                         "/com/canonical/powerd",
-                         "com.canonical.powerd",
-                         "getBrightnessParams",
-                         NULL,
-                         G_VARIANT_TYPE("((iiiib))"),
-                         G_DBUS_CALL_FLAGS_NONE,
-                         -1, /* default timeout */
-                         p->cancellable,
-                         on_powerd_brightness_params_ready,
-                         self);
-}
-
-static void
-on_powerd_appeared(GDBusConnection * connection,
-                   const gchar     * bus_name     G_GNUC_UNUSED,
-                   const gchar     * name_owner   G_GNUC_UNUSED,
-                   gpointer          gself)
-{
-  IndicatorPowerBrightness * self = INDICATOR_POWER_BRIGHTNESS(gself);
-  priv_t * p = get_priv(self);
-
-  /* keep a handle to the system bus */
-  g_clear_object(&p->system_bus);
-  p->system_bus = g_object_ref(connection);
- 
-  /* update our cache of powerd's brightness params */
-  call_powerd_get_brightness_params(self);
-}
-
-static void
-on_powerd_vanished(GDBusConnection * connection  G_GNUC_UNUSED,
-                   const gchar     * bus_name    G_GNUC_UNUSED,
-                   gpointer          gself)
+  else if (error != NULL)
+    {
+      if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+        g_warning("Unable to get system bus: %s", error->message);
+
+      g_error_free(error);
+    }
+}
+
+static void
+on_powerd_name_owner_changed(GDBusProxy * powerd_proxy,
+                             GParamSpec * pspec         G_GNUC_UNUSED,
+                             gpointer     gself)
 {
   priv_t * p = get_priv(INDICATOR_POWER_BRIGHTNESS(gself));
-
-  p->have_powerd_params = FALSE;
+  gchar * owner = g_dbus_proxy_get_name_owner(powerd_proxy);
+
+  if (g_strcmp0(p->powerd_name_owner, owner))
+    {
+      p->have_powerd_params = FALSE;
+
+      if (owner != NULL)
+        {
+          dbus_powerd_call_get_brightness_params(DBUS_POWERD(powerd_proxy),
+                                                 p->cancellable,
+                                                 on_powerd_brightness_params_ready,
+                                                 gself);
+        }
+
+      g_free(p->powerd_name_owner);
+      p->powerd_name_owner = owner;
+  }
+}
+
+static void
+on_powerd_brightness_changed(DbusPowerd * powerd_proxy,
+                             GParamSpec * pspec         G_GNUC_UNUSED,
+                             gpointer     gself)
+{
+  set_brightness_local(gself, dbus_powerd_get_brightness(powerd_proxy));
+}
+
+static void
+on_powerd_proxy_ready(GObject      * source_object G_GNUC_UNUSED,
+                      GAsyncResult * res,
+                      gpointer       gself)
+{
+  GError * error;
+  DbusPowerd * powerd_proxy;
+
+  error = NULL;
+  powerd_proxy = dbus_powerd_proxy_new_for_bus_finish(res, &error);
+
+  if (powerd_proxy != NULL)
+    {
+      priv_t * p;
+      p = get_priv(INDICATOR_POWER_BRIGHTNESS(gself));
+
+      /* keep a handle to the system bus */
+      g_clear_object(&p->system_bus);
+      p->system_bus = g_object_ref(g_dbus_proxy_get_connection(G_DBUS_PROXY(powerd_proxy)));
+
+      /* keep the proxy and listen to owner changes */
+      p->powerd_proxy = powerd_proxy;
+      g_signal_connect(p->powerd_proxy, "notify::g-name-owner",
+                       G_CALLBACK(on_powerd_name_owner_changed), gself);
+      g_signal_connect(p->powerd_proxy, "notify::brightness",
+                       G_CALLBACK(on_powerd_brightness_changed), gself);
+      on_powerd_name_owner_changed(G_DBUS_PROXY(powerd_proxy), NULL, gself);
+    }
+  else if (error != NULL)
+    {
+      if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+        g_warning("Unable to get powerd proxy: %s", error->message);
+
+      g_error_free(error);
+    }
 }
 
 /**
@@ -433,13 +461,13 @@
       g_settings_schema_unref(schema);
     }
 
-  p->powerd_name_tag = g_bus_watch_name(G_BUS_TYPE_SYSTEM,
-                                        "com.canonical.powerd",
-                                        G_BUS_NAME_WATCHER_FLAGS_NONE,
-                                        on_powerd_appeared,
-                                        on_powerd_vanished,
-                                        self,
-                                        NULL);
+  dbus_powerd_proxy_new_for_bus (G_BUS_TYPE_SYSTEM,
+                                 G_DBUS_PROXY_FLAGS_GET_INVALIDATED_PROPERTIES,
+                                 "com.canonical.powerd",
+                                 "/com/canonical/powerd",
+                                 p->cancellable,
+                                 on_powerd_proxy_ready,
+                                 self);
 }
 
 static void

=== added file 'src/com.canonical.powerd.xml'
--- src/com.canonical.powerd.xml	1970-01-01 00:00:00 +0000
+++ src/com.canonical.powerd.xml	2015-05-20 19:36:35 +0000
@@ -0,0 +1,84 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<node name="/">
+  <interface name="com.canonical.powerd">
+    <!-- Properties -->
+
+    <property name="brightness" type="i" access="readwrite">
+    </property>
+
+
+    <!-- Functions -->
+    <method name="requestSysState">
+      <arg type="s" name="name" direction="in" />
+      <arg type="i" name="state" direction="in" />
+      <arg type="s" name="cookie" direction="out" />
+    </method>
+
+    <method name="clearSysState">
+      <arg type="s" name="cookie" direction="in" />
+    </method>
+
+    <method name="requestWakeup">
+      <arg type="s" name="name" direction="in" />
+      <arg type="t" name="time" direction="in" />
+      <arg type="s" name="cookie" direction="out" />
+    </method>
+
+    <method name="enableProximityHandling">
+      <arg type="s" name="name" direction="in" />
+    </method>
+
+    <method name="disableProximityHandling">
+      <arg type="s" name="name" direction="in" />
+    </method>
+
+    <method name="clearWakeup">
+      <arg type="s" name="cookie" direction="in" />
+    </method>
+
+    <method name="registerClient">
+        <arg type="s" name="name" direction="in" />
+    </method>
+
+    <method name="unregisterClient">
+        <arg type="s" name="name" direction="in" />
+    </method>
+
+    <method name="ackStateChange">
+        <arg type="i" name="state" direction="in" />
+    </method>
+
+    <!-- User settings -->
+    <method name="userAutobrightnessEnable">
+        <arg type="b" name="enable" direction="in" />
+    </method>
+
+    <method name="getBrightnessParams">
+        <!-- Returns min, max, and default brighness and whether or not
+             autobrightness is supported, in that order -->
+        <arg type="(iiiib)" name="params" direction="out" />
+    </method>
+
+    <method name="setUserBrightness">
+        <arg type="i" name="brightness" direction="in" />
+    </method>
+
+    <!-- for debug/testing -->
+    <method name="listSysRequests">
+      <arg type="a(ssi)" name="requestList" direction="out" />
+    </method>
+
+    <method name="getSysRequestStats">
+      <arg type="a(ssuttt)" name="requestStats" direction="out" />
+    </method>
+
+    <!-- Signals -->
+    <signal name="SysPowerStateChange">
+      <arg type="i" name="sysState" direction="out" />
+    </signal>
+
+    <signal name="Wakeup">
+    </signal>
+
+  </interface>
+</node>