Skip to content

Conversation

@XINJIANGMO
Copy link
Contributor

🦟 Bug fix

Fixes #2981

Summary

Gazebo allows loading static models without any links. However, if a /world/<world>/set_pose service call is made targeting such a model, the physics system crashes , because the underlying physics code assumes every model has at least one link.
After Fix : Before executing set_pose, the code now checks whether the target model contains at least one link.

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

@XINJIANGMO XINJIANGMO requested a review from azeey as a code owner July 22, 2025 04:01
@github-actions github-actions bot added the 🏛️ ionic Gazebo Ionic label Jul 22, 2025
Signed-off-by: momo <[email protected]>
@github-project-automation github-project-automation bot moved this from Inbox to In review in Core development Jul 22, 2025
@codecov
Copy link

codecov bot commented Jul 22, 2025

Codecov Report

Attention: Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 68.95%. Comparing base (5aa528a) to head (8fc6f45).
Report is 5 commits behind head on gz-sim9.

Files with missing lines Patch % Lines
src/systems/physics/Physics.cc 33.33% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           gz-sim9    #2988      +/-   ##
===========================================
- Coverage    68.97%   68.95%   -0.03%     
===========================================
  Files          345      345              
  Lines        33538    33572      +34     
===========================================
+ Hits         23134    23148      +14     
- Misses       10404    10424      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

iche033
iche033 previously approved these changes Jul 25, 2025
@azeey azeey added this to the Jetty Release milestone Jul 28, 2025
@iche033 iche033 dismissed their stale review July 31, 2025 01:04

this stops set_pose from working with nested models

@iche033
Copy link
Contributor

iche033 commented Jul 31, 2025

found that this changes in this PR stop set_pose from working with nested models, e.g.

    <!-- set_pose should work with this nested model -->
    <model name="box">
      <pose>0 0 2.5 0 0 0</pose>
      <static>true</static>
      <model name="inner_box">
        <pose>0 0 0.0 0 0 0</pose>
        <link name="link"/>
      </model>
    </model>

Did some debugging and noticed that the crash happens only in dartsim when a nested model tree has no links. A simple model (non-nested) with no links but cause the crash. I found the root cause and created a fix in gazebosim/gz-physics#765.

@XINJIANGMO
Copy link
Contributor Author

Thank you for your correcting ! My PR really doesn't take nested models into account. Your fix is right !
In next few months, I'll continue to find and fix crashes for Gazebo, and let me know if you have any suggestions.

@iche033
Copy link
Contributor

iche033 commented Jul 31, 2025

Thank you for your correcting ! My PR really doesn't take nested models into account. Your fix is right ! In next few months, I'll continue to find and fix crashes for Gazebo, and let me know if you have any suggestions.

great thanks for all your contributions!

@azeey azeey merged commit aab29bc into gazebosim:gz-sim9 Aug 7, 2025
14 checks passed
@github-project-automation github-project-automation bot moved this from In review to Done in Core development Aug 7, 2025
@iche033
Copy link
Contributor

iche033 commented Aug 8, 2025

oh this PR stops set_pos from working with a nested model, see #2988 (comment). Can we revert this?

@azeey
Copy link
Contributor

azeey commented Aug 8, 2025

Oh, sorry. Please go ahead and revert.

iche033 added a commit that referenced this pull request Aug 8, 2025
iche033 added a commit that referenced this pull request Aug 8, 2025
@iche033
Copy link
Contributor

iche033 commented Aug 8, 2025

Oh, sorry. Please go ahead and revert.

created #3023 to revert this change

iche033 added a commit that referenced this pull request Aug 9, 2025
@XINJIANGMO XINJIANGMO deleted the fixbug-2981 branch September 17, 2025 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏛️ ionic Gazebo Ionic

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Crash on set_pose when included model (via <include>) has no link element

4 participants