[CM] Use robot_description topic instead of parameter and don't crash on empty URDF 🦿

[mamueluth]

The robot description file is no longer passed to controller manager directly. Instead, if no urdf is passed to controller_manager_node, a subscription on the /robot_description topic is created and urdf is received via callback (needed for my thesis on distributed control).

What has been done:

• Deprecated passing of robot description file directly to controller manager.
• Subscribe to robot_state_publisher to get robot_description_file.

Things to consider:

• Controllers are in "unconfigured" state if robot description file arrives after ControllerManager creates subscription. This way, controller have to be configured and activated by hand.
• Maybe we should not call init_resource_manager multiple times if we have already received a valid urdf. Should i check this via a flag or if hardware_info_map_ is empty and ignore? Suggestions would be appreciated.

---

Starting of the concept can be tested with the :

• ros2 launch ros2_control_demo_bringup empty_robot.launch.py
• ros2 launch ros2_control_demo_bringup publish_description.launch.py
  in this ros2_control_demos_repo

---

• Add tests

[destogl]

I commented about some small changes.

We should also add test for this new functionality. Also with different cases, i.e., publishing description before and later. This is in the core of the framework's functionalities and should be 100% tested.

[bmagyar]

Can we have tests please?

[codecov-commenter]

Codecov Report

Merging #940 (c4825b1) into master (925f5f3) will decrease coverage by 1.97%.
The diff coverage is 35.61%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #940      +/-   ##
==========================================
- Coverage   34.61%   32.65%   -1.97%     
==========================================
  Files          52       91      +39     
  Lines        2981     9563    +6582     
  Branches     1855     6441    +4586     
==========================================
+ Hits         1032     3123    +2091     
- Misses        310      716     +406     
- Partials     1639     5724    +4085     

Flag	Coverage Δ
unittests	32.65% <35.61%> (-1.97%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
controller_manager/src/controller_manager.cpp	37.81% <ø> (-1.90%)	⬇️
controller_manager/src/ros2_control_node.cpp	0.00% <0.00%> (ø)
..._interface/include/hardware_interface/actuator.hpp	100.00% <ø> (ø)
...re_interface/include/hardware_interface/sensor.hpp	100.00% <ø> (ø)
...re_interface/include/hardware_interface/system.hpp	100.00% <ø> (ø)
hardware_interface/src/resource_manager.cpp	49.91% <ø> (-3.72%)	⬇️
hardware_interface/src/sensor.cpp	50.52% <ø> (ø)
hardware_interface/src/system.cpp	55.45% <ø> (ø)
...rface/test/mock_components/test_generic_system.cpp	9.14% <ø> (ø)
...dware_interface/test/test_component_interfaces.cpp	32.44% <ø> (+4.25%)	⬆️
... and 68 more

... and 19 files with indirect coverage changes

[mamueluth]

Can we have tests please?

Added some are those enough?

[destogl]

What about controller manager tests?

[mamueluth]

Added test for the cm site. Ended up doing a workaround and unfortunately had to make the callback function public. This needs to be addressed in the future but does not seem like a big issue for me at the moment...
First tried to pass the urdf via topic while testing, the way how it's supposed to work later, but this did not work properly. The problem here is how the tests are executed. If the urdf arrives to late then no hardware is found and tests a failing. Waiting and so on didn't work for me.

[bmagyar]

@destogl @mamueluth what's the status on this sirs?

[destogl]

We didn't mange to get it tested properly yet, and @mamueluth is now finalizing his thesis. I'll check to come to it in the next week or two, since I need this future for a scenario I am working at.

[Noel215]

Hi!

I am also interested on this feature!!

I've been testing it on a TIAGo Base and I can get the robot working by using the robot_description from the topic. :)
I only had to cherry-pick the commits to humble (+ solve merge conflicts) and change the subscriber topic from "~/robot_description" to "robot_description".

[bmagyar]

Let's get this guy rolling

[destogl]

Now happy!

[Noel215]

Could you backport this feature to humble by any chance?

[bmagyar]

Note that this is only the simplest implementation of the feature, it still doesn't allow for updating the URDF on-the-fly and if we agree with my fellow maintainers about adding this to Humble (as it is indeed slightly breaking...) we shouldn't expect anything other than this basic functionality.

[Noel215]

I'm aware of that, thanks!! :)

[destogl]

@Mergifyio backport humble

[mergify]

backport humble

✅ Backports have been created

• #1052 [CM] Use robot_description topic instead of parameter and don't crash on empty URDF 🦿 (backport #940) has been created for branch humble