Rockbox

Tasklist

FS#12502 - Rework powermgmnt for RaaA/Sim

Attached to Project: Rockbox
Opened by Thomas Martitz (kugel.) - Sunday, 01 January 2012, 21:05 GMT
Last edited by Thomas Martitz (kugel.) - Tuesday, 03 January 2012, 23:45 GMT
Task Type Patches
Category Operating System/Drivers
Status Closed
Assigned To Thomas Martitz (kugel.)
Operating System All players
Severity Low
Priority Normal
Reported Version Release 3.10
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

This patch reworks powermgmt.c and related files to allow it to compile for hosted targets.

Since it assumed the ability to read voltage, I made it aware of other concepts in order to make it useful on RaaA. Some targets can only read battery level (android), some can read level & estimated time (Maemo, making our own estimation superflous). And some can read nothing (SDL app).

This patch introduces new defines CONFIG_BATTERY_MEASURE and compiles powermgmt.c for all of them, including the simulator. This makes hosted/powermgnt.c unneeded. Runtime estimation works with voltage and level reading, provided a battery capacity is given. power_history[] tracks battery level if voltage can't be measured.

Furthermore, the behavior of the sim is changed to implement a targets API (i.e. _battery_voltage()) instead of re-implementing powermgmt.c so it's much closer to real targets now.

Rename battery_adc_voltage() to _battery_voltage() for consistency.


So, I would like some tests, and naturally comments are welcome as well.
This task depends upon

Closed by  Thomas Martitz (kugel.)
Tuesday, 03 January 2012, 23:45 GMT
Reason for closing:  Accepted
Additional comments about closing:  r31548
Comment by Thomas Martitz (kugel.) - Monday, 02 January 2012, 07:00 GMT
Updated patch. No functional change.
Comment by Thomas Jarosch (thomasjfox) - Monday, 02 January 2012, 08:31 GMT
Nice cleanup you're doing here. I'll give it a spin on maemo tomorrow evening.

What's the reason the battery voltage is stored as "int" instead of "unsigned int"?
Comment by Thomas Martitz (kugel.) - Monday, 02 January 2012, 08:46 GMT
Can be -1 if the target cannot report voltages (android,maemo).
Comment by Thomas Martitz (kugel.) - Monday, 02 January 2012, 15:06 GMT
Sync
Comment by Boris Gjenero (dreamlayers) - Monday, 02 January 2012, 19:19 GMT
Nice cleanup! CONFIG_BATTERY_MEASURE is definitely needed, and the sim change is a good idea.

Why are target API functions (eg. _battery_voltage()) and data (eg. percent_to_volt_discharge[BATTERY_TYPES_COUNT][11]) needed in powermgmt.c when the target doesn't have that capability? Wouldn't it be better to instead have preprocessor conditionals so those aren't used?

I can't compile a 5G iPod sim due to:
- powermgmt-sim.c: accessory_supply_set() and lineout_set() removed
- powermgmt-sim.c: battery_voltage() should be _battery_voltage()
- undefined reference to lcd_shutdown(). Apparently, shutdown_hw() is not needed in the sim.
Comment by Thomas Martitz (kugel.) - Monday, 02 January 2012, 22:16 GMT
one reason this cleanup is needed is that powermgmt.c is a mess because its one hell of ifdef. so I prefer to collect the ifdef at one place and work with stubs/wrappers from there, to make it more maintainable. the functions and data is too tiny to worry about anyway.
Comment by Nick Peskett (nickp) - Tuesday, 03 January 2012, 13:50 GMT
I'm having problems compiling a clip+ sim too;

mkdir clipsim && cd clipsim && ../tools/configure --target=62 --type=S && make

/home/nick/rockbox-temp/clipsim2/firmware/libfirmware.a(powermgmt.o): In function `battery_read_info':
/home/nick/rockbox-temp/firmware/powermgmt.c:147: undefined reference to `_battery_voltage'
[etc..]
Comment by Thomas Martitz (kugel.) - Tuesday, 03 January 2012, 14:22 GMT
Rename battery_adc_voltage to _battery_voltage in powermgmt-sim.c. I upload an updated patch this evening.
Comment by Nick Peskett (nickp) - Tuesday, 03 January 2012, 14:59 GMT
That did the trick, thanks.
Comment by Thomas Martitz (kugel.) - Tuesday, 03 January 2012, 20:14 GMT
Updated.
EDIT: Updated again, for maemo

Loading...