Controllers not deactivated on error in update on Humble

[jellehierck]

Describe the bug
When a controller returns return_type::ERROR in its update() function, it does not get deactivated.

For example, using following snippet:

controller_interface::return_type MyController::update(
  const rclcpp::Time & /*time*/, const rclcpp::Duration & /*period*/
) {
  if (something_bad == true) {
    RCLCPP_ERROR(get_node()->get_logger(), "Something bad happened");
    return controller_interface::return_type::ERROR;
  }
  ...
  return controller_interface::return_type::OK;
}

The controller will spam Something bad happened to the console continuously instead of stopping the controller.

Expected behavior
I expected a controller to get deactivated once it returns return_type::ERROR.

Environment (please complete the following information):

• OS: Ubuntu 22
• Version: Humble

Additional context
This functionality is already implemented in the master branch (#1499). Is the choice to not include in Humble conscious? Are there any workarounds? Or would I need to build from source from the master branch?

Thanks!

[christophfroehlich]

@saikishor can we backport #1499?

[saikishor]

@saikishor can we backport #1499?

It shouldn't be hard to backport it. The problem is this is a change in behaviour, Moreover, I'm not sure if @ros-controls/ros2-maintainers are fine with this kind of change

[christophfroehlich]

It is a change of behavior only if a user implemented an ERROR return value, which was just ignored up to now. However, I would not put too much effort in the backport, users always can build the rolling version from source to have all the new features.

[jellehierck]

However, I would not put too much effort in the backport, users always can build the rolling version from source to have all the new features.

That would work for me.

I can see how the change in behaviour is something to consider carefully. But arguably, the current behaviour is wrong ;) We can leave that up to @saikishor and the other @ros-controls/ros2-maintainers.

[christophfroehlich]

We could log a warning if an ERROR was returned, but don't change the behavior of it?

[jellehierck]

Yes that would also be an acceptable change which clearly lets the users know what is happening instead of (somewhat) silently failing on Humble!

[saikishor]

I can see how the change in behaviour is something to consider carefully. But arguably, the current behaviour is wrong ;) We can leave that up to @saikishor and the other @ros-controls/ros2-maintainers.

I do agree that it is wrong, that why I proposed this change for the Jazzy. The problem is the Humble doesn't have the disabling of controller when an ERROR is received in read and write cycles. That's why I'm hesitant to the change. Moreover, if the user doesn't have a robust hardware Component, then any NaN in the Handles might trigger this