-
Notifications
You must be signed in to change notification settings - Fork 483
Document design decisions that were made for statically typed parameters #1568
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
Merged
ivanpauno
merged 10 commits into
master
from
ivanpauno/add-docs-about-statically-typed-parameters
Mar 18, 2021
Merged
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
2987e63
Document design decisions that were made for statically typed parameters
ivanpauno 80624f0
fix typo
ivanpauno f245d42
Address peer review comments
ivanpauno c789228
Improve sentence
ivanpauno 4167e5c
grammar
ivanpauno 75691e5
wording
ivanpauno f78327a
typo
ivanpauno 13fc4c8
Correct typo
ivanpauno 46a773e
Document possible improvement
ivanpauno 2576bc0
One sentence per line
ivanpauno File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| # Notes on statically typed parameters | ||
|
|
||
| ## Introduction | ||
|
|
||
| Up to ROS 2 Foxy, all parameters could be changed of type anytime, except the user installed a parameter callback to reject that change. | ||
| This could generate confusing errors, for example: | ||
|
|
||
| ``` | ||
| $ ros2 run demo_nodes_py listener & | ||
| $ ros2 param set /listener use_sim_time not_a_boolean | ||
| [ERROR] [1614712713.233733147] [listener]: use_sim_time parameter set to something besides a bool | ||
| Set parameter successful | ||
| $ ros2 param get /listener use_sim_time | ||
| String value is: not_a_boolean | ||
| ``` | ||
|
|
||
| For most use cases, having a static parameter types is enough. | ||
ivanpauno marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| This notes document some of the decisions that were made when implementing static parameter types enforcement in: | ||
ivanpauno marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| * https://github.com/ros2/rclcpp/pull/1522 | ||
| * https://github.com/ros2/rclpy/pull/683 | ||
|
|
||
| ## Allowing dynamically typed parameters | ||
|
|
||
| There might be some scenarios were dynamic typing is desired, so a `dynamic_typing` field was added to the [parameter descriptor](https://github.com/ros2/rcl_interfaces/blob/09b5ed93a733adb9deddc47f9a4a8c6e9f584667/rcl_interfaces/msg/ParameterDescriptor.msg#L25). | ||
ivanpauno marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| It defaults to `false`. | ||
|
|
||
| For example: | ||
|
|
||
| ```cpp | ||
| rcl_interfaces::msg::ParameterDescriptor descriptor; | ||
| descriptor.dynamic_typing = true; | ||
|
|
||
| node->declare_parameter("dynamically_typed_parameter", rclcpp::ParameterValue{}, descriptor); | ||
| ``` | ||
|
|
||
| ```py | ||
| rcl_interfaces.msg.ParameterDescriptor descriptor; | ||
| descriptor.dynamic_typing = True; | ||
|
|
||
| node.declare_parameter("dynamically_typed_parameter", None, descriptor); | ||
| ``` | ||
|
|
||
| ## How is the parameter type specified? | ||
|
|
||
| The parameter type will be inferred from the default value provided when declaring it. | ||
|
|
||
| ## Statically typed parameters when allowing undeclared parameters | ||
|
|
||
| When undeclared parameters are allowed and a parameter is set without a previous declaration, the parameter will be dynamically typed. This is consistent with other allowed behaviors when undeclared parameters are allowed, e.g. trying to get an undeclared parameter returns "NOT_SET". | ||
ivanpauno marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| Parameter declarations will remain special and dynamic or static typing will be used based on the parameter descriptor (default to static). | ||
|
|
||
| ## Declaring a parameter without a default value | ||
|
|
||
| There might be cases were a default value does not make sense and the user must always provide an override. | ||
| In those cases, the parameter type can be specified explicitly: | ||
|
|
||
| ```cpp | ||
| // method signature | ||
| template<typename T> | ||
| Node::declare_parameter<T>(std::string name, rcl_interfaces::msg::ParameterDescriptor = rcl_interfaces::msg::ParameterDescriptor{}); | ||
| // or alternatively | ||
| Node::declare_parameter(std::string name, rclcpp::ParameterType type, rcl_interfaces::msg::ParameterDescriptor = rcl_interfaces::msg::ParameterDescriptor{}); | ||
|
|
||
| // examples | ||
| node->declare_paramter<int64_t>("my_integer_parameter"); // declare an integer parameter | ||
| node->declare_paramter("another_integer_parameter", rclcpp::ParameterType::PARAMETER_INTEGER); // another way to do the same | ||
| ``` | ||
|
|
||
| ```py | ||
| # method signature | ||
| Node.declare_parameter(name: str, param_type: rclpy.Parameter.Type, descriptor: rcl_interfaces.msg.ParameterDescriptor = rcl_interfaces.msg.ParameterDescriptor()) | ||
|
|
||
| # example | ||
| node.declare_paramter('my_integer_parameter', rclpy.Parameter.Type.INTEGER); # declare an integer parameter | ||
| ``` | ||
|
|
||
| If the parameter may be unused, but when used requires a parameter override, then you could conditionally declare it: | ||
|
|
||
| ```cpp | ||
| auto mode = node->declare_parameter("mode", "modeA"); // "mode" parameter is an string | ||
| if (mode == "modeB") { | ||
| node->declare_parameter<int64_t>("param_needed_when_mode_b"); // when "modeB", the user must provide "param_needed_when_mode_b" | ||
| } | ||
| ``` | ||
|
|
||
| ## Other migration notes | ||
|
|
||
| Declaring a parameter with only a name is deprecated: | ||
|
|
||
| ```cpp | ||
| node->declare_parameter("my_param"); // this generates a build warning | ||
| ``` | ||
|
|
||
| ```py | ||
| node.declare_parameter("my_param"); # this generates a python user warning | ||
| ``` | ||
|
|
||
| Before, you could initialize a parameter with the "NOT SET" value (in cpp `rclcpp::ParameterValue{}`, in python `None`). | ||
| Now this will throw an exception in both cases: | ||
ivanpauno marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| ```cpp | ||
| node->declare_parameter("my_param", rclcpp::ParameterValue{}); // not valid, will throw exception | ||
| ``` | ||
|
|
||
| ```py | ||
| node.declare_parameter("my_param", None); # not valid, will raise error | ||
| ``` | ||
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.
Uh oh!
There was an error while loading. Please reload this page.