Expand ${VAR} env references in obelisk config values - #172
Merged
Conversation
load_config_file previously passed every YAML string through verbatim,
forcing downstream configs (e.g. legged_locomotion_rl's nav/mpc configs)
to hard-code absolute paths like /home/<user>/.../nav_graphs.
Add _expand_env_vars, run per-file in _load_config_recursive right after
load (before include-processing and merging). This expands ${VAR} in every
string leaf -- include paths, node params, sim sensor config_path, etc. --
in the launch process, before values are serialized into ROS parameters, so
downstream nodes (Python and C++) receive fully-resolved absolute paths and
need no env-var awareness.
- Only the braced ${VAR} form is expanded; a bare $ is left untouched.
- An undefined variable raises KeyError naming the offending file, so a typo
fails fast at launch instead of surfacing as a file-not-found in a node.
- No-op on any string without ${, so existing configs are unaffected.
Add coverage in test_launch_utils.py: value expansion (params + sim
config_path), undefined-var raise, scalar/bare-$ passthrough, and ${VAR}
inside an include path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
load_config_file previously passed every YAML string through verbatim, forcing downstream configs (e.g. legged_locomotion_rl's nav/mpc configs) to hard-code absolute paths like /home//.../nav_graphs.
Add _expand_env_vars, run per-file in _load_config_recursive right after load (before include-processing and merging). This expands ${VAR} in every string leaf -- include paths, node params, sim sensor config_path, etc. -- in the launch process, before values are serialized into ROS parameters, so downstream nodes (Python and C++) receive fully-resolved absolute paths and need no env-var awareness.
Add coverage in test_launch_utils.py: value expansion (params + sim config_path), undefined-var raise, scalar/bare-$ passthrough, and ${VAR} inside an include path.