FS#12338 : Option to restart running sleep timer on keypress

Status Closed
Percent Complete
100%
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

Attached to Project: Rockbox
Opened by nickp - 2011-10-18
Last edited by nickp - 2011-12-26

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.

keypress_resets_sleep_timer_3... (4.4 KiB)

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

r31437

Comments (13)
Related Tasks (0/0)

nickp commented on 2011-10-18 03:49

This patch relies on >= r30777.

dfkt commented on 2011-10-18 08:31

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.

nickp commented on 2011-10-18 08:49

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.

nickp commented on 2011-10-26 08:39

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.

keypress_resets_sleep_timer_3... (5.7 KiB)

nickp commented on 2011-10-27 04:26

Warning: Undefined array key "useheading" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 1099 Warning: Undefined array key "target" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 557 Warning: Trying to access array offset on value of type null in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 557

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.

keypress_resets_sleep_timer_3... (5 KiB)

nickp commented on 2011-12-08 09:53

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

keypress_resets_sleep_timer_n... (0.6 KiB)

fml2 commented on 2011-12-14 16:00

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.

nickp commented on 2011-12-15 11:06

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.

keypress_resets_sleep_timer_n... (0.9 KiB)

keypress_resets_sleep_timer_3... (6.4 KiB)

fml2 commented on 2011-12-15 13:33

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?

nickp commented on 2011-12-15 13:51

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.

fml2 commented on 2011-12-15 17:27

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.

nickp commented on 2011-12-16 03:42

Seems obvious now, silly me! Thanks for your patience.

keypress_resets_sleep_timer_n... (0.8 KiB)

keypress_resets_sleep_timer_3... (6.4 KiB)

nickp commented on 2011-12-26 09:25

Moved to the new General Settings > Startup/Shutdown menu, prior to committing.

keypress_resets_sleep_timer_3... (5.9 KiB)