- Status Closed
- Percent Complete
- Task Type Patches
- Category Settings
- Assigned To No-one
- Operating System All players
- Severity Low
- Priority Very Low
- Reported Version Release 3.9
- Due in Version Undecided
-
Due Date
Undecided
- Votes
- Private
FS#12338 - Option to restart running sleep timer on keypress
If set and a sleep timer is running, the sleep timer is restarted with the value of the “sleeptimer duration” setting each time a key is pressed.
Closed by nickp
2011-12-26 09:34
Reason for closing: Accepted
Additional comments about closing: Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407
2011-12-26 09:34
Reason for closing: Accepted
Additional comments about closing: Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407
r31437
Loading...
Available keyboard shortcuts
- Alt + ⇧ Shift + l Login Dialog / Logout
- Alt + ⇧ Shift + a Add new task
- Alt + ⇧ Shift + m My searches
- Alt + ⇧ Shift + t focus taskid search
Tasklist
- o open selected task
- j move cursor down
- k move cursor up
Task Details
- n Next task
- p Previous task
- Alt + ⇧ Shift + e ↵ Enter Edit this task
- Alt + ⇧ Shift + w watch task
- Alt + ⇧ Shift + y Close Task
Task Editing
- Alt + ⇧ Shift + s save task
This patch relies on >= r30777.
Wow, thank you for that patch, Nick. Being aware of each and any key press looks like the most universal approach to reset the sleep timer.
I should add, as this patch currently piggy-backs on the poweroff_timer code (idle poweroff?), volume up/ down and scrolling up/ down lists seem to be ignored, but not in/ out of options or anything else I tried.
I think this was probably done to stop the interface becoming sluggish, and if so, seemed a good idea.
I was wrong about volume and menu up/down being exceptions, I was testing with a Sansa e200 and it turns out the scroll wheel was activating the back light, but not resetting the idle state. It was only after testing on a Clip+ I realised all buttons should restart the timer.
Here’s an update with a fix to the e200 scroll wheel code.
It now also includes a manual entry.
I’ve split out the e200 fix and reported it as a bug;
http://www.rockbox.org/tracker/task/12351
Updated revision, purely implementing the restarting the sleep timer on keypress.
If you’ve already applied this patch, there have been no functional changes since the first version.
Here’s a version which makes it the default behaviour (no setting to enable/ disable it).
As discussed on IRC;
http://www.rockbox.org/irc/log-20111208#10:12:43
I think the code in the firmware layer should not access any app level data. In this patch, however, just that happens since “global_setting” lives in the app layer. A more correct (and less pragmatic) solution would be to have a (static) var at the firmware level which would be set every time the global_setting changes. The code in powermgmt.c would only use that static var.
Thanks for the advice Alexander.
Here’s an updated couple of patches; the first one applies the restart by default, the second as an option.
Hello Nick.
You did exactly what I proposed, thank you! I have some notes:
1. It would be good if the new variable (sleeptimer_duration) would be commented, specifically, in what units it is (I think, it’s seconds).
2. In “set_sleep_timer”, the variable can be set unconditionally, i.e. the assignment can be pukked out of if-then-else.
As to the setting… I can’t comment on that but has anything changed since the last discussion? I.e. do we need it?
Good idea about the commenting, I’m a bit tied up but will add that soon.
Not sure what you mean about 2, sleeptimer_duration is just keeping track of what the sleep timer was last instantiated with so it can be reinstated on keypress, or am I missing the point?
I plan to put the question re a setting to the dev list soon. Rasher wasn’t keen on it being a default, there may be others;
http://www.rockbox.org/irc/log-20111208#10:40:52
I think if another sleep timer setting is added, there ought to be a dedicated sub-menu under settings, rather than squatting in time&date (not sure what its got to do with t&d anyway).
As it is, I think there’s a case for a dedicated sub-menu, if only to stop polluting the root settings menu with the couple of existing sleep timer options for non-RTC targets.
Re 1: I mean that you do not need to write “sleeptimer_duration = seconds;” and “sleeptimer_duration = 0;” in the branches, but can just write “sleeptimer_duration = seconds;” parallel to the if.
Seems obvious now, silly me! Thanks for your patience.
Moved to the new General Settings > Startup/Shutdown menu, prior to committing.