Merge lp:~3v1n0/unity-settings-daemon/kbd-brightness-update into lp:unity-settings-daemon

[Marco Trevisan (Treviño)]

GsdPowerManager: don't cache kbd backlight current value

Keyboard backlight is now always fetched from UPower, without keeping
a cached value around.

This allows to properly Toggle and save/restore the backlight keyboard even
in hardwired configurations, where no ACPI events are emitted to request
a keyboard backlight change (and the bios handles the change silently).

This needs upower patched with the patch I've pushed at https://bugs.freedesktop.org/show_bug.cgi?id=95457

[lp:~3v1n0/unity-settings-daemon/kbd-brightness-update]

4136. By Marco Trevisan (Treviño) on 2016-05-20

GsdPowerManager: always update current kbd brightness value before evaluating it

[Sebastien Bacher (seb128)]

what would happen if that landed/was used without the upower patch? if the patch is needed to avoid a regression we need to wait for upower to be uploaded and update the depends

review: Needs Fixing

[Marco Trevisan (Treviño) (3v1n0)]

> what would happen if that landed/was used without the upower patch? if the
> patch is needed to avoid a regression we need to wait for upower to be
> uploaded and update the depends

Simply nothing.

If the upower change doesn't land or is delayed, the fix just doesn't work since upower will always return the cached brightness value on upower_kbd_update_brightness call, then this patch is quite useless (but not dangerous) in that case.

So, there's not much need of bumping the required versions, although this could ensure the fix will land alltogether.
Just let me know what you prefer.

[Sebastien Bacher (seb128)]

if there is no regression no reason to enforce the patched upower version

review: Approve

[lp:~3v1n0/unity-settings-daemon/kbd-brightness-update]

4137. By Marco Trevisan (Treviño) on 2016-05-30

Merging with lp:~3v1n0/unity-settings-daemon/keep-cached-kbd-backlight-updated

4138. By Marco Trevisan (Treviño) on 2016-06-13

GsdPowerManager: don't cache kbd backlight current value

Keyboard backlight is now always fetched from UPower, without keeping
a cached value around.

This allows to properly Toggle and save/restore the backlight keyboard even
in hardwired configurations, where no ACPI events are emitted to request
a keyboard backlight change (and the bios handles the change silently).

=== modified file 'plugins/power/gsd-power-manager.c'
--- plugins/power/gsd-power-manager.c	2016-06-13 11:19:59 +0000
+++ plugins/power/gsd-power-manager.c	2016-06-13 11:19:59 +0000
@@ -192,7 +192,6 @@
 
         /* Keyboard */
         GDBusProxy              *upower_kdb_proxy;
-        gint                     kbd_brightness_now;
         gint                     kbd_brightness_max;
         gint                     kbd_brightness_old;
         gint                     kbd_brightness_pre_dim;
@@ -1931,14 +1930,41 @@
 }
 
 static gboolean
+upower_kbd_get_brightness (GsdPowerManager *manager)
+{
+        GVariant *k_now = NULL;
+        GError *error = NULL;
+        gint now;
+
+        k_now = g_dbus_proxy_call_sync (manager->priv->upower_kdb_proxy,
+                                        "GetBrightness",
+                                        NULL,
+                                        G_DBUS_CALL_FLAGS_NONE,
+                                        -1,
+                                        NULL,
+                                        &error);
+        if (k_now == NULL) {
+                if (error->domain != G_DBUS_ERROR ||
+                    error->code != G_DBUS_ERROR_UNKNOWN_METHOD) {
+                        g_warning ("Failed to get brightness: %s",
+                                   error->message);
+                }
+
+                g_error_free (error);
+                return -1;
+        }
+
+        g_variant_get (k_now, "(i)", &now);
+        g_variant_unref (k_now);
+
+        return now;
+}
+
+static gboolean
 upower_kbd_set_brightness (GsdPowerManager *manager, guint value, GError **error)
 {
         GVariant *retval;
 
-        /* same as before */
-        if (manager->priv->kbd_brightness_now == value)
-                return TRUE;
-
         /* update h/w value */
         retval = g_dbus_proxy_call_sync (manager->priv->upower_kdb_proxy,
                                          "SetBrightness",
@@ -1950,8 +1976,6 @@
         if (retval == NULL)
                 return FALSE;
 
-        /* save new value */
-        manager->priv->kbd_brightness_now = value;
         g_variant_unref (retval);
         return TRUE;
 }
@@ -1976,7 +2000,7 @@
         } else {
                 g_debug ("keyboard toggle on");
                 /* save the current value to restore later when untoggling */
-                manager->priv->kbd_brightness_old = manager->priv->kbd_brightness_now;
+                manager->priv->kbd_brightness_old = upower_kbd_get_brightness (manager);
                 ret = upower_kbd_set_brightness (manager, 0, error);
                 if (!ret) {
                         /* failed, reset back to -1 */
@@ -2063,7 +2087,7 @@
         }
 
         if (policy == GSD_POWER_ACTION_NOTHING) {
-		inhibit_lid_switch (manager);
+                inhibit_lid_switch (manager);
                 manager->priv->inhibit_lid_switch_action = TRUE;
         } else {
                 uninhibit_lid_switch (manager);
@@ -2281,7 +2305,7 @@
         if (manager->priv->upower_kdb_proxy == NULL)
                 return TRUE;
 
-        now = manager->priv->kbd_brightness_now;
+        now = upower_kbd_get_brightness (manager);
         max = manager->priv->kbd_brightness_max;
         idle = PERCENTAGE_TO_ABS (0, max, idle_percentage);
         if (idle > now) {
@@ -2848,25 +2872,10 @@
 }
 
 static void
-upower_kbd_signal_cb (GDBusProxy *proxy,
-                      const gchar *sender_name,
-                      const gchar *signal_name,
-                      GVariant *parameters,
-                      gpointer user_data)
-{
-        GsdPowerManager *manager = GSD_POWER_MANAGER (user_data);
-
-        if (g_strcmp0 (signal_name, "BrightnessChanged") == 0) {
-                g_variant_get (parameters, "(i)", &manager->priv->kbd_brightness_now);
-        }
-}
-
-static void
 power_keyboard_proxy_ready_cb (GObject             *source_object,
                                GAsyncResult        *res,
                                gpointer             user_data)
 {
-        GVariant *k_now = NULL;
         GVariant *k_max = NULL;
         GError *error = NULL;
         GsdPowerManager *manager = GSD_POWER_MANAGER (user_data);
@@ -2879,26 +2888,6 @@
                 goto out;
         }
 
-        g_signal_connect (manager->priv->upower_kdb_proxy, "g-signal",
-                          G_CALLBACK (upower_kbd_signal_cb), manager);
-
-        k_now = g_dbus_proxy_call_sync (manager->priv->upower_kdb_proxy,
-                                        "GetBrightness",
-                                        NULL,
-                                        G_DBUS_CALL_FLAGS_NONE,
-                                        -1,
-                                        NULL,
-                                        &error);
-        if (k_now == NULL) {
-                if (error->domain != G_DBUS_ERROR ||
-                    error->code != G_DBUS_ERROR_UNKNOWN_METHOD) {
-                        g_warning ("Failed to get brightness: %s",
-                                   error->message);
-                }
-                g_error_free (error);
-                goto out;
-        }
-
         k_max = g_dbus_proxy_call_sync (manager->priv->upower_kdb_proxy,
                                         "GetMaxBrightness",
                                         NULL,
@@ -2907,17 +2896,20 @@
                                         NULL,
                                         &error);
         if (k_max == NULL) {
-                g_warning ("Failed to get max brightness: %s", error->message);
+                if (error->domain != G_DBUS_ERROR ||
+                    error->code != G_DBUS_ERROR_UNKNOWN_METHOD) {
+                        g_warning ("Failed to get max brightness: %s",
+                                   error->message);
+                }
                 g_error_free (error);
                 goto out;
         }
 
-        g_variant_get (k_now, "(i)", &manager->priv->kbd_brightness_now);
         g_variant_get (k_max, "(i)", &manager->priv->kbd_brightness_max);
 
         /* set brightness to max if not currently set so is something
          * sensible */
-        if (manager->priv->kbd_brightness_now < 0) {
+        if (upower_kbd_get_brightness (manager) < 0) {
                 gboolean ret;
                 ret = upower_kbd_set_brightness (manager,
                                                  manager->priv->kbd_brightness_max,
@@ -2930,8 +2922,6 @@
                 }
         }
 out:
-        if (k_now != NULL)
-                g_variant_unref (k_now);
         if (k_max != NULL)
                 g_variant_unref (k_max);
 }
@@ -3553,6 +3543,7 @@
                              GVariant *parameters,
                              GDBusMethodInvocation *invocation)
 {
+        gint now;
         gint step;
         gint value = -1;
         gboolean ret;
@@ -3561,15 +3552,17 @@
 
         if (g_strcmp0 (method_name, "StepUp") == 0) {
                 g_debug ("keyboard step up");
+                now = upower_kbd_get_brightness (manager);
                 step = BRIGHTNESS_STEP_AMOUNT (manager->priv->kbd_brightness_max);
-                value = MIN (manager->priv->kbd_brightness_now + step,
+                value = MIN (now + step,
                              manager->priv->kbd_brightness_max);
                 ret = upower_kbd_set_brightness (manager, value, &error);
 
         } else if (g_strcmp0 (method_name, "StepDown") == 0) {
                 g_debug ("keyboard step down");
+                now = upower_kbd_get_brightness (manager);
                 step = BRIGHTNESS_STEP_AMOUNT (manager->priv->kbd_brightness_max);
-                value = MAX (manager->priv->kbd_brightness_now - step, 0);
+                value = MAX (now - step, 0);
                 ret = upower_kbd_set_brightness (manager, value, &error);
 
         } else if (g_strcmp0 (method_name, "Toggle") == 0) {