Design choice: why is the `ros2_control_node` reimplemented?

[danielsanjosepro]

Hi!
Thanks for the repo, I like the idea of having an unified place for this. :)
I have a question about the design choice the MujocoRosControl class.
Why are you not just creating a HardwareInterface like in cartesian_controllers ?
In your code, you implement a class that instantiates a pointer to the control manager instead of just creating a HardwareInterface that runs mujoco in an infinite while loop. This makes the launch files look a bit odd, since they do not use the controller_manager as default. (I am referencing the code down here).

mujoco_ros2_control/mujoco_ros2_control/include/mujoco_ros2_control/mujoco_ros2_control.hpp

Lines 50 to 60 in 9e82f24

	mjModel *mj_model_;
	mjData *mj_data_;
	rclcpp::Logger logger_;
	std::shared_ptr<pluginlib::ClassLoader<MujocoSystemInterface>> robot_hw_sim_loader_;
	std::shared_ptr<controller_manager::ControllerManager> controller_manager_;
	rclcpp::Executor::SharedPtr cm_executor_;
	std::thread cm_thread_;
	bool stop_cm_thread_;
	rclcpp::Duration control_period_;

Is there a reason for this? I have been using a modified version of cartesian_controllers (since it has some mistakes, see Issue 217) and it has been working perfectly without modifying the controller manager.

Thanks in advance for the clarification.
Dani

[pucciland95]

Hi,

I am not one of the original authors of the code. However, I think that the design for this code was inspired by gz_ros2_control, where it first loads the Hardware Interface as a plugin and then passes it to the Controller Manager. This way, the Controller Manager is aware of the existence of the plugin.

In my fork, I tried to remove this pattern, hoping that this part of the URDF would be enough to let the Controller Manager load the Hardware Interface:

<hardware>
  <plugin>mujoco_ros2_control/MujocoSystem</plugin>
</hardware>

Whenever I tried to launch this, the Controller Manager would complain that the Hardware Interface could not be found.

If you know a way to fix this I would be more than glad to hear it :-D

[sangteak601]

The reason is that mujoco_ros2_control_node is a replacement for ros2_control_node, and ros2_control_node contains a controller_manager.

I'm not sure which parts of the launch files look odd to you. To me, using a different controller for simulation seems more unusual than replacing the node.

[danielsanjosepro]

Hi,
Thank you for the answer. You are right, I might have explained the problem too vaguely. The is a node that is spinned in . As you said, you replace the ros2_control_node with a custom mujoco_ros2_control_node but why are you not simply using the HardwareComponent abstraction for the sim?
In the of ros2_control, they even discourage users to not use the default-node setup.

I might need to go into more detail to explain my problem. I will try to explain it with some dummy code snippets. If the urdf of my should probably look something like this:

<?xml version='1.0' encoding='utf-8'?>
<robot xmlns:xacro="http://www.ros.org/wiki/xacro" name="my_robot">
  <xacro:arg name="use_sim" default="True" />

 <!-- Include robot description -->
  <xacro:include filename="$(find my_robot_description)/urdf/my_robot_base.urdf.xacro"/>

 <!-- Include ros2_control definition -->
  <xacro:include filename="$(find my_robot_description)/urdf/my_robot_ros2_control.urdf.xacro"/>
  <xacro:my_robot_ros2_control use_sim="$(arg use_sim)"></my_robot_ros2_control>
    
</robot>

Where the ros2 control include file might look like this:

<?xml version='1.0' encoding='utf-8'?>
<robot xmlns:xacro="http://www.ros.org/wiki/xacro">
<xacro:macro name="my_robot_ros2_control"  params="use_sim">
  <ros2_control name="MyRobotHardwareComponent" type="system">
    <xacro:if value="${use_sim}">
       <hardware>
          <plugin>mujoco_ros2_control/MujocoSystem</plugin>
          <param name="mujoco_model">path/to/mujoco_file.xml</param>
        </hardware>
    </xacro:if>
    <xacro:unless value="${use_sim}">
       <hardware>
          <plugin>my_robot/MyRealRobotSystem</plugin>
        </hardware>
    </xacro:unless>

   ...

  </ros2_control>
</xacro:macro>
</robot>

Then I would expect in my launch file, that simply by setting the flag use_sim to true, that the MyRobotHardwareComponent uses mujoco as an interface and I do not need to modify my launch file in any other way. My launch file would look something like this:

robot_description_content = Command(
        [
            PathJoinSubstitution([FindExecutable(name="xacro")]),
            " ",
            PathJoinSubstitution(
                [FindPackageShare("my_robot_description"), "urdf", "my_robot.urdf.xacro"]
            ),
            " ",
            "use_sim:=<True|False>",
        ]
    )
robot_description = {"robot_description": robot_description_content}

...

# The rest stay untouched, it should not really matter if we are using sim or not
controller_manager = Node(
          package="controller_manager",
          executable="ros2_control_node",
          parameters=[controllers_config],
  )
  controllers = [
      Node(
          package="controller_manager",
          executable="spawner",
          arguments=["joint_state_broadcaster"],
          output="screen",
      ),
     Node(
          package="controller_manager",
          executable="spawner",
          arguments=["joint_trajectory_controller"],
          output="screen",
      ),
      ...
  ]

Now however, we need a different ros2_control_node, but why? The HardwareComponent abstraction is here to serve the purpose of having different systems for the robot. Actually, one could also have multiple ros2_control tags, one for sim and one for real and then let the controller_manager decide which component should be unconfigured, inactive or active depending on the config of hardware_components_initial_state (see the ).

I hope that I was a bit more clear on the issue now :)
@pucciland95 I think understood the problem as well. I modified the title of the issue to make it more clear!

Best,
Daniel

[pucciland95]

I think the main difference wrt the repo you linked is that this repo does not want to solve only the mujoco<->ros2 control problem.

In particular, despite the repo's name, the last commits are bridging camera images in mujoco to Ros2 through topics. Binding the Mujoco simulation with the ros2 control hardware interface (like the repo you linked is doing) would not make sense from a design point of view in this case.

In other words: the mujuco<->ros2 integration does not comprehend only the control problem but also camera images streaming, object poses, sensor reading ecc...