-
Notifications
You must be signed in to change notification settings - Fork 292
Fix: Validate SphereShape radius to prevent assertion failures (DART 6.16) #2441
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: Validate SphereShape radius to prevent assertion failures (DART 6.16) #2441
Conversation
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef60a68990
ℹ️ 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".
a1a90aa to
8677786
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 @@
## release-6.16 #2441 +/- ##
================================================
+ Coverage 58.74% 58.81% +0.07%
================================================
Files 385 385
Lines 32194 32201 +7
Branches 3884 3888 +4
================================================
+ Hits 18911 18939 +28
+ Misses 13283 13262 -21
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Replace DART_ASSERT with validation that rejects NaN, Infinity, and non-positive radius values with a warning instead of crashing. Fixes: gazebosim/gz-physics#843
69ccb9e to
a9db95d
Compare
|
@codex review This PR branch is up to date with release-6.16. The validation approach using |
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ 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.Changes
dart/dynamics/SphereShape.cpp: Added validation with graceful rejectiontests/unit/dynamics/test_SphereShape.cpp: New test file with 5 test casestests/unit/dynamics/CMakeLists.txt: New CMake config for dynamics teststests/unit/CMakeLists.txt: Added dynamics subdirectoryCHANGELOG.md: Added entry under DART 6.16.5Testing
World::setTimeStep()