[PATCH][RFC] Backlight addon for dell laptops

[Richard Hughes]

On Wed, 2007-01-03 at 22:08 +0100, Erik Andrén wrote:
> I've developed an addon for hal, making it possible to control the AC
> and the battery backlight values for Dell laptops using SMBIOS.

Nice one!

> My first approach was to try to emulate the work the c++ libsmbios
> library developed by Dell did in C. I quickly discovered  that the code
> got quite messy and thus I decided to abandon this approach in favor of
> linking against the libsmbios library.

Good plan IMO. This is what we talked about doing a few months ago.

> This of course meant that I
> needed to write the addon in c++. (I don't know if there is a policy in
> the hal development against having c++-programs shipped).

I don't think it's ever come up before. The addon looks very "c'ish"
apart from a couple of lines so I don't think it's an issue. David?

> Anyway, I got away with this implementation rather cheaply, only needing
> to introduce a slight modification to logger.h in hald/
> 
> The addon currently works quite well for me but I still have the
> following issues:
> 
> 1. There seems to be something funky going on in gnome-power-manager
> while changing the brightness in battery mode. It thinks the current
> level is 7 all the time. This bug is problably related to the
> gnome-power-manager as the Power Manager Brightness Applet works correctly.

I think you are confusing the point of the two sliders. The one in the
gnome-power-preferences sets the policy for the change from ac->battery
or vice-versa. The Brightness Applet talks to gnome-power-manager
(rather than hal) so it can work with the session policy, for instance
saving the value in gconf, or not changing a idle-dimmed screen. Maybe
it's a bug if the policy slider doesn't move in response to the policy
brightness changing, or maybe it should just be in the help file.

> 2. Brightness turnaround speed isn't great. It's definitely an
> improvement when
> compared with the patch developed by Richard Hughes in July but when
> going from 7 (highest) to 0 (lowest) you can definitely feel the lag.

Ohh yes, I was calling a shell script, which called a large smbios
utility program - I make no apology for that being slow. :-)

> 3. No support for if you have protected you bios with a password. It's
> quite possible to workaround this issue by adding some kind of a
> mechanism throwing an exception prompting for the password. This
> problably need to be coordinated with gnome-power-manager.

And PolicyKit. I'm not sure how the interaction with the desktop should
work, I'm guessing if we try to set a brightness without a password
(when one is required) then we get a RequiredPassword exception or
something. g-p-m can then ask for a password using gnome-keyring and
save it there. We then need a SetPassword method in the addon, although
I'm not sure how secure this would be going plaintext over session-dbus.

> The attached patch cleanly applies to current git.
> 
> Please comment.

Patch looks clean, and coded well. I have little experience with C++ so
I'm not the best person to review the addon, but it looks very similar
to the C macbook one.

Richard.