After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 328927 - g-p-m should ignore large values of time remaining
g-p-m should ignore large values of time remaining
Status: RESOLVED FIXED
Product: gnome-power-manager
Classification: Deprecated
Component: gnome-power-manager
SVN TRUNK
Other Linux
: Normal normal
: ---
Assigned To: GNOME Power Manager Maintainer(s)
GNOME Power Manager Maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2006-01-28 02:00 UTC by Richard Hughes
Modified: 2006-02-13 11:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
exponential average for the rate (3.29 KB, patch)
2006-02-10 18:20 UTC, Richard Hughes
needs-work Details | Review

Description Richard Hughes 2006-01-28 02:00:18 UTC
My trucked battery just told me I had to wait many hundreds of hours until charged. We should probably limit this to < 5hours, and then it becomes "Invalid"

Richard.
Comment 1 Jaap A. Haitsma 2006-01-28 10:11:21 UTC
Richard,

Limiting is not the right approach I think. I have for example around 7 hours of battery power if I use two batteries. There should be laptops/batteries out there which can give even more battery time.

My guess is that you saw this very large value just after you unplugged your adapter. Is this so?

As I mentioned before we should probably ignore the first remaining_time update after an ac adapter change, because charge_rate is an average over a certain time and if during that time the laptop was partially using AC and partially using battery you get a value for charge_rate that is too low. Other option would be that we store in gconf a value of what the discharge_rate on average is. This average we can obtain while running gpm.

Jaap

BTW Can I already apply the patch I sent you yesterday? I have other patches coming up :-). So it would be handy if that went in
Comment 2 Richard Hughes 2006-01-28 10:28:01 UTC
Okay, what about a sanity check of < 100 hours? The value was displayed on my new iBook with a broken battery, displayed all the time.

>As I mentioned before we should probably ignore the first remaining_time update
>after an ac adapter change, because charge_rate is an average over a certain
>time and if during that time the laptop was partially using AC and partially
>using battery you get a value for charge_rate that is too low.

Yes, this is logically correct.

>Other option
>would be that we store in gconf a value of what the discharge_rate on average
>is. This average we can obtain while running gpm.

No so keen on this, as it gets messy dealing with the battery serials in gconf, and an average rate may change dramatically on load, and off load.

Maybe one to think about tho.

Richard.
Comment 3 Jaap A. Haitsma 2006-01-28 10:43:26 UTC
(In reply to comment #2)
> Okay, what about a sanity check of < 100 hours? The value was displayed on my
> new iBook with a broken battery, displayed all the time.
> 
That seems OK to me
What do you mean with broken? The battery doesn't work or HAL does not report any sensible values?

> 
> >Other option
> >would be that we store in gconf a value of what the discharge_rate on average
> >is. This average we can obtain while running gpm.
> 
> No so keen on this, as it gets messy dealing with the battery serials in gconf,
> and an average rate may change dramatically on load, and off load.

I was not proposing to have battery serials in gconf, just the average discharge rate. That we could make very slowly varying in a way like this.

avg_charge_rate = 0.98 * avg_charge_rate + 0.02 * currently_measured_charge_rate

if (currently_measured_charge_rate < 0.20 * avg_charge_rate)
     time = "Unkown"

> Maybe one to think about tho.
> 
I agree
Comment 4 Richard Hughes 2006-01-28 10:48:53 UTC
Broken as in the rate goes from zero, to negative, to massivly positive values. The battery does not charge past 4%, and holds no charge what-so-ever. Basically it's fubar'd. :-)

And your running average could work well. We wouldn't need to touch gconf and just store it in a manager->priv->running_average field. The lag (0.98 in your case) should be configurable whilst testing. Fancy writing a test patch for discussion?

Richard.
Comment 5 Jaap A. Haitsma 2006-01-28 11:11:57 UTC
If it's completely fubar'd even a running average won't help. You just get an average of complete garbage. That just should be fixed in ACPI (or HAL) first before we can do anything at all :-(

Because of the large lag to get a stable average you need quite some time to get this average. So you would need to have gpm running for quite some time to get a good average. Therefore I was thinking of storing it in gconf so at startup you immediately have a reasonable value.

The case of charge_rate being negative we should check for anyway. Otherwise we might get weird times. Though I think get_timestring saves us there at the moment.
I'll cover that in my next patch as I've changed code in that part of the code anyway
Comment 6 Richard Hughes 2006-01-29 23:55:47 UTC
I meant the running average could work well on a good battery, I think mine is too far gone...

I think limiting the time to less that 50 hours would be a good compromise for really broken batteries -- do you agree? Else I get "350 hours 34 minutes" until charged every now and then.

As for the average, I dont think storing it in gconf is a good idea -- what if we switch batteries, or add another?
Comment 7 Richard Hughes 2006-02-10 18:20:39 UTC
Created attachment 59092 [details] [review]
exponential average for the rate

What do you think of this patch? Seems to work well for me.
Comment 8 Jaap A. Haitsma 2006-02-12 17:46:21 UTC
Seems OK to me.
Comment 9 Richard Hughes 2006-02-13 11:12:01 UTC
Thanks Jaap.

2006-02-13 Richard Hughes <richard@hughsie.com>
 * src/gpm-power.c (battery_device_cache_entry_update_key): Do a exponentially weighted average for the rate so that high frequency changes are smoothed. This should mean the time_remaining does not change drastically between updates. (battery_kind_cache_update) Also limit the time_remaining to 100 hours. Fixes bug #328927.