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

Richard Hughes hughsient at gmail.com
Mon Nov 27 08:38:26 PST 2006


On Mon, 2006-11-06 at 18:18 -0500, Stu Hood wrote:
> 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!

David, what do you think? I've fixed these little nits, and really want
to get this in well before the next HAL release.

Richard.

> 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
>         
>         
>         
> 



More information about the hal mailing list