-
Notifications
You must be signed in to change notification settings - Fork 540
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 use_padding flag + deprecate checkCollisionUnpadded() functions #3088
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just small comments. But my first one on variable naming is probably the more important.
Also, this change is probably worth adding to the corresponding CHANGELOG.rst
moveit_core/collision_detection/include/moveit/collision_detection/collision_common.h
Outdated
Show resolved
Hide resolved
moveit_core/planning_scene/include/moveit/planning_scene/planning_scene.h
Outdated
Show resolved
Hide resolved
Can you also add to the CHANGELOG.rst? |
@sea-bass Sorry, missed that but now it's done ✔️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Sebastian, nice cleanup!
Just a couple of questions.
moveit_core/planning_scene/include/moveit/planning_scene/planning_scene.h
Outdated
Show resolved
Hide resolved
Sorry @sjahr -- I was putting together a Humble release and realized it's not CHANGELOG.rst that needs modification, as that is put together by Could you move that stuff to Also changes LGTM, but will let @marioprats have the approval since he still had some open questions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Sebastian, just a nit to update the comment, since the PR changes the check to return early
Description
I wanted to perform a padded self-collision check with a robot state and realized that this is not possible with the current API. So I decided to enable it + clean-up the checkCollision API of the planning scene a bit. This PR:
Checklist