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

Add acceleration limits to all robot configs #62

Closed
wants to merge 2 commits into from

Conversation

sea-bass
Copy link
Contributor

@sea-bass sea-bass commented Apr 24, 2023

Based on the discussion in #46, MoveIt 2 now requires defined acceleration limits for the Time-Optimal Trajectory Generation (TOTG) adapter to function correctly.

This adds acceleration limits of 5.0 rad/s^2 to all joints, which I'm happy to change if we have something more up-to-date to use.

I also rearranged the structure of the limits to be a bit easier to read and modify, and submitted UniversalRobots/Universal_Robots_ROS2_Driver#639 so the joint limits are loaded from the config file.

Closes #46

@fmauch
Copy link
Contributor

fmauch commented Apr 25, 2023

Thank you, this is very much appreciated! We've been discussing this internally already and this was one of the next things we wanted to tackle. However, we came to the conclusion that this should not be part of the description, but of the MoveIt! config for the following reason:

As discussed in #46, the robot itself does not have any specific acceleration limits, but rather torque limits. The acceleration that can be achieved depends very much on the actual joint configuration the robot is currently in. Since a fixed (conservative) acceleration limit is required by MoveIt! it should be specified inside the MoveIt! configuration directly. Hence, our preferred approach would be to add the limits inside the ur_moveit_config package similar to here.

@sea-bass
Copy link
Contributor Author

This seems good, and I tested the alternative PR. Worked great.

Shall we close this PR, or do you want any of the parameter rearranging (but leave the accel limits to false)?

@fmauch
Copy link
Contributor

fmauch commented Apr 26, 2023

Closing this, since UniversalRobots/Universal_Robots_ROS2_Driver#645 was merged.

@fmauch fmauch closed this Apr 26, 2023
@fmauch
Copy link
Contributor

fmauch commented Apr 27, 2023

@sea-bass I was told, that I apparently ignored your second sentence, sorry for that. Yes, the reordering would make sense, indeed.

@sea-bass sea-bass mentioned this pull request Apr 27, 2023
@sea-bass
Copy link
Contributor Author

@sea-bass I was told, that I apparently ignored your second sentence, sorry for that. Yes, the reordering would make sense, indeed.

Ah no worries, without the acceleration limits it's purely cosmetic. If you want these changes, I opened #63

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.

No acceleration limits given in joint_limits.yaml
2 participants