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

Export interfaces created in init #83

Merged
merged 4 commits into from
Jul 21, 2021
Merged

Conversation

bjsowa
Copy link
Contributor

@bjsowa bjsowa commented Jun 24, 2021

Looking at the code, I noticed that the GazeboSystem creates state and command interfaces in 2 places:

  1. In the initSim method. It creates only the handles for the interfaces defined in the URDF.
  2. In export methods. It creates handles for all interfaces (effort, velocity, position), regardless of whether they are defined in the URDF, and exports them to the Resource Manager.

This makes the handles created in 1. completely redundant (unless I'm missing something), as they are not used throughout the code.
I'm new to ros2_control but from what I understand the sole reason for creating the state and command interfaces is for them to be exported to the Resource Manager and not be cared about anymore.

In this PR, the export methods "move" interfaces created in initSim to the Resource Manager instead of creating new ones. This let's the user define in URDF what interfaces should a joint export. This also simplifies the code.

@shonigmann
Copy link

I was just about to submit an issue on the exact same topic. would be nice to get this merged in!

@ahcorde
Copy link
Collaborator

ahcorde commented Jul 5, 2021

To be honest, I'm no sure which is the right way to use it. I coded this based on the examples.

Maybe @Karsten1987 or @bmagyar can add some light here.

@bjsowa
Copy link
Contributor Author

bjsowa commented Jul 5, 2021

Let me explain it more based on the example you mentioned:

The RRBotSystemPositionOnlyHardware is an example of system which controls a type of joints that only has position command interface and position state interface. In the configure method, it asserts that only these interfaces are defined in URDF and in the export methods it creates handles for these interfaces which are sent to the Resource Manager.

It naturally comes to mind that a GazeboSystem should be able to simulate any arbitrary system. Therefore, it should not assume what interfaces a joint could be controlled with and instead export only interfaces that are defined in URDF.

Currently, the GazeboSystem actually creates handles for the interfaces defined in URDF, but for some reason it later creates handles for all possible interfaces and these handles end up being exported to the Resource Manager, making the former ones useless.

@bmagyar
Copy link
Member

bmagyar commented Jul 6, 2021

To add a new angle to this discussion: what if the gazebo plugin supported mode switching on joints that define multiple interfaces? I saw this topic came up recently already, perhaps it is a good time to implement that?

@bjsowa
Copy link
Contributor Author

bjsowa commented Jul 6, 2021

To add a new angle to this discussion: what if the gazebo plugin supported mode switching on joints that define multiple interfaces? I saw this topic came up recently already, perhaps it is a good time to implement that?

Mode Switching is the next thing I would like to add to gazebo_ros2_control. I have also some other changes on the way, but I need to have this merged first.

@ahcorde
Copy link
Collaborator

ahcorde commented Jul 16, 2021

@bjsowa, do you mind to fix the conflicts? then it's good to go in

@bjsowa
Copy link
Contributor Author

bjsowa commented Jul 19, 2021

I fixed the conflicts, made similar changes for the IMU and FR interfaces. I also added some small cosmetic changes and I made sensor-only systems (without joints defined) allowed.

@ahcorde ahcorde merged commit 04b2c6f into ros-controls:master Jul 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants