[RFC, patch] Toshiba LCD support, now also sony, panasonic, asus and ibm

[David Zeuthen]

On Tue, 2005-08-30 at 01:42 +0100, Richard Hughes wrote:
> On Mon, 2005-08-29 at 18:45 +0100, Richard Hughes wrote:
> > Guys, Girls.
> > 
> > Here is a patch to add Toshiba LCD support into HAL.
> > 
> > Please review and comment.
> 
> With lots of help from the #ubuntu-devel guys, I've added sony,
> panasonic, asus and ibm to the patch. And I've had lots of reports of
> success, which  is always nice.
> 
> I've also added support for the method: SetLCDBrightness ()
> 
> To test do something like the following:
> 
> dbus-send --system --print-reply --dest=org.freedesktop.Hal \
> /org/freedesktop/Hal/devices/acpi_toshiba \

Care to paste the output of lshal for this device object?

> org.freedesktop.Hal.Device.SystemPowerManagement.SetLCDBrightness \
> int32:2

Hmm, bad choice of interface name; this has nothing to do with
powermanagement. Suggest

 org.freedesktop.Hal.Device.LCDPanel

instead.

+ <entry><literal>lcdpanel.brightness.raw</literal> (int)</entry>
+ <entry><literal>lcdpanel.brightness.percent</literal> (int)</entry>
+ <entry><literal>lcdpanel.brightness.levels</literal> (int)</entry>

Not ideal names IMO... Suggest

 .num_levels
 .current_level   # can assume {0, 1, 2, ..., num-levels - 1}

+		 * echo "level {0..15}" > /proc/acpi/ibm/brightness

This is not true on my system with the latest ibm-acpi code :-).

 [root at daxter ibm]# cat /proc/acpi/ibm/brightness
 level:          7
 commands:       up, down
 commands:       level <level> (<level> is 0-7)

Suggest to double-check all the code for vendor modules... 

You also want to patch hal.conf.in so only the user at the console can
invoke these methods (and please test that this actually works using a
dummy user).

> Please have a look and tell me what you think.

Apart from that I think this looks good. Care to clean this up and send
a new patch? 

Btw, there were some comments a few weeks back on this list... did you
take these into account?

    David