Rockbox

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

   sleep_timer_menu.patch (13.1 KiB)

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.

   sleep_timer_menu.patch (16.4 KiB)

Synced to r25926.

   sleeptimer_submenu_25926.patch (16.4 KiB)

Synced to r26142.

   sleeptimer_submenu_26142.patch (15.2 KiB)

Synced to r27736.

   sleeptimer_submenu_27736.patch (15.2 KiB)

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

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.

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

   sleeptimer_submenu_27990.patch (15.5 KiB)

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

   sleeptimer_submenu_28291.patch (15.3 KiB)

Synced to r28701.

   sleeptimer_submenu_28701.patch (15.2 KiB)

Synced to r29159.

   sleeptimer_submenu_29159.patch (15.1 KiB)

Synced to r29198.

   sleeptimer_submenu_29198.patch (15.2 KiB)

Synced to 3.8 (r29442).

   sleeptimer_submenu-3.8.patch (15.2 KiB)

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

   sleeptimer_submenu_29474.patch (14 KiB)

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

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

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?

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

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…

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);

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”

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.

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 @@

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.

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.

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.

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.

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.

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.?

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

Synced to r30267.

   sleeptimer_submenu_30267.patch (14 KiB)

Introduces menu option re-organisation discussed on IRC;

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

   sleeptimer_submenu_30270.patch (14.8 KiB)

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.

Thanks sideral, appreciated.

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.

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.]

   sleeptimer-v3.diff (13.4 KiB)

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"

Ha ha, beat me to it!

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.

   sleeptimer_submenu_30324.patch (8.3 KiB)

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

   sleeptimer_submenu_30355.patch (8.1 KiB)

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.

   0001-Move-Time-Date-menu-from... (1.4 KiB)
   0002-Rename-System-in-the-mai... (2 KiB)
   0003-Sleep-timer-options-pers... (12 KiB)

Looks good, thanks for picking this up kugel.

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.