bullet-featherstone: Fix bounding box for collisions with pose offset #647
Conversation
Signed-off-by: Ian Chen <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## gz-physics7 #647 +/- ##
===============================================
+ Coverage 78.32% 79.06% +0.74%
===============================================
Files 140 140
Lines 8069 8235 +166
===============================================
+ Hits 6320 6511 +191
+ Misses 1749 1724 -25 ☔ View full report in Codecov by Sentry. |
| { | ||
| data.pose = convert(model->body->getBaseWorldTransform()) | ||
| * model->baseInertiaToLinkFrame; | ||
| data.pose = convert(model->body->getBaseWorldTransform()); |
There was a problem hiding this comment.
Don't we have to account for
- The offset from the model to the base link, i.e, the links pose in SDF? Do we have a test that covers this?
- The offset of the link frame to the center of mass? I thought that's what
model->baseInertiaToLinkFramewas there for .
There was a problem hiding this comment.
This line computes the pose for the model. The next 2 lines takes into account the pose of the link (and inertia offset if any) which uses model->baseInertiaToLinkFrame. I think that's correct? I added a test in 5c6ec0f that checks the frame data for a link and a collision, both of which have a pose offset.
There was a problem hiding this comment.
expanded test to cover both base and non-base links. 84054fa
There was a problem hiding this comment.
It looks like this test is not testing model frame semantics at all. Fortunately, we currently don't use it in gz-sim, but it would be good to fix (not in this PR). I've added a failing test in the azeey/kinematic_feature_failing_test branch. Evidently, dartsim fails the test as well.
Signed-off-by: Ian Chen <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
| { | ||
| data.pose = convert(model->body->getBaseWorldTransform()) | ||
| * model->baseInertiaToLinkFrame; | ||
| data.pose = convert(model->body->getBaseWorldTransform()); |
There was a problem hiding this comment.
It looks like this test is not testing model frame semantics at all. Fortunately, we currently don't use it in gz-sim, but it would be good to fix (not in this PR). I've added a failing test in the azeey/kinematic_feature_failing_test branch. Evidently, dartsim fails the test as well.
|
Thanks for adding the test. I'm tracking all frame data issues in #648 |
Follow-up PR: #649 |
🦟 Bug fix
hide whitespace changes for better diff:
https://github.com/gazebosim/gz-physics/pull/647/files?w=1
Summary
Currently if a collision has a pose offset, the bullet-featherstone plugin return an incorrect axis aligned bounding box for the link containing the collision. This is found to be caused by incorrect frame data pose computed for collisions - It currently ignores the collision pose offset.
This PR makes a few changes to
KinematicsFeatures::FrameDataRelativeToWorldin bullet-featherstone plugin:The link_features's bounding box test was previously skipped for bullet-featherstone plugin. This PR enables this test for bullet-featherstone by requiring only a minimal set of features needed to run the test.
Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-bymessages.