Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10698 - Cowon D2: Tactile button functions with hold switch on

Attached to Project: Rockbox
Opened by Andrew Beveridge (andrewthecoder) - Wednesday, 21 October 2009, 06:15 GMT+2
Task Type Patches
Category User Interface
Status Unconfirmed
Assigned To No-one
Player Type Cowon D2
Severity Low
Priority Normal
Reported Version Release 3.4
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Private No

Details

This has been (slightly) discussed before, but this is a new take on the matter, not a repeat of  FS#10683 .

With this patch applied, the following functionality is recognised with the hold switch in place:

On any list/menu screen or the while playing screen;
pressing MINUS decreases volume by one,
pressing PLUS increases volume by one,
pressing MENU switches between Play and Pause,
holding MINUS skips back a track,
holding PLUS skips forward a track,
holding MENU advances to the next folder.

I feel that anybody using a Cowon D2 will find this functionality essential, not only because it is present in the original firmware, but because it is necessary for in-pocket operation.
With a small DAP like this, with tactile buttons as well as a touch screen, it makes sense to maximize convenience.

I have added as much functionality as I could without requiring pressing multiple buttons at once. I have made several similar patches and built them for members on the iAudiophile forums, and this configuration has consistently been the most popular.

I welcome any comments/suggestions.
-Andrew
   tactile-buttons-in-hold-mode.patch (5.9 KiB) 
 apps/action.h                                        |   11 +++++-
 apps/gui/list.c                                      |   34 +++++++++++++++++++
 apps/keymaps/keymap-cowond2.c                        |   17 +++++++++
 firmware/export/config-cowond2.h                     |    5 ++
 firmware/target/arm/tcc780x/cowond2/button-cowond2.c |   17 +++++++--
 firmware/target/arm/tcc780x/cowond2/button-target.h  |    5 ++
 6 files changed, 85 insertions(+), 4 deletions(-)

Comment by Tomer Shalev (tomers) - Wednesday, 21 October 2009, 07:41 GMT+2
As an owner of D2 (without RB yet), I think this is a 'can't live without' feature.
Comment by Jonathan Gordon (jdgordon) - Wednesday, 21 October 2009, 07:49 GMT+2
this is an almost definite "not gonna happen" feature :) the hold button shouldnt be used as a modifier... it should be used to lock out input so you dont accidently track skip and stuff...
Comment by Rob Purchase (shotofadds) - Wednesday, 21 October 2009, 23:22 GMT+2
@JdGordon, the OF already has this feature on the D2, so users aren't going to be confused by this behaviour (and indeed will expect it). It especially won't be aproblem if it's behind a "enable track skipping on hold" menu setting, disabled by default. I cannot for the life of me see a problem with that.
Comment by Jonathan Gordon (jdgordon) - Wednesday, 21 October 2009, 23:54 GMT+2
lets move this discussion to the dev mailing list... My immediate answer to that point though is we dont try to emulate the OF on any targets, prime example is the ipods menu button works completly differently in the OF (although their usage is retarted...)
Comment by Rob Purchase (shotofadds) - Thursday, 22 October 2009, 00:43 GMT+2
This isn't simply as trying to emulate the OF's button usage - it's a useful feature in the OF, which Rockbox is capable of providing. I thought one of the Rockbox goals was to become a complete OF replacement?

But yes.. a dev ML thread is needed. Right now I need sleep, I'll kick it off tomorrow.
Comment by Dave Hooper (stripwax) - Thursday, 22 October 2009, 21:03 GMT+2
One possible way to do this, which might be globally useful for rockbox on other/all devices, would be to have an option that defines Hold switch behaviour:
1. Lock out all input
2. Lock out touchscreen only (on touchscreen devices) / lock out scrollwheel only (on scrollwheel devices) - keeping buttons enabled

Thoughts?
Comment by Koptelov Nikita (DaMaGeD) - Saturday, 24 October 2009, 16:02 GMT+2
@stripwax
There's a lot of work,but i think it's worth it.
Will post code here for menus,then would port it on other targets.
Comment by Koptelov Nikita (DaMaGeD) - Sunday, 25 October 2009, 10:03 GMT+2
Here's patch with added menus.
I'm used the HAVE_MUSIC_CONTROL_IN_LIST define to control the appearance of menu.
   d2-tactile-v2.patch (10.2 KiB) 
 apps/action.h                                        |   11 +++++-
 apps/gui/list.c                                      |   34 +++++++++++++++++++
 apps/keymaps/keymap-cowond2.c                        |   17 +++++++++
 apps/lang/english.lang                               |   19 ++++++++++
 apps/lang/russian.lang                               |   28 +++++++++++++++
 apps/menus/settings_menu.c                           |   12 ++++++
 apps/settings.h                                      |    3 +
 apps/settings_list.c                                 |    4 ++
 firmware/export/config-cowond2.h                     |    5 ++
 firmware/target/arm/tcc780x/cowond2/button-cowond2.c |   19 ++++++++--
 firmware/target/arm/tcc780x/cowond2/button-target.h  |    5 ++
 11 files changed, 152 insertions(+), 5 deletions(-)

Comment by Koptelov Nikita (DaMaGeD) - Monday, 26 October 2009, 14:25 GMT+2
I think it would be nice to port it on other devices and add setup for actions of buttons.
Comment by Rob Purchase (shotofadds) - Monday, 09 November 2009, 23:19 GMT+2
I've updated this patch with the following:

- rename the setting to "Playback Control in Hold Mode", and move it to the Playback Settings menu
- don't access global_settings directly from button-cowond2.c (this isn't allowed since settings are apps/ code and this is firmware/)
- add separate #defines for HAVE_PLAYBACK_CONTROL_IN_LIST and HAVE_BUTTONS_IN_HOLD_MODE
- add an entry to apps/features.txt, so that the lang entry is only built for targets with this feature
- rename a few variables

I intend to commit this once the following bug is fixed:

- the backlight sometimes behaves strangely (eg. fading in more than once) when turning hold off after having used buttons in hold mode. This does not happen when the option is disabled.
   playback_control_in_hold_v3.diff (11.2 KiB) 
 apps/action.h                                        |   11 ++++-
 apps/lang/english.lang                               |   17 ++++++++
 apps/lang/russian.lang                               |   17 ++++++++
 apps/gui/list.c                                      |   39 +++++++++++++++++++
 apps/settings.c                                      |    4 +
 apps/settings.h                                      |    3 +
 apps/menus/playback_menu.c                           |    7 +++
 apps/settings_list.c                                 |    5 ++
 apps/features.txt                                    |    4 +
 apps/keymaps/keymap-cowond2.c                        |   27 ++++++++++++-
 firmware/export/config-cowond2.h                     |    9 ++++
 firmware/target/arm/tcc780x/cowond2/button-cowond2.c |   27 +++++++++++--
 firmware/target/arm/tcc780x/cowond2/button-target.h  |   12 +++++
 docs/CREDITS                                         |    2 
 14 files changed, 179 insertions(+), 5 deletions(-)

Comment by Brian DIll (bkd11) - Tuesday, 17 November 2009, 21:50 GMT+2
I think the backlight issue you mentioned is related to FS#10362 because it only does this for me when the 'Backlight fade in' is enabled. And in fact it still does it with 'Buttons in Hold' function turned off, it just takes longer to happen. Try letting music play for a few hours to see. Either way it doesn't do it when 'Backlight fade in' is turned off.
Comment by Koptelov Nikita (DaMaGeD) - Saturday, 21 November 2009, 18:58 GMT+2
Found a bug- while pressing minus button more than approx. 1sec, track not skips back,but playback goes to start of current track.
Comment by Rob Purchase (shotofadds) - Monday, 23 November 2009, 22:05 GMT+2
@DaMaGeD, I think this is actually the correct behaviour. The "skip back" function will always jump to the beginning of the track, unless the current playback position is right at the start of the track.(within the first few seconds). It's the same as using the normal track skip function.

@bkd11, that certainly looks related, but I'd like to get to the bottom of it before committing this since this patch exaggerates the problem somewhat.
Comment by Nick Peskett (nickp) - Tuesday, 08 December 2009, 05:50 GMT+2
Synced to trunk revision 23894.
   playback_control_in_hold_v3_23894.diff (11.1 KiB) 
 docs/CREDITS                                         |    2 
 apps/action.h                                        |   11 ++++-
 apps/settings.c                                      |    4 +
 apps/lang/russian.lang                               |   17 ++++++++
 apps/lang/english.lang                               |   17 ++++++++
 apps/gui/list.c                                      |   39 +++++++++++++++++++
 apps/settings.h                                      |    3 +
 apps/menus/playback_menu.c                           |    7 +++
 apps/settings_list.c                                 |    5 ++
 apps/features.txt                                    |    4 +
 apps/keymaps/keymap-cowond2.c                        |   25 ++++++++++++
 firmware/export/config/cowond2.h                     |    9 ++++
 firmware/target/arm/tcc780x/cowond2/button-cowond2.c |   27 +++++++++++--
 firmware/target/arm/tcc780x/cowond2/button-target.h  |   12 +++++
 14 files changed, 178 insertions(+), 4 deletions(-)

Comment by Brian DIll (bkd11) - Wednesday, 16 December 2009, 22:49 GMT+2
Synced to revision 24036
   playback_control_in_hold_v3.diff (11.2 KiB) 
 docs/CREDITS                                         |    2 
 apps/action.h                                        |   11 ++++-
 apps/lang/russian.lang                               |   17 ++++++++
 apps/lang/english.lang                               |   17 ++++++++
 apps/settings.c                                      |    4 +
 apps/gui/list.c                                      |   39 +++++++++++++++++++
 apps/settings.h                                      |    3 +
 apps/menus/playback_menu.c                           |    7 +++
 apps/settings_list.c                                 |    5 ++
 apps/features.txt                                    |    4 +
 apps/keymaps/keymap-cowond2.c                        |   25 ++++++++++++
 firmware/export/config/cowond2.h                     |    9 ++++
 firmware/target/arm/tcc780x/cowond2/button-cowond2.c |   27 +++++++++++--
 firmware/target/arm/tcc780x/cowond2/button-target.h  |   12 +++++
 14 files changed, 178 insertions(+), 4 deletions(-)

Comment by Andrew Beveridge (andrewthecoder) - Sunday, 31 January 2010, 03:36 GMT+2
Just thought I'd add this as a comment here as food for thought.
pixelma made a suggestion in IRC yesterday which I suspect may be a good compromise to alleviate the controversy around committing this patch; http://www.rockbox.org/irc/log-20100129#07:32:02

I don't think I have the knowledge to implement a "special keymap mode" which would be triggered by holding down one of the tactile buttons while sliding the hold switch, but perhaps somebody does, or if this seems like a good idea to other people, I'm sure I could find the time to learn (s/learn/fiddle around and break things till it works/).

-Andrew
Comment by Rob Purchase (shotofadds) - Sunday, 31 January 2010, 12:59 GMT+2
We did actually reach a general consensus on the dev mailing list that no-one was going to veto this if I committed it as-is. I just haven't done it yet... but now we're out of feature freeze it might be a good time to do so.
Comment by Brian DIll (bkd11) - Wednesday, 10 February 2010, 20:56 GMT+2
I changed a few lines in the keymap-cowond2.c from:
{ ACTION_WPS_SKIPPREV, BUTTON_HOLDMINUS|BUTTON_REL, BUTTON_HOLDMINUS|BUTTON_REPEAT },
{ ACTION_WPS_SKIPNEXT, BUTTON_HOLDPLUS|BUTTON_REL, BUTTON_HOLDPLUS|BUTTON_REPEAT },
to:
{ ACTION_WPS_SKIPPREV, BUTTON_HOLDMINUS|BUTTON_REPEAT, BUTTON_HOLDMINUS },
{ ACTION_WPS_SKIPNEXT, BUTTON_HOLDPLUS|BUTTON_REPEAT, BUTTON_HOLDPLUS },
and
{ ACTION_LIST_SKIPPREV, BUTTON_HOLDMINUS|BUTTON_REL, BUTTON_HOLDMINUS|BUTTON_REPEAT },
{ ACTION_LIST_SKIPNEXT, BUTTON_HOLDPLUS|BUTTON_REL, BUTTON_HOLDPLUS|BUTTON_REPEAT },
to
{ ACTION_LIST_SKIPPREV, BUTTON_HOLDMINUS|BUTTON_REPEAT, BUTTON_HOLDMINUS },
{ ACTION_LIST_SKIPNEXT, BUTTON_HOLDPLUS|BUTTON_REPEAT, BUTTON_HOLDPLUS },

It seems to make it slightly more functional to not 'have to' release the button first to skip the tracks. ie. You can just hold + or - until it skips.

What do you think?
Comment by Brian DIll (bkd11) - Wednesday, 10 February 2010, 20:58 GMT+2
Here's the patch.
   playback_control_in_hold_v4.diff (11.2 KiB) 
 docs/CREDITS                                         |    2 
 apps/action.h                                        |   11 ++++-
 apps/lang/russian.lang                               |   17 ++++++++
 apps/lang/english.lang                               |   17 ++++++++
 apps/settings.c                                      |    4 +
 apps/gui/list.c                                      |   39 +++++++++++++++++++
 apps/settings.h                                      |    3 +
 apps/menus/playback_menu.c                           |    7 +++
 apps/settings_list.c                                 |    5 ++
 apps/features.txt                                    |    4 +
 apps/keymaps/keymap-cowond2.c                        |   25 ++++++++++++
 firmware/export/config/cowond2.h                     |    9 ++++
 firmware/target/arm/tcc780x/cowond2/button-cowond2.c |   27 +++++++++++--
 firmware/target/arm/tcc780x/cowond2/button-target.h  |   12 +++++
 14 files changed, 178 insertions(+), 4 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Wednesday, 10 February 2010, 21:02 GMT+2
Rob, this feature might actually be usefull for all other targets with a software hold switch, so can you wait for someone to do the version described in the irc log above instead? (i.e holding a button while enabling hold put it into a different mode wherre the buttons work)
Comment by Rob Purchase (shotofadds) - Wednesday, 10 February 2010, 22:05 GMT+2
@bkd11, that change looks sensible, yes.

@jdgordon, I like that suggestion a bit more having thought about it a 2nd time. Now all we need is someone to do the work.... ;-)
Comment by Jens Theeß (punkt) - Monday, 05 April 2010, 12:02 GMT+2
Synced to r25479.
   playback_control_in_hold_v4.diff (11 KiB) 
 apps/action.h                                        |   11 ++++-
 apps/lang/russian.lang                               |   17 ++++++++
 apps/lang/english.lang                               |   17 ++++++++
 apps/settings.c                                      |    4 +
 apps/gui/list.c                                      |   39 +++++++++++++++++++
 apps/settings.h                                      |    4 +
 apps/menus/playback_menu.c                           |    7 +++
 apps/settings_list.c                                 |    5 ++
 apps/features.txt                                    |    4 +
 apps/keymaps/keymap-cowond2.c                        |   25 ++++++++++++
 firmware/export/config/cowond2.h                     |    9 ++++
 firmware/target/arm/tcc780x/cowond2/button-cowond2.c |   27 +++++++++++--
 firmware/target/arm/tcc780x/cowond2/button-target.h  |   12 +++++
 13 files changed, 177 insertions(+), 4 deletions(-)

Comment by Tomer Shalev (tomers) - Monday, 05 April 2010, 13:36 GMT+2
> Synced to r25479.
Can you please update the manual accordingly? Thanks.
Comment by Ivan Goncharov (Dromok) - Tuesday, 20 April 2010, 22:50 GMT+2
In FS#11213 I post patch which implement mentioned alternative mode.
Comment by Nick Peskett (nickp) - Wednesday, 22 December 2010, 12:44 GMT+2
Synced to r28404 (3.7)
   playback_control_in_hold_v4_28404.patch (11 KiB) 
 apps/action.h                                        |   11 ++++-
 apps/lang/russian.lang                               |   17 ++++++++
 apps/lang/english.lang                               |   17 ++++++++
 apps/settings.c                                      |    4 +
 apps/gui/list.c                                      |   39 +++++++++++++++++++
 apps/settings.h                                      |    4 +
 apps/menus/playback_menu.c                           |    7 +++
 apps/settings_list.c                                 |    5 ++
 apps/features.txt                                    |    4 +
 apps/keymaps/keymap-cowond2.c                        |   25 ++++++++++++
 firmware/export/config/cowond2.h                     |    9 ++++
 firmware/target/arm/tcc780x/cowond2/button-cowond2.c |   27 +++++++++++--
 firmware/target/arm/tcc780x/cowond2/button-target.h  |   12 +++++
 13 files changed, 177 insertions(+), 4 deletions(-)

Comment by Andrew Beveridge (andrewthecoder) - Friday, 14 January 2011, 02:21 GMT+2
Not too sure what the current status regarding this is now, I vaguely remember a general consensus of "fine, well nobody will veto it if shotofadds commits it", but I'm not sure it matters too much now anyway - the D2 is now a discontinued product, and popularity of it is decreasing. The few people who took a strong interest in rockbox on it and use it regularly know about this patch and a fair number of them use builds supplied by my script here;
http://www.d2rockbox.co.uk/?a=patches

I'm posting that link here because this patch, FS#10698, is kept up-to-date there now - at least once a week it is synchronised with the latest rockbox revision and built for the D2. Thus, any D2 owner interested in this functionality will probably have more luck getting it there.
Comment by Sergey Petrushkin (psn84) - Friday, 07 December 2012, 13:14 GMT+2
Synced to current HEAD (how to know current revision?)
   tactile-buttons-in-hold-mode-20121207.patch (11.1 KiB) 
 b/apps/action.h                                        |    9 +++
 b/apps/features.txt                                    |    4 +
 b/apps/gui/list.c                                      |   39 +++++++++++++++++
 b/apps/keymaps/keymap-cowond2.c                        |   25 ++++++++++
 b/apps/lang/english.lang                               |   18 +++++++
 b/apps/lang/russian.lang                               |   17 +++++++
 b/apps/menus/playback_menu.c                           |    6 ++
 b/apps/settings.c                                      |    4 +
 b/apps/settings.h                                      |    4 +
 b/apps/settings_list.c                                 |    5 ++
 b/firmware/export/config/cowond2.h                     |    9 +++
 b/firmware/target/arm/tcc780x/cowond2/button-cowond2.c |   29 +++++++++++-
 b/firmware/target/arm/tcc780x/cowond2/button-target.h  |   13 +++++
 13 files changed, 179 insertions(+), 3 deletions(-)

Loading...