ros-controls / ros2_control

Generic and simple controls framework for ROS 2
https://control.ros.org
Apache License 2.0
504 stars 300 forks source link

Compatibility with ROS 2 Components #1704

Open jcarpinelli-bdai opened 2 months ago

jcarpinelli-bdai commented 2 months ago

Summary

Can we use rclcpp_components, and Components more generally, to more effectively separate manager node / managed node semantics in the ControllerManager class?

Issue

When using the ControllerManager class in a new process (not the project-provided ros2_control_node process), renaming the controller manager might cause every underlying controller to have its private member variable node to change its name to the name of the controller manager. This is a known issue, and is described in the ros2_control documentation.

Composition

In ROS 2, Node Composition seems to be the general solution to the issue described above. Components can be renamed independently of the component manager.

Question

Does composition (in the ROS 2 sense of the word) offer any benefits over the existing design of the controller manager, and the controller interface? Instead of inheriting a std::shared_ptr<rclcpp_lifecycle::LifecycleNode> in the controller interface, could controllers act as Node components? Could the controller manager be a component manager? Thank you for reading!

[!WARNING]

I am not a ROS 2 expert, so my understanding could definitely be incorrect. Please let me know if I'm misunderstanding ros2_control, or ROS 2 features more broadly.

saikishor commented 2 months ago

Hello @jcarpinelli-bdai!

This is no longer a problem because the issue you are describing is solved with the https://github.com/ros-controls/ros2_control/pull/1694. The ros2 components that you are describing internally use the same approach of setting use_global_arguments to false.

Let us know if this fixes your issue.

Thank you!

jcarpinelli-bdai commented 2 months ago

Thanks for the quick reply! In my own testing, setting use_global_arguments(false) does solve the re-naming issue, but it also disables parameter setting for the composed nodes. I am using humble, so perhaps this behavior has changed in more recent ROS versions?

saikishor commented 2 months ago

Thanks for the quick reply! In my own testing, setting use_global_arguments(false) does solve the re-naming issue, but it also disables parameter setting for the composed nodes. I am using humble, so perhaps this behavior has changed in more recent ROS versions?

Well that's true, that part will be affected. That's why the changes are not backported to older distros. However, in order to have it working use --param-file option from spawner, and this will help you set it to the controller nodes

christophfroehlich commented 2 months ago

Thanks for the detailed summary of your issue. @saikishor Isn't the part in docs outdated after #1694?

saikishor commented 2 months ago

Thanks for the detailed summary of your issue. @saikishor Isn't the part in docs outdated after #1694?

Yes, it is outdated as now such individual node remapping is no longer needed, even though the same trick will still work. Should we cleanup this part of the docs?

jcarpinelli-bdai commented 2 months ago

Well that's true, that part will be affected. That's why the changes are not backported to older distros. However, in order to have it working use --param-file option from spawner, and this will help you set it to the controller nodes

Ah, I understand. Thank you for the clarification. I have been using global arguments passed to the controller manager node to configure each of the controllers, as shown in this humble demo. In demos for later releases, the --params-file argument is used instead. This all fully addresses my issue. Thanks again.

I am also interested, just to learn, how these "composed lifecycle nodes" used in ros2_control relate to rclcpp_components. If anyone is feeling like explaining... do components address the same needs as this pattern of "composing full lifecycle nodes"?