Rockbox

Tasklist

FS#8070 - Sansa battery_bench plugin buffer overflow

Attached to Project: Rockbox
Opened by Eddy (bascule) - Friday, 02 November 2007, 09:17 GMT
Last edited by Jonathan Gordon (jdgordon) - Saturday, 16 August 2008, 09:44 GMT
Task Type Bugs
Category Plugins
Status Closed
Assigned To No-one
Operating System Sansa e200
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

As detailed on this wiki page (http://www.rockbox.org/twiki/bin/view/Main/SandiskE200BatteryMeasurements), the battery_bench plugin does not work correctly on the Sansa e2x0 series.

When I run the plugin, it outputs this message to the log (file attached):

-Skipped 1722 measurements from 00:00:23 to 11:00:05-

The file contains exactly 1000 readings, the same as the quoted buffer size. The M/DA reading also looks screwed up.

I am assuming the problem is the fact that it's a flash player and the problem is somehow associated with an inability to skip some readings between disk accesses, as the 'disk' access functionality is not the same as a hard disk player.

This results in losing the readings from the buffer at the beginning of the test and only retaing the last 1000 readings (approx 5 hours-worth in my test).

I would presume the plugin is affected by the same behaviour on other flash-based players.

I've also attached a patch file (with fix code as per the Wiki page mentioned above) to make it easier for others to fix the plugin and run their own battery tests.
This task depends upon

Closed by  Jonathan Gordon (jdgordon)
Saturday, 16 August 2008, 09:44 GMT
Reason for closing:  Out of Date
Additional comments about closing:  new batt_bench was commit which shouldnt have this problem
Comment by Marianne Arnold (pixelma) - Friday, 02 November 2007, 13:28 GMT
Interesting that you got a log at all, I got no values when I last tried on my c200 and just letting it run out of battery (I get something when I cancel battery_bench).

If my grasp of it is now remotely correct, the real problem is that battery_bench relies on monitoring disk activity to write away the values that it collected in the buffer (to avoid extra disk spinups) and that this isn't possible on the Sansas (on Ondios the driver was written differently so it is possible, IIUC; don't know about the Nano). The stored values could be written away everytime it has to rebuffer - which seems sufficient for your numbers. The "quick and dirty fix" in the wiki prevents that "collecting" and forces a write everytime something changes. There was a short discussion between JdGordon and amiconn about the right fix in IRC around http://www.rockbox.org/irc/reader.pl?date=20071025#12:13:57 - if it helps to shed some light on it.

Edit: I just tried again and I get a log now too (but had less than 1000 entries in the end so the described problem couldn't occur). So I guess that for some reason battery_bench is now able to write the last values once before shutdown.
Comment by David Hall (Soap) - Wednesday, 16 January 2008, 11:43 GMT
What effect will this patch have on HDD based players?
Comment by Eddy (bascule) - Wednesday, 16 January 2008, 12:47 GMT
I don't know what effect it will have on HDD players. In fact, it does not entirely fix the problem for me on my Sansa.

I really just posted this bug report to ensure the issue did not become lost on the Wiki page and to provide the patch as a possible starting point for rectification.
My coding skills are limited to very minor changes to others' code, so I am not anticipating working on the problem.
Comment by Tomasz Wasilczyk (tomkiewicz) - Wednesday, 09 April 2008, 08:18 GMT
i think, updating batt_bench.txt file everytime, when new data comes may affect running time - i wrote another patch to write down data not everytime, but only if buffer is full. I have tested it and its works fine for me, but i have found another bug - last (in my case two) measurements have bad "time left" value

IMHO, that way of buffering may be applied to hdd based players too - i.e. to test their FM listening time
Comment by Tomasz Wasilczyk (tomkiewicz) - Thursday, 10 April 2008, 23:31 GMT
finally, I got it to work correctly and without "dirty fixes" ;)

1) lines 342-348 (one hour buffer-flush timeout):
Removed checking if mp3 is played - in case of HDD-based players it almost isn't used, because buffer is flushed more frequent than every hour (because disk activity checking works), but in case of sansa (and i guess other flash-based players) that check was breaking every-hour-buffer-flushing. On the other hand benchmark without playing of mp3 have no big sense (maybe FM benchmark), so i think that lines wasn't necessary.

2) rest of patch (added "timeflag" flag):
I had another problem, like described by pixelma - frequently (about 2 of 3 tries) last buffer isn't wrote in battery_benchmark.txt file. That was happened, because when "exit" action was used, it was too little time to write down changes to disk (because of low voltage, device may sometime turn off too fast). I found solution - below 10% of battery, buffer is flushed every 1% change; below 1% there is no buffering.

That patch doesn't breaks buffer-skip feature ("skipped 1234 measurements ...") - that may be useful, if there is too much measurements (over 1000/h), which may make battery_bench.txt file too big.

Frequent saving feature also may be useful both for flash and harddisk based devices - in 2-10% range thats doesn't affects too much battery runtime (it makes about 10 more disk spindowns) and keeps buffer quite small (very useful for bad-calibrated batteries). In 0-1% range there is much bigger chance to loose data, because of low voltage, so buffering seems to be harmful.
Comment by Tomasz Wasilczyk (tomkiewicz) - Friday, 11 April 2008, 19:11 GMT
small mistake - in 2) "added timeflag flag" it should be "added lowbattflag flag"

Loading...