Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Battery/Charging
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Version 3.0
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Bertrik Sikken - 2008-07-05
Last edited by Bertrik Sikken - 2008-08-14

FS#9155 - Simplified battery bench plugin

Attached patch contains the current work-in-progress for a simplified version of the battery bench plugin. The main difference with the old one is that the battery measurements themselves are now done purely on a time basis (once a minute) and do not depend on disk activity or disk spindown times. Writing the measurements to disk is still as unobtrusive as possible (written when the disk is active anyway).

Other changes:
* Now uses the ata idle callback mechanism to flush measurements to disk, instead of polling for disk activity with ata_disk_is_active(). Infact, this patch removes ata_disk_is_active from the plugin API and adds the ata idle register/unregister functions.
* Skipped measurements are no longer kept track of, because they are nearly impossible. The only problem occurs when the measurement buffer gets full after > 16 hours of no disk activity, which is rather unusual. The current implementation chooses to save the data in that case (and spin up the disk).
* Measurements/disk activity is no longer logged, because it makes no sense with time-based logging.

Things to consider:
* The measurement thread simply sleeps for 60 seconds between measurements, resulting in a measurement interval that is actually slightly more than 60 seconds.
* Only tested on my sansa e200 (which is a flash target), so it has not been tested on a DAP with a hard disk yet. I tested the buffer-full scenario by modifying the source for a small measurement buffer (500 bytes) and short measurement interval (10s).

Closed by  Bertrik Sikken
2008-08-14 22:46
Reason for closing:  Accepted
Additional comments about closing:  

Committed as svn r18281

Jonathan Gordon commented on 2008-07-05 12:04

much better :)

one thing to remember is that on the flash targets, the ata callback is called every 3s or so (iirc)….

Andree Buschmann commented on 2008-07-09 19:24

Worked fine on my iPod 5.5G. I will perform the next run with full charged battery and switched off accessory supply to check whether the expected results come up.

Bertrik Sikken commented on 2008-07-10 10:24

jdgordon, as far as I interpreted the code the ata idle callbacks are called 3 secs after the last ata activity, not every 3 secs.

Buschel, thanks for testing so far. Did you notice anything strange? Can you verify that it does no unnecessary disk spinups and that logging is complete (measurement cache should be flushed on shutdown)?

Andree Buschmann commented on 2008-07-11 06:16

Bertrik, here is the second bench. This time the runtime result is quite near to what I’ve expected. So, there seems to be no additional disk spin-up or anything else that might impact battery life. Nevertheless I am confused that the bench again stops at 3.5V or 3-4%. Normally my benchs show voltages down to 3.3V (0%) – which equals up to 10-20min additional runtime.

E.g. http://www.rockbox.org/twiki/bin/viewfile/Main/IpodRuntime?rev=1;filename=battery_bench_080505_mpc_24_nolineout.txt.

Maybe the flush at shutdown is not performed?

Bertrik Sikken commented on 2008-07-13 11:29

Buschel, in your nolineout bench around 14:20:25 the voltage dips from 3.559 to 3.395. This dip is about the same voltage (3.557) and at about the same time (14:24:08) as the last voltage logged with the simplified battery bench. I think that somehow the player powered down because of this dip during the battery bench with my patch. There is an explicit flush in the simplified battery bench that is called when the plugin is stopped during the power-down procedure, so I think that the flush was performed correctly. I could add a (temporary) line to the text file (”End of battery bench” or something) on shutdown to verify that.

I don’t know what your player’s shutdown voltage is exactly, or what caused the dip. BTW, there seem to be some bugs in the battery runtime calculation code. At a voltage of 3.431, the estimated runtime is 0 minutes, but at 3.430 it becomes 2:20 again.

Attached is the patch updated to current SVN. It’s a bit smaller because I stripped most of the non-essential white-space changes, but it’s otherwise the same.

Thomas Martitz commented on 2008-07-21 02:13

Just a small thing: AFAIK you either bump the plugin api version (not only the min version) or you add new functions to the end of the api (then you’ll only need to bump the min version). Now, you’ve put new functions to into the middle of the api, but only bumped the min version.

See comments in plugin.h before and at the end of the plugin api.

Thomas Martitz commented on 2008-07-21 02:20

Ah, since you even removed a function, I think you actually have to bump the whole version.

But, I possibly confused the two versions. Now, that I’ve read the comment again, I think you’ve done it correctly.
Anyway, you didn’t take the opportunity to sort in any new function which are “waiting” at the end of the function table :D

Meh, I’m too tired right now…

Tomasz Wasilczyk commented on 2008-07-21 12:07

there is related task:
http://www.rockbox.org/tracker/task/8070 maybe some ideas (ie. more frequent flushing when battery is under 10%) would be helpful

Bertrik Sikken commented on 2008-07-21 18:08

Thomas, you’re right about me having to move the waiting functions, I’ll do that when this finally gets committed.

Tomasz, I added task 8070 as a related task. I’d like to keep the amount of special cases (like more frequent flushing) to an absolute minimum. I don’t want to fix something if it isn’t clear that it is a problem in the first place. Besides, more frequent flushing will make the battery bench less true to the normal situation (because of the extra disk spinups).

Attached is a new version of the patch. It is updated to svn r18813 and fixes the problem that it would not compile in the simulator.

Bertrik Sikken commented on 2008-07-27 13:03

Attached is a new version of the battery bench, updated to svn r18127. Changes:
* on exit, the battery bench now logs a final line with the reason for exiting (normal plugin exit or power off). This way you can detect if a battery bench terminated abnormally.
* fixed compilation issue for ifp7xx.
* minor bugfix in buffer overflow handling.

Bertrik Sikken commented on 2008-07-27 16:55

Stupid bug on my part when combining an assignment and an if statement. Fixed in attached patch.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing