-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Update new_declare_or_get_parameter API for collision_monitor and map_server #5588
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
Update new_declare_or_get_parameter API for collision_monitor and map_server #5588
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.
Also, can use we a node->declare_or_get_parameter()API instead of
nav2::declare_or_get_parameter(node, ...)`? If that's not in the node wrapped capabilities, it might be good to add that.
Can you also expand on the test changes?
bf3a2dd
to
18cd86b
Compare
I have added the missing definition of
I have removed all |
Signed-off-by: Leander Stephen D'Souza <[email protected]>
…eter. Signed-off-by: Leander Stephen D'Souza <[email protected]>
Signed-off-by: Leander Stephen D'Souza <[email protected]>
18cd86b
to
887f016
Compare
54b1023
to
ace49b9
Compare
… template. Signed-off-by: Leander Stephen D'Souza <[email protected]>
Signed-off-by: Leander Stephen D'Souza <[email protected]>
ace49b9
to
5efd816
Compare
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.
Generally LGTM!
…docstring. Signed-off-by: Leander Stephen D'Souza <[email protected]>
…_server (ros-navigation#5588) * Use the new_declare_or_get_parameter API for nav2_collision_monitor. Signed-off-by: Leander Stephen D'Souza <[email protected]> * Removed redundant set_parameter calls when preceeded by declare_parameter. Signed-off-by: Leander Stephen D'Souza <[email protected]> * Use declare_or_get_parameter without a default value. Signed-off-by: Leander Stephen D'Souza <[email protected]> * Simplified definitions that required to determine parameter type from template. Signed-off-by: Leander Stephen D'Souza <[email protected]> * Removed self-definition of node_utils.hpp. Signed-off-by: Leander Stephen D'Souza <[email protected]> * Address PR feedback: Remove unnecessary linebreaks and improve param docstring. Signed-off-by: Leander Stephen D'Souza <[email protected]> --------- Signed-off-by: Leander Stephen D'Souza <[email protected]>
Basic Info
Description of contribution in a few bullet points
declare_or_get_parameter
without the default value to the lifecycle node.get_parameter
anddeclare_parameter
fields to use the newdeclare_or_get_parameter
API for nav2_collision_monitor.rclcpp::exceptions::ParameterUninitializedException
torclcpp::exceptions::InvalidParameterValueException
to match the exception raised indeclare_or_get_parameter
if the default value is not set.set_parameter()
calls when preceeded bydeclare_parameter
.Description of documentation updates required from your changes
Description of how this change was tested
pre-commit run --all
and testing locally usingcolcon test
Future work that may be required in bullet points
For Maintainers:
backport-*
.