Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#9368 - Add settings tag to WPS

Attached to Project: Rockbox
Opened by Antoine Cellerier (dionoea) - Friday, 05 September 2008, 16:01 GMT+2
Last edited by Antoine Cellerier (dionoea) - Sunday, 07 December 2008, 17:21 GMT+2
Task Type Patches
Category Themes
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
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.
   wps_settings_tag.patch (2.8 KiB) 
 apps/gui/gwps-common.c |    7 +++++++
 apps/gui/gwps.h        |    5 ++++-
 apps/gui/wps_parser.c  |   36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 1 deletion(-)

   cfg_to_string.patch (4.7 KiB) 
 apps/settings.c |  101 ++++++++++++++++++++++++++++++--------------------------
 apps/settings.h |    1 
 2 files changed, 56 insertions(+), 46 deletions(-)

This task depends upon

Closed by  Antoine Cellerier (dionoea)
Sunday, 07 December 2008, 17:21 GMT+2
Reason for closing:  Accepted
Comment by PaulJam (PaulJam) - Friday, 05 September 2008, 17:18 GMT+2
Can this tag be used as a conditional?
Comment by Antoine Cellerier (dionoea) - Friday, 05 September 2008, 17:26 GMT+2
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, 19:44 GMT+2
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, 19:48 GMT+2
Great job !
Comment by Antoine Cellerier (dionoea) - Saturday, 06 September 2008, 00:52 GMT+2
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, 17:21 GMT+2
Updated WPS patch enables use as a conditional, prevents matching on partial setting names and adds documentation to the manual.
   wps_settings_tag-2.patch (5.8 KiB) 
 apps/gui/gwps-common.c       |   48 +++++++++++++++++++++++++++++++++++++++++++
 apps/gui/wps_parser.c        |   38 ++++++++++++++++++++++++++++++++++
 apps/gui/gwps.h              |    5 +++-
 manual/appendix/wps_tags.tex |    9 ++++++++
 4 files changed, 99 insertions(+), 1 deletion(-)

Comment by Antoine Cellerier (dionoea) - Saturday, 06 September 2008, 17:37 GMT+2
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|)
   wps_settings_tag-3.patch (5.8 KiB) 
 apps/gui/gwps-common.c       |   48 +++++++++++++++++++++++++++++++++++++++++++
 apps/gui/wps_parser.c        |   38 ++++++++++++++++++++++++++++++++++
 apps/gui/gwps.h              |    5 +++-
 manual/appendix/wps_tags.tex |    9 ++++++++
 4 files changed, 99 insertions(+), 1 deletion(-)

Comment by Alexander Levin (fml2) - Monday, 08 September 2008, 21:10 GMT+2
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, 21:27 GMT+2
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...