Add namespaced controllers

[hellantos]

Enabling namespacing for controllers.

Currently, it is not possible to set a controller specific namespace. Controllers simply add the namespace of the controller manager if it has one. Having a bit more flexibility can be pretty handy in larger systems.

This PR enables setting the namespace when loading a controller by name, by the following naming conventions.
It should not have any effect on existing systems.

controller name starts with '/'

• The controller name passed in when loading is taken as fully qualified name. This means relative to the root namespace.
• The controller type parameter for the manager is the qualified name with '/' replaced by '.' and without leading '.'

controller name starts without '/'

• The controller name is taken as relative to the controller_managers namespace.
• The controller type parameter for the manager is the passed in controller name with '/' replaced by '.'

If the passed in name starts with a '/' it is assumed to be relative to the root namespace. In this case the parameter
is the full name with the initial '/' removed and all other '/' replaced with '.'.

To send us a pull request, please:

• Fork the repository.
• Modify the source; please focus on the specific change you are contributing. If you also reformat all the code, it will be hard for us to focus on your change.
• Ensure local tests pass. (colcon test and pre-commit run (requires you to install pre-commit by pip3 install pre-commit)
  • colcon test 100%
  • precommit fails for master branch as well...
• Commit to your fork using clear commit messages.
• Send a pull request, answering any default questions in the pull request interface.
• Pay attention to any automated CI failures reported in the pull request, and stay involved in the conversation.

[saikishor]

Hello @ipa-cmh,

Can you explain a bit on your current usecase? I would like to understand better which limitations you are planning to address.
Adding this feature might have some affects with the current approach. Can you also please add some tests?.

Thank you,

[hellantos]

We are working on systems that will have 10 - 100 devices to control. From simple IO devices and single servos to robot systems (the general machines you find in automation) and there will be mutiple devices of the same kind in one machine. We will also have machines with different variations of devices. Conseuqently, we want to standardise the interface to the different devices via namespaces. Currently, we have to options to do that.

a) one controller manager per device and setting a namespace for the controller manager
b) naming conventions for the controllers
c) enable namespacing for controllers in the controller manager

We want to avoid a) because it would lead to hundreds of unnecessary threads and have negatie performance impact.
We want to avoid b) because we would need naming conventions that do not leverage the given namespacing options of the ROS 2 ecosystem and also lead to very large and difficult to understand configuration files. It would also clutter the root namespace.

Therefore, c) seems to be a good solution. We can give controllers namespaces independent of the controller manager. We are able to freely define how many controller managers we run to manage hardware and controllers. To me, it simply makes sense, that the namespace of the controller should be independent of the namespace of the controller manager. I usually would choose the number of control loops (controller managers) depending on the available RT cores. However my application should not need to know on which controller manager the currently used controller runs, but rather which device it belongs to.

This is what an example system would look like:

/controller_manager_1 # runs on core 1 in RT 
/controller_manager_2 # runs on core 2 in RT
/cylinder1/operation_mode_manager
/cylinder1/forward_position_controller # loaded by controller manager 1
/cylinder2/operation_mode_manager
/cylinder2/forward_position_controller # loaded by controller manager 1
/robot1/operation_mode_manager
/robot1/forward_position_controller # loaded by controller manager 2
/robot1/joint_trajectory_controller # loaded by controller manager 2
/robot2/operation_mode_manager
/robot2/forward_position_controller # loaded by controller manager 2
/robot2/joint_trajectory_controller # loaded by controller manager 2
/system_operation_mode_manager
/joint_state_publisher
/robot_state_publisher
...

The operation_mode_manager is basically a more intelligent spawner that spawns and enables controllers for a specific device and also does some monitoring. The system_operation_mode_manager takes care of operation mode of the system (i.e. the mode of the different devices). For multiple devices otf the same type, we can just feed a namespace and the same configuration file to the operation mode manager (which will load the necessary parameters into the controller manager).

It is certainly possible to implement this without namespacing support, but it would just be harder. Having namespacing available just gives everyone more freedom in designing their ROS 2 driver/controller interfaces.

[hellantos]

I'll add some tests in the next days.

[hellantos]

Okay, I have taken existing namespace test as base and created the following tests:

• Normal behavior (controller manager and controller in same namespace)
• Relative namespace (controller in namespace relative to controller manager), controller name used to load the controller does not have a leading '/' but contains a '/'
• Absolute namespace (controller in namespace relative to '/'), controller name has a leading '/'.

[destogl]

Generally, I find the idea quite nice, but we have to clean up and clarify some more details in this PR.

[destogl]

Is this sentence complete?

[destogl]

Why not use controller manager's logger

Suggested change

	rclcpp::get_logger("split_namespace_and_name"),
	get_logger(),

[saikishor]

Therefore, c) seems to be a good solution. We can give controllers namespaces independent of the controller manager. We are able to freely define how many controller managers we run to manage hardware and controllers. To me, it simply makes sense, that the namespace of the controller should be independent of the namespace of the controller manager. I usually would choose the number of control loops (controller managers) depending on the available RT cores. However my application should not need to know on which controller manager the currently used controller runs, but rather which device it belongs to.

Hello @ipa-cmh,

Thank you for such a nice explanation. I think your use case is valid, and I like the idea of the namespacing to be able to solve this. It's so nice to see tests.

ros2_control/controller_manager/src/controller_manager.cpp

Lines 1202 to 1205 in 228373a

	std::string controller_namespace_, controller_name_, controller_param_name_;
	calculate_controller_naming(
	controller.info.name, get_namespace(), controller_namespace_, controller_name_,
	controller_param_name_);

However, I see a possible issue that the controller.info.name is always the original name of the controller with namespace, this might create problems for the controller chaining case here:

ros2_control/controller_manager/src/controller_manager.cpp

Lines 157 to 160 in 228373a

	auto interface_prefix = interface_name.substr(0, split_pos);
	following_controller_it = std::find_if(
	controllers.begin(), controllers.end(),
	std::bind(controller_name_compare, std::placeholders::_1, interface_prefix));

In the above case, the interfaces will have the controller name without the namespace, and then when comparing it will be compared with the total name. This might probably affect the name-spaced controller chaining.

I think it would be nice to add a field called namespace in the ControllerInfo and then be able to set it separately as a parameter as how we do with the type of the controller. It would be much clear I believe.

ros2_control/hardware_interface/include/hardware_interface/controller_info.hpp

Lines 27 to 37 in 228373a

	struct ControllerInfo
	{
	/// Controller name.
	std::string name;
	/// Controller type.
	std::string type;
	/// List of claimed interfaces by the controller.
	std::vector<std::string> claimed_interfaces;
	};

What do you think?

Best Regards,
Sai

[hellantos]

Sorry for the long radio silence some family things came in between.

@saikishor
Good that you point that out. I do not really have a full overview over the code. Adding a field in ControllerInfo should be feasible.

@destogl
I will include your changes in the PR as soon as possible.

[mergify]

This pull request is in conflict. Could you fix it @ipa-cmh?

[mergify]

This pull request is in conflict. Could you fix it @hellantos?