Check joint parent/child names in Root::Load #727
Conversation
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
include/sdf/parser.hh
Outdated
| /// \param[in] _root SDF Root object to check recursively. | ||
| /// \param[out] _errors Detected errors will be appended to this variable. | ||
| SDFORMAT_VISIBLE | ||
| void checkJointParentChildLinkNames(const sdf::Root *_root, Errors &_errors); |
There was a problem hiding this comment.
now that I think about it, we could change this function name since parent and child aren't just links anymore
-
checkJointParentChildNames -
checkJointParentChildFrameNames
There was a problem hiding this comment.
Can they be frames or links? If so, I think checkJointParentChildNames would be better and the error messages should be updated to be "link or frame" (and not one or the other)
There was a problem hiding this comment.
the parent and child names can refer to a //link, //joint, //frame, or //model, so anything with a frame in the attached-to graph. I'll change to checkJointParentChildNames
Codecov Report
@@ Coverage Diff @@
## sdf11 #727 +/- ##
==========================================
+ Coverage 88.09% 88.21% +0.11%
==========================================
Files 73 73
Lines 11030 11021 -9
==========================================
+ Hits 9717 9722 +5
+ Misses 1313 1299 -14
Continue to review full report at Codecov.
|
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
| @@ -322,6 +322,17 @@ TEST(check, IGN_UTILS_TEST_DISABLED_ON_WIN32(SDF)) | |||
| EXPECT_EQ("Valid.\n", output) << output; | |||
| } | |||
|
|
|||
| // Check an SDF file with the infinite values for joint axis limits. | |||
There was a problem hiding this comment.
Not sure if we need to add this test if it's covered by other tests.
There was a problem hiding this comment.
it helps with coverage of ign sdf --check; I think it doesn't hurt anything except test runtime. I would keep it personally
|
This pull request has been mentioned on Gazebo Community. There might be relevant details there: https://community.gazebosim.org/t/new-ignition-releases-2022-03-25-fortress-edifice-citadel/1343/1 |
🦟 Bug fix
Fixes #719 for
sdf11, similar to #726Summary
As noted in #710, several example files in the
test/sdf/folder specify non-existent links in the//joint/parentelement, which violates the spec. I added anign sdf --check test/sdf/joint_axis_infinite_limits.sdftest case in 893f2c8, which properly notes the failure, though thesdf::Root::Loadcall inINTEGRATION_joint_axis_domdoes not report an error. To detect the error insdf::Root::Load, I first added a version ofcheckJointParentChildLinkNamesthat outputs ansdf::Errorstoparser.hh(49b432b) and then call that function fromRoot::Load(1d21294). This goes beyond the change in #726 by checking that each joint's parent and child frames resolve correctly. Finally, I fixed thetest/sdf/files in 08ac2b9 and made a minor fix to an error message in 628f382.This will likely reduce coverage due to redundancy in our error checking.
Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge