Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Operating System/Drivers
  • Assigned To
    kugel.
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Release 3.10
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by kugel. - 2012-01-01
Last edited by kugel. - 2012-01-03

FS#12502 - Rework powermgmnt for RaaA/Sim

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.

Closed by  kugel.
2012-01-03 23:45
Reason for closing:  Accepted
Additional comments about closing:  

r31548

Updated patch. No functional change.

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"?

Can be -1 if the target cannot report voltages (android,maemo).

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.

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.

nickp commented on 2012-01-03 13:50

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..]

Rename battery_adc_voltage to _battery_voltage in powermgmt-sim.c. I upload an updated patch this evening.

nickp commented on 2012-01-03 14:59

That did the trick, thanks.

Updated.
EDIT: Updated again, for maemo

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing