-
Notifications
You must be signed in to change notification settings - Fork 161
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
Fix how ros2 param interprets command-line arguments. #684
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this makes sense. (probably we would want to have documentation how to override with !!str
?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me with green CI.
@ros-pull-request-builder retest this please |
Basically ensure that we are *always* using the parsed yaml string, so that users can override types as appropriate. This makes it possible for users to specify something like: ros2 param set /node param '!!str off' for force a string to be "off" as opposed to having YAML intepret it as a boolean. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
After doing some additional testing, I found a bug in the code, so I fixed that and added a couple more tests. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
f839780
to
e1569a4
Compare
All right, I found some time to do additional testing. This led to a bug which I fixed, and I added more tests. I'm going to mark this as ready for review and run CI on it. |
I'm going to go ahead and merge this with green CI. See ros2/ros2_documentation#2212 for documentation about this feature. |
Basically ensure that we are always using the parsed yaml
string, so that users can override types as appropriate.
This makes it possible for users to specify something like:
ros2 param set /node param '!!str off'
for force a string to be "off" as opposed to having YAML
intepret it as a boolean.
Signed-off-by: Chris Lalancette clalancette@openrobotics.org
This is still a draft because I'm kind of nervous about the change; this has the potential to break some use-cases that worked before. All of the tests pass, and basic testing here has worked fine for me, but I want to do more testing before marking it as ready.
Fixes #677 (at least, as far as I think we can fix it)