Implement get_parameter_or_set_default.#551
Conversation
|
CI:
(note that the test failures are ros2/rmw_fastrtps#226, so unrelated to this PR) |
sloretz
left a comment
There was a problem hiding this comment.
Just 1 nitpick.
Also, it looks like this method won't be available to a lifecycle node, but I don't think anything needs to be done since get_parameter_or() is the same.
rclcpp/include/rclcpp/node.hpp
Outdated
| /// Get the parameter value; if not set, set the "alternative value" and store it in the node. | ||
| /** | ||
| * If the parameter is set, then the "value" argument is assigned the value | ||
| * in the parameter. If the parameter is not set, then the "value" argument |
rclcpp/include/rclcpp/node_impl.hpp
Outdated
|
|
||
| template<typename ParameterT> | ||
| void | ||
| Node::get_parameter_or_set_default( |
There was a problem hiding this comment.
I think this could be shortened to get_parameter_or_set, but that's subjective of course.
There was a problem hiding this comment.
I think this could be shortened to
get_parameter_or_set, but that's subjective of course.
True. I don't have a strong opinion, though I typically prefer a bit more verbose. I'm going to leave it as get_parameter_or_set_default unless someone voices a strong opinion for something else.
(and thanks for the review!)
There was a problem hiding this comment.
Since it isn't setting the default value (since a parameter in the server side doesn't have a default value) I would second the suggestion from @sloretz.
There was a problem hiding this comment.
All right, I renamed to get_parameter_or_set in 0bd546d.
This is syntactic sugar to allow the user to get a parameter. If the parameter is already set on the node, it gets the value of the parameter. If it is not set, then it gets the alternative value and sets it on the node, ensuring that it exists. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
220a74d to
7bdee2b
Compare
|
New CI:
|
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
|
Thanks for the review, merging. |
This is syntactic sugar to allow the user to get a parameter.
If the parameter is already set on the node, it gets the value
of the parameter. If it is not set, then it gets the alternative
value and sets it on the node, ensuring that it exists.
I personally think that the name makes some sense, but I'm definitely open to other ideas for the API. If approved and merged, this will be used in ros2/teleop_twist_joy#8 . Test for this new functionality is in ros2/system_tests#296