Legitimate and global remaps (SetRemap) also triggers deprecation warning

[robin-zealrobotics]

The new --controller-ros-args-related deprecation warning is shown also when passing the remaps through the spawners as the deprecation warning suggests.
Unless I did something wrong, but I doubt it. No remaps are passed to ros2_control_node and instead they are passed to the arguments of spawner. I see the Setting controller param "node_options_args" to... log statement and my topics are correctly mapped. Having 4 spawners of which 3 have remappings, I thus see the deprecation warning 3 times.

See comment below

[saikishor]

Hello!

Thank you for reporting the issue. Do you mind sharing the log?

Thank you

[saikishor]

Hello!

Ideally, the deprecation notice should only be shown when the remapping is done through the ros2_control_node

ros2_control/controller_manager/src/controller_manager.cpp

Lines 3508 to 3518 in 54a9226

	// Add deprecation notice if the arguments are from the controller_manager node
	if (
	check_for_element(node_options_arguments, RCL_REMAP_FLAG) ||
	check_for_element(node_options_arguments, RCL_SHORT_REMAP_FLAG))
	{
	RCLCPP_WARN(
	get_logger(),
	"The use of remapping arguments to the controller_manager node is deprecated. Please use the "
	"'--controller-ros-args' argument of the spawner to pass remapping arguments to the "
	"controller node.");
	}

If you can share more logs, it would be great,.

Thank you!

[robin-zealrobotics]

Here you have the log files for the ros2_control_node and the 4 spawners. You see the 3 spawners that have remap rules print out the log statement that only triggers when --controller-ros-args is used. And you see ros2_control_node print out the deprecation warning 3 times, which suggests that the code snippet you referenced is in fact also called when a spawner passes those arguments to the controller_manager and not just when you specify the remappings directly on the ros2_control_node.
(Unless there's already been a bugfix that's not part of 4.24)

  ros2_control_node = Node(
      package="controller_manager",
      executable="ros2_control_node",
      parameters=[ros2_control_params],
  )
  joint_state_broadcaster_spawner = Node(
      package="controller_manager",
      executable="spawner",
      arguments=["joint_state_broadcaster", "-c", "controller_manager"],
  )
  odometry_broadcaster_spawner = Node(
      package="controller_manager",
      executable="spawner",
      arguments=[
          "odometry_broadcaster",
          "-c",
          "controller_manager",
          "--controller-ros-args",
          "-r odometry_broadcaster/odom:=localisation/odom",
      ],
  )
  velocity_controller_spawner = Node(
      package="controller_manager",
      executable="spawner",
      arguments=[
          "velocity_controller",
          "-c",
          "controller_manager",
          "--controller-ros-args",
          " ".join(
              [
                  "-r velocity_controller/cmd_vel:=cmd_vel",
                  "-r velocity_controller/cmd_vel_actuation:=cmd_vel_actuation",
                  "-r velocity_controller/kinematic_profile:=kinematic_profile",
              ]
          ),
      ],
  )
  torque_controller_spawner = Node(
      package="controller_manager",
      executable="spawner",
      arguments=[
          "torque_controller",
          "-c",
          "controller_manager",
          "--inactive",
          "--controller-ros-args",
          "-r torque_controller/cmd_torque:=cmd_torque",
      ],
  )

python3_72886_1737452967043.log
python3_72885_1737452967036.log
python3_72884_1737452966970.log
python3_72883_1737452967065.log
ros2_control_node_72882_1737452966652.log

[robin-zealrobotics]

I realized what is going on, so the previous information I gave is not that relevant anymore.
When using e.g. these types of remaps

        SetRemap("/diagnostics", ("/", robot_name, "/diagnostics")),
        SetRemap("/parameter_events", ("/", robot_name, "/parameter_events")),

This will also trigger the deprecation warning.
Given that these are not remaps intended for the spawner-nodes specifically, but generally for all nodes, probably we wouldn't want deprecation warnings being printed out.
Not sure if it's easy to determine though which kinds of remaps should print the deprecation and which should not.

It's also very confusing that in this unintended warning case, the warning gets printed out for each spawner.

[saikishor]

Given that these are not remaps intended for the spawner-nodes specifically, but generally for all nodes, probably we wouldn't want deprecation warnings being printed out.
Not sure if it's easy to determine though which kinds of remaps should print the deprecation and which should not.

We do understand that it is hard to differentiate which type of remaps belongs to what, that's why the depreciation is printing irrespective of what it belongs to. We want users to use the new ways to remap for controllers and we don't want it to be a breaking change.

The issue you are mentioning should be solved soon in some upcoming releases for rolling