Rockbox

  • 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.4
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by nickp - 2009-12-13
Last edited by kugel. - 2011-10-18

FS#10849 - Sleep timer options: persistent duration and start on boot.

[EDIT: More accurate description of latest patch posted in comments section, requested by nickp]

This patch adds two sleep timer options and a sub-menu to hold them (Settings > General Settings > System > Sleep Timer).

The two options are;

Sleep Timer Duration: Allows the value to be set permanently (as ‘sleeptimer duration’ in the config file). The value will be used until reset to a new value.

Start Sleep Timer On Boot: Whether you would like a sleep timer to automatically be started when the device boots up (stored in the config file as ‘sleeptimer on startup’).

These options are particularly useful for users who have a dedicated DAP by their bedside, or a specific config file used when sleeping. When the later is set, powering the device up will play for the users specified duration, then switch the device off.

The existing (System > (Date & Time) > Sleep Timer) option has been modified to be a switch showing “Start Sleep Timer (duration)” when inactive, or “Stop Sleep Timer (remaining)” when active. Duration and remaining being the current setting and remaining minutes left on the timer, respectively.

Closed by  kugel.
2011-10-18 19:08
Reason for closing:  Accepted
Additional comments about closing:  

r30777. Thank you for this.

Just an update to the above patch for revision 24705 - hope I did right, its my first try to add something like that.

As I see now the file apps/menus/sleeptimer_menu.c is missing.
I made this patch by “svn diff >sleep_timer_menu.patch”.
Which option brings svn diff to include the a new file?

Ok - found out - had to add the file to the svn itself - here is the patch, finally.

nickp commented on 2010-05-10 13:31

Synced to r25926.

nickp commented on 2010-05-18 13:29

Synced to r26142.

nickp commented on 2010-08-06 13:18

Synced to r27736.

nickp commented on 2010-08-26 15:25

If you find this patch interesting, you should also check out;

http://www.rockbox.org/tracker/task/11042

just curious, but if fs#11042 is a more sophisticated version of this patch, why not close this task? or are there differences between this task and fs#11042

nickp commented on 2010-08-28 00:59

While  FS#11042  has more features, at this point I’m not convinced it is more sophisticated.

This is mainly due to to the unsolved issue (which you have also commented on) regarding cancelling of timers when pause is pressed in wps, and the timer not being reinstated unless sleeptimer_on_startup is set.

Depending on this getting resolved, I’ll either request this is closed or import the uisimulator improvements into this patch.

nickp commented on 2010-09-02 23:15

Sleep Timer menu moved to /Settings/General Settings/System/Sleep Timer.

nickp commented on 2010-10-16 22:52

Synced to r28291.
Updated manual removal of wake-up alarm warning from r28263.

nickp commented on 2010-11-29 05:07

Synced to r28701.

nickp commented on 2011-01-29 06:36

Synced to r29159.

nickp commented on 2011-02-03 07:13

Synced to r29198.

nickp commented on 2011-02-28 08:18

Synced to 3.8 (r29442).

nickp commented on 2011-03-01 02:14

Synced to r29474.
Got rid of redundant uisimulator/common/stubs.c code, it seems something very similar is now in trunk.

dfkt commented on 2011-04-27 11:47

Could someone please re-sync this patch? With r29788 it appears to not compile anymore.

nickp commented on 2011-04-27 12:18

Hi dfkt,

It’s working for me after an svn up to r29788.

Could you try the following in your rockbox checked out root and attach /tmp/reject.txt?

wget http://www.rockbox.org/tracker/task/10849?getfile=23392 -O /tmp/sleeptimer_submenu_29474.patch
patch -p0 -i /tmp/sleeptimer_submenu_29474.patch –global-reject-file=/tmp/reject.txt
svn info » /tmp/reject.txt

Maybe another patch is conflicting with it?

Cheers

nickp commented on 2011-04-27 12:33

Ah sorry, you said compile not apply patch.

I just tried a compile for the Clip+ and it seems to run fine on the device - what errors are you getting?

dfkt commented on 2011-04-27 13:00

Hello Nick,

Thanks for your fast reply. I tried the patch on a clean source, no other patches applied. Applying the patch normally (patch -p0 < sleeptimer_submenu_29474.patch) gives no error. Trying your trouble shooting above gives no reject.txt. However, when actually compiling a clean source with the patch, it spits out hundreds of errors.

Here’s the last few lines of the terminal buffer, after trying make: http://pastie.org/1839139 - there’s hundreds more errors like these.

Applying any other patches to the same clean source gives no issues.

I’m trying to build it for a Clip+, on Ubuntu 10, by the way.

Cheers,
Martin

nickp commented on 2011-04-27 13:11

Thanks Martin.

Are you sure it isn’t  FS#11042  you’re applying? I only ask because menu_sleeptimer_pause_mode only exists there, not in this patch (which just powers off the device when the timer completes). That patch is pretty neglected nowadays.

If not I’ll have a go downloading and installing a Ubuntu 10 over the weekend, I’m still on an ancient 8.04 server here…

nickp commented on 2011-04-27 13:25

After applying the patch, hopefully “sed -n 93,94p apps/menus/sleeptimer_menu.c” at the command line should return;

MAKE_MENU(sleeptimer_menu, ID2P(LANG_SLEEP_TIMER), NULL, Icon_NOICON,

          &menu_toggle_sleeptimer, &menu_sleeptimer_duration, &menu_sleeptimer_on_startup);
nickp commented on 2011-04-27 13:41

I just noticed, you’re getting errors way beyond the end of the file;

/home/dfkt/rockbox-clean/apps/menus/sleeptimer_menu.c:649: error: redefinition of ‘menu_toggle_sleeptimer’

apps/menus/sleeptimer_menu.c should only have 94 lines - I think there’s something odd going on there…

Try “wc -l apps/menus/sleeptimer_menu.c”

dfkt commented on 2011-04-27 15:25

Nick, we’re getting closer… :)

I re-downloaded the patch from the site, to make sure it’s not  FS#11042  I wrongly renamed or similar. It was the correct patch I used.

The “sed” command indeed does return the lines you specified. The “wc” command returns “848 apps/menus/sleeptimer_menu.c”. I checked that file and it indeed contains 848 lines, several slightly different copies one after each other.

I did a “svn revert -R .” after that, and applied the patch again, normally. This is what sleeptimer_menu.c looks like after applying the patch: http://pastie.org/1839628 - one too many copies.

dfkt commented on 2011-04-27 18:25

Might this be causing issues with the patch, on some computers?

— apps/menus/sleeptimer_menu.c (revision 0)
+++ apps/menus/sleeptimer_menu.c (revision 0)
@@ -0,0 +1,94 @@

nickp commented on 2011-04-27 22:14

This patch adds sleeptimer_menu.c, so it shouldn’t exist in a pristine checkout of the rockbox trunk (that’s why it’s revision 0 - not yet checked in).

I think the root of the problem is the revert doesn’t seem to be getting rid of the file/ maybe other stuff before reapplying it.

A quick hack might be to just “rm apps/menus/sleeptimer_menu.c” before you start.

I personally always keep a pristine checkout in one directory (rockbox_orig), then if I want to apply any patches do a “svn up rockbox_orig/ && cp -a rockbox_orig rockbox && cd rockbox” before I start, then “cd .. && rm -rf rockbox” after I’ve finished.

Could you face a fresh checkout of rockbox? I know it’s quite a hit to take but I think your current one might have become a bit corrupted.

dfkt commented on 2011-05-01 00:10

Nick, I finally founds some time to try your suggestions. I checked out a fresh version in a new folder, and the sleep timer patch indeed applied and compiled without issues.

After successfully compiling a build with the sleep timer patch, I did a “svn revert -R .” on the new source. Lo and behold, sleeptimer_menu.c didn’t get removed by that, and reapplying the patch to the presumably “clean” source led to sleeptimer_menu.c becoming filled with duplicates. I didn’t have that behavior with any other patch yet, but at least now I know that “svn revert -R .” doesn’t fully clean up the source with this one patch. I wonder if anyone else ever had those issues, or why I’m having them…

Thanks for your help, Nick - highly appreciated. I’m happy I can use your sleep timer again in my builds.

dfkt commented on 2011-05-01 00:12

I forgot to mention, “svn diff” didn’t show any clues as well, that sleeptimer_menu.c was still present after reverting all changes to svn.

nickp commented on 2011-05-01 02:10

Thanks for persevering and keeping me updated Martin.

It all makes sense now, even though you wrote “svn revert -R .”, for some reason I was thinking you were doing a patch -R.

Due to sleeptimer_menu.c not being in the repository the code was checked out from (the patch creates it from scratch), it’s necessary for me to do a “svn add apps/menus/sleeptimer_menu.c” before svn diff to create the patch. Otherwise (as you found) svn diff doesn’t even mention the file.

This means when you checkout from trunk, svn knows nothing about sleeptimer_menu.c, so doesn’t bother reverting it. The next time you apply the patch, it would apply the change to line 0 of the sleeptimer_menu.c, leaving the previous change at the end.

I think even patch -R would leave an empty file rather than deleting it, not ideal.

I just checked svn help revert, there doesn’t even seem to be an option to delete unknown files… Oh well.

dfkt commented on 2011-05-07 20:32

I forgot to mention, “svn diff” didn’t show any clues as well, that sleeptimer_menu.c was still present after reverting all changes to svn.

dfkt commented on 2011-05-07 20:34

Ooops, browser cache in my virtual machine messed up and resent the old post. Please delete. Sorry!

Works great with rev.29474. My favorite patch; no idea, why this is not in standard Rockbox branch.
any chance to get this updated to 3.9 or current cvs rev.?

nickp commented on 2011-07-03 00:06

Hi Hemingway,

I just tried it here, apart from a few offsets, the current patch seems to work with v3_9;

svn switch svn://svn.rockbox.org/rockbox/tags/v3_9
wget http://www.rockbox.org/tracker/task/10849?getfile=23392 -O /tmp/sleeptimer_submenu_29474.patch
patch -p0 -i /tmp/sleeptimer_submenu_29474.patch

I tend to only update the patch when changes to the code base break the patch. Are you getting any failures? If so, could you post them?

Thanks,

Nick

Hi Nick,

switched to v3_9, patched, recompiled and installed it on my Sansa Clip+:
works like a charm.

Thanks!
Miguel

nickp commented on 2011-08-08 05:16

Synced to r30267.

nickp commented on 2011-08-09 10:12

Introduces menu option re-organisation discussed on IRC;

http://www.rockbox.org/irc/log-20110808#14:03:21

This looks fine to me. I like the new menu/settings structure. I’d like to see this committed.

I’ll test this for a few days and then report back. Unless something surprising surfaces, it’s time to brings this up on the developer’s mailing list.

nickp commented on 2011-08-09 13:31

Thanks sideral, appreciated.

dfkt commented on 2011-08-09 15:12

Thanks for the improvements, nickp. I will try it ASAP in my next build.

Regarding the one nifty feature  FS#11042  has over this one (which obviously caused some issues) - may I ask if it would be possible/easier to add an option for “reset timer on *any* key press” instead of having it only linked with the pause function? Creative players stock firmwares have this feature, and it seems rather comfortable to me.

nickp commented on 2011-08-09 22:32

It’s a feature I’ve be meaning to implement for ages, dfkt, but I’m not happy with how FS #11042 just intercepts a wps pause and cancels the current timer (only restarting if sleeptimer_on_startup is set).

I think the most elegant implementation might be to tap into the code which wakes the display on key press, refreshing the sleep timer if it’s active. This might also be good to be optional as a setting too.

At this stage though, sideral and I think that the best chance to get this accepted is to introduce as few changes as possible initially, presenting new features as further patches.

Some minor changes after reviewing this patch:

* Lang additions have to be at the end of english.lang
* Removed sleeptimer_menu.c to minimize diff; options live in settings_menu.c for the time being
* Moved sleep-timer settings next to idle-poweroff settings
* Some whitespace and formatting changes
* The formats using LANG_SLEEP_TIMER_START and LANG_SLEEP_TIMER_STOP needed to be made a part of the corresponding translation strings to cater for right-to-left languages.

Still missing:
[EDIT: nothing right now.]

nickp commented on 2011-08-13 23:04

Thanks sideral.

Re LANG_SLEEP_TIMER_(START|STOP), do you think we could get away with (stripped for brevity)?

if lang_is_rtl()

"%s (%d:%02d)"

else

"(%02d:%d) %s"
nickp commented on 2011-08-13 23:05

Ha ha, beat me to it!

nickp commented on 2011-08-17 10:26

sideral: I couldn’t resist implementing Thomas Jarosch’s suggested changes (thanks Thomas). I did this in the full knowledge that rockbox-dev opinion may sway in a different direction and the work would have to be done again ;). The idea was a good one though and curiosity got the better of me.

I think it turned out well, the patch is even smaller now and I think this actually adds functionality.

nickp commented on 2011-08-25 23:05

After further discussions on IRC earlier and the rockbox-dev list, here’s a new version.

All Sleep Timer entries are now all in System (> Time & Date)[if real time clock].

Existing sleep timer entry now also sets the persistent sleep timer value.

“Start sleep timer on boot” option added after “Alarm Wake up Screen” in same menu.

http://www.rockbox.org/irc/log-20110825#14:17:04

nickp commented on 2011-09-22 04:51

sideral: Are you tied up with something else at the moment? Would you prefer I found somebody else to test and commit this?

I’m sorry you’re waiting. I’ll try to get this in.

Thank you, kugel, that would be great!

This is the patch series I would like to commit. It implements the proposal from the mailing list.

The third patch is based on sleeptimer_submenu_30355.patch, however I added that the sleep timer menu item transforms into “cancel running timer (XX min)” and cancels the current timer on selection (took me longer than I anticipated :\ )

Perhaps I split the last one for commit, but that’s a minor topic.

nickp commented on 2011-10-09 02:28

Looks good, thanks for picking this up kugel.

nickp commented on 2011-10-18 03:46

kugel: Thanks for submitting this, I really appreciate it.
dfkt: I just added another patch which adds a “restart running sleep timer on keypress” option:  FS#12338 . Sorry it took a while, I was holding off until the persistent sleep timer duration value was available.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing