Rockbox

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.

   keypress_resets_sleep_timer_3... (5.7 KiB)

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)

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)

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.

   keypress_resets_sleep_timer_n... (0.9 KiB)
   keypress_resets_sleep_timer_3... (6.4 KiB)

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.

   keypress_resets_sleep_timer_n... (0.8 KiB)
   keypress_resets_sleep_timer_3... (6.4 KiB)

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

   keypress_resets_sleep_timer_3... (5.9 KiB)