Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category User Interface → Simulator
  • Assigned To No-one
  • 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 nickp - 2012-01-03
Last edited by nickp - 2012-01-08

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

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.

Closed by  nickp
2012-01-08 12:20
Reason for closing:  Accepted
Additional comments about closing:  

r31635

I think  FS#12502  supersedes this.

nickp commented on 2012-01-03 13:55

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.

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.

nickp commented on 2012-01-03 14:57

OK, I didn't see the charger status getting through to the skin yet, I'll try later.

I committed my other patch. I think this can be closed now as well.

nickp commented on 2012-01-04 08:23

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.

Charger simulation should work in SVN, that's why I suggested closing this. I'd consider it a bug if it's not working.

nickp commented on 2012-01-04 12:03

Synced to r31574.

nickp commented on 2012-01-05 06:00

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?

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.

nickp commented on 2012-01-05 21:45

OK, fair point, I'll fix what's in SVN and then work from there.

nickp commented on 2012-01-05 22:53

For the time being, synced to r31588.

nickp commented on 2012-01-06 05:21

By popular demand, a version where everything is handled by battery_status_update().

Looks good to me

nickp commented on 2012-01-08 11:52

Some tidying before committing.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing