Rockbox

Tasklist

FS#3001 - Improved power management

Attached to Project: Rockbox
Opened by Jvo Studer (vinylivo) - Wednesday, 08 February 2006, 19:40 GMT
Last edited by Brandon Low (lostlogic) - Monday, 06 March 2006, 16:58 GMT
Task Type Patches
Category
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

This Patch improves the power managment especially on
the iriver series.

new features:
- calculate remaining charging time (in place of
remaining running time) when charger is plugged in
- switches off jukebox when battery reaches dangerous
voltage level (less than 2.95V)

improvements:
- doesn't spin up harddisk at low battery level (<10%)
for shutting down
- shows 'Low Battery' or 'Battery empty' warning when
shutting down
- nicer filtering of battery voltage for less
variations in remain time
- slightly readjusted voltage levels (for iriver lipo
1300) to give more linear battery level condition
- reduced stack size (there's still enough room for
more) by half

This patch should solve all the low voltage battery
problems on the iriver. If battery is below 10% there
is no more spinning up for shutting down. but it can
still happen that the HDD spins up for reading another
track. but if the voltage falls below 3V a forced
shutdown is initiated and a warning message displayed.
This task depends upon

View Dependency Graph

This task blocks these from closing
 FS#4786 - Rockbox doesn't shut down gracefully on low battery. 
Closed by  Barry Wardell (barrywardell)
Saturday, 11 November 2006, 01:39 GMT
Reason for closing:  Accepted
Additional comments about closing:  Committed to CVS
Comment by Brandon Low (lostlogic) - Thursday, 09 February 2006, 23:03 GMT

+#elif defined(IRIVER_H100_SERIES)
+#define BATT_AVE_SAMPLES 128 /* slow filter for iriver */
+#else
+#define BATT_AVE_SAMPLES 64 /* medium filter constant
for all others */
+#endif

why is this only for H100 series, shouldn't it be battery or
charge type dependent?
Comment by Brandon Low (lostlogic) - Thursday, 09 February 2006, 23:07 GMT

Are all of these changes designed to only impact the iRiver
H1x0 series? It looks like they should supercede the
HAVE_CHARGE_STATE stuff that I did for H3x0 instead of
fighting with it?
Comment by Jvo Studer (vinylivo) - Sunday, 12 February 2006, 12:21 GMT

Does the battery voltage reading on the H300 series work now?

Yes probably it's better to relate the filter constant to
the battery type (LIPOL1300) for the irivers instead of
IRIVER_H100_SERIES definition. I'll change that.

What do you mean by fighting with HAVE_CHARGE_STATE? For the
H100 there's only HAVE_CHARGING defined. I'll have a look
into this. Updated patch will follow.
Comment by Jvo Studer (vinylivo) - Wednesday, 15 February 2006, 00:00 GMT

updated patch to work on H300_SERIES too. I can't really
test it because I don't own a H340, so comments would be good.

new feature: takes RECORDING, REMOTE and SPDIF-OUT current
consumption into account.

moved shutdown from power_thread_sleep to handle_auto_poweroff.

should work and improve safe shutdown for all targets.
shutdown level is in battery_level_shutoff table.

some code cleanup.
Comment by Brandon Low (lostlogic) - Friday, 17 February 2006, 13:21 GMT

I think with this I can drop the HAVE_CHARGE_STATE define
and use your patch, I'll try to do that and apply it this
weekend.

Anything else you think needs doing with the power management?
Comment by Jvo Studer (vinylivo) - Friday, 17 February 2006, 22:29 GMT

Unfortunately there was a bug (with the handover from
power_thread_sleep to handle_auto_poweroff) and this is
solved now. This only affected when drive was spinning and
battery level got low, but actually this is the most
critical part.

The charge_state is checked (so it works with current build
for H300 too, I checked that it compiles with no errors) but
it will work without too.

The advantage of using the LIPOL1300 part in
battery_status_update is that the voltage change from
plugging in or unplugging is compensated, otherwise the
display would show wrong values for several minutes.

As mentioned in my last post current consumption of
recording, remote and spdif-out (H100 only of course) are
checked. therfore I needed to make some changes in the
header file.
Here it would make sense to move the device specific current
values to the corresponding config files instead of having
them in powermgmt.h, but I may be wrong.

I added some power saving stuff in handle_auto_poweroff (no
more fade-in / -out of backlight and short timeout) when
battery gets low. This could be improved by adjusting disk
spindown timeout or other means of saving power.

I would like to issue a 'Warning Low Battery' splash message
once a threshold is passed but unfortunately this is not
possible because powermgmt has no access to the apps
(folder). Perhaps this could be done by using an event.

Otherwise I think this makes the battery more save, I never
had a hardware shutdown anymore. A patch for a save
bootloader follows...
Comment by Jvo Studer (vinylivo) - Saturday, 18 February 2006, 00:48 GMT

@ lostlogic:

I just forgot what I really wanted to implement too:

Is there a way to instruct the playback engine to start
rebuffering the remaing playlist? Because this could be cool
to have some more final listening time for low battery
condition because normally the harddisk is the problem
powerwise seen.
So if it could spin up a very last time at a certain
threshold we could have a final listening of perhaps another
half hour, of course this depends on the bitrate and the
buffer memory buts its the most we can can squeeze out of
the battery before it finally dies... (hopefully not)
Comment by needleboy (needleboy) - Sunday, 19 February 2006, 08:26 GMT

Gives an error when patching to latest CVS:
patching file firmware/powermgmt.c
Hunk #3 FAILED at 117.
1 out of 21 hunks FAILED -- saving rejects to file
firmware/powermgmt.c.rej
Comment by Jvo Studer (vinylivo) - Sunday, 19 February 2006, 12:55 GMT

Sorry I don't know why this happened. Attached a working patch.
Comment by Brandon Low (lostlogic) - Thursday, 02 March 2006, 13:43 GMT
This patch probably doesn't apply cleanly ... again ... I should have a chance today to adapt it to the changes I just made and commit it. Sorry for the delay and thanks for your patience.
Comment by Jvo Studer (vinylivo) - Monday, 06 March 2006, 02:49 GMT
Updated patch to work with 060305.
Added localization (language support) for low battery warning messages. English and German.
Added short disk spindown time and ata_power_off for devices that support it when battery is low to save some power.
Comment by Linus Nielsen Feltzing (linusnielsen) - Monday, 06 March 2006, 12:47 GMT
Looks promising. Does it compile cleanly on all targets?

I see that you have adjusted the battery_level_dangerous() function, but you still use "if(battery_percent > 9)" when checking. Is that intentional?
Comment by Brandon Low (lostlogic) - Monday, 06 March 2006, 16:57 GMT
Sorry, I shouldn't be the assignee for this, I've been too busy and unfairly ignoring it.
Comment by Jvo Studer (vinylivo) - Tuesday, 07 March 2006, 13:53 GMT
@ linus: Yes that's intentional because you normally rarely discharge the battery this low and if you do you probably need all the remaining power. By not spinning up the harddisk just for shutting down (for saving settings which have been saved on last spinup anyway) we can save some valuable juice. Typically the only thing that's not updated in settings is the current playback postition.
If you don't like that I can change it to 'if(battery_level_safe())'.

I didn't install other crosscompilers than the 'm68k-elf' yet, so I couldn't check. I hope I can install 'sh-elf' on OSX then I'll give it a try.
Comment by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 07 March 2006, 23:00 GMT
Maybe we should add another threshold, battery_level_critical() or something. The percentage value doesn't really tell much, as it depends on the battery capacity. 9 percent can be a whole lot for a high-capacity battery.

You should try to compile the other targets, just to make sure. It helps us committers a lot.
Comment by Jvo Studer (vinylivo) - Friday, 10 March 2006, 22:56 GMT
Ok here is a recent version of the power patch.
For the 'powersaving' shutdown I added a new function 'battery_level_critical()' which is now battery capacity dependent. This means if you have an original battery with relatively low capacity the threshold for not saving is at 9%, if you installed a more powerful battery this is decreased linearly. This is valid for all targets.
I also moved the shutoff during recording to the powermanagement to have control over the longer needed shutdown_timeout for doing a complete save. I tested this several times and it works safe now (tested with 1300mAh & 1600mAh battery on H140).

There was at least one bug I found for other targets. But I'm unable to check the Archos targets because I can't build the 'sh-elf' compiler. Also building the simulator failed on my powerbook (Darwin BSD), see mail on dev-list. Because I don't have a PC and I can't install linux on my laptop (simply not enough space available) I can't do anything about that for now. Perhaps someone can help me with these problems...
All coldfire and arm targets build fine.
Comment by Nicolas Pennequin (nicolas_p) - Thursday, 16 March 2006, 16:21 GMT
I synched this patch with the latest CVS (2006.03.16), based on the code that was included with a recent recording AGC patch (seems to be the same as v2.5). I had to adapt to the changes made to powermgmt.c ( http://www.rockbox.org/viewcvs.cgi/firmware/powermgmt.c.diff?r1=1.103&r2=1.104 ), so i'm not sure i did it all right, but it compiles.

Use -p0 to apply it.
Comment by glam (b00st4) - Monday, 03 April 2006, 18:22 GMT
hunk #20 failed at 1230...pls fix
Comment by David Rothenberger (drothenberger) - Monday, 03 April 2006, 21:03 GMT
Here's a version of the patch that applies cleanly to the latest CVS. It does *not* use the new voltages from <http://www.rockbox.org/viewcvs.cgi/firmware/powermgmt.c.diff?r1=1.103&r2=1.104>.
Comment by David Rothenberger (drothenberger) - Monday, 03 April 2006, 23:04 GMT
And here's another version that again applies cleanly after all the lang changes.
Comment by David Rothenberger (drothenberger) - Tuesday, 04 April 2006, 20:25 GMT
Here's another updated patch. This incorporates the latest change to disable backlight fadeout when shutting down. It also incorporates a few changes that got lost in previous merges to turn off the wmcodec and clear the iPod background.
Comment by glam (b00st4) - Friday, 07 April 2006, 12:15 GMT
hunk #21 failes again at 1231
Comment by David Rothenberger (drothenberger) - Friday, 07 April 2006, 18:49 GMT
> hunk #21 failes again at 1231

Which patch are you applying? What version of CVS are you applying it to? Are there other patches?

powermanagement-20060404.patch applies (with fuzz in the *.lang files) for me with latest CVS as of now. I've attached another patch that applies without fuzz to latest CVS, but if powermanagement-20060404.patch isn't working for you, this one won't either.

If you're still having problems, please the entire patch output around the section that's not applying. Specifically, the file where it's having problems. Might be useful to attach the *.rej file, too.

Comment by David Rothenberger (drothenberger) - Monday, 17 April 2006, 00:54 GMT
Updated for latest CVS changes.
Comment by David Rothenberger (drothenberger) - Thursday, 10 August 2006, 20:08 GMT
Updated for the latest CVS. Also fixed a few compilation errors so it now compiles for all targets, real and sim.
Comment by Norbert Preining (norbusan) - Thursday, 10 August 2006, 20:58 GMT
Hi David!
Thanks for the updated, I have included this patch again in my h300 builds. Currently testing.
Best wishes
Norbert
Comment by David Rothenberger (drothenberger) - Wednesday, 27 September 2006, 18:25 GMT
Updated for the latest CVS. Included guess for the H10 5/10GB shutoff voltages. Also, fixed a compilation error when building bootbox bootloader targets.
Comment by Barry Wardell (barrywardell) - Friday, 10 November 2006, 20:28 GMT
Updated to latest CVS. There is a problem with the H10 when using this patch and the charger is connected. For some reason, the first reading from the ADC is 0 which results in the H10 shutting down immeadiately when I try to turn it on. This is more a problem with how the ADC is used than with this patch, but should probably still be accounted for. Also, devices with non LiIon batterys shouldn't ever shut off when the power gets low. They should be just allowed to power off by themselves.
Comment by Barry Wardell (barrywardell) - Friday, 10 November 2006, 23:42 GMT
Updated version. Now makes sure the ADC on the H10 reads properly on startup. Also, doesn't shut Rockbox down if it's using NiMH or Alkaline batteries

Loading...