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

[Stu Hood]

Quick nitpicks:

   - @330 - Slight indentation problem
   - @364 - TRUE in boolean statement
   - @400 - Check against some ratio of bat->last_full instead: For
   instance, (rate > 0.25 * last_full)? (huge/server batteries could have
   rate >50000)
   - @923 - Broken comment

Otherwise, brilliant!


Stu Hood


On 11/6/06, Richard Hughes <hughsient at gmail.com> wrote:
>
> 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
> hal at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/hal
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.freedesktop.org/archives/hal/attachments/20061106/b1e61665/attachment.html