add supervisor.get_setting(); change os.getenv() behavior#10819
Open
dhalbert wants to merge 1 commit intoadafruit:mainfrom
Open
add supervisor.get_setting(); change os.getenv() behavior#10819dhalbert wants to merge 1 commit intoadafruit:mainfrom
dhalbert wants to merge 1 commit intoadafruit:mainfrom
Conversation
987d9d5 to
4f9929b
Compare
…etch typed objects
4f9929b to
4b2111e
Compare
This file contains hidden or 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
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.
See #9113 for the motivation for this PR.
os.getenv(key, default=None)now always returns a string. It never raises an error. It will return whatever is on the right hand side ofkey = <value>insettings.tomlafter removing leading and trailing whitepsace. The value is returnedeven if that value is not valid TOML, such as an unquoted string. If the key cannot be found or there is a pathological problem (bad Unicode escape, etc.), it will return thedefaultvalue.supervisor.get_setting(key, default=None)is added. It parseskey = <value>. The value must be one these kinds of valid TOML values:str.int.trueorfalse, returned as the Python booleanTrueorFalse.ValueError. Ifkeyis not present, then the value ofdefaultis returned.Internally, these changes were made:
shared-module/os/getenv.cwas moved tosupervisor/shared/settings.c, and was slightly simplified. The routines were renamed fromcommon_hal_os_getenv_*tosettings_*. The error reporting is simpler and doesn't bother to report the particular offending bad character. Errors that occur when trying to read, say,CIRCUITPY_WIFI_SSIDare still printed in the REPL stream for the user's benefit.CIRCUITPY_OS_GETENVis nowCIRCUITPY_SETTINGS_TOML.os.getenv()in theunixmicropythonvariant=COVERAGEbuild is now simpler and smaller. The test was updated to handle the new behavior ofos.getenv().The original code is now slightly smaller, but adding
supervisor.get_setting()added about 260 bytes.The original version of this code was developed using Copilot CLI using the GPT-5.3-Codex model and some fairly specific prompts about what I wanted. The code worked, but I then had further ideas about how to structure the code and do some cleanup, and did most of that by hand. I also reworked the documentation Codex wrote. Some refactoring and renaming was left to Copilot. I reviewed all the new diffs and new code.
Tested on Metro M4 and Metro ESP32-S3. Automatic WiFi access point connection worked as expected with
settings.tomlvalues as before.supervisor.get_setting()was tested manually.Aside: I'd like to have written automated tests for
supervisor.get_setting(), but that's not possible with the current UNIX micropython executable. The POSIX port (#9581) would be a move in that direction, as would, say an ARM executable on qemue or something like rp2040js/wokwi.