Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Community feature settings in main settings menu #59

Merged

Conversation

PaulFreund
Copy link
Collaborator

@PaulFreund PaulFreund commented Jun 15, 2023

In accordance with the requirement in PR #56 for Pull requests altering user facing behavior or have a massive impact in constant cycle load (see here) this Pull request aims to implement:

  • A new menu item "Community features" in main menu settings (Shift+Pressing Selection knob) above Firmware version
  • Once opened the user shall be presented with a list of all community features that can be enabled/disabled or selected (in case features are not compatible with each other, for example Row style vs Column style clip launching)
  • It shall be very easy and worry free for Pull requests to implement new settings, there will be a module global instance of the settings that can be accessed through it's header file in all other objects
  • The settings shall be stored as an XML file on the SD card instead of the flash for easy access in case manual intervention is required (e.g. testing a beta firmware and going back to an official release without a certain feature).
  • The XML file shall be upward and downward compatible as much as possible. For this reason all settings unknown to the firmware will persist reading and writing a new config file.

As always all feedback is welcome!

Usage for feature devlopers currently looks like this:

  • To add a setting add a new block in RuntimeFeatureSettings.h along with new enum defines for the possible values:
[RuntimeFeatureSettingType::FileFolderSorting] =  { 
    .displayName = "File/Folder sorting",
    .xmlName = "fileFolderSorting",
    .value = RuntimeFeatureStateToggle::Off, // Default value
    .options = { 
        { .displayName = "Off", .value = RuntimeFeatureStateToggle::Off }, 
        { .displayName = "On", .value = RuntimeFeatureStateToggle::On },
        { .displayName = NULL, .value = 0 }
    }
},

The flags can be easily checked anywhere in the code by including RuntimeFeatureSettings.h and adding a condition like this:

runtimeFeatureSettings.get(RuntimeFeatureSettingType::FileFolderSorting) == RuntimeFeatureStateToggle::On

Example file:

<?xml version="1.0" encoding="UTF-8"?>
<runtimeFeatureSettings
	firmwareVersion="4.1.4-alpha"
	earliestCompatibleFirmware="4.1.3">
	<setting name="settingA" value="1"></setting>
	<setting name="settingB" value="2"></setting>
	<setting name="settingC" value="0"></setting>
	<setting name="settingZ" value="9"></setting>
</runtimeFeatureSettings>

@litui litui added the enhancement New feature or request label Jun 20, 2023
@PaulFreund PaulFreund changed the title [Draft] Community feature settings in main settings menu Community feature settings in main settings menu Jun 20, 2023
@PaulFreund PaulFreund marked this pull request as ready for review June 20, 2023 20:05
@jamiefaye
Copy link
Collaborator

Testing on a 7SEG here. When I click on Comm, I see an L and nothing else. If I twist the select knob, it will show SONG. Am I seeing the right thing?

@PaulFreund
Copy link
Collaborator Author

PaulFreund commented Jun 22, 2023

Hi @jamiefaye, I have to admit I didn't test the menu empty on 7seg (@sichtbeton was so nice to test it for me with three example settings, video in discord). Can you please uncomment the example block in RuntimeFeatureSettings.h as can be seen in the description? Since there are no settings yet I left it out. Edit: Sorry and the referenced enum value FileFolderSorting above.
Edit2: And of course you can define additional ones for testing If you want. Number of options is also dynamic

@sichtbeton
Copy link
Contributor

Any build i can test?

@jamiefaye
Copy link
Collaborator

Please update your MR as needed. Then I won't mess things up by uncommenting stuff and losing sync.

@PaulFreund
Copy link
Collaborator Author

PaulFreund commented Jun 23, 2023

The menu only makes sense as soon as other developers add new features so right now it is intentionally empty. For testing to see an entry and check XML generation and parsing I left an entry as a comment but that should not be in the merged code.

Since I expected the menu to ship only after we have runtime options like the new sorting behavior I didn't test it empty. Do you want me to add code to not display it if the list is empty?

PS: Thank you @sichtbeton, I will ping you on Discord if the need arises again 💯

@PaulFreund
Copy link
Collaborator Author

PaulFreund commented Jun 26, 2023

From my side this PR is still ready to merge. Here is a link to the binary I created with some mock options as can be seen in the video above: https://discord.com/channels/608916579421257728/1107026299945496577/1120692404031340594. I'm 90% sure it goes back to SONG because there is no entry in the menu.

As soon as the first feature is merged like PR #66 with an option to enable/disable the menu starts to make sense and give value. It's a hen and egg type situation :)

@sichtbeton
Copy link
Contributor

this is great! so now feature developers could use this option mechanism?

@jamiefaye jamiefaye merged commit 203b128 into SynthstromAudible:community Jun 27, 2023
@m-m-adams m-m-adams mentioned this pull request Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants