Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#5294 - Quick exit for menu screen

Attached to Project: Rockbox
Opened by Alistair Marshall (amar) - Friday, 05 May 2006, 20:44 GMT+1
Last edited by Paul Louden (darkkone) - Wednesday, 07 June 2006, 16:21 GMT+1
Task Type Patches
Category User Interface
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

press play in the menu and it will quit to the previous screen (I have been wanting this for a while, not sure if it counts as a bug though)

Though ATM it is only set up for H300 and H100, it just needs button asignments for the other targets
(application/octet-stream)    quickexit.patch (2 KiB) 
 apps/main_menu.c |    2 ++
 apps/menu.c      |    6 +++++-
 apps/menu.h      |    2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

This task depends upon

Closed by  Steve Bavin (pondlife)
Friday, 02 March 2007, 11:22 GMT+1
Reason for closing:  Out of Date
Additional comments about closing:  I believe the root menu UI changes have superceded this patch. Re-open if I\'m wrong!
Comment by Dominik Riebeling (bluebrother) - Friday, 05 May 2006, 20:54 GMT+1
thinking of the button assignment in the menu on my h120 I think it's a bit strange Play doesn't do anything and Stop goes one level back. Similar to the file browser I think it would be good (and somewhat consistent) to have it the same in them menu:
- Play to return to wps
- Stop to stop playback
- Left to go a level up (as it already does)
Comment by Alistair Marshall (amar) - Friday, 05 May 2006, 21:30 GMT+1
- Play to return to wps (this is what the patch does)
- Stop to stop playback (personaly I would like the oposet, stop to take you up one level but it doesn't bother me)
- Left to go a level up (as it already does)

amar aka Pi
Comment by Alistair Marshall (amar) - Tuesday, 16 May 2006, 16:08 GMT+1
Hi updated it a fair bit, now it should exit from all parts of the menu (including browse plugins etc)
Please test/post comments thanks
(as before it is atill only maped for the H300 and H100 main unit but adding remot and other targets should be simple
(application/octet-stream)    qexit.patch (8.2 KiB) 
 apps/main_menu.c        |    2 ++
 apps/menu.c             |    6 +++++-
 apps/menu.h             |    2 ++
 apps/settings.c         |   10 ++++++++++
 apps/tree.c             |   22 ++++++++++++++++------
 apps/gui/color_picker.c |   13 +++++++++----
 apps/gui/select.c       |    5 +++++
 apps/gui/select.h       |    1 +
 8 files changed, 50 insertions(+), 11 deletions(-)

Comment by Dave Chapman (linuxstb) - Tuesday, 16 May 2006, 16:59 GMT+1
Following the discussion in IRC, my suggestion would be to do the following:

1) Implement a "quick exit" button in the menus - this would (where possible) be the same button used to enter the menu. So for example, on the h1x0 it would be BUTTON_MODE, and on the ipods, BUTTON_MENU.

2) Make the PLAY button resume playback (or take the user to the WPS) in more places in Rockbox - an extension of this patch could be used to implement that in the menus.

i.e. don't try and mix the two features.
Comment by Alistair Marshall (amar) - Tuesday, 16 May 2006, 18:31 GMT+1
Thanks

OK I have changed the assignments to the same button used to enter the menu and added support to all platforms however the ipods use BUTTON_MENU in the colour picker and in select.h and I don't know what should be used instead. I _think_ remote support should be working now.

I only have an H300 <no remote> so if others could test the other platforms I would be grateful
(application/octet-stream)    qexit.patch (10.7 KiB) 
 apps/main_menu.c        |    2 ++
 apps/menu.c             |   13 +++++++------
 apps/menu.h             |    1 +
 apps/settings.c         |   10 ++++++++++
 apps/tree.c             |   18 +++++++++++++-----
 apps/gui/color_picker.c |   16 ++++++++++++----
 apps/gui/select.c       |    5 +++++
 apps/gui/select.h       |   19 ++++++++++---------
 8 files changed, 60 insertions(+), 24 deletions(-)

Comment by Alistair Marshall (amar) - Wednesday, 17 May 2006, 18:15 GMT+1
Now implemented part 2 of your suggestion, now when you quit the menu it always returns you to the WPS if music is playing and the file tree if no music is playing.

Press play, if no music is playing it resumes play list, then does a quick return.
Press stop, it stops current playback
Press menu, does a quick return

Buttons mapped on other targets based on the mappings in the file tree
Would appreciate it of people could check on other targets particularly the ONDIO (I have a bad feeling about the stop play in the file tree with that one)
(application/octet-stream)    menu_buttons.patch (16.5 KiB) 
 apps/main_menu.c        |    2 +
 apps/menu.c             |   47 ++++++++++++++++++++++++++++++++++----
 apps/menu.h             |   13 +++++++++-
 apps/settings.c         |   10 ++++++++
 apps/tree.c             |   58 +++++++++++++++++++++++++++++-------------------
 apps/gui/color_picker.c |   16 +++++++++----
 apps/gui/select.c       |    5 ++++
 apps/gui/select.h       |   19 ++++++++-------
 8 files changed, 129 insertions(+), 41 deletions(-)

Comment by Alistair Marshall (amar) - Friday, 19 May 2006, 17:45 GMT+1
One more go, I think it covers everything asked for
Still not tested on any other targets!
(application/octet-stream)    menu_buttons.patch (18.1 KiB) 
 apps/menu.c             |   58 ++++++++++++++++++++++++++++++++++++----
 apps/menu.h             |   15 +++++++++-
 apps/settings.c         |   13 ++++++++-
 apps/tree.c             |   69 ++++++++++++++++++++++++++++++++----------------
 apps/gui/color_picker.c |   16 ++++++++---
 apps/gui/gwps.c         |    3 +-
 apps/gui/select.c       |    5 +++
 apps/gui/select.h       |   19 ++++++-------
 8 files changed, 153 insertions(+), 45 deletions(-)

Comment by Michael DiFebbo (medifebbo) - Sunday, 04 June 2006, 14:04 GMT+1
Tested on iPod Video 60 GB. The patch applies and builds cleanly, and all functions appear to work as designed.
Comment by Norbert Preining (norbusan) - Tuesday, 06 June 2006, 00:56 GMT+1
Hi all!
I have added this patch to my experimental builds for h300, but I don't get any special action. Now I can press the A-B button (Menu button I assume) to get one step up, but pressing "Play" does not jump to the WPS screen. Any ideas about this?
Best wishes
Norbert
(for my builds, see http://www.misticriver.net/showthread.php?t=42675
Comment by Norbert Preining (norbusan) - Monday, 12 June 2006, 20:56 GMT+1
Here is an updated patch for current cvs (made on 2006-06-12 20:00 +0200).
   menu_buttons_NP20060612.patch (16.7 KiB) 
 ./apps/gui/color_picker.c |   16 +++++++---
 ./apps/gui/gwps.c         |    3 +
 ./apps/gui/select.c       |    5 +++
 ./apps/gui/select.h       |   19 ++++++------
 ./apps/menu.c             |   58 ++++++++++++++++++++++++++++++++++----
 ./apps/menu.h             |   15 +++++++++
 ./apps/settings.c         |   13 +++++++-
 ./apps/tree.c             |   70 ++++++++++++++++++++++++++++++----------------
 8 files changed, 153 insertions(+), 46 deletions(-)

Comment by Dominik Riebeling (bluebrother) - Tuesday, 13 June 2006, 01:32 GMT+1
Similar as suggested on the mailing list, it would also be nice to have
- press rec (short) to enter the playlist. Rockbox is playlist based, and atm switching / showing the playlist is a bit clumsy -- rec doesn't do anything at all atm.
- press rec (long) to enter the recording screen. Not to any importance to me as I seldomly use recording, but it would be nice, and I guess really useful for the users that use recording frequently.
I think this would make the button behavior even more consistent.

(the above does of course only apply on targets with a dedicated rec button, in my case h120)
Comment by Simon Menzel (Rincewind) - Tuesday, 13 June 2006, 03:13 GMT+1
@bluebrother: I suppose this should be a different task (or maybe two) as it is an addition to wps and not to the menu handling.
Anyway here is my patch for "press rec (short) enters current playlist:

(application/octet-stream)    rec_in_wps.patch (1.8 KiB) 
 d:/devel/apps/gui/gwps.h |    4 ++++
 d:/devel/apps/gui/gwps.c |   28 ++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

Comment by Dominik Riebeling (bluebrother) - Tuesday, 13 June 2006, 09:46 GMT+1
Ricewind: I think this is somewhat related to this task as I thought also on handling it from within the menu and file browser -- mainly having toggle buttons for the playlist, so mode toggles the menu, rec the playlist, play the wps. Moving and selecting is completely handled by the "joystick" (haven't looked on this for other targets than h100 yet).
But you're right, it doesn't only affect the menu screen so it isn't related only to this task. Maybe we need a new task "make button handling more consistent" if we want to cover all this ideas in a single task (or two different tasks ;-)
Comment by Robert Kukla (roolku) - Friday, 16 June 2006, 15:45 GMT+1
I only just noticed this patch and I love it. The only problem I have found with menu_buttons_NP20060612.patch so far is that if you enter a menu from the radio screen and press the play button there, it will bring you to the WPS as expected with audio resuming and radio stopping - however the peakmeter is not working and I haven't found a way to make it start work again without a reboot. Cheers, Roolku
Comment by Alistair Marshall (amar) - Tuesday, 18 July 2006, 18:20 GMT+1
OK as the freeze is over i have updated against CVS again and added the fade(o); a nd bookmark fuction calls that were missing. Please check on other other targets (particularly ONDIO still not shure if i got it correct here
   menu_buttons.patch (11.2 KiB) 
 apps/menu.c     |   64 ++++++++++++++++++++++++++++++++++++++++++++++----
 apps/menu.h     |   15 ++++++++++-
 apps/tree.c     |   71 +++++++++++++++++++++++++++++++++++++-------------------
 apps/gui/gwps.c |    3 +-
 4 files changed, 121 insertions(+), 32 deletions(-)

Comment by Michael DiFebbo (medifebbo) - Tuesday, 18 July 2006, 21:12 GMT+1
I've noticed one small bug. When in the menu system, the play button returns to the WPS, but it also pauses playback, which I believe is unintended. I have tested this on an iPod 5g using the most recent patch above. I have not yet had a chance to test on any other target.
Comment by Alistair Marshall (amar) - Tuesday, 18 July 2006, 21:20 GMT+1
This doesn't happen on my h300, so without an ipod to playwith, i can't think of any way to fix this.
Anyone?
Comment by Ryan Sawhill (ryran) - Wednesday, 19 July 2006, 03:58 GMT+1
Oh wow. I always wondered about that, but just assumed it was intended.. for some reason.
Comment by Michael DiFebbo (medifebbo) - Thursday, 20 July 2006, 14:24 GMT+1
I've tested this patch on Archos platforms on the sim, and it compiles and appears to work fine on all platforms except for the Archos Player, which breaks the compiler with the following error:

menu.c:209: error: `BUTTON_OFF' undeclared (first use in this function)
menu.c:209: error: (Each undeclared identifier is reported only once
menu.c:209: error: for each function it appears in.)


Comment by Michael DiFebbo (medifebbo) - Saturday, 22 July 2006, 16:39 GMT+1
This patch corrects the problem noted in my previous comment.

This patch now compiles on all platforms.
   menubutton.patch (11.6 KiB) 
 ./apps/menu.c     |   64 ++++++++++++++++++++++++++++++++++++++++++++----
 ./apps/menu.h     |   20 +++++++++++----
 ./apps/tree.c     |   71 +++++++++++++++++++++++++++++++++++-------------------
 ./apps/gui/gwps.c |    3 +-
 4 files changed, 123 insertions(+), 35 deletions(-)

Comment by Linus Nielsen Feltzing (linusnielsen) - Tuesday, 25 July 2006, 15:33 GMT+1
I liked this patch for a while, until you started adding more stuff than just the quick exit.
Comment by Simon Menzel (Rincewind) - Tuesday, 25 July 2006, 18:23 GMT+1
just a note: this patch makes it very hard to enter the id3-info screen from the wps (on irver with PLAY + A-B). you have to release both buttons at the exact same time, otherwise the screen is only shown for the time the buttons are pressed and is exited directly afterwards.
Comment by Dominik Riebeling (bluebrother) - Tuesday, 25 July 2006, 19:03 GMT+1
on my h120 I usually access the id3 screen via the wps context menu. I wasn't aware there is this button combination. Is this intended? Also, I have this "very hard" behaviour also without this patch -- when holding A-B too long the quick screen gets triggered sometimes, when holding play a bit too long it returns to wps. Also, sometimes the status bar (enabled, disabled in wps) doesn't show up correctly.
Accessing the id3 screen from the context menu (as I'm used to) works without any problems.
Comment by Simon Menzel (Rincewind) - Tuesday, 25 July 2006, 19:47 GMT+1
yes, this button combo is intended. I only discovered it by examining the gwps.c source, I didn't know it for a long time. But it is very handy to take a short look at your replaygain values.
If my problems aren't related to your patch, then I suppose I just had more shaky hands today than usual (I was using a build without your patch for some time up to yesterday).
Comment by David Rothenberger (drothenberger) - Thursday, 17 August 2006, 00:56 GMT+1
I updated the patch for CVS HEAD with all the new button changes. I modified the iriver h1x0-h3x0 keymaps so that
1. Main unit ON will start playback in the menu.
2. Main unit OFF will stop playback in the menu.

The updated keymap generates ACTION_WPS_STOP and ACTION_WPS_PLAY for OFF and ON. I modified menu.c to recognize those actions. I'm not sure whether this is appropriate or whether ACTIONs should have been added to action.h (e.g., ACTION_MENU_PLAY, ACTION_MENU_STOP).

I did not adjust any other keymaps.

NOTE: In CVS HEAD, there is no way to move up a level in the list tree using the remote. I submitted patch 5830 to correct that problem. I did not include that fix in this patch.
   menu_buttons.patch (12.6 KiB) 
 apps/tree.c                     |   81 +++++++++++++++++++++++++---------------
 apps/gui/gwps.c                 |    3 -
 apps/menu.c                     |   60 ++++++++++++++++++++++++++++-
 apps/menu.h                     |   20 +++++++--
 apps/keymaps/keymap-h1x0_h3x0.c |   10 ++++
 5 files changed, 135 insertions(+), 39 deletions(-)

Comment by David Rothenberger (drothenberger) - Monday, 21 August 2006, 19:07 GMT+1
Updated for CVS HEAD. Verified it builds for all targets and sim targets.
   menu_buttons.patch (12.6 KiB) 
 apps/tree.c                     |   81 +++++++++++++++++++++++++---------------
 apps/gui/gwps.c                 |    3 -
 apps/menu.c                     |   60 ++++++++++++++++++++++++++++-
 apps/menu.h                     |   20 +++++++--
 apps/keymaps/keymap-h1x0_h3x0.c |   10 ++++
 5 files changed, 135 insertions(+), 39 deletions(-)

Comment by Ryan Sawhill (ryran) - Sunday, 07 January 2007, 08:25 GMT+1
any chance someone more knowledgeable than me could fix this bad boy? it's pretty out of synch...
Comment by Alistair Marshall (amar) - Sunday, 07 January 2007, 11:40 GMT+1
I gave up trying to keep this synced when it became apparent that it was not going to be included in CVS. Currently my computer can't compile rockbox but this may give me a reason to try again if i have some spare time.
Comment by Simon Menzel (Rincewind) - Sunday, 07 January 2007, 14:58 GMT+1
I would also like to see an update of this useful patch!
The last time I used it I removed some code from the patch that wasn't needed with the new button action system but I don't remember if this was before the last update here or not. Unfortunately I don't have those sources anymore. If you don't get around to it in the next time I might update this patch (only if it is not very difficult and I need to have the time, too)
Comment by Norbert Preining (norbusan) - Monday, 08 January 2007, 16:20 GMT+1
Here is a sync against todays cvs. This is more or less the patch I am using in my build. Only compile time tested.
   menu_buttons_np_070108.diff (11.7 KiB) 
 ./apps/gui/gwps.c                 |    3 -
 ./apps/keymaps/keymap-h1x0_h3x0.c |   10 ++++
 ./apps/tree.c                     |   81 ++++++++++++++++++++++++--------------
 ./apps/menu.c                     |   60 ++++++++++++++++++++++++++--
 ./apps/menu.h                     |   15 +++++--
 5 files changed, 132 insertions(+), 37 deletions(-)

Comment by Brock Norman (mightybrick) - Thursday, 25 January 2007, 04:21 GMT+1
Hunk error with today's SVN. Hunk#1 failed at 509.
Comment by Jon (ace214) - Monday, 05 February 2007, 02:00 GMT+1
sync
(application/octet-stream)    05-menu_buttons_20070204_ace214.diff (11.7 KiB) 
 ./apps/gui/gwps.c                 |    3 -
 ./apps/keymaps/keymap-h1x0_h3x0.c |   10 ++++
 ./apps/tree.c                     |   81 ++++++++++++++++++++++++--------------
 ./apps/menu.c                     |   60 ++++++++++++++++++++++++++--
 ./apps/menu.h                     |   15 +++++--
 5 files changed, 132 insertions(+), 37 deletions(-)

Comment by Jon (ace214) - Monday, 05 February 2007, 03:32 GMT+1
sorry- typo. didn't compile. this one works.
(application/octet-stream)    menu_buttons_20070204_ace214.diff (11.7 KiB) 
 ./apps/gui/gwps.c                 |    3 -
 ./apps/keymaps/keymap-h1x0_h3x0.c |   10 ++++
 ./apps/tree.c                     |   81 ++++++++++++++++++++++++--------------
 ./apps/menu.c                     |   60 ++++++++++++++++++++++++++--
 ./apps/menu.h                     |   15 +++++--
 5 files changed, 132 insertions(+), 37 deletions(-)

Comment by Paul van der Heu (paulheu) - Saturday, 10 February 2007, 19:11 GMT+1
The patch is seriously broken against SVN right now..

Loading...