configuration system #220
Merged
configuration system #220
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
My initial first pass is really positive! I think this is a great step in providing the optional ability to add per-user configurations. I'll give a more in-depth review later. Nice work. 👍 |
awonak
approved these changes
Feb 10, 2023
|
LGTM, I'll leave the merge up to you when you're ready. |
- introduce ConfigPoints and ConfigPointsBuilder classes - make configurable the value that the TM write switch writes - add config point for pico board type
fall back to defaults properly
add int config point add tests for different config point failures make the TM pulses bits configurable move europi_config.py to the firmware diagnostic temp units are configurable also black format diagnostic
- removed builder - simplified EuroPiScript.config_point() - introduced ConfigPoint class - rename config_points.py -> configuration.py - move configPoint helpers out of ConfigSpec class - move europi config to its own var, this removes the need care about conflicts and namespacing - only load config points that exist in the spec - take a range object for IntegerConfigPoint instead of recreating its api - europi_script validates config when loading it - move generic file utils out of europi_script - moved config file methods out of EuroPiScript
add config point for cpu freq
add make target to copy config files
mjaskula
force-pushed
the
mjj/config_script
branch
from
3989d15
to
f8ddba5
Compare
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a rework of (closed) PR #185 and attempts to address the problems identified in issue #163 by way of the addition of a configuration system to the europi firmware. As mentioned in the comments of #185, we have pushed the actual UI portion of this idea out to later work.
This PR adds a configuration system and API, but not a UI, as described in issue #163. The intention of this configuration system is to allow for config file based (rather than in code or UI) user editing parameters that aren't expected to change very often. These are 'set and forget' type options that the user would not expect to be able to change while the script is running. This allows the script itself to forgo the UI complexities of making these options editable. Similarly, it also avoids asking users to edit and re-deploy code to make a change to a setting.
This PR consists of a few parts:
configurationmodule which contains code for:ConfigPoint)ConfigSpec)ConfigPoint)EuroPiScriptto make configuration available to running scriptsAdditional notable changes to support this work:
Future work: