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

Fix bullet-featherstone collisions #387

Conversation

mxgrey
Copy link
Contributor

@mxgrey mxgrey commented Jul 19, 2022

This PR fixes collisions for bullet-featherstone, and also updates the CMakeLists.txt to be compatible with the latest main branches of gz-cmake.

I've tested these changes with a modified shapes.sdf. The first modification is to raise up the shapes by 1.0m to make the collision behavior more clear. Doing that results in most of the shapes just dropping through the plane.

It looks to me like Bullet's btStaticPlaneShape is only able to detect collisions with boxes. I think this would qualify as an unexpected behavior that could negatively impact users, so we may want to consider replacing instances of PlaneShape with a big box instead.

When I further modify shapes.sdf to use a big box for the ground plane, I get these two results:

  1. Everything works as expected
  2. The ellipsoid drops through the ground

I haven't found any pattern for when/why the ellipsoid drops. I run the simulation with the exact same SDF repeatedly and I'd estimate the ellipsoid drops through the ground ~80% of the time. That's an awfully large false negative, so I don't think we should just ignore it. We may want to declare that ellipsoid shapes aren't supported for bullet-featherstone if we can't solve the failure to detect the collision.

mxgrey added 4 commits July 18, 2022 14:03
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
@mxgrey mxgrey requested review from mjcarroll and ahcorde July 19, 2022 07:24
@mxgrey mxgrey requested review from azeey and scpeters as code owners July 19, 2022 07:24
@codecov
Copy link

codecov bot commented Jul 19, 2022

Codecov Report

❗ No coverage uploaded for pull request base (ahcorde/add-bullet-featherstone/shapesFeatures@90f4701). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 4e7178d differs from pull request most recent head bf8f0f0. Consider uploading reports for the commit bf8f0f0 to get more accurate results

@@                                Coverage Diff                                @@
##             ahcorde/add-bullet-featherstone/shapesFeatures     #387   +/-   ##
=================================================================================
  Coverage                                                  ?   67.15%           
=================================================================================
  Files                                                     ?      138           
  Lines                                                     ?     6469           
  Branches                                                  ?        0           
=================================================================================
  Hits                                                      ?     4344           
  Misses                                                    ?     2125           
  Partials                                                  ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90f4701...bf8f0f0. Read the comment docs.

Copy link
Contributor

@ahcorde ahcorde 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 minor comment, if we merge this PR we should open an issue with the ellipsoid issue.

double mu = 1.0;
double mu2 = 1.0;
double restitution = 0.0;
// double mu = 1.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I was experimenting with the default parameters to see how they impact the collision behavior. I've returned them back to their default values here 862411b

Signed-off-by: Michael X. Grey <grey@openrobotics.org>
@mxgrey
Copy link
Contributor Author

mxgrey commented Jul 20, 2022

Thanks for the review, @ahcorde . I've addressed your inline comment and opened an issue for this: #389

@ahcorde ahcorde merged commit 87d434a into ahcorde/add-bullet-featherstone/shapesFeatures Jul 21, 2022
@ahcorde ahcorde deleted the bullet-featherstone-collisions branch July 21, 2022 11:54
mjcarroll added a commit that referenced this pull request Sep 16, 2022
* Added Shapes Features to bullet-featherstone
* make linters happy
* Improve implementation
* make linters happy
* Fix bullet-featherstone collisions (#387)
* Fixed collision issues
* Improved joints in bullet featherstone

Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: Michael X. Grey <grey@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Co-authored-by: Michael X. Grey <grey@openrobotics.org>
Co-authored-by: Michael Carroll <michael@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants