Fix static URDF models with fixed joints #1193
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This demonstrates a bug that prevents models with fixed joint reduction from being declared as static. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Add a boolean flag to indicate when setStaticFlag has been set, since all the other variables in SDFExtension have these flags. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
github-actions
bot
added
Gazebo 1️1️
Codecov Report
@@ Coverage Diff @@
## sdf9 #1193 +/- ##
==========================================
+ Coverage 87.94% 88.33% +0.39%
==========================================
Files 64 63 -1
Lines 10118 10075 -43
==========================================
+ Hits 8898 8900 +2
+ Misses 1220 1175 -45
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
scpeters
force-pushed
the
scpeters/urdf_static_fixed_joint
branch
from
699faa3 to
985f38a
Compare
adityapande-1995
approved these changes
Nov 4, 2022
src/SDFExtension.cc
Outdated
| @@ -74,6 +75,7 @@ SDFExtension::SDFExtension(const SDFExtension &_ge) | |||
| this->material = _ge.material; | |||
| this->visual_blobs = _ge.visual_blobs; | |||
| this->collision_blobs = _ge.collision_blobs; | |||
| this->isSetStaticFlag = _ge.isSetStaticFlag; | |||
There was a problem hiding this comment.
Can we do something about this line in codecov ?
There was a problem hiding this comment.
I just removed the SDFExtension copy constructor and destructor so the class can follow the rule of zero. That should improve coverage.
There was a problem hiding this comment.
coverage checker is green; removed 45 missed lines
Delete destructor and copy constructor. Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🦟 Bug fix
Allows URDF models with fixed joint reduction to be declared as
<static>Summary
Our URDF parser extensions allow a
<static>1</static>tag inside a<gazebo/>element (with noreferenceattribute) to declare the model as static. Currently, theSDFExtensiondata structure does not indicate whether any value for<static>was specified or not, so every<gazebo>block will cause a<static>0</static>value to be added to the resulting SDFormat model. As of #1148, every URDF fixed joint that is reduced (ie. without<preserveFixedJoint>1</preserveFixedJoint>) will add a<gazebo>block with<frame>elements, which would prevent those models from being marked as static. This is illustrated with thefixed_joint_static.urdfexample added in 2dc5c1f with a failing test.A fix is included in 6580927?w=1 (without whitespace) by adding a flag to indicate when the
<static>element has been set in anSDFExtension.Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-bymessages.