Rockbox

Tasklist

FS#9368 - Add settings tag to WPS

Attached to Project: Rockbox
Opened by Antoine Cellerier (dionoea) - Friday, 05 September 2008, 14:01 GMT
Last edited by Antoine Cellerier (dionoea) - Sunday, 07 December 2008, 16:21 GMT
Task Type Patches
Category Themes
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

wps_settings.patch adds a %St|<some setting name>| tag that can be used to display the value of any rockbox setting in the WPS.

cfg_to_string.patch adds a new function called cfg_to_string and exports it.
This task depends upon

Closed by  Antoine Cellerier (dionoea)
Sunday, 07 December 2008, 16:21 GMT
Reason for closing:  Accepted
Comment by PaulJam (PaulJam) - Friday, 05 September 2008, 15:18 GMT
Can this tag be used as a conditional?
Comment by Antoine Cellerier (dionoea) - Friday, 05 September 2008, 15:26 GMT
I don't have a clue. This is the first time I've looked at the WPS code. Is there some specific code to add to be conditional compliant?
Comment by Alexander Levin (fml2) - Friday, 05 September 2008, 17:44 GMT
I think, as it is now, it can't be used as a conditional switcher. In order to do this, you also have to write the value of the setting to the address pointed to by intval. Compare e.g. WPS_TOKEN_REPEAT_MODE. The value written to the string buffer is used when the tag is used 'standalone,' and the int value written to (*intval) is used as the branch number of the conditional.
Comment by Clément Pit--Claudel (CFP) - Friday, 05 September 2008, 17:48 GMT
Great job !
Comment by Antoine Cellerier (dionoea) - Friday, 05 September 2008, 22:52 GMT
So, basically I should put "setting value as an int" + 1 in the *initval variable? (which would be equivalent to setting value position in the settings menu)

I also had a question about the WPS_REFRESH_ setting. I'm currently using WPS_REFRESH_STATIC. When is that refreshed? on track change only? Should I switch to WPS_REFRESH_DYNAMIC? or something else?
Comment by Antoine Cellerier (dionoea) - Saturday, 06 September 2008, 15:21 GMT
Updated WPS patch enables use as a conditional, prevents matching on partial setting names and adds documentation to the manual.
Comment by Antoine Cellerier (dionoea) - Saturday, 06 September 2008, 15:37 GMT
Changed to WPS_REFRESH_DYNAMIC. This fixes updating the value of some settings which can be changed without quiting the WPS screen. (For example %St|volume|)
Comment by Alexander Levin (fml2) - Monday, 08 September 2008, 19:10 GMT
While this tag looks nice and useful at a first glance, it also has a drawback IMO: it exposes the internal setting names to the user. So they need to be documented. Also, if a setting's name is changed (unlikely but possible), WPS's using it must be changed.

I also have an idea: in the WPS debug mode (or always?), if an unknown name is used as the parameter in the tag, we could display e.g. ??<bad_name>?? (<bad_name> is the value specified) instead of returning INVALID_PARAM.
Comment by Antoine Cellerier (dionoea) - Monday, 08 September 2008, 19:27 GMT
Setting names are documented (in an appendix in the documentation). They're also pretty stable as any changes to a setting name would break the config loading. (well it would disregard the old value).

I'll try to implement the debug mode idea.

Loading...