Rockbox

Tasklist

FS#12506 - Attempt to emulate the charge/ discharge cycle more completely.

Attached to Project: Rockbox
Opened by Nick Peskett (nickp) - Tuesday, 03 January 2012, 08:56 GMT
Last edited by Nick Peskett (nickp) - Sunday, 08 January 2012, 12:20 GMT
Task Type Patches
Category Simulator
Status Closed
Assigned To No-one
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

Currently the simulator only emulates cycling the battery level without setting the charger states.

In addition, this cycling was only advanced when battery_status_update is called.

This patch sets up an independent mock battery thread which handles continually discharging the battery, emulating a charger being attached, charging the battery and then trickle charging for a defined period (if CONFIG_CHARGING is defined and high enough).

The motivation for this change was to enable all battery/power states to be tested when designing skins.
   sim_discharge_charge_cycle.pa... (4.8 KiB)
This task depends upon

Closed by  Nick Peskett (nickp)
Sunday, 08 January 2012, 12:20 GMT
Reason for closing:  Accepted
Additional comments about closing:  r31635
Comment by Thomas Martitz (kugel.) - Tuesday, 03 January 2012, 12:34 GMT
I think  FS#12502  supersedes this.
Comment by Nick Peskett (nickp) - Tuesday, 03 January 2012, 13:55 GMT
Ah, does it? I had a look through the code there, but the bulk of battery_status_update in powermgmt-sim.c seemed unchanged.

I couldn't see any emulation of chargers in there, so thought the two patches were fairly unrelated and could co-exist.

At the moment I'm having problems compiling  FS#12502  for a sim though, I left a comment.
Comment by Thomas Martitz (kugel.) - Tuesday, 03 January 2012, 14:23 GMT
It is unchanged yes, but it it's periodically called by the power thread (the same one which runs on native too) which is now compiles for hosted. Charger insertion is also simulated with that patch.
Comment by Nick Peskett (nickp) - Tuesday, 03 January 2012, 14:57 GMT
OK, I didn't see the charger status getting through to the skin yet, I'll try later.
Comment by Thomas Martitz (kugel.) - Tuesday, 03 January 2012, 23:46 GMT
I committed my other patch. I think this can be closed now as well.
Comment by Nick Peskett (nickp) - Wednesday, 04 January 2012, 08:23 GMT
As far as I can see, there's still no working charger emulation on the simulator (the reason I wrote this patch in the first place).

Try it for yourself: Build a Clip+ sim, watch the battery icon until it's empty, when it starts charging again there ought to be a power plug icon (charger attached). Ideally, when full, it would then spend some time "trickle" charging when complete so you can also see the external power attached icon.

I'd prefer to keep this patch open until there's something in trunk that makes it possible to test theme charging icons using the simulator.

Is there something about the implementation that makes you keen to close it? I'm going to have to resync anyway, is there something you'd like changed?

I think the thread-based battery discharger/ charger is a cleaner solution than hacking battery_status_update, after all it's simulating something that is a sort of external thread IRL (i.e the battery itself and the hand plugging in a charger).

This approach also makes the trickle charge phase easier to implement too.
Comment by Thomas Martitz (kugel.) - Wednesday, 04 January 2012, 08:45 GMT
Charger simulation should work in SVN, that's why I suggested closing this. I'd consider it a bug if it's not working.
Comment by Nick Peskett (nickp) - Wednesday, 04 January 2012, 12:03 GMT
Synced to r31574.
   sim_discharge_charge_cycle_31... (5 KiB)
Comment by Nick Peskett (nickp) - Thursday, 05 January 2012, 06:00 GMT
kugel: Until now I'd been assuming that when you were saying "closed" you meant "rejected & closed", but it just occurred to me you might have meant "committed & closed" (in which case my rant wouldn't have made much sense, sorry!).

I've tested it with a few targets now and it seems ok, is there anything that might cause problems with RaaA?
Comment by Thomas Martitz (kugel.) - Thursday, 05 January 2012, 15:53 GMT
Alright, if I see this right, the bug in SVN is that power_input_status() returns POWER_INPUT_NONE instead of POWER_INPUT_CHARGER right? That would be unintended and a bug.

FWIW, I don't think another thread is needed. The power thread calls battery_update_status() periodically so I don't see the need for another thread. But even then, there's the "sim_tasks" thread which exists explicitly to drive some simulation functionalities so that could be used as well.

Also, abusing storage_init() is nasty. We don't want that.
Comment by Nick Peskett (nickp) - Thursday, 05 January 2012, 21:45 GMT
OK, fair point, I'll fix what's in SVN and then work from there.
Comment by Nick Peskett (nickp) - Thursday, 05 January 2012, 22:53 GMT
For the time being, synced to r31588.
   sim_discharge_charge_cycle_31... (4.8 KiB)
Comment by Nick Peskett (nickp) - Friday, 06 January 2012, 05:21 GMT
By popular demand, a version where everything is handled by battery_status_update().
   sim_discharge_charge_cycle_no... (3 KiB)
Comment by Thomas Martitz (kugel.) - Friday, 06 January 2012, 15:40 GMT
Looks good to me
Comment by Nick Peskett (nickp) - Sunday, 08 January 2012, 11:52 GMT
Some tidying before committing.
   sim_discharge_charge_cycle_31... (4 KiB)

Loading...