Quick nitpicks: <ul><li>@330 - Slight indentation problem</li><li>@364 - TRUE in boolean statement</li><li>@400 - Check against some ratio of bat->last_full instead: For instance, (rate > 0.25 * last_full)? (huge/server batteries could have rate >50000) </li><li>@923 - Broken comment</li></ul>Otherwise, brilliant! Stu Hood <div>On 11/6/06, Richard Hughes <<a href="mailto:hughsient@gmail.com"> hughsient@gmail.com</a>> wrote:<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">On Mon, 2006-11-06 at 12:03 -0500, David Zeuthen wrote: > On Sun, 2006-11-05 at 22:54 +0000, Richard Hughes wrote: > > Trivial fix attached, stops a weird error message: > > > > 22:51:02.523 [D] acpi.c:173: Current voltage is unknown, smaller than > > 501002010575r greater than design > > > > Okay to commit? > > Certainly. Please commit. Thanks. Nahh, I got hacking and other stuff started happening: b/hald/Makefile.am  |    1 b/hald/linux/acpi.c |  226 +++++++---------------- b/hald/linux/apm.c  |    7 b/hald/linux/pmu.c  |  188 +++++++++++-------- b/hald/util_pm.c    |  162 ---------------- b/hald/util_pm.h    |    8 b/hald/util_battery.c |  508 ++++++++++++++++++++++++++++++++++++++++++++++++++++ b/hald/util_battery.h |   64 ++++++ 8 files changed, 770 insertions(+), 394 deletions(-) What I've basically done is expand the existing struct: typedef struct {        int last_level;        int last_chargeRate;        time_t last_time; } batteryInfo; So that we store the per-battery state in a quick to use structure so we can start doing some clever things with multiple batteries and rate calculations (like exponential discharge curve fitting). This has been the result of a question "why does g-p-m have a GpmPower class to abstract the data from HAL" when really it was HAL that needed improving to do the stuff I was doing in g-p-m - and the only way to do this was to rip out the bad battery handling code, and add it to a new module with proper logic, formatting and comments. Note: this refactoring work out of acpi.c is also required when sysfs starts giving us battery class objects like that OLPC and PMU now produce. The old code can only do the checks in acpi.c, and even then some were inconsistent, some only done on 2nd read and some just wrong. The acpi battery code has been cleaned up and made easier to read. The acpi.c code was especially *horrible* to work on before, due to the very specific ordering of checks and conversions, something that is in somepart my fault. Using a private battery object alongside HAL device allows us to add private properties to a hal device that we can use for clever things like rate smoothing and better time remaining calculation that can work for pmu, apm, acpi and battery class drivers. You can see the start of this in the attached patch where the rate is exponentially smoothed by 50%. This makes the time remaining much more stable. I moved the driver out of util_pm and into util_battery as the HalDevice dep was needed, and this was the cleanest way to still let the addons use the standalone util_pm power stuff without the deps. I think it's cleanest to use HalDevice, but I can add a match to a static hashtable using the UDI if you want, although this seems a bit of a bodge to me. Maybe even a static hashtable might be best for multiple batteries, I dunno. Most of the logic is copy/paste from the acpi.c, although it's quite a lot easier to follow as there are loads of comments. This is the first version, so there might be loads of really big problems, although it's been running like a champ on my laptop for 12 hours. PMU is converted, but untested. APM is lightly tested. ACPI works really well. This also makes my Lenovo laptop report rate correctly with and without experimental BIOS from IBM. Code review and comments welcomed. Richard. _______________________________________________ hal mailing list <a href="mailto:hal@lists.freedesktop.org">hal@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/hal">http://lists.freedesktop.org/mailman/listinfo/hal</a> </blockquote></div>