Remove ros2_control tag from package (and move it to ur_robot_driver)

[fmauch]

There definitively is a need for using the robot description without the ros2_control tag in general e.g. when simply viewing the robot model (also see #52).

This commit removes the dependency to the ros2_control tag from the macro instantiating a robot and adds new files describing the control mechanisms of the individual joints and sensors. These files can be included in ros2_control files setting up a specific control structure (e.g. hardware driver, mock hardware, gazebo).

This change would require packages like driver, gazebo startup, etc. to provide their own descriptions with their own ros2_control tags but I personally don't see this that negative. As also raised in https://github.com/UniversalRobots/Universal_Robots_ROS2_Description/pull/52#issuecomment-1511073354 users will have to provide their own ros2_control tag for their own configuration as soon as there is more than only the (or more than one) UR involved.

I think the approach from this PR leaves a simple to use UR macro, a standalone (non-control) description and a couple of helper macros to create your own ros2_control tag.

[bmagyar]

I personally see a lot of value in a well-tested, officially supported ros2_control tag that comes with the robot's description files. If I had to deploy a UR-based system, I'd always try using the official tags once I'm past the prototyping stage.

Speaking of which, many tools used for prototyping may want to supply/generate their own ros2_control tag content (or not technically need it at all) which should also be supported somehow. I'm wondering if this could not be achieved with a much simpler new required parameter that turns ros2_control support on-off.

  <xacro:macro name="ur_robot" params="
    name
    tf_prefix
    parent
    *origin
    joint_limits_parameters_file
    kinematics_parameters_file
    physical_parameters_file
    visual_parameters_file
    generate_ros2_control_tag
    ...
  >

Please tell me if this is outlandishly oversimplified :D

[fmauch]

I personally see a lot of value in a well-tested, officially supported ros2_control tag that comes with the robot's description files. If I had to deploy a UR-based system, I'd always try using the official tags once I'm past the prototyping stage.

Speaking of which, many tools used for prototyping may want to supply/generate their own ros2_control tag content (or not technically need it at all) which should also be supported somehow. I'm wondering if this could not be achieved with a much simpler new required parameter that turns ros2_control support on-off.

One issue I have with keeping this inside here is that the whole hardware interface parametrization is piped through the description. In my point of view that carries mainly two negative implications:

• The description contains a wall of parameters that are only relevant if it is ought to be used in conjunction with the ur_robot_driver. Even more we have to define bogus default values since we cannot pass empty values to the hardware interface (which I realize while writing this is no longer true).
• Extending the driver's functionalities with new data being used in controllers requires a change on the description and the driver simultaneously (such as #110 / https://github.com/UniversalRobots/Universal_Robots_ROS2_Driver/pull/856)

I am not 100% sure about the cleanest solution and I know that this discussion has been done before, never coming to a final solution which I want to change with this PR.

So thank you for joining the discussion I hope that we find a solution that everybody can agree on. I'm not a huge fan of at-mentioning people, but since we had this discussion before, I would love to get input from @ipa-cmh, @destogl, @gavanderhoorn, @RobertWilbrandt on this suggestion.

To not seem like I am ignoring input from previous discussions:

• I did not create a separate package for the ros2_control tag mainly because of the two points raised above. The biggest part of the existing tag is parametrizing the ur_robot_driver's hardware interface. From my point of view this part should live with the driver. I could also imagine a separate package for this living in the driver's repo, though.
• I don't think this will drop a "well-tested, officially supported ros2_control tag", it will just not live with the description anymore.
• Although the title says "remove" more appropriately it should probably say "move to driver", see https://github.com/UniversalRobots/Universal_Robots_ROS2_Driver/pull/877
• I would like to minimize code duplication, hence I've created the helper files here.

[bmagyar]

Thanks for the great writeup!

What can the user expect to change on their side?

Going from the current

  <xacro:ur_robot
    name
    tf_prefix
    parent
    *origin
    joint_limits_parameters_file
    kinematics_parameters_file
    physical_parameters_file
    visual_parameters_file
    ... (one gazillion potentially irrelevant parameters)
  >

to

  <xacro:ur_robot
    name
    tf_prefix
    parent
    *origin
    joint_limits_parameters_file
    kinematics_parameters_file
    physical_parameters_file
    visual_parameters_file
    ... only description-relevant parameters
  >

  <xacro:ur_ros2_control
    ... relevant parameters for real HW driver or SIM
  >

or if you don't want ros2_control at all, to simply

  <xacro:ur_robot
    name
    tf_prefix
    parent
    *origin
    joint_limits_parameters_file
    kinematics_parameters_file
    physical_parameters_file
    visual_parameters_file
    ... only description-relevant parameters
  >

I quite like this :point_up:

[bmagyar]

Here's my next question: could this be made part of Humble in any way?

Perhaps for backward compatibility you could add the new macro to the driver there (same as w rolling) and a define_ros2_control_tag boolean param to the main macro?

[fmauch]

Here's my next question: could this be made part of Humble in any way?

Perhaps for backward compatibility you could add the new macro to the driver there (same as w rolling) and a define_ros2_control_tag boolean param to the main macro?

Yes, that sounds reasonable. Nobody plans to actively break Humble anyway 8-)

[destogl]

This move sounds reasonable to me also. In few more recent drivers we started to split things in similar way. We are even adding "ros2_control_support" packages with all ros2_control-related things and then this package can load description of a robot needed. Since here you have only ros2_control as driver, moving this to driver is good.

Only things to consider to not get lost. We have to update gazebo and igniton packages add add those two URDF snippets for those there. For now I would be happy with just a draft PR adding this somewhere. Those two packages needed anyway some more attention (at least when I checked them last time - and I feel responsible for that, but...)