[patch] Reworking HAL's battery handling, was:Trivial error message fix

Richard Hughes hughsient at gmail.com
Mon Nov 6 12:38:10 PST 2006


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.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: hal-rewrite-battery-handling.patch
Type: text/x-patch
Size: 47042 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/hal/attachments/20061106/3007cd33/hal-rewrite-battery-handling-0001.bin


More information about the hal mailing list