Skip to content

Conversation

@AndyZe
Copy link
Member

@AndyZe AndyZe commented Feb 3, 2021

This sets Bullet as the exclusive collision detection algorithm and adds a comment. I believe it's not well-known that MoveIt was designed to maintain a list of collision detection algorithms and switch between them at runtime. However, this is an advanced feature that is not well-tested and I already found one bug. So I don't think we should be recommending it to the world.

The documentation on this exclusive arg is here.

@AndyZe AndyZe requested a review from j-petit February 3, 2021 23:31
Copy link
Contributor

@felixvd felixvd left a comment

Choose a reason for hiding this comment

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

I didn't know that that's possible. I can't think of many reasons why you would want to switch collision detection algorithms, so if this works it's fine with me.

Co-authored-by: Felix von Drigalski <FvDrigalski@gmail.com>
Copy link
Contributor

@j-petit j-petit 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 to me.

Copy link
Member

@davetcoleman davetcoleman left a comment

Choose a reason for hiding this comment

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

Just a thought: should this exclusive argument default to true in the main moveit repo?

@davetcoleman
Copy link
Member

@AndyZe needs clang-format run to pass CI

@AndyZe
Copy link
Member Author

AndyZe commented Mar 10, 2021

Just a thought: should this exclusive argument default to true in the main moveit repo?

I don't think it's worth changing. In MoveIt2, this argument went bye-bye completely.

@v4hn v4hn merged commit 8fd9264 into moveit:master Apr 28, 2021
Abishalini pushed a commit to Abishalini/moveit_tutorials that referenced this pull request Apr 29, 2021
Co-authored-by: Felix von Drigalski <FvDrigalski@gmail.com>
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