This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#12555 - Displayed battery level is not smoothed
Attached to Project:
Rockbox
Opened by Boris Gjenero (dreamlayers) - Friday, 20 January 2012, 02:49 GMT+2
Last edited by Boris Gjenero (dreamlayers) - Sunday, 22 January 2012, 00:51 GMT+2
Opened by Boris Gjenero (dreamlayers) - Friday, 20 January 2012, 02:49 GMT+2
Last edited by Boris Gjenero (dreamlayers) - Sunday, 22 January 2012, 00:51 GMT+2
|
DetailsI recently found that displayed battery level can change very rapidly after the hard drive spins down. This is very easy to see on my 5G iPod with the original 30GB hard drive, running 007f61f. It happens because battery_read_info() calls _battery_voltage(), which reads battery voltage from the hardware. I changed it to call battery_voltage(), which returns smoothed voltage, and that restored the familiar old behaviour. I'm attaching the patch.
I think some smoothing is needed. Maybe the old behaviour was doing too much smoothing, but total lack of smoothing is worse. This only affects targets which measure battery voltage. The changed code also builds on other targets, because they provide a battery_voltage() function which just returns -1. 
read_smoothed_voltage.patch
(0.4 KiB)
b/firmware/powermgmt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) |
This task depends upon
Closed by Boris Gjenero (dreamlayers)
Sunday, 22 January 2012, 00:51 GMT+2
Reason for closing: Fixed
Additional comments about closing: 109084d
Sunday, 22 January 2012, 00:51 GMT+2
Reason for closing: Fixed
Additional comments about closing: 109084d
ETA: And battery_adc_voltage was renamed regardless of the fact that the "adc" was an important hint as to what it did.
Micheal, I don't see how having adc in name is an important hint. It's exposing an unintersting implementation detail in my book (fwiw, some hosted targets that can report it don't even use adc for that).
The function names here aren't very helpful. _battery_voltage() makes me think it's internal or more low level than battery_voltage(), but it doesn't explain the difference and battery_adc_voltage() seems more descriptive. If the value is processed a lot, for example, being smoothed by lower level code, then the name hints that the value needs to be treated differently (instead of being returned via battery_adc_voltage() and smoothed again). Also battery_read_info() isn't descriptive. Why should one expect that it returns values that aren't smoothed? However, I don't think these things are worth worrying about now.
battery_adc_voltage doesn't exist anymore, it has been renamed to _battery_voltage because a) adc is an implementation details which is not necessarily true, and b) _battery_time and _level where added and it should be consistent (battery_adc_time and battery_adc_level would have been absolutely wrong).