Fix: Validate SphereShape radius to prevent assertion failures (DART 7) #2442
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d3a483603
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
jslee02
changed the title
jslee02
force-pushed
the
fix/gz-physics-843-sphere-nan-validation
branch
from
fcaa0bd to
d822d4a
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2442 +/- ##
=======================================
Coverage 66.49% 66.49%
=======================================
Files 407 407
Lines 37254 37256 +2
Branches 4901 4902 +1
=======================================
+ Hits 24772 24774 +2
Misses 12482 12482
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
jslee02
force-pushed
the
fix/gz-physics-843-sphere-nan-validation
branch
from
9ec25b5 to
e5dbe9e
Compare
Replace DART_ASSERT(radius > 0.0) with proper validation using std::isfinite() and DART_WARN(). Invalid values (NaN, infinity, non-positive) are now rejected with a warning instead of crashing. This addresses gz-physics#843 where DART crashes when SphereShape receives invalid radius values from upstream physics interfaces. Includes unit tests for: - Valid radius acceptance - NaN rejection - Infinity rejection - Non-positive value rejection
jslee02
force-pushed
the
fix/gz-physics-843-sphere-nan-validation
branch
from
e5dbe9e to
2389de2
Compare
|
@codex review This PR has been rebased on main to incorporate the new error handling infrastructure from #2445. The validation approach using |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
DART_ASSERT(radius > 0.0)with proper validation usingstd::isfinite()andDART_WARN()Motivation
Addresses gz-physics#843 where DART crashes with an assertion failure when
SphereShape::setRadius()receives invalid radius values from upstream physics interfaces.Related PR
Changes
dart/dynamics/SphereShape.cpp: Added validation with graceful rejectiontests/unit/dynamics/test_SphereShape.cpp: New test file with 5 test casestests/unit/CMakeLists.txt: Added SphereShape test entryCHANGELOG.md: Added entry under Collision and Geometry sectionTesting
World::setTimeStep()