non-standard interfaces for IMUSensor

[christianrauch]

The semantic_components::IMUSensor is registering interfaces with the name pattern angular_velocity.* and linear_acceleration.*. This is quite different from other interfaces, which use something like */velocity and */acceleration as defined in the hardware_interface namespace:

ros2_control/hardware_interface/include/hardware_interface/types/hardware_interface_type_values.hpp

Lines 18 to 50 in 23337e5

	namespace hardware_interface
	{
	/// Constant defining position interface name
	constexpr char HW_IF_POSITION[] = "position";
	/// Constant defining velocity interface name
	constexpr char HW_IF_VELOCITY[] = "velocity";
	/// Constant defining acceleration interface name
	constexpr char HW_IF_ACCELERATION[] = "acceleration";
	/// Constant defining effort interface name
	constexpr char HW_IF_EFFORT[] = "effort";
	/// Constant defining torque interface name
	constexpr char HW_IF_TORQUE[] = "torque";
	/// Constant defining force interface name
	constexpr char HW_IF_FORCE[] = "force";
	/// Constant defining current interface name
	constexpr char HW_IF_CURRENT[] = "current";
	/// Constant defining temperature interface name
	constexpr char HW_IF_TEMPERATURE[] = "temperature";
	/// Gains interface constants
	/// Constant defining proportional gain interface name
	constexpr char HW_IF_PROPORTIONAL_GAIN[] = "proportional";
	/// Constant defining integral gain interface name
	constexpr char HW_IF_INTEGRAL_GAIN[] = "integral";
	/// Constant defining derivative gain interface name
	constexpr char HW_IF_DERIVATIVE_GAIN[] = "derivative";
	/// Constant defining integral clamp interface name
	constexpr char HW_IF_INTEGRAL_CLAMP_MAX[] = "integral_clamp_max";
	/// Constant defining integral clamp interface name
	constexpr char HW_IF_INTEGRAL_CLAMP_MIN[] = "integral_clamp_min";
	/// Constant defining the feedforward interface name
	constexpr char HW_IF_FEEDFORWARD[] = "feedforward";
	} // namespace hardware_interface

This non-standard pattern prevents its use in other controllers. E.g. if someone wants to use the sensor as state interface for a pid_controller/PidController to control the rotation velocty of a robot like this:

pid_rotate:
  ros__parameters:
    dof_names: [
      "my_robot/yaw",
    ]
    command_interface: "velocity"
    reference_and_state_interfaces: ["velocity"]
    reference_and_state_dof_names: [
      "my_imu/angular_velocity.z",
    ]

then the interface my_imu/angular_velocity.z/velocity will not be available.

You could do:

pid_rotate:
  ros__parameters:
    reference_and_state_interfaces: ["angular_velocity.z"]
    reference_and_state_dof_names: [
      "my_imu",
    ]

but this restricts the controller to a single DoF.

To follow the standard interface pattern, shouldn't the semantic_components::IMUSensor interface names be something like angular.*/velocity and linear.*/acceleration?:

"angular.x/velocity"
"angular.y/velocity"
"angular.z/velocity"
"linear.x/acceleration"
"linear.y/acceleration"
"linear.z/acceleration"

[christophfroehlich]

We agreed that your proposal makes sense, but we were not able to decide if your proposal of "standard interface patterns" is the way to go. Maybe @saikishor can comment on his thoughts in more detail.

Another thing we have to figure out is how to do that change gracefully, otherwise we will break every setup with the IMUSensor. Maybe add a parameter for using one or another, to opt-in the new values for now.

[christianrauch]

Well, with "standard interface pattern", I mean that it appears that the rest of the ros2_control ecosystem and other controllers use the same scheme of exporting the DoFs individually, e.g. joint1, joint2, ..., together with an indicator about the physical state/control unit as standardised by the hardware_interface.

Indeed, this is a breaking change affecting many other third-party components. But I found that ros2_control is not afraif of breaking changes :-)

I would suggest to simply deploy this change to rolling, maybe announce this in advance, and have packages adapt to the new strings. Since there is no behaviour change, this should be rather simple to adapt.

[christophfroehlich]

Yes we are not afraid in breaking things, but we try to be graceful where we can ;) and this is such a non-safety-critical breaking change