On 27/03/2012 14:20, Jonathan Gordon wrote:
Hi,

This wasnt caught when the sleep timer changes were commited, and its
not really a big deal but it should be changed.

IIUC the sleep timer is now broken into a few different settings:
1) set timer on boot
2) restart timer on keypress
3) start the timer with the chosen minutes value (arbitrarily set to
5min increments).

Both the first two settings use the 3rd to know how long to start the
timer for manually. The issue in the task is:
1) It isnt a normal setting so cant be added to the quickscreen and
shortcuts menu (minor inconvinience)
2) cancelling the setting screen enables the timer anyway. - BAD!

Unfortunately this has been the case with the callback since 2007;

http://svn.rockbox.org/viewvc.cgi/trunk/apps/menus/main_menu.c?view=diff&pathrev=30777&r1=12548&r2=12549

As you move through the menu items the sleep timer continually gets reset, so when you cancel the menu item the last set remains.


I don't understand why setting the sleep timer duration for boot
should be linked to the sleep timer duration i want to set
temporarily. (i.e the current system means unless I have a fixed.cfg
it is impossible for me to set a 30min timer before i go to bed and
have it turn on with a 90min timer).

The duration should be a separate setting and the "start sleep timer"
menu item should then be changed to start at "off" so pressing cancel
will not enable a timer.
In addition, extra handling for the sleep timer can be manually added
to the shortcuts screen to start/stop a timer.

This is pretty much how it was in the original patch before many opinions on IRC and the ML stripped it down to the bare minimum changes acceptable to get it committed.

http://www.rockbox.org/tracker/task/10849#comment40259

http://www.rockbox.org/mail/archive/rockbox-dev-archive-2011-08/0018.shtml

It might be difficult to implement this sort of flexibility without complaints about it getting too complicated/ too many options/ a dedicated sub-menu.

Cheers,

Nick

Reply via email to