Standardize usage of ROS Parameters

[henningkayser]

MoveIt should follow a unified approach of loading and using parameters.

This includes:

• Declare all parameters explicitly
• Cleanup parameter declare/get/set/validate implementation (i.e. use struct/class templates based on rosparam_shortcuts)
• Improve debug output (pretty-print parameters and errors, handle accordingly)
• Apply repo-wide refactoring

In the future, this unified (struct/class) approach could be used to implement parameter syncing between nodes.

[tylerjw]

Ideally, I think this parameter struct should be populated before starting any ROS interfaces (pub/sub/etc) so the node can fail quickly with the minimal unrelated output if parameters are misconfigured in a way that would prevent the node from working correctly. By doing this it also has the benefit that the struct can be stored as immutable (const) and passed around by reference to anything that needs it will be easy to reason about immutability.

An example of what I'm talking about is in moveit_servo as ported by Adam last summer.

[MarqRazz]

Are there any parameters that we would like to allow the user to update after startup? I think most of them could be cosnt but on a few occasions I have wished I could reload the robot_description on the fly for optimization/reach studies. I have read that the new parameter system supports dynamic reconfig by default now (but is currently still missing some functionality). I know that robot_description might be a pain to support reloading and with python launch scripts that might be a better answer but I figured I would ask while we were looking at this.

Other parameters we may want to consider supporting reloading...

• If we dont want to support reloading the entire robot_description how about just the joint angle limits?
• allowed_execution_duration_scaling and allowed_goal_duration_margin
• does the octomap plugin support switching camera topics?

[pac48]

The main benefit of parameters is that the user can dynamically configure compiled code without the need to recompile. However, the biggest issue I see with the current approach is that there is no clear protocol defining what parameters are required and what are optionally.

Consider moveit2/moveit_ros/planning/trajectory_execution_manager/src/trajectory_execution_manager.cpp as an example. The source code calls get_parameter for the following parameters.

• moveit_manage_controllers
• trajectory_execution.execution_duration_monitoring
• trajectory_execution.allowed_execution_duration_scaling
• trajectory_execution.allowed_goal_duration_margin
• trajectory_execution.allowed_start_tolerance

These parameters are required to exist at runtime. Currently, these parameters are specified using a user provided yaml like this

But how is the user supposed to know what parameters need to be in their yaml? If documentation is good and there is an example for everything, then they can copy it. However, there is still nothing preventing them for accidentally typing in a field incorrectly. It would be really great if the parameter protocol was defined in the package itself.

One solution is to create a new directory moveit2/moveit_ros/planning/trajectory_execution_manager/include/parameter_defintion with two files: required.yaml and optional.yaml. Those files would explicitly show what they need to include in their yaml, also they use them as a template for creating their parameter yaml.

A benefit of this approach is that lots of boiler plate code for parameter handling can be automated. A struct can be generated according to the required.yaml and optional.yaml files with automatic updating. See here for an example implementation. Additionally, the fields defined in the user's yaml can be checked against the required.yaml at compile time and output an error if they forgot to define something. This approach will help eliminate runtime errors like this https://github.com/ros-planning/moveit2/blob/f4fc946f78cc8e1bb81d19d795454e8581873b58/moveit_ros/planning/trajectory_execution_manager/src/trajectory_execution_manager.cpp#L137 There still is the case where the user is completely relying on compiled binaries and they are tuning their yaml file. If they make a mistake there, then there is nothing we can to prevent a runtime error. Still, these checks can be added automated since we have an explicit protocol.

There are probably other ways to tackle this problem. But I believe the root of the problem is that our source code expects certain parameters, but does not require the user to define them.

[tylerjw]

Here is a Discussion about this work: https://github.com/ros-planning/moveit2/discussions/1403

[tylerjw]

ros2_controllers

generate_parameter_library is being dog-fooded into ros2_controllers first. This is enabling us to figure out what is missing as ros2_controllers has many uses of ros2 parameters.

• [x] diff_drive_controller: https://github.com/ros-controls/ros2_controllers/pull/386
• [x] force_torque_sensor_broadcaster: https://github.com/ros-controls/ros2_controllers/pull/395
• [x] forward_command_controller: https://github.com/ros-controls/ros2_controllers/pull/396
• [x] gripper_controllers: https://github.com/ros-controls/ros2_controllers/pull/398
• [x] imu_sensor_broadcaster: https://github.com/ros-controls/ros2_controllers/pull/399
• [x] joint_state_broadcaster: https://github.com/ros-controls/ros2_controllers/pull/401
• [x] joint_trajectory_controller: https://github.com/ros-controls/ros2_controllers/pull/384

What's next after that?

• [x] Catalog all the parameter usage in MoveIt - https://github.com/ros-planning/moveit2/discussions/1486

[YeThawy]

Ideally, I think this parameter struct should be populated before starting any ROS interfaces (pub/sub/etc) so the node can fail quickly with the minimal unrelated output if parameters are misconfigured in a way that would prevent the node from working correctly. By doing this it also has the benefit that the struct can be stored as immutable (const) and passed around by reference to anything that needs it will be easy to reason about immutability.

An example of what I'm talking about is in moveit_servo as ported by Adam last summer.

true