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

Mimic joints independent of the combination of command interfaces #111

Closed

Conversation

christophfroehlich
Copy link
Contributor

I updated the mimic joint options to be independent on the command interfaces and also added an offset tag like the URDF-cousin has.

See the readme:

You can specify a mimicked joint independent of the combination of command interfaces. E.g., if you use an effort command interface for joint 1 but want to let joint 2 mimic the position of joint 1, set

<joint name="right_finger_joint">
  <command_interface name="effort"/>
  <state_interface name="position"/>
  <state_interface name="velocity"/>
  <state_interface name="effort"/>
</joint>
<joint name="left_finger_joint">
  <param name="mimic">right_finger_joint</param>
  <param name="multiplier">1</param>
  <param name="offset">0</param>
  <command_interface name="position"/>
  <state_interface name="position"/>
  <state_interface name="velocity"/>
  <state_interface name="effort"/>
</joint>

Be aware that these mimicked joints do not preserve any conservation of energy, i.e.,
the necessary effort of joint 1 won't be changed.

Let me know if this is not the intention of the maintainers, but for my robot it was a necessary fix. More elegant would be the usage of transmission tags by ros2_control, but it seems that they are not implemented yet.

@ahcorde
Copy link
Collaborator

ahcorde commented Feb 15, 2022

@bmagyar and @destogl what do you think about this ?

@bmagyar
Copy link
Member

bmagyar commented Feb 15, 2022 via email

@christophfroehlich
Copy link
Contributor Author

christophfroehlich commented Feb 15, 2022

Transmissions are coming in the very near future. Can you take a look whether that format would work for you? ros-controls/ros2_control#633

Thanks for this hint, great to see it coming :)
My real hardware has a mechanically linked telescope arm (one actuator, two prismatic joints). From the PR alone I do not get if this is supported yet, but I'll give it a try within the next days..

@bmagyar
Copy link
Member

bmagyar commented Feb 15, 2022 via email

@christophfroehlich
Copy link
Contributor Author

@ahcorde It seems that my extension of the mimic joints does not meet with approval... Should I rewrite it by adding the offset only, and adding a paragraph to the readme discussing the limits of the mimicked joints?

@destogl
Copy link
Member

destogl commented Feb 18, 2022

@christophfroehlich since we are adding more parameters for mimic joints (rightly so!) I would propose that we update our parser on that.

The proposal would be to implement this syntax (@bmagyar what do you think?)

<joint name="right_finger_joint">
  <command_interface name="effort"/>
  <state_interface name="position"/>
  <state_interface name="velocity"/>
  <state_interface name="effort"/>
</joint>
<joint name="left_finger_joint">
  <mimic joint="right_finger_joint" multiplier="1" offset="0" />
  <command_interface name="position"/>
  <state_interface name="position"/>
  <state_interface name="velocity"/>
  <state_interface name="effort"/>
</joint>

This would make the syntax the same as for the rest of URDF. The changes are fairly simple to do on ros2_control repository. If you would like to contribute, I can give you exact pointers what should be done. I don't have time unfortunately to implement this, but can support you with pointers and reviews.

@christophfroehlich
Copy link
Contributor Author

@destogl I can give it a try if you point me into the right direction where to start ;)
just a sidenote: your code example with different command_interfaces won't work without my proposed changes by this PR.

@destogl
Copy link
Member

destogl commented Feb 18, 2022

just a sidenote: your code example with different command_interfaces won't work without my proposed changes by this PR.

Yes, of course! Your code is great, but I would just like to have a long-term solution to this that is well integrated with other features.

@destogl I can give it a try if you point me into the right direction where to start ;)

Please check ros-controls/ros2_control#652 for detailed description, and please give your feedback there on how would you like this to be integrated and described in URDF? Since you are using it, you probably have a good idea how to make this nice and straightforward to use in URDF. Then we adjust the parser to it.

@christophfroehlich christophfroehlich marked this pull request as draft November 17, 2022 06:39
Copy link
Member

@destogl destogl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christophfroehlich I think we should continue with merging this ros-controls/ros2_control#652 could take few more months, and we should not block this feature for you.

@bmagyar @ahcorde do you agree?

@destogl
Copy link
Member

destogl commented Nov 22, 2022

@christophfroehlich I hope I didn't break your idea in #154. Can you comment if that merge changes something here?

@christophfroehlich
Copy link
Contributor Author

you did ;) But I fixed this with 0d4468a already. I'm not sure if this is an elegant way, but it works at least. I'm not exactly sure what to do with the mimic joints in case of a command_mode_switch.
I had a look into ros-controls/ros2_control#652 and tried to understand how big this change would be. But as I understand now, the plan is to always mimic the position in any case, isn't it? Then we have to rewrite this part again anyways.

@christophfroehlich christophfroehlich marked this pull request as ready for review November 22, 2022 15:38
@destogl
Copy link
Member

destogl commented Nov 22, 2022

But as I understand now, the plan is to always mimic the position in any case, isn't it? Then we have to rewrite this part again anyways.

Why do you think so?

ros-controls/ros2_control#652 needs definitely multiple (breaking) changes so it can take time. If you can or would like to tackle it I can give you pointers where to start.

@roncapat
Copy link

Any updates on this?

@christophfroehlich
Copy link
Contributor Author

Unfortunatley not yet, I never started with ros-controls/ros2_control#652
Do you have a use case for this change?

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.

5 participants