-
Notifications
You must be signed in to change notification settings - Fork 41
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 restitution coefficient support for bouncing #139
Conversation
Signed-off-by: Diego Ferigo <diego.ferigo@iit.it>
cf486f8
to
f370ea6
Compare
Closes #84 |
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 for the contribution @diegoferigo. The PR looks good. It does demonstrate the limitation of the current API that uses SDFormat objects for construction because it make it possible for two physics engine plugins to implement the Construct*
feature, but have different capabilities in reality. i.e, in this case, if there is another physics engine plugin that implements the ConstructCollision
feature, but doesn't support restitution, we now have a discrepancy.
For now though, I think we should go forward with this PR. Would it be possible for you to add a test?
@azeey Thanks for the feedback. I get back to this issue to say that lately I'm running a bit low with time, not sure if I manage to add the test in the next days.
Yes, I think this is the trade off between API complexity and flexibility. I'm not sure what it the best solution here, I just say that often the flexibility is good to have, especially in this development phase, and it can be compensated with a proper documentation so that downstream users know what they can and cannot expect. |
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.
LGTM! I took the liberty to add a small test.
Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
@osrf-jenkins run tests please |
Thanks @azeey, few weeks ago I tried to add a test but I couldn't find anything simple enough (and delayed again). I was thinking something about computing the average velocity of the spheres and checking that those with bigger restitution coefficients were higher. Though, it was a bit convoluted and in any case it was not checking against a ground truth. |
CI looks good. Merging! |
This PR adds the support of simulating bouncing using
//collision/surface/bounce/restitution_coefficient
. It calls the methods introduced in dartsim/dart#1369.This is the resulting behaviour simulating 11 spheres with a different restitution coefficient and setting the coefficient of the ground to
1.0
:Script (requires third party libraries)
This PR fills one of the gaps described in https://www.allisonthackston.com/articles/ignition_vs_gazebo.html.