search

UniversalRobots / Universal_Robots_ROS2_Description

ROS2 URDF description for Universal Robots
BSD 3-Clause "New" or "Revised" License
110 stars 107 forks source link

Add acceleration limits to all robot configs #62

Closed sea-bass closed 1 year ago

sea-bass commented 1 year ago

Based on the discussion in https://github.com/UniversalRobots/Universal_Robots_ROS2_Description/issues/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 https://github.com/UniversalRobots/Universal_Robots_ROS2_Driver/pull/639 so the joint limits are loaded from the config file.

Closes https://github.com/UniversalRobots/Universal_Robots_ROS2_Description/issues/46

fmauch commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Closing this, since https://github.com/UniversalRobots/Universal_Robots_ROS2_Driver/pull/645 was merged.

fmauch commented 1 year ago

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

sea-bass commented 1 year ago

@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 https://github.com/UniversalRobots/Universal_Robots_ROS2_Description/pull/63