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 initialization of world components when enabling velocity checks (backport #2777) #2811

Merged
merged 1 commit into from
Mar 7, 2025

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Mar 6, 2025

🦟 Bug fix

Fixes #2774

Summary

When using the link API to enable velocity checks for a link that does not yet have the world ECM components (WorldPose, WorldLinearVelocity, WorldAngularVelocity), typically before the physics system plugin does its first update loop, such components will be initialized with their default constructor, zeroing values that might not be zero. One more evident example is the link world pose that could have already been initialized.

  1. Add the same unit test added by this MR to gz-sim/src/Link_TEST.cc
  2. Build the package: colcon build --packages-select gz-sim9
  3. Execute the test from your build folder: ./bin/UNIT_Link_TEST --gtest-filter=*EnableVelocityChecks*
  4. You should verify the tests will fail because all world components are zero

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.


This is an automatic backport of pull request #2777 done by [Mergify](https://mergify.com).

@mergify mergify bot requested a review from mjcarroll as a code owner March 6, 2025 16:59
@mergify mergify bot added the conflicts label Mar 6, 2025
Copy link
Contributor Author

mergify bot commented Mar 6, 2025

Cherry-pick of 4ae75e5 has failed:

On branch mergify/bp/gz-sim8/pr-2777
Your branch is up to date with 'origin/gz-sim8'.

You are currently cherry-picking commit 4ae75e5b.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   include/gz/sim/Util.hh
	modified:   src/Link.cc
	modified:   src/Link_TEST.cc
	modified:   src/Util_TEST.cc

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   test/integration/mesh_inertia_calculation.cc

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@gabrielfpacheco
Copy link
Contributor

gabrielfpacheco commented Mar 7, 2025

@scpeters, thanks for triggering the automatic creation of this backport PR. There is a simple conflict to be solved in test/integration/mesh_inertial_calculation.cc, though.

How should we proceed in this case? I don't have the permission to update this branch, but I can create a new PR if this is the right process.

…2777)

Retain already initialized world component values when
enabling velocity checks and adjust expectations
accordingly in mesh_inertia_calulation integration tests.

Signed-off-by: Gabriel Pacheco <[email protected]>
Co-authored-by: Gabriel Pacheco <[email protected]>
(cherry picked from commit 4ae75e5)
@scpeters scpeters force-pushed the mergify/bp/gz-sim8/pr-2777 branch from cc3c040 to 21d6c7d Compare March 7, 2025 18:43
@scpeters
Copy link
Member

scpeters commented Mar 7, 2025

@scpeters, thanks for triggering the automatic creation of this backport PR. There is a simple conflict to be solved in test/integration/mesh_inertial_calculation.cc, though.

How should we proceed in this case? I don't have the permission to update this branch, but I can create a new PR if this is the right process.

I resolved conflicts and amended the commit

@scpeters scpeters merged commit ff6c047 into gz-sim8 Mar 7, 2025
10 checks passed
@scpeters scpeters deleted the mergify/bp/gz-sim8/pr-2777 branch March 7, 2025 21:32
@gabrielfpacheco
Copy link
Contributor

Thanks a lot, guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants