Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
bullet-featherstone: Fix bounding box for collisions with pose offset #647
bullet-featherstone: Fix bounding box for collisions with pose offset #647
Changes from 1 commit
4f35c1e
5c6ec0f
84054fa
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 97 in bullet-featherstone/src/KinematicsFeatures.cc
Codecov / codecov/patch
bullet-featherstone/src/KinematicsFeatures.cc#L95-L97
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.
Don't we have to account for
model->baseInertiaToLinkFrame
was there for .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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expanded test to cover both base and non-base links. 84054fa
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.
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.