It seems that I'm frequently involved in a recurring scenario: I tweak something, a couple of days later I discover that an unrelated thing was broken by the tweak, I fall down the rabbit hole to fix the issue.
Today is the notebook's keyboard backlight, I don't even use the thing as I always keep it off, I find the light distracting.
So, a few months ago I started using a recent kernel otherwise a third party application causes the Intel Xorg driver to segfault. I think last week I spotted that a new kernel package was available, so I installed the new version without too much attention, but it wasn't a minor release and only today I realized that the backlight of the keyboard now turns on every time the notebook is resumed after a suspend or simply unlocking the screen lock.
A quick Google search pointed to a mix of a new kernel feature1 and UPower behavior, digging some more I found the freedesktop.org bug #95457 with a patch that should have fixed the issue in the upstream version.
- "It will took me half an hour tops to port it to the version in Debian stable and creating a package" - or that's what I thought.
It's a straight forward patch, it changes the behavior of the UPower daemon to check the brightness level instead of using the last saved value. The problem was that my modified daemon didn't fully worked, the backlight seemed to behave correctly or at least it didn't turn on after resume, but I couldn't query the brightness level using D-Bus and the "failed to convert brightness" error was present on syslog, so something was wrong with my version of the patch.
Bummer.
I spent a considerable amount of time staring at my code, searching for the error, but all looked consistent with what was applied upstream. So I rebuilt the package with the debug symbols2,
$ DEB_BUILD_OPTIONS='nostrip noopt debug' dpkg-buildpackage -b -uc -us
installed the new package, killed the daemon, launched it attached with gdb and set a break-point on the up_kbd_backlight_brightness_read function, in other words, standard boring C development.
A couple of minutes later, it turned out that I didn't introduced new bugs, the issue was with the upstream patch itself in the *end != '\0' test case, a off by 1 example. I found the confirmation in the UPower's git log and it was also tracked by the bug #96215.
So now I can query and set the brightness using D-Bus3 and the state of the backlight is preserved like I was used to:
$ gdbus call --system \n
--dest org.freedesktop.UPower \n
--object-path /org/freedesktop/UPower/KbdBacklight \n
--method org.freedesktop.UPower.KbdBacklight.GetBrightness
If someone is interested in the patch for Debian Jessie the diff below apply to the upower_0.99.1-3.2_amd64.deb package:
--- upower-0.99.1/src/up-kbd-backlight.c 2013-10-29 11:37:08.000000000 +0100
+++ upower-0.99.1.mod/src/up-kbd-backlight.c 2017-01-23 15:37:54.022599766 +0100
@@ -47,7 +47,6 @@
struct UpKbdBacklightPrivate
{
gint fd;
- gint brightness;
gint max_brightness;
DBusGConnection *connection;
};
@@ -62,10 +61,41 @@
G_DEFINE_TYPE (UpKbdBacklight, up_kbd_backlight, G_TYPE_OBJECT)
/**
+ * up_kbd_backlight_brightness_read:
+ **/
+static gint
+up_kbd_backlight_brightness_read (UpKbdBacklight *kbd_backlight)
+{
+ gchar buf[16];
+ gchar *end = NULL;
+ ssize_t len;
+ gint64 brightness = -1;
+
+ g_return_val_if_fail (kbd_backlight->priv->fd >= 0, brightness);
+
+ lseek (kbd_backlight->priv->fd, 0, SEEK_SET);
+ len = read (kbd_backlight->priv->fd, buf, G_N_ELEMENTS (buf) - 1);
+
+ if (len > 0) {
+ buf[len] = '\0';
+ brightness = g_ascii_strtoll (buf, &end, 10);
+
+ if (brightness < 0 ||
+ brightness > kbd_backlight->priv->max_brightness ||
+ end == buf) {
+ brightness = -1;
+ g_warning ("failed to convert brightness: %s", buf);
+ }
+ }
+
+ return brightness;
+}
+
+/**
* up_kbd_backlight_brightness_write:
**/
static gboolean
-up_kbd_backlight_brightness_write (UpKbdBacklight *kbd_backlight, gint value)
+ up_kbd_backlight_brightness_write (UpKbdBacklight *kbd_backlight, gint value)
{
gchar *text = NULL;
gint retval;
@@ -96,9 +126,8 @@
}
/* emit signal */
- kbd_backlight->priv->brightness = value;
g_signal_emit (kbd_backlight, signals [BRIGHTNESS_CHANGED], 0,
- kbd_backlight->priv->brightness);
+ value);
out:
g_free (text);
@@ -113,8 +142,20 @@
gboolean
up_kbd_backlight_get_brightness (UpKbdBacklight *kbd_backlight, gint *value, GError **error)
{
+ gint brightness;
+
g_return_val_if_fail (value != NULL, FALSE);
- *value = kbd_backlight->priv->brightness;
+
+ brightness = up_kbd_backlight_brightness_read (kbd_backlight);
+
+ if (brightness >= 0) {
+ *value = brightness;
+ } else {
+ g_set_error_literal (error, UP_DAEMON_ERROR,
+ UP_DAEMON_ERROR_GENERAL,
+ "error reading brightness");
+ }
+
return TRUE;
}
@@ -226,23 +267,13 @@
goto out;
}
- /* read brightness */
+ /* open the brightness file for read and write operations */
path_now = g_build_filename (dir_path, "brightness", NULL);
- ret = g_file_get_contents (path_now, &buf_now, NULL, &error);
- if (!ret) {
- g_warning ("failed to get brightness: %s", error->message);
- g_error_free (error);
- goto out;
- }
- kbd_backlight->priv->brightness = g_ascii_strtoull (buf_now, &end, 10);
- if (kbd_backlight->priv->brightness == 0 && end == buf_now) {
- g_warning ("failed to convert brightness: %s", buf_now);
- goto out;
- }
- /* open the file for writing */
kbd_backlight->priv->fd = open (path_now, O_RDWR);
- if (kbd_backlight->priv->fd < 0)
+
+ /* read brightness and check if it has an acceptable value */
+ if (up_kbd_backlight_brightness_read (kbd_backlight) < 0)
goto out;
/* success */
@@ -315,4 +346,3 @@
{
return g_object_new (UP_TYPE_KBD_BACKLIGHT, NULL);
}