[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