release_interfaces does not seem to work (or not work as expected)

[firesurfer]

Describe the bug

I have a controller which may decide to return return controller_interface::CallbackReturn::FAILURE; in case my hardware is in a state that is not usable in combination with the controller.

Prior to this I call release_interfaces but this does not seem to call a command_mode switch in the corresponding hardware interface in order to stop the interfaces.

In the corresponding header file controller_interface_base.hpp there is no available documentation for this method:

   CONTROLLER_INTERFACE_PUBLIC
  void release_interfaces();

To Reproduce

Return return controller_interface::CallbackReturn::FAILURE from on_activate. And call release_interfaces before.

Expected behavior

In general I would expect a controller that can not be successfully activated to release the claimed interfaces.

Environment (please complete the following information):

• OS: Ubuntu 22.04
• Version Iron binary install

[saikishor]

Hello @firesurfer!

The release_interfaces just clears the loaned interfaces within the controller, it is clear from the following lines of code:

ros2_control/controller_interface/src/controller_interface_base.cpp

Lines 104 to 108 in b989f70

	void ControllerInterfaceBase::release_interfaces()
	{
	command_interfaces_.clear();
	state_interfaces_.clear();
	}

I've a question if the activation is failure from the controller side, why should the hardware switch the interfaces?, here you are not providing a new interface switching right?. You can always switch to a different interface by launching a different controller.

If you are interested in switching back to the old hw state if the activation fails, then maybe we can always have the old HW state and switch back if it fails. I believe this could be implemented. if this is the case, feel free to open a PR.

[firesurfer]

Hi @saikishor
The behavior of release_interfaces makes sense.

I've a question if the activation is failure from the controller side, why should the hardware switch the interfaces?, here you are not providing a new interface switching right?. You can always switch to a different interface by launching a different controller.

From the point of view of a hardware interface I get a list of interfaces to be started and a list of interfaces to be stopped. In my implementation I actually remember which interface was claimed and only release it when it was in the stop list at some point.

If I fail the activation of a control an interface will be in the start list by will never be in the stop list. So if I start another controller my hardware interface says: Hey this interface has been claimed.

The reason why I store which interface was claimed is: My hardware supports three modes (position, velocity,torque). My hardware interface manages multiple axes at one.

This means I have per axis the following three interface.

axis_A/position
axis_A/velocity
axis_A/torque

So for example if the position interface of axis_A was claimed I store that axis_A is claimed so that no controller can claim the corresponding velocity or torque interface.

As far as I know there is no other way in ros2control at the moment to have exclusive command interfaces for a certain axis/joint?

[saikishor]

@firesurfer I've already noticed that there is no release interfaces called in the following condition when the activation is failed

ros2_control/controller_manager/src/controller_manager.cpp

Lines 1507 to 1515 in b989f70

	if (new_state.id() != lifecycle_msgs::msg::State::PRIMARY_STATE_ACTIVE)
	{
	RCLCPP_ERROR(
	get_logger(),
	"After activation, controller '%s' is in state '%s' (%d), expected '%s' (%d).",
	controller->get_node()->get_name(), new_state.label().c_str(), new_state.id(),
	hardware_interface::lifecycle_state_names::ACTIVE,
	lifecycle_msgs::msg::State::PRIMARY_STATE_ACTIVE);
	}

[firesurfer]

@saikishor As I wrote I call release_interfaces manually. But this does not solve the issue with interfaces still being claimed (Or marked as claimed in the hardware interface)

[saikishor]

@firesurfer Ideally when you clear the list, it should be released as the release_claimed_interface, is called by the deleter method

ros2_control/hardware_interface/src/resource_manager.cpp

Lines 981 to 985 in b989f70

	resource_storage_->claimed_command_interface_map_[key] = true;
	std::lock_guard<std::recursive_mutex> guard(resource_interfaces_lock_);
	return LoanedCommandInterface(
	resource_storage_->command_interface_map_.at(key),
	std::bind(&ResourceManager::release_command_interface, this, key));

I'll take a closer look later. 👍

[firesurfer]

@saikishor I see your point. I think by issue is of a different nature (exclusive interfaces). Shall I open a different issue for that or is there already a solution that I am not aware of?

[saikishor]

@firesurfer I recommend adding some introspection logs or some debug info to see if it is really calling the release_command_interface method. Yes, this is something exclusive to interfaces. We can come to conclusions once we know some info from more introspection.

[firesurfer]

@saikishor I am pretty sure that release_interfaces() is called from my code:

The structure is:

on_activate {
  if(condition) {
      Error log...
      release_interfaces()
      return controller_interface::CallbackReturn::FAILURE;
  }


}

[saikishor]

@firesurfer I meant to add some logs to the release_command_interface method to see if it is being called when you are clearing the lists. If I have some time today, I will take a look.

Have a great day :)

[firesurfer]

Hi @saikishor as I really need to get my controller working this week I won't be able to put any time into debugging this and will just work around this issue. :(

I also wish you a great day!

[saikishor]

OK when you have time, debug it and let us know. Thanks

[firesurfer]

Hey @saikishor I didn't debug this further but I think the issue I am facing is not originating from release_command_interface. In case the command interfaces would not have been released I would for sure see another error printed by ros2control when trying to loading the controller again.

As I wrote above I have a hardware_interface::SystemInterface which in the end controls a set of servo motors. For each servo axis I have three command interfaces (position, velocity, torque) and I need to make sure that only one of them per axis is used at a time.
As far as I know ros2control does not support exclusive command interfaces. So therefore my hardware_interface keeps a list internally which axis have one of those interfaces claimed (started).
The issue is now: When a controller is not sucessfully activated these interfaces are started from the point of view of the hardware_interface, but are never stopped again.
Additionally as far as I know there is know way to manually trigger a stop (To be precise a perform_command_mode_switch with a specific interface in the stop list)

EDIT:
Btw. I worked around this issue by always activating the controller regardless the system state and checking in the update method if the system is in the correct starting state.

[saikishor]

@firesurfer you mentioned earlier that the interfaces are left in claimed state in the Resource Manager, is this is the case I don't think it is in your hardware component. I suggest to introspect further. If you are fine with your work around and not clear of the exact issue, then I recommend to close this issue.

Thanks. Have a great day

[firesurfer]

@saikishor I think I got the terminology wrong here (Or as I wrote in the title does not work as I expected it).

I guess the issue is rather about: How to safely implement exclusive command interfaces.
As it seems there is not easy/quick answer to this shall I open a new issue for that and close this one ?

[saikishor]

@saikishor I think I got the terminology wrong here (Or as I wrote in the title does not work as I expected it).

I guess the issue is rather about: How to safely implement exclusive command interfaces.
As it seems there is not easy/quick answer to this shall I open a new issue for that and close this one ?

If this is the case, it's upto the Hardware components to decide it they allow both interfaces to be enable at same time or not, so maybe we should rethink the strategy for exclusive interfaces.

Do you have an idea on how to deal with it?. Can you come up with a PR so we can get it in for Jazzy?

[saikishor]

@firesurfer will you be able to write a test that reproduces the issue?

[firesurfer]

@saikishor I close this issue in favor of #1487