Rockbox

Tasklist

FS#7980 - Select default option in an option list

Attached to Project: Rockbox
Opened by Stephane Doyon (sdoyon) - Wednesday, 17 October 2007, 04:57 GMT
Last edited by Stephane Doyon (sdoyon) - Sunday, 20 April 2008, 14:13 GMT
Task Type Patches
Category Settings
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

This patch adds an action to the option_screen to cause the default value
for a particular setting to become the current selection. It allows you
to see which value is the default, and at that point you can confirm it
or not. The code to do this is all in there, it just needs to be moved to
separate functions so we can reuse it.

Particularly useful for settings with long ranges of options, like a lot
of the sound settings such as base and treble for example. I'm blind, and
I find it takes a lot of button'ing around to put the setting back to
zero. It's also great for when music is playing and the sound menus are
not spoken. And as a bonus, it lets one see what the safe default option
is supposed to be when browsing an obscure option.

Do others think this is useful?

The only tough part is the button assignment of course. This patch has
button mappings for x5 and e200 only, and at this point I've just added
them to the list context, but perhaps a separate context could be
created. For X5 I chose SELECT+POWER. (I thought the awkwardness wasn't
a big issue in this case.) For e200 it's POWER+SELECT.
This task depends upon

Closed by  Stephane Doyon (sdoyon)
Sunday, 20 April 2008, 14:13 GMT
Reason for closing:  Accepted
Comment by Jonathan Gordon (jdgordon) - Wednesday, 17 October 2007, 07:11 GMT
I like the idea, but not really sure how useful it would be.

also, instead of adding a new action, I tinhk ACTION_STD_CONTEXT would work fine in the option screen for reset as it shouldnt be used currently anyway.
Comment by Daniel Dalton (ddalton) - Wednesday, 17 October 2007, 09:36 GMT
Sdoyon:
I know you prefer me to not modify your patches.

But is it ok that I added a keymap for h300(and h100 I think)?
Sorry if you didn't want this.
Hopefully I didn't do anything wrong.
Anyway the keymap for h300 is play and rec.

Not the easiest key press but should work for this.
Here it is...
Comment by Stephane Doyon (sdoyon) - Wednesday, 17 October 2007, 12:38 GMT
>also, instead of adding a new action, I tinhk ACTION_STD_CONTEXT would work
>fine in the option screen for reset as it shouldnt be used currently anyway.

Oh! Don't know how I missed that. Well then that makes the patch
really simple!
Comment by Stephane Doyon (sdoyon) - Thursday, 18 October 2007, 01:40 GMT
Version using ACTION_STD_CONTEXT to trigger the reset, so no need
to modify keymaps.
Comment by Daniel Dalton (ddalton) - Thursday, 18 October 2007, 06:24 GMT
Cool. I will try it out. I used it last might on my player and it was great.
Thanks.
BTW Were my changes ok? Or was the indenting and stuff wrong. I know you have a better way now but just wanting feedback mainly on what I did wrong and can improve on.
I know I only added one line.
Thx.
Comment by Stephane Doyon (sdoyon) - Thursday, 18 October 2007, 12:43 GMT
>BTW Were my changes ok? Or was the indenting and stuff wrong.

Well... you had an extra space between ACTION_SETTINGS_RESET
and BUTTON_ON|BUTTON_REC. They normally have the columns aligned in a
nice table format. I know that's hard to get right with speech though.
Comment by Stephane Doyon (sdoyon) - Wednesday, 28 November 2007, 04:16 GMT
Sync.

Any objections to committing this?
Comment by Stephane Doyon (sdoyon) - Wednesday, 28 November 2007, 04:19 GMT
Oops, That was an old version. Here it is. Sorry.
Comment by Stephane Doyon (sdoyon) - Thursday, 17 April 2008, 01:15 GMT
I still find this somewhat useful. So here's a sync, and a commit
warning: unless there are objections, I'll commit this in a couple of days.
Comment by Jonathan Gordon (jdgordon) - Thursday, 17 April 2008, 01:36 GMT
looks ok to me..
except, in setting_reset() the string resseting is wrong.. it should be using the length struct member, not MAX_FILENAME (although, admitadly thats my mistake to being with.. but please fix in the commit)
Comment by Stephane Doyon (sdoyon) - Thursday, 17 April 2008, 03:11 GMT
Sure. Like this?
Comment by Linus Nielsen Feltzing (linusnielsen) - Thursday, 17 April 2008, 06:16 GMT
Looks OK to me.
Comment by Steve Bavin (pondlife) - Thursday, 17 April 2008, 09:48 GMT
And me.

Loading...