HAL ported to FreeBSD

[Jean-Yves Lefort]

On Sat, 29 Apr 2006 11:27:20 +0100
Richard Hughes <hughsient at gmail.com> wrote:

> On Fri, 2006-04-28 at 19:22 +0200, Jean-Yves Lefort wrote:
> > Hi,
> > 
> > Joe Marcus Clarke and I have ported HAL to FreeBSD. The attached patch
> > contains various portability/bug fixes as well as a few
> > improvements. It'd help me if you could apply them.
> 
> +if [ "$HAL_PROP_LAPTOP_PANEL_ACCESS_METHOD" = "toshiba" ]; then
> +       if [ "x`uname -s`" = "xFreeBSD" ]; then
> +               value="`sysctl -n hw.acpi.toshiba.lcd_brightness`"
> +       else
> +               # cat /proc/acpi/toshiba/lcd
> +               #  brightness:              5
> +               #  brightness_levels:       8
> +               value="`cat $HAL_PROP_LINUX_ACPI_PATH | grep brightness:
> | awk '{print $2;}'`"
> +       fi
> 
> Wouldn't it be better to just :
> 
> if [ "x`uname -s`" = "xFreeBSD" ]
> 
> the whole section, and then do two blocks of:
> 
> if [ "$HAL_PROP_LAPTOP_PANEL_ACCESS_METHOD" = "toshiba" ]; then....
> 
> Would certainly look cleaner.
> 
> ------------------------------------
> This looks wrong I think:
> 
> -if [ ${value} -lt 0 ] || [ ${value} -ge
> $HAL_PROP_LAPTOP_PANEL_NUM_LEVELS ]; then
> +if [ ${value} -lt 0 ] || [ ${value} -gt
> $HAL_PROP_LAPTOP_PANEL_NUM_LEVELS ]; then
> 
> NUM_LEVELS can be 15, where valid values are 0-14 -- the comment may
> have to be updated I think.
> 
> ------------------------------------
> And then there is this:
> 
> -               dbus-send --system --print-reply
> --dest=org.freedesktop.Hal \
> +               dbus-send --system --dest=org.freedesktop.Hal \
>                           $device org.freedesktop.Hal.Device.Rescan
> 
> Which is wrong because the rescan doesn't happen when we don't use
> --print-reply... I think this is in the archives.

Right.

> I really think a smaller patches would allow us to dribble this into CVS
> checking each part rather than just merging one patch that could break
> lots of stuff in random ways.

Maybe you can commit the undiscussed parts and let the others out; I'd
then submit a modified patch for these.

-- 
Jean-Yves Lefort

jylefort at brutele.be
http://lefort.be.eu.org/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/hal/attachments/20060429/30d90575/attachment.pgp