Skip to content
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

Enhance parameters API - declarations and descriptors #321

Closed
jubeira opened this issue Apr 24, 2019 · 2 comments · Fixed by #325
Closed

Enhance parameters API - declarations and descriptors #321

jubeira opened this issue Apr 24, 2019 · 2 comments · Fixed by #325
Assignees
Labels
enhancement New feature or request

Comments

@jubeira
Copy link
Contributor

jubeira commented Apr 24, 2019

The purpose of this issue is to describe proposed changes to rclpy considering changes in rclcpp regarding parameter handling: ros2/rclcpp#495, and gather feedback if necessary.

Changes to rclcpp: summary

Considering the changes in the PR mentioned above, this is what the API in rclcpp looks like for parameter handling (Node.hpp):

  • declare_parameter: declares and initialize a parameter, returns the effective value.
    • Has overload to specify type.
  • declare_parameters: declares and initializes several parameters with the same namespace and type.
    • Has overload with map + key / value pairs.
  • undeclare_parameter: undeclares previously declared parameter.
  • has_parameter: returns true if a parameter is declared. Doesn't throw exceptions if not.
  • set_parameter: sets the given parameter and then returns result of the set action.
  • set_parameters: sets multiple parameters, one at the time.
  • set_parameters_atomically: sets multiple parameters at once.
  • get_parameter: returns parameter.
    • Has overload that doesn't throw; returns false when it's not possible to get the parameter.
  • get_parameter_or: gets parameter, or returns a given default.
  • get_parameters: gets multiple parameters at once.
    • Has overload for prefixes.
  • describe_parameter: returns parameter descriptor.
  • describe_parameters: returns vector of descriptors.
  • get_parameter_types: returns vector of parameter types.
  • list_parameters: returns list of parameter with any of given prefixes
  • set_on_parameters_set_callback: registers callback anytime a parameter is about to be changed.

As a general rule, any method that gets or sets undeclared parameters might throw an exception if undeclared parameters are not allowed, or return a sane default if undeclared parameters are allowed. Methods that don't throw exceptions are explicity documented as such. The list above doesn't consider deprecated functions.

Current API in rclpy

The current API in rclpy is briefer:

  • get_parameter.
  • set_parameters.
  • set_parameters_atomically.
  • set_parameters_callback.

Proposal

Changes to rclpy should:

  • Allow the user to declare and undeclare parameters.
  • Allow getting and setting undeclared parameters, throwing exceptions in the case the node doesn't allow it.
  • Handle parameter descriptors.

Methods to add

def declare_parameter(parameter: Parameter) -> Parameter:
    """Declares parameter with a given descriptor; returns effective parameter value assigned to the parameter."""
 
def declare_parameters(namespace: str, parameters: List[Parameter])) -> List[Parameter]:
    """ 
    Same as declare_parameter, for multiple parameters. Returns list of parameters containing the effective values assigned to each of them.
    """

def undeclare_parameter(name: str):
    """Undeclares a given parameter"""

def describe_parameter(name: str) -> ParameterDescriptor:
    """Gets parameter descriptor for a given parameter"""
    
def describe_parameters(name: List[str]) -> List[ParameterDescriptor]:
    """Same as describe_parameter, for multiple parameters at once."""

def has_parameter(name: str) -> bool:
    """Returns true if a parameter has been declared; false otherwise (doesn't throw)."""

def get_parameter_or(name: str, alternative_value: Parameter) -> Parameter:
    """Returns a parameter if declared or the alternative value otherwise.
    Option B: return Tuple(bool, Parameter), with the boolean indicating whether the parameter was declared or not.
    """

Methods to modify

These methods will have the same API, but their behavior will be modified:

  • set_parameters(List[Parameter]) -> List[SetParametersResult]
    • Will throw if parameter is not declared and undeclared parameters are not allowed.
    • If one fails beacuse it was not declared, stop there. If the callback fails, that will be reflected in the returned list as in the current implementation.
  • set_parameters_atomically(List[Parameter]) -> SetParametersResult
    • Will throw if parameter is not declared and undeclared parameters are not allowed.
    • If one fails, none is set (i.e. all have to be declared for this to succeed).
  • get_parameter(name: str) -> Parameter
    • Will throw if not declared and undeclared not allowed. Returns PARAMETER_NOT_SET if allowed as in the current implementation.
  • get_parameters(names: List[str]) -> List[Parameter]
    • Will throw if a parameter is not declared and undeclared are not allowed. Returns PARAMETER_NOT_SET if allowed as in the current implementation in the corresponding position of the list.

Additional modifications

These changes shouldn't modify the API either if default values are used:

  • allow_undeclared_parameters flag to shall be added to the node constructor, set to True by default for backwards compatibility.
  • Parameter class should support custom ParameterDescriptors other than the default one.

To be verified

Regarding callbacks when parameters are set, rclpy has the method set_parameters_callback. In rclcpp, set_on_parameters_set_callback was introduced; these callbacks can reject a parameter from being set.
At first sight it looks like the callbacks in rclpy can do the same; I need to double check if this is the case.

@jubeira jubeira self-assigned this Apr 24, 2019
@jubeira
Copy link
Contributor Author

jubeira commented Apr 24, 2019

@wjwwood here's the issue with the description taking into account your feedback (thanks again!).
I think it's taking shape; any other feedback is always welcome!
/cc @gonzodepedro

@jubeira jubeira added ready Work is about to start (Kanban column) enhancement New feature or request in progress Actively being worked on (Kanban column) and removed ready Work is about to start (Kanban column) labels Apr 24, 2019
@wjwwood
Copy link
Member

wjwwood commented Apr 27, 2019

Just re-reviewed it, lgtm.

jubeira pushed a commit that referenced this issue Apr 30, 2019
- See issue #321 for details.

Signed-off-by: Juan Ignacio Ubeira <[email protected]>
jubeira pushed a commit that referenced this issue May 2, 2019
- See issue #321 for details.

Signed-off-by: Juan Ignacio Ubeira <[email protected]>
@jubeira jubeira removed the in progress Actively being worked on (Kanban column) label May 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants