acpi updates

David Zeuthen david at fubar.dk
Mon Jan 31 21:41:10 PST 2005


On Sun, 2005-01-30 at 14:40 +0000, Richard Hughes wrote:
> Okay, I know the initial patch hasn't even been merged yet, but I've
> been working more on the code...(*)
> 
> Patch to HEAD (assuming nobody has committed anything yet...) is here:
> 
> http://hughsie.no-ip.com/write/hal/hal-patch-procfs-compile3.patch
> 

Hi,

So I had a look at this today and tonight. Some general comments

 o Missing CVSID: $Id$ in files.

 o Missing #ifndef guards for header files

 o Header files shouldn't include config.h. Source files should
   indeed include config.h (with HAVE_CONFIG_H guards)

 o Please use named parameters in header file prototypes, e.g.

    void foo (const char *someParam, int someOtherParam)

   rather than

    void foo (const char *, int)

 o Coding standards; basically

    - if ( foo ) should be if (foo); and
    - some_function() should be some_function ()
    - Use tabs of width 8
    - 120 characters per line maximum; don't cram things
      into 80 characters
    - UNIX text format; no \r\n, only \n

   and other small stuff. For details see
   http://developer.gnome.org/doc/guides/programming-guidelines/code-
style.html
   since hal is using more or less this coding style.

 o Please use the Doxygen format for documenting functions instead
   of a non-standard format (and you don't need to document trivial
   internal functions)

 o License boilerplate; please include the full GPLv2 license like
   the other files in the project

 o You exported a lot of functions you didn't need to (e.g.
   procfs_acpi.h) - this confused me a bit.

I fixed all. I also ripped out the caching parts since it made
things more unreadable for no real gain. Instead, I've added
some new functions to util.c that does all the heavy-lifting.
I also ripped out the PMU and APM files since it was only boiler-
plate. And I made a few other changes and I only ported battery,
button and ac_adapter over.

So, uhm, the patch I committed is very different from the
patch you supplied. It's also a bit shorter and IMHO a bit
more readable :-) - please have a look and let me know what
you think.

For fun, I added some polling, it works (you need to run
./hal-device-manager from the tools directory; you may need
to chmod a+x that file), but we need to replace it by
listening to the ACPI socket in a not too distant future.

Cheers,
David


_______________________________________________
hal mailing list
hal at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/hal



More information about the Hal mailing list