• Status Closed
  • Percent Complete
  • Task Type Patches
  • Category Settings
  • Assigned To No-one
  • Operating System All players
  • Severity Medium
  • Priority Very Low
  • Reported Version
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by jdgordon - 2006-11-30

FS#6399 - Menu/Settings recoding

OK, I think I’m finally happy with this version. Apart from the EQ menu all the settings menus should be using the new system and woring correctly.
The only tihng I’m not sure if its working or not is the RTC settings code, I dont have a target that uses this so I cant test.

How does it look?

Closed by  jdgordon
2007-01-22 04:09
Reason for closing:  Rejected
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

started again, so closing

The simulator build is still broken by this patch, under Cygwin at least:

… CC stubs.c
In file included from /home/Steve/rockbox-devel/apps/menu.h:25,

               from stubs.c:27:

./sound.h:36:5: warning: #warning “No sound yet in win32” In file included from /home/Steve/rockbox-devel/apps/menu.h:25,

               from stubs.c:27:

./sound.h:42: error: parse error before ‘*’ token
./sound.h:43: error: parse error before ‘*’ token
./sound.h:44: error: parse error before ‘*’ token
./sound.h:45: error: parse error before ‘*’ token
In file included from /home/Steve/rockbox-devel/apps/menus/exported_menus.h:25,

               from /home/Steve/rockbox-devel/apps/menu.h:26,
               from stubs.c:27:

/home/Steve/rockbox-devel/apps/playlist.h:24:18: file.h: No such file or directory
In file included from /home/Steve/rockbox-devel/apps/playlist.h:26,

               from /home/Steve/rockbox-devel/apps/menus/exported_menus.h:25,
               from /home/Steve/rockbox-devel/apps/menu.h:26,
               from stubs.c:27:

/home/Steve/rockbox-devel/firmware/export/id3.h:24:18: file.h: No such file or directory
In file included from /home/Steve/rockbox-devel/apps/playlist.h:26,

               from /home/Steve/rockbox-devel/apps/menus/exported_menus.h:25,
               from /home/Steve/rockbox-devel/apps/menu.h:26,
               from stubs.c:27:

/home/Steve/rockbox-devel/firmware/export/id3.h:136: error: `MAX_PATH’ undeclared here (not in a function)
make[1]: *** [/home/Steve/rockbox-devel/build/comsim/stubs.o] Error 1

Ah, the normal H300 build also fails here:
… CC alarm_menu.c
CC abrepeat.c
In file included from abrepeat.h:32,

               from abrepeat.c:20:

settings.h:31:27: settings_list.h: No such file or directory
In file included from abrepeat.h:32,

               from abrepeat.c:20:

settings.h:579: warning: “struct settings_list” declared inside parameter list
settings.h:579: warning: its scope is only this definition or declaration, which is probably not what you want
make[1]: *** [/home/Steve/rockbox-devel/build/apps/abrepeat.o] Error 1

Maybe it’s just me…?

damn, ok, new patch which shuold be fine

fix the sim build

That looks pretty good. The only problem I get is that the sim doesn’t load the configured WPS when I restart. This is probably because the WPS is saved in a relative, not absolute format now and no longer has an extension.
OLD: wps: /.rockbox/wps/ipodColor.wps
NEW: wps: ipodColor

Even if this isn’t the problem here, it might be a good idea to add file extensions if they’re needed to maintain cross-compatibility of .CFG files.

? I thought the file locations were stored in the short form… (unless it is on the target and long form i the sim?)

new version,
Due to discussion on irc today the full path is no stored in RAM for all the files.
also fixed contrast settings. Adds flags to allow settinigs to have functions which handle the max/min/default values instead of hard coding them.
other minor fixups. (the sim seems to crash, but target works fine, will hunt this down tomorrow)

OK, sim doesn’t crash for me, but still doesn’t load the WPS.

On the H340 (and pixelma’s Ondio), the playback track and position isn’t saved during rebuffering (e.g. if the reset button is pressed). Apparently it used to be.
And I’m seeing CPU boost problems again, but haven’t compared it to an unpatched CVS build yet, so don’t panic!

I just “upgraded” and test now a clean CVS build as of Sunday (before Slasheri’s commit) against a patched one from the same source (changes.patch from the 2nd)… - I can confirm now that the settings and playback data not being saved during rebuffering must have something to do with the patch - CVS works fine
- the font browser problem (that I reported in IRC) isn’t solved yet - the menu stays and the fonts list doesn’t show
- during the upgrade most of the settings were loaded correctly except fonts, wps, language, fmpresets - but I compared the old(er) config.cfg with the new one, and it now saved the full path (and looks like the one that I use for restoring the settings manually) so I expect it to be working next time. Just one small thing though… it took me a while to find those settings at the end of the list (as they are at the beginning ATM and I believe that’s the better place). I don’t care much but I guess lots of theme designers do.
- (just to have it in written) I wanted to remind you of the odd “lcd inverse” setting which leads to a change of the line selector.

thanks, noted.

An observation on my Ondio which hopefully could help tracking down the boost problem on swcodec targets:
for example loading a new wps through “browse wps-files” reports memory activity (in the statusbar) even after it has finished (splash disappeared and taken back to the display settings menu). It stays this way as long as I don’t do anything else.

Ok, new patch.
- fixes the font browser bug.
- maybne fixes the lcd invert.
I’m hoping that the settins not saving correctly is due to bugged ata_idle_notify code which was fixed last week (let me know of course if it still is buggered).

Now, to get this commited the following needs to happen (really appreciate help with these)
each setting in global_setting needs to be checked for the following
(1) it exsits in setting_list.c
(2) is in the correct #if(n)def block
(3) has the correct .cfg string attached
(4) if the settign needs a callback, then it has the correct one attached.
(5) is in the menu at the correct place in the correct #if(n)def block

sounds like fun aye?
The weather is apparently going to be really bad this week so ill try to get this done for all ~200 settings ASAP so we can call this finished.
I’m starting from the top and working down the list… help very much appreciated. (just make sure you say whch you have checked and what needs fixing)

the first of possibly a few updates today.
- moved the filename settings to the top of the setting list so they are easy to find in the .cfg
- add the mdb_* settings menu.
- the above list is done for volume → crossfade_fade_out_mixmode (inclusive) settings in settings.h
- put recording_menu.c in a HAVE_RECORDING ifdef.

update #2 for today,
fixed the option callbacks not working with bool options.
fixed the status bar not redrawing in the menu (caused the hdd activity light apparently staying on until something happens problem reported above)
JhMikeS did the recording settings so I am assuming they are correct.
so, all settings up to repeat_mode are setup correctly. should all be working correctly also..

RE: resume info not saving correctly.
There should be no change between cvs and this patch. both call the settings_save() function (which causes a config flush) right before a disk spin up. Also, even if config isnt saved often enough, it is always saved on a clean shutdown, so i’m inclined to ignore this “problem”.

voice for some reason isnt working for some reaso so i’m trying to hunt that down now.

The patch needs another resync with pcm_record.c, I’m afraid.

Yay! found the problem.
Apparently the way settings are currently being saved is more hacky than would be liked (good thing I asked about this 2 months ago on the mailing list….)

Anywho, this version has a temporary fix, shhuold work on all targets. (except ondio sim builds… but that hopefully is no big deal, and will be fixed properly shortly.

You’re missing settings_list.h, at least. I get:
CC abrepeat.c
In file included from abrepeat.h:32,

               from abrepeat.c:20:

settings.h:31:27: settings_list.h: No such file or directory

damn, woops, i got a new cvs tree and forgot to do cvsdos add on the 2 settings_list files… here is the proper patch (yet again :p)

ok, synced with current cvs, checked the patch and hopefullly it applies cleanly.
still dont know why voice doesnt work.

settins flushing is now done in the ata thread which simplifies things a bit

Nitpicking maybe, but the sim still doesn’t load the WPS upon initial start up. However the correct data is stored in config.cfg, and if I load config.cfg manually, the correct WPS is loaded. Otherwise this looks good (so far!).

Playlist position and settings changes get written successfully during playback -
one thing I noticed though is that writing these takes a noticeable longer time than in cvs (guess it’s because of it being a text file now).
I’m speaking about 3 seconds versus less than 1 second in CVS (watching the virtual led on Ondio as it is done some seconds later than the actual rebuffering).
Other than voice not working I couldn’t find any severe problems so far - I will go on testing more and more settings.
Still there is the odd “LCD mode inverse/Selector type” behaviour which isn’t very important and could be ironed out later but would be nice if it was fixed before.


Available keyboard shortcuts


Task Details

Task Editing