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