Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling of exclusive command interfaces #1487

Open
firesurfer opened this issue Apr 18, 2024 · 12 comments · May be fixed by #1522
Open

Handling of exclusive command interfaces #1487

firesurfer opened this issue Apr 18, 2024 · 12 comments · May be fixed by #1522

Comments

@firesurfer
Copy link
Contributor

@saikishor Let's continue the discussion here as in #1476 there was clearly a misunderstanding on my side regarding the right terminology.

Description

Let's assume a hardware_interface::SystemInterface which handles multiple servo axes. Each of the axis has three modes (position, velocity, torque). These interfaces have to be used exclusive ( The servo can only be in one of those modes)

Currently the only solution I see is to have a list in the hardware_interface where the started interfaces for each axis are tracked. The issue with this solution is: When a controller is does not activate properly it will start the interfaces but will never stop the interfaces. So afterwards no other controller can ever start these interfaces again.

Workaround

Allow a controller to always activate successfully but check in the update method if the system is in a state where we can/want to control.

Possible solution

Quick and Dirty - make sure to stop interfaces

Make sure that if a controller starts any interface it will stop it in case it can not be configured/activated.

Advantage would be that the hardware interface has still the maximum amount of flexibility

Allow to mark interfaces as exclusive

In the export_command_interfaces of a hardware_interface we could introduce the concept of an ExclusiveGroup. Afterwards it is probably the job of the resource manager to handle this kind of resource lock.

Additional thoughts

What happens if one controller wants to switch from one interface to another in the same ExclusiveGroup ? (I actually have a use case where I would really like to do that)

@saikishor I can probably provide a test that reproduces this issue but it will take some time to setup as I need to implement a demo hardware_interface + a demo controller which takes some time to setup.

@saikishor
Copy link
Member

@firesurfer Thank you for taking time and create a new issue. Once you have the testcase, we can try to find a solution. It would be really nice to have such testcase, so we don't introduce the bug in future

@firesurfer
Copy link
Contributor Author

@saikishor good morning. Could you please point me out where the best place is to add such a test?

I found so far:
https://github.com/ros-controls/ros2_control/blob/master/hardware_interface_testing/test/test_components/test_actuator.cpp

Shall I just add an exclusive actuator there ?

@saikishor
Copy link
Member

saikishor commented Apr 19, 2024

@firesurfer Yes you can add it over there. Can you name it as test_actuator_exclusive_interfaces.cpp because you are focusing on the interfaces part

@saikishor
Copy link
Member

I went through the code roughly, it is more or less good. Now, the part is to reproduce the same failure you have in the perform command switch in the test :)

@firesurfer
Copy link
Contributor Author

That should be rather easy. I will add a test controller (prob. here? https://github.com/ros-controls/ros2_control/tree/master/controller_manager/test) that fails during activation and claims some of the interfaces.

@firesurfer
Copy link
Contributor Author

Regarding the testing procedure:

  1. Setup a system that uses the hardware interface in Exlusive hw interface rolling #1492
  2. Load and activate the controller from added a test controller that fails at activation #1493

The hw interface should now be in a state in which it has a "started" command interface that has not been stopped.

  1. Try to load and activate the failing controller or another one that uses the command interface.

The hw interface should now deny the prepare_command_mode_switch

@saikishor
Copy link
Member

@firesurfer add both #1492 and #1483 in the same PR, so you can clearly write a TEST case to reproduce it

@saikishor
Copy link
Member

@firesurfer
When I meant test, If you can write a test like how the tests are written in the tests folder of the packages, that's why merging both PRs into one would make sense

@firesurfer
Copy link
Contributor Author

Sorry for that.
But for someone who is not really involved in the ros2control development adding something, especially adding test looks rather difficult/ time consuming at the moment, as there are quite a lot of infrastructure and internal internal interfaces involved. Additionally I couldn't find any documentation about the test infrastructure and best practices.

I am really willing to provide the test code but I could really use some help setting up the necessary glue/infrastructure code.

I merged both PRs in #1492

@saikishor
Copy link
Member

@firesurfer Right now we are occupied with the current developments for Jazzy, if we are done with it, I can take a look at this later. I hope this is not a problem 👍

@firesurfer
Copy link
Contributor Author

@saikishor Thanks a lot!

@destogl
Copy link
Member

destogl commented May 8, 2024

@firesurfer In the meantime, you can check how different tests are written. The integration tests are in the hardware_interface_testing package. I usually find a similar test, copy it, and then start changing it. You can add it to any file, and we will easily move it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants