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

bullet-featherstone: support off-diagonal inertia #544

Merged
merged 3 commits into from
Nov 9, 2023

Conversation

scpeters
Copy link
Member

🦟 Bug fix

Allows loading models with off-diagonal inertia terms in bullet-featherstone

Summary

Currently gz-physics prints error messages and fails to load portions of models that contain links with off-diagonal inertia terms with magnitude larger than 0.001.

The bullet APIs expect the moment of inertia matrix to already be diagonalized. This changes the bullet-featherstone plugin to compute the principal moments of inertia in the same manner as the bullet plugin (see bullet's SDFFeatures.cc). It will now load models with off-diagonal inertia values without giving errors.

I'd really like to add a test for this, but I'm having trouble figuring out a simple test.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

The bullet APIs expect the moment of inertia matrix
to already be diagonalized. This changes the
bullet-featherstone plugin to compute the principal
moments of inertia in the same manner as the bullet
plugin. It will now load models with off-diagonal
inertia values without giving errors.

Signed-off-by: Steve Peters <[email protected]>
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Sep 20, 2023
@codecov
Copy link

codecov bot commented Sep 20, 2023

Codecov Report

Merging #544 (ee23790) into gz-physics6 (1260ef2) will increase coverage by 0.14%.
The diff coverage is 100.00%.

❗ Current head ee23790 differs from pull request most recent head 2fe5481. Consider uploading reports for the commit 2fe5481 to get more accurate results

@@               Coverage Diff               @@
##           gz-physics6     #544      +/-   ##
===============================================
+ Coverage        77.02%   77.16%   +0.14%     
===============================================
  Files              143      143              
  Lines             7321     7309      -12     
===============================================
+ Hits              5639     5640       +1     
+ Misses            1682     1669      -13     
Files Coverage Δ
bullet-featherstone/src/SDFFeatures.cc 76.30% <100.00%> (+2.15%) ⬆️

@scpeters
Copy link
Member Author

scpeters commented Nov 9, 2023

I've been working on a test for this, and while I'm not ready to add one to this branch, I've done enough testing that I'm confident enough about this change to merge once this has an approval. I will post the branches I have been using for testing once I have a chance to clean them up.

cc @iche033

@scpeters scpeters added the Bullet Bullet engine label Nov 9, 2023
@scpeters scpeters merged commit cf74036 into gz-physics6 Nov 9, 2023
8 checks passed
@scpeters scpeters deleted the scpeters/bullet_offdiagonal_inertia branch November 9, 2023 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bullet Bullet engine 🌱 garden Ignition Garden
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants