-
Notifications
You must be signed in to change notification settings - Fork 221
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
Allow declaring uninitialized parameters #798
Conversation
Parameters can now be declared without a default value and without an override. Attempting to access a statically typed parameter that does not have value will raise an exception. Getting dynamically typed parameters will return an unset parameter value. This change is equivalent to that made in rclcpp: ros2/rclcpp#1673 Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
@@ -364,7 +364,6 @@ def declare_parameters( | |||
parameters: List[Union[ | |||
Tuple[str], | |||
Tuple[str, Parameter.Type], | |||
Tuple[str, Any], |
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.
wasn't this for passing a value, e.g.: declare_parameters('', [('my_param', 5)])
?
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 the annotation was incorrect before. IIUC, we only expect a tuple up to length 3
. The last value of the tuple can have type ParameterDescriptor
or Any
(for the case of passing a value).
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.
Line 630 in 1b335d3
('baz', 2.41), |
We actually can be more specific than using Any
, I think Union[int, float, str, bytes, None]
would be fine
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 and some manual testing to check for sanity.
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
This is the same behavior as rclcpp. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Thanks for the reviews. I think I've addressed everything. PTAL. |
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
Thanks for triggering CI! |
* Allow declaring uninitialized parameters Parameters can now be declared without a default value and without an override. Attempting to access a statically typed parameter that does not have value will raise an exception. Getting dynamically typed parameters will return an unset parameter value. This change is equivalent to that made in rclcpp: ros2/rclcpp#1673 Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Fix flake8 Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Minor refactor Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Don't allow static parameters to be undeclared Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Return alternative for uninitialized parameters in get_parameter_or Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Ditto for dynamic parameters This is the same behavior as rclcpp. Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Fix lint Signed-off-by: Jacob Perron <jacob@openrobotics.org>
* Allow declaring uninitialized parameters Parameters can now be declared without a default value and without an override. Attempting to access a statically typed parameter that does not have value will raise an exception. Getting dynamically typed parameters will return an unset parameter value. This change is equivalent to that made in rclcpp: ros2/rclcpp#1673 Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Fix flake8 Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Minor refactor Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Don't allow static parameters to be undeclared Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Return alternative for uninitialized parameters in get_parameter_or Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Ditto for dynamic parameters This is the same behavior as rclcpp. Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Fix lint Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Parameters can now be declared without a default value and without an override.
Attempting to access a statically typed parameter that does not have value will raise an exception.
Getting dynamically typed parameters will return an unset parameter value.
This change is equivalent to that made in rclcpp: ros2/rclcpp#1673