Re: [gpm] [PATCH] Start implementation of keyboard backlight control.

[Richard Hughes <hughsient gmail com>]

On 9 January 2011 16:11, Alex Launi <alex launi gmail com> wrote:
> First draft of a patch to apply keyboard backlight support to gpm. Has
> support for using the keyboard brightness keys, and also dimming on idle.
> Applies against current master. Please review :)

First of all, thanks. Comments below:

+#define GPM_DBUS_PATH_KBD_BACKLIGHT	"/org/gnome/PowerManager/KeyboardBacklight"

Not KbdBacklight?

+/* misc math macros */
+#define GPM_MIN(a,b)					((a) < (b) ? (a) : (b))
+#define GPM_MAX(a,b)					((a) > (b) ? (a) : (b))

Please use the existing MIN and MAX macros.

+/*#define GPM_SETTINGS_KBD_BRIGHTNESS_IDLE_DIM_AC		"kbd-brightness-idle-dim-on-ac"
+#define GPM_SETTINGS_KBD_BRIGHTNESS_IDLE_DIM_BY_BATT
"kbd-brightness-idle-dim-by-on-battery"*/

Should be in gpm-common.h

Also, as a more general point, gpm-kbd-brightness.c should probably be
merged with gpm-kbd-backlight.c so there is only one new GObject. The
only reason we have the abstraction for backlight devices is that we
can control the backlight either through XOrg or the silly little
policykit helper.

Other than that, looks good. Thanks.

Richard.