Remove comments from yaml file before applying substitutions #322
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
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
| @@ -240,7 +240,13 @@ def evaluate(self, context: LaunchContext) -> Path: | |||
| with open(param_file_path, 'r') as f, NamedTemporaryFile( | |||
| mode='w', prefix='launch_params_', delete=False | |||
| ) as h: | |||
| parsed = perform_substitutions(context, parse_substitution(f.read())) | |||
| try: | |||
| file_without_comments = yaml.safe_dump(yaml.safe_load(f.read())) | |||
There was a problem hiding this comment.
It seems to break a case, which I am not sure if it's a correct case, that could be parsed successfully before.
/**:
ros__parameters:
mix_subs: "$(command pwd) $(dirname)"
before:
$ ros2 param get /parameter_blackboard mix_subs
String value is: /home/chenlh/Projects/ROS2/ros2-master /home/chenlh/Projects/ROS2/ros2-master/install/share/demo_nodes_cpp/launch/topics
after using 95d4699:
$ ros2 launch demo_nodes_cpp parameter_blackboard.xml
[INFO] [launch]: All log files can be found below /home/chenlh/.ros/log/2022-08-30-11-39-18-674265-OptiPlex-7080-460434
[INFO] [launch]: Default logging verbosity is set to INFO
tree Tree(template, [Tree(fragment, [Token(UNQUOTED_STRING, 'demo_nodes_cpp')])])
tree Tree(template, [Tree(fragment, [Token(UNQUOTED_STRING, 'parameter_blackboard')])])
tree Tree(template, [Tree(fragment, [Token(UNQUOTED_STRING, 'parameter_file_with_substitutions.yaml')])])
tree Tree(template, [Tree(fragment, [Token(UNQUOTED_STRING, '/**:\n ros__parameters:\n mix_subs: ')]), Tree(fragment, [Tree(substitution, [Token(IDENTIFIER, 'command'), Tree(arguments, [Tree(value, [Tree(part, [Token(UNQUOTED_RSTRING, 'pwd')])])])])]), Tree(fragment, [Token(UNQUOTED_STRING, ' ')]), Tree(fragment, [Tree(substitution, [Token(IDENTIFIER, 'dirname')])]), Tree(fragment, [Token(UNQUOTED_STRING, '\n')])])
[ERROR] [launch]: Caught exception in launch (see debug for traceback): The substituted parameter file is not a valid yaml file
There was a problem hiding this comment.
It seems that we need to invoke yaml.safe_dump with default_style='"', and then to support the yaml tag for !!int, !!float in the rcl which will be similar to ros2/rcl#999.
the data is dumped with default_style='"', and then parsed by substitutions.
"/**":
"ros__parameters":
"array":
- !!int "1"
- !!int "2"
- !!int "3"
"double": !!float "3.141592653"
"filename": "run /home/chenlh/Projects/ROS2/ros2-master/install/share/demo_nodes_cpp/launch/topics/parameter_blackboard.xml on 2022年 08月 30日 星期二 13:50:26 CST
"
"filename2": "2022年 08月 30日 星期二 13:50:26 CST
/home/chenlh/Projects/ROS2/ros2-master/install/share/demo_nodes_cpp/launch/topics/parameter_blackboard.xml"
"float": !!float "3.1"
"int": !!int "3"
There was a problem hiding this comment.
@iuhilnehc-ynos thanks for showing an example when the PR doesn't work correctly.
It seems that we need to invoke yaml.safe_dump with default_style='"'
This will cause issues in yaml files like:
/**:
ros__parameters:
my_integer: $(env MY_INTEGER)There was a problem hiding this comment.
A better way to fix this issue might need to update the grammar.lark to ignore the comment. I am not familiar with lark.
+COMMENT: "#" /[^\n]/*
+%ignore COMMENT
+
IDENTIFIER: LETTER (LETTER | DIGIT | "_" | "-")*
I am not sure if it covers all comment cases. I have tested the following cases, and it seems good with the result.
yaml file:
```yaml
# int: $(env MY_INT)
/**:
ros__parameters:
# int: $(env MY_INT)
filename: "run $(filename) on $(command date)" # int: $(env MY_INT)
filename2: "$(command date) $(filename) #fdas"
filename3: $(env MY_INT) $(filename) #fdas
filename4: $(env MY_INT) # $(filename) #fdas
int2: $(env MY_INT)
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.
Fixes ros2/ros2#1317.
I'm not sure if it's the best solution, but it's one.