Feature requests for hal battery backend.

Ryan Lortie desrt at desrt.ca
Mon Aug 1 13:26:06 PDT 2005


Le lundi 01 août 2005 à 14:03 -0400, David Zeuthen a écrit :
> On Mon, 2005-08-01 at 04:15 -0400, Ryan Lortie wrote:
> > A patch to implement the second half of the feature requests is
> > attached.
> > 
> > This patch is 100% completely untested so it might segfault hal on
> > startup.  I have no way to test it as my laptop isn't ACPI (Powerbook
> > here, too).
> > 
> > All I can say is "it compiles" and "the rough idea is there". :)
> > 
> > Please test it out and give feedback.
> 
> A few comments
> 
>  - should probably use something else than battery.remaining_time,
>    suggest battery.combined_remaining_time which is exactly the
>    combined time of all available batteries. Need small patch for 
>    the spec too to spell this out.

>  - Suggest to just searching for batteries by capability rather than
>    keeping a list around as Richard suggests (there is no IPC involved
>    here). Also put this function in the common code so it can be
>    reused for other things than ACPI. Need to check that battery.type
>    is 'primary' to avoid UPS's and wireless mice.

I think this should only apply to ACPI since the other 'primary' battery
backends don't have proper multiple-battery support and with everything
else (including UPSes) it's not possible to determine what multiple
batteries mean.  Also, by putting it in ACPI it serves to keep the
calculation separate from the other battery devices in the system.

I am worried about the performance overhead of scanning the entire HAL
device database by capability and then doing a check for type "primary"
times the number of all battery devices in the system on every single
ACPI event/poll.  No IPC, but an awful lot of computation.

The linked list serves to keep things nicely separated and reduces the
computation load on updates.

> 
>  -  +	int am_discharging = 0;
>     +	int am_charging = 0;
>     +	int total_rate = 0;
> 
>     should use gboolean here.
Ah.  Had I know that glib was used by HAL I'd have used GSList instead
of hacking up my own linked-list implementation.

...

I'm actually starting to wonder if this patch is appropriate at all.  It
would make it a lot easier for HAL clients to report accurate time
remaining information to the user, but it's not really a nice fit as
it's currently written (the hackish nature of my patch is a bit of an
indication of this).

What might be nice is some overall "time remaining" property on the core
computer device or a virtual composite battery device for all primary
batteries in the entire system (this composite device could also report
overall percentage).

Something like:

  Computer
   |
   +- Batteries
   |   |
   |   +- Battery Bay 1
   |   +- Battery Bay 2
   |
   +- AC Adaptor
    etc...

And the parent "Batteries" would contain sane composite information
about all "primary" battery devices (and be flagged as such).  The
correct place to put the combined time remaining information would then
be in this new virtual device.

I think the mWh patch should absolutely go through since there's no
doubt that it's the right thing to do.  Maybe my ideas about the time
remaining stuff could use a little more thinking through, though.

As always, comments and input are greatly appreciated.  The feedback
that has occured so far on this list has been amazing.  Many thanks.

Cheers.
_______________________________________________
hal mailing list
hal at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/hal



More information about the Hal mailing list