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

Update acceleration limits for robot configs #195

Merged
merged 3 commits into from
Jan 8, 2024

Conversation

pac48
Copy link
Contributor

@pac48 pac48 commented Dec 27, 2023

See #194 for more detail. I think some discussion on how to manage different sets of joint_limits.yaml could be beneficial.

Signed-off-by: Paul Gesel <[email protected]>
@pac48 pac48 changed the title Update acceleration limits in for robot configs Update acceleration limits for robot configs Dec 28, 2023
Copy link
Contributor

@sjahr sjahr left a comment

Choose a reason for hiding this comment

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

Do you mind adding a comment why has_jerk_limits: false for all joints in a file called joint_limits_jerk_limited? I think that might be confusing otherwise

@pac48
Copy link
Contributor Author

pac48 commented Jan 4, 2024

@sjahr That is a good point. The problem is that joint_limits.yaml was created before MoveIt supported jerk limits. So the acceleration limits were intentionally low to implicitly enforce jerk constraints. Now that there is some support for jerk constraints, it is reasonable to remove the artificially low acceleration limits in some configurations. The idea was to make a new config with the actual hardware acceleration values. I guess the name is not very clear. Do you have any suggestions? There is some discussion in this issue: #194

@pac48
Copy link
Contributor Author

pac48 commented Jan 5, 2024

Do you mind adding a comment why has_jerk_limits: false for all joints in a file called joint_limits_jerk_limited? I think that might be confusing otherwise

@sjahr What if we call it hard_joint_limits.yaml It looks like this is already a convention in here

@sjahr
Copy link
Contributor

sjahr commented Jan 5, 2024

👍 Yes, I like that. Let's do it

Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

Looks good.

So the plan is to use this new file for our servo examples going forward, then?

@pac48
Copy link
Contributor Author

pac48 commented Jan 7, 2024

Looks good.

So the plan is to use this new file for our servo examples going forward, then?

Yes, we can use these limits in servo for better control.

@sjahr sjahr merged commit 53da532 into moveit:ros2 Jan 8, 2024
5 checks passed
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.

3 participants