-
Notifications
You must be signed in to change notification settings - Fork 91
launch_ros action to set parameters for all nodes in scope using a global parameter yaml file #260
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
Conversation
Signed-off-by: Aditya Pande <[email protected]>
Signed-off-by: Aditya Pande <[email protected]>
|
This is only adding the parameterers to Node actions but not to loaded components, which might be a bit unexpected by users. |
Signed-off-by: Aditya Pande <[email protected]>
Signed-off-by: Aditya Pande <[email protected]>
Signed-off-by: Aditya Pande <[email protected]>
Signed-off-by: Aditya Pande <[email protected]>
Signed-off-by: Aditya Pande <[email protected]>
TODO
|
Signed-off-by: Aditya Pande <[email protected]>
Signed-off-by: Aditya Pande <[email protected]>
… arguments Signed-off-by: Aditya Pande <[email protected]>
|
Parameters are now passed in order , e.g. this launch file will run -- Similarly, will produce: TODO
|
ivanpauno
left a comment
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.
Implementation LGTM!
This looks ready to go with some tests added.
Signed-off-by: Aditya Pande <[email protected]>
Signed-off-by: Aditya Pande <[email protected]>
Signed-off-by: Aditya Pande <[email protected]>
Signed-off-by: Aditya Pande <[email protected]>
Signed-off-by: Aditya Pande <[email protected]>
Signed-off-by: Aditya Pande <[email protected]>
ivanpauno
left a comment
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.
LGTM!
test_launch_ros/test/test_launch_ros/actions/test_set_parameters_from_file.py
Show resolved
Hide resolved
Signed-off-by: Aditya Pande <[email protected]>
ivanpauno
left a comment
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.
LGTM with green CI
jacobperron
left a comment
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.
LGTM! Just a couple minor comments.
test_launch_ros/test/test_launch_ros/actions/test_set_parameters_from_file.py
Outdated
Show resolved
Hide resolved
Signed-off-by: Aditya Pande <[email protected]>
This PR aims to add a new
launch_rosaction,SetParametersFromFilewhich can accept a yaml file with ROS parameters which can be passed to all nodes in scope.Example usage :
The parameter file:
The parameter yaml file must be written in the following format:
Launch file in xml:
demo_node_3is not in the same scope, so theSetParametesFromFileaction won't affect it.Equivalent usage in python:
TODO
@propertymethods required forSetParametersFromFileclass ? For e.g.launch_ros/launch_ros/launch_ros/actions/set_parameter.py
Line 76 in ef1502e
SomeSubstitutionTypeobjects when parsing xml launch files