ros2_control icon indicating copy to clipboard operation
ros2_control copied to clipboard

Rethink parameter handling

Open Karsten1987 opened this issue 4 years ago • 4 comments

TL;DR: I propose to enhance the service call for loading a controller with an optional parameters file, similar to what the ros2 composition does: ros2 composition load -p <parameters> .... That way we can load controllers and their types dynamically, even though they weren't declared beforehand.


The current parameter handling is IMO a bit cumbersome and quite error prone - user unfriendly to say the least.

To bring everybody on the same page, a quick recap how parameters work in ROS2, but foremost in the context of ros2_control.

  • There is no global parameter server as in ROS1, every node has their own local set of parameters.
  • Parameters can be set (only) when starting a node by passing in a ros-parameter yaml file (there's other CLI args as well, but for simplicity let's focus on the yaml file).
    • The parameter file as a whole gets parsed through the call to rclcpp::init(argc, argv). The parameters are attached to a process-wide context object which holds these parameters as so-called overrides.
    • When a node is spawned, the context object gets probed for overrides according to the node name and the overrides are passed implicitly through the NodeOptions to the node constructor and declared (c.f. node_options.automatically_declare_parameters_from_overrides(true); in here.
  • All nodes spawned in a future time point will get these overrides passed in the same way, given that all nodes are launched within the same process, a.k.a share the same context object. That's the reason why in our case, the controllers which are being loaded later on still get all the parameters set in the initial parameters file as they are spawned in the same context as the controller manager which initially loads that parameters file.

Given that primer, we take the following necessary precautions, i.e.:

controller_manager:
  ros__parameters:
    update_rate: 2  # Hz

    forward_command_controller_position:
      type: forward_command_controller/ForwardCommandController

    joint_state_controller:
      type: joint_state_controller/JointStateController

forward_command_controller_position:
  ros__parameters:
    joints:
      - joint1
      - joint2
    interface_name: position

The parameter file above shows two problems in my eyes:

  1. All controllers (and their types) have to be declared within the controller manager before loading.
  2. Controller and their respective parameters have to be declared in two different location (once in the controller manager, once for their actual parameters).

The first problem is the more severe in this case as it's prohibiting to load an unknown controller if their type and parameters are not set. Take the example above and imagine to load a third controller such as a JointTrajectoryController. Without restarting and modifying the yaml file there's currently no way to do so. A more advanced occurrence of that problem happened within the gazebo_ros2_control packages, as there's currently no way to pass in a yaml file to the plugins/gazebo's context. See this PR for more context (pun intended!). The gazebo plugin as-is, wo/ the PR has to load all parameters manually. That is, when not the controllers are not loaded at start-up time you won't be able to load them afterwards as the parameters were never set on the context.

I thus propose to enhance the controller manager's logic of loading controllers and passing in additional arguments to its service call. I'd love to load a controller and pass along a set of parameters with it.

ros2 control load_controller <controller_name> <controller_type> --parameter-file </path/to/file.yaml>

Alternatively, I could see a solution in which all parameters are set at the controller manager level, which will forward these parameters to the individual controllers upon loading. The obvious downside to this is that we have no a duplicated parameter system on the ros-graph - once in the controller manager namespace, once in the controller namespace.

Karsten1987 avatar Feb 25 '21 02:02 Karsten1987

Have you seen https://github.com/ros-controls/ros2_control/pull/310?

The idea is that you can: ros2 run controller_manager spawner.py my_controller_name --controller-type my_controller_pkg/ControllerType --param-file </path/to/file.yaml>

--controller-type and --param-file are optional.

We can refactor it into a ros2control verb, but I also like the idea of having it as a node, as you can include in a launch file, as in https://github.com/ros-controls/ros2_control_demos/pull/66/files#diff-cee04bac3ad5cc54b7318abebf3cb3480a2b8ff0c9701f275fe73f21c768f52d

It also has the options of the ROS1 spawner of stopping and unloading when you kill it.

v-lopez avatar Feb 25 '21 10:02 v-lopez

TL;DR: I propose to enhance the service call for loading a controller with an optional parameters file, similar to what the ros2 composition does: ros2 composition load -p <parameters> .... That way we can load controllers and their types dynamically, even though they weren't declared beforehand.

I find the idea very good. For completeness, I would like to add/comment on a few points.

2. Controller and their respective parameters have to be declared in two different location (once in the controller manager, once for their actual parameters).

I don't think we have to declare parameters in the two places. First, you have to declare the controller's type for CM to know which plugin to load. The second declaration are the controller's parameters in their namespace. I find this very natural and intuitive. would rather say that ROS1 was confusing since CM has access to the controller's namespace to load it. It's kind of "memory violation" IMHO. (I had to say this :D)

The first problem is the more severe in this case as it's prohibiting to load an unknown controller if their type and parameters are not set. Take the example above and imagine to load a third controller such as a JointTrajectoryController. Without restarting and modifying the yaml file there's currently no way to do so.

You are defining the issue very well. IMO it is in the CM's namespace. I think @v-lopez solved this very nice in #310.

ros2 control load_controller <controller_name> <controller_type> --parameter-file </path/to/file.yaml>

If we go in this direction. Do we need to do file reading tasks in CM or can ROS support that similar to when starting a node?

Alternatively, I could see a solution in which all parameters are set at the controller manager level, which will forward these parameters to the individual controllers upon loading. The obvious downside to this is that we have no a duplicated parameter system on the ros-graph - once in the controller manager namespace, once in the controller namespace.

I don't like this alternative....

destogl avatar Mar 01 '21 15:03 destogl

@Karsten1987 ist this resolved by merging spawner/unspawner PR?

destogl avatar Apr 02 '21 07:04 destogl