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!