FS#5833 - Display/Edit Settings Within Menus
Attached to Project:
Rockbox
Opened by Andreas Mattsson (AM) - Thursday, 17 August 2006, 16:18 GMT
Last edited by Jonathan Gordon (jdgordon) - Wednesday, 01 August 2007, 14:09 GMT
Opened by Andreas Mattsson (AM) - Thursday, 17 August 2006, 16:18 GMT
Last edited by Jonathan Gordon (jdgordon) - Wednesday, 01 August 2007, 14:09 GMT
|
DetailsAllows values for settings to be viewed and modified from directly within the menus (you don't have to select them to display them on a separate page).
Activated via General Settings -> Display -> Settings Display. So far only the aforementioned menu, plus Playback Settings and its submenus, have been changed to allow for this. I thought I'd await comments/suggestions/testing before I go ahead and convert all settings menus. Apply patch with -p0 |
This task depends upon
Closed by Jonathan Gordon (jdgordon)
Wednesday, 01 August 2007, 14:09 GMT
Reason for closing: Out of Date
Wednesday, 01 August 2007, 14:09 GMT
Reason for closing: Out of Date
1) I would like a better way to edit the settings inline, especially when the line gets so long that it has to scroll.
2) I think it would be nicer to have two lines per setting, the first line with the left-adjusted title, and the second line with the right-adjusted value.
Linus:
1) So would I, but can't really think of a better way to do it... :) Other than perhaps that it would be nice if the line started scrolling in its rightmost position when in edit mode, but I'm unsure how to do this. (I'm a little wary of messing with the scrolling code, it's a bit daunting the way it seems to have multiple implementations in separate lcd-*.c files)
2) Might be a good idea, but a problem with that is I believe the way lists work right now yo'u cant have differing amounts of lines for different menu options.
- how about the following idea for inline editing:
o on the highlighted entry "select" or "right" enters edit mode
o when in edit mode, "right" also changes the values. "select" leaves the edit mode, "stop" cancels, possibly also use "left" for changing. Not sure if this is a good idea though ...
(of course this will only work on target with enough buttons)
Apart from that I think this makes the settings menus less confusing.
The up and down do seem inverted don't they? At least on integer selection settings. However I'm not really sure it's a good idea to have separate button handling depending on whether selection is done inline or on a separate page. This doesn't really feel like an issue specific to this patch, perhaps the order of integers settings should reversed for both inline and list selection...?
I'm not sure how to handle this best but completely reversing the order in both view will be strange the other way round.
* Removed the 1px right margin for now, probably won't add it again until I've figured out a better way to do this.
* Added a Boolean Quick Toggle option: when enabled it's enough to hit select or right in the menu to toggle a boolean setting.
* Converted all of General Settings (well almost all, anything still left as ordinary menus was done so intentionally)
TODO now: Lots of testing on different targets...
Menu conversion is now complete.
* Added code and macros to allow for display of custom (descriptive?) strings on the second line in multiline mode. Nothing I'd want or plan to do myself, just making things easier for anyone who may want to do this in the future - after this has been included in CVS (*crosses fingers*).
I just applied the latest patch and found some menu entries missing (target is h120):
- General Settings -> System -> Idle Poweroff
- General Settings -> System -> Sleep Timer
- General Settings -> System -> Car Adapter Mode
- General Settings -> Display -> Default Codepage
Also, General Settings and Display doesn't use the multiline view in the top level -- to have it consistent that menus (even if they only contain subentries) should use that multiline view with the extension mark ">" too IMO.
Also, extending this view (I really like the multiline view) up to the main menu level would be nice I guess because this would make it more consistent.
However, I have still not changed any menus that do not directly contain any settings - that's not what this patch was intended for (if it was my intent to replace all menus I'd probably have structured my code in a different manner).
I tried it with the simulator and it works
but not on my x5 player
I need to change your patch a little bit to make it compile even for the
simulator.
The problem is the following:
...
CC codeclib.c
AR+RANLIB /home/maxl/workspace/rockbox/build-uisimulator/libcodec.a
make[2]: *** No rule to make target `button-target.h', needed by `/home/maxl/workspace/rockbox/build-uisimulator/apps/codecs/vorbis.o'. Stop.
I think the problem is your #include "select.h" in settings.h
this ends up in including button-target.h but bulding the codecs
does not include the folder where button-target.h is located.
So the dependencies are unresolved
When changing the includes I can make it compile
Another general question:
Whatever I change in "Settings Display" the "result" is always the same
means: I can edit the values for a single setting with the keys without a separate page
What is the difference of the four values of "Display Values In Menus"?
Also the meaning of "Show Edit Indicator" and "Change Settings In Menu" is unclear to me
and I dont see any difference changing the values for any of them
This is for the simulator
I can see all the new settings under "Display" and can change
the values like in the simulator.
But the menu behaviour is not changing and still the
old one (using a separate page)
scrobbler patch working. There seems to be some problem
of memory corruption.
When adding a new variable at the end of global_settings
it will get corrupt at some point. Adding the variable
"higher" in the struct and everything works.
When debugging I noticed that some variables of the
struct got overwritten and changed e.g. from true to false :-(
But this kind problems really makes me nervous :-(
If it is really some problem of memory corruption
we may end in big trouble sooner or later
BTW: I noticed that the location of the new file in the SOURCES file
makes also a difference sometime :-)
You may try that too
It seems the earlier fix for the X5 also fixed the Toshiba GigaBeat sim build as well. I now believe this patch no longer breaks anything.
And now I also see the differences between the options
that I asked before
In theory, adding settings to the end of the settings struct should be the right way to go, otherwise you have to increase the settings version counter. If you apply an updated patch and run the new rockbox.iriver when there was a patched binary before, it is always a good idea to clear the settings at boot (holding rec after bootloader). But if you increase the version counter in your patch there can be serious problems if it gets increased by a cvs update, because then rockbox won't recognize it (would be the same number) and possibly uses values in wrong places. CVS should show a conflict in this case, though.
They don't give conflict directly, I suppose it should be possible to figure out where the problem is. If I have my own patch ready again I try this a second time, because I like both patches very much.
Unfortunatly for us, Jonathan Gordon is going to do a complete rework of the menu code (see the mailing list), this will break all patches again in a few weeks.
I am also creating builds for the people from the iaudiophile.net forums
So there will be a lot of testers for this patch :-)
I am also creating builds for the people from the iaudiophile.net forums
including this patch together with some others (e.g. album_art ...)
So there will be a lot of testers for this patch :-)
Anyhow, I won't be able to work on this for a couple of days (off to Dublin over the weekend). Eager to check up on testing evaluations when I get back.
One footnote: Don't know if this could be fixed in your patch or even if the problem lies there; when changing the volume in the menu you can blast you ears out when you reach the bottom and click trough going right to maximal output!
Anyway great patch!
during the last days on x5
short answer: no problems so far! :-)