Feature requests for hal battery backend.

[Ryan Lortie]

Le samedi 30 juillet 2005 à 16:20 +0100, Richard Hughes a écrit :
> On Sat, 2005-07-30 at 09:22 -0400, Ryan Lortie wrote:
> > I think the easiest thing to do here is just to do the calculation using
> > the normalised mWh figures for charge and full.
> 
> See below, I think you have a good point here.
> 
> > I think mWh is appropriate.  Perhaps we could have a separate set of
> > fields for it, though?  (ie: so that the original information is still
> > available if you want it, but on primary batteries, there are 4
> > additional fields using normalised units to facilitate accurate
> > calculations).
> > > What would a patch look like that converted mA (or other units) into
> > > mWh? I think mWh is the defacto std unit.
> > 
> > I'm not completely sure on this one.  Just looking at the units:
> > 
> > watts = volts * amps
> 
> What about a part-conversion:
> 
> char *unit;
> int current, lastfull, rate;
> unit = hal_device_property_get_string (d, "battery.charge_level.unit");
> 
> current = hal_device_property_get_int (d,
> "battery.charge_level.current");
> lastfull = hal_device_property_get_int (d,
> "battery.charge_level.last_full");
> rate =hal_device_property_get_int (d, "battery.charge_level.rate");
> 
> if (strcmp (unit, "mWh") == 0) {
> 	hal_device_property_set_int (d, "battery.mwh.current", current);
> 	hal_device_property_set_int (d, "battery.mwh.last_full", lastfull);
> 	hal_device_property_set_int (d, "battery.mwh.rate", rate);
> } else if (strcmp (unit, "mWh") == 0) {
> 	int vcurrent, vdesign;
> 	vcurrent = hal_device_property_get_int (d, "battery.voltage.current");
> 	vdesign = hal_device_property_get_int (d, "battery.voltage.design");
> 	hal_device_property_set_int (d, "battery.mwh.current", current *
> vcurrent);
> 	hal_device_property_set_int (d, "battery.mwh.last_full", lastfull *
> vdesign);
> 	hal_device_property_set_int (d, "battery.mwh.rate", rate * vcurrent);
> }

This looks like just about what I want.  Just a warning: you have a
typo: else if (strcmp (unit, "mWh") == 0) should read "mAh".

You should probably also have a default case.  I think it would be
vaguely OK to just copy the values in verbatim if we don't know what the
unit is rather than just not have the property at all.  Alternatively,
leaving the properties blank and writing code that depend on them is a
great way to get new information from people with strange laptops in the
form of bug reports.  :)

If 99% of laptops out there report mWh or mAh then I'd say don't include
the default case for weird units, and let them come to us.  If more than
a few people will be negatively impacted by this, then we should address
it now even if it means getting less useful feedback (since they'll
assume that it's working fine but is just inaccurate).

Also: May as well throw in design capacity for good measure.  Also, we
might want to look at where we use the design voltage and where we use
the current voltage.  I'm happy with your decisions for now until
someone who really knows what they're talking about thinks that it
should be different.

> > It will change the semantics for the case where the batteries discharge
> > at the same time, but I think it will be a change for the better.
> 
> Okay, how do you work this out?
Either something like a global variable (yick) or every time a battery
update occurs then HAL walks through the entire tree looking for
batteries.

Maybe we could just keep a global variable that is a linked list of all
"primary" batteries?

It actually even gets a bit worse.  When the current rate on a given
battery changes then you have to go around and update the "remaining
time" on all of the other batteries.

I think the linked list idea is probably the best.  Unfortunately, it
means that all ACPI events are now O(n) instead of O(1) (where n is the
number of batteries in the system) but I can't imagine a reasonable case
where n is greater than 3 :)

Algorithm would go something like

update_global_list()
{
  llist *item;
  char *udi;
  int totalrate = 0;
  
  /* iterate over all batteries */
  for( item = global_linked_list; item; item = item->next )
  {
    udi = item->data;
    totalrate += hal_get_prop( udi, "rate" );
  }

  for( item = global_linked_list; item; item = item->next )
  {
    udi = item->data;
    hal_set_prop( udi, "remaining time", util_compute( ..., totalrate ) );
  }
}

on_battery_update()
{
  normal_update_stuff();

  if( not on AC power )
  {
    update_global_list();
  }
}

The proper thing to do to calculate time remaining while charging is
left as an exercise to the reader.  This is really a more difficult
problem since we don't know if the rate of charge of one battery will
increase once the other battery finishes charging or if it will remain
the same.  I don't think we can know.  It might even make sense to
completely disable "time remaining" guesses for dual battery systems.  

Given the current situation, the best thing to do as an application
developer would probably be to look at all of the batteries and report
the "time remaining" as the highest one.  On the other hand, it would be
nice for simplicity sake to be able to always do the sum thing on
time_remaining.

(Personally, I find that HAL gives wildly inaccurate "time remaining"
figures even when charging my single-battery powerbook due to its
assumption that charge rate will remain constant as the battery charges
linearly (not the case, it seems).  Perhaps for PMU as well as on
single-battery systems it would be appropriate to disable time remaining
guesses on charge?) 

> > 
> > However, again, I don't have a laptop to test it on :)
> 
> Heh, theory is great, right!

Let's hope so :)


I'd really like to get this code in HAL 0.5.4 so that GNOME 2.12 can use
it, but I don't expect that to occur (but I'd be very happy if it
did) :)  For now, assuming that it does not, I am going to take as many
of my ideas as possible from these mails and manually code them into the
battery status applet until they are moved into the HAL.

I'm on GIMPnet as desrt, by the way, if you'd like to talk about any of
this.

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