Draft: Parser warning message when <gazebo> reference does not exist in URDF #1392

aagrawal05 · 2024-04-05T00:39:56Z

🎉 New feature

Summary

Added functionality to emit a warning when a reference to a link does not exists by adding a check in the InsertSDFExtensionLink method.

Test it

I've added the ParseGazeboRefDoesntExistWarningMessage test which adds the test case mentioned from #1372 with conflicting U+0069 and U+0456 characters.

Checklist

Signed all commits for DCO
Added tests
Added example and/or tutorial
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

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

ahcorde

If you check the diff, the indentation in your changes is very wierd. Do you mind to fix it ?

aagrawal05 · 2024-04-05T08:33:16Z

If you check the diff, the indentation in your changes is very wierd. Do you mind to fix it ?

Hi, I took a look at it—I think there was something wrong with my text editor, it showed a different output and I hadn't noticed earlier. Let me fix it now.

aagrawal05 · 2024-04-05T09:03:23Z

Ok, sorry about the many commits—I think the formatting should be fixed now. I'll make sure everything fits with rigorously with codecheck afterwards.

Signed-off-by: Aditya Agrawal <agraw17107@gapps.uwcsea.edu.sg>

Signed-off-by: Aditya Agrawal <58559719+aagrawal05@users.noreply.github.com> Signed-off-by: Aditya Agrawal <agraw17107@gapps.uwcsea.edu.sg>

azeey · 2024-08-06T03:53:26Z

@traversaro would you be able to take a look at this?

traversaro · 2024-08-06T08:08:22Z

Thanks @aagrawal05 ! I guess it could make sense to do something similar with joints as well, but even just with links this PR is already quite useful.

aagrawal05 · 2024-08-15T08:43:59Z

Thanks @aagrawal05 ! I guess it could make sense to do something similar with joints as well, but even just with links this PR is already quite useful.

Thanks @traversaro, I will take a look into it. Please let me know if anything further is to be done.

azeey · 2024-08-15T14:32:18Z

@aagrawal05 did you mean to close this PR?

aagrawal05 · 2024-08-16T00:09:35Z

@aagrawal05 did you mean to close this PR?

Whoops yeah sorry, I must have clicked the wrong button!

aagrawal05 · 2024-08-16T11:52:21Z

Ok I've added the other extensions and tests, please take a look.

Signed-off-by: Aditya Agrawal <agraw17107@gapps.uwcsea.edu.sg>

src/parser_urdf.cc

src/parser_urdf_TEST.cc

Signed-off-by: Aditya Agrawal <agraw17107@gapps.uwcsea.edu.sg>

aagrawal05 · 2024-08-17T09:25:11Z

Ok all the above changes should be addressed—I was mistaken about how the reference works.

This passes all the tests now and should be ready. Please let me know if anything further is needed, @azeey.

Signed-off-by: Aditya Agrawal <agraw17107@gapps.uwcsea.edu.sg>

azeey · 2024-08-19T22:46:20Z

src/parser_urdf_TEST.cc

+ sdf::ParserConfig config;
+ parser.InitModelString(str, config, &sdfResult);
+
+ EXPECT_PRED2(sdf::testing::contains, buffer.str(),


Can you add a comment here why the two link1s are not the same? I believe the і (https://www.compart.com/en/unicode/U+0456) in <gazebo reference="lіnk1"> is the issue.

azeey · 2024-08-19T22:52:05Z

src/parser_urdf_TEST.cc

+ parser.InitModelString(str, config, &sdfResult);
+
+ EXPECT_PRED2(sdf::testing::contains, buffer.str(),
+ "<link> tag reference[link1] does not exist"


Shouldn't the error message reference visual1?

Suggested change

"<link> tag reference[link1] does not exist"

"<link> tag reference[visual1] does not exist"

Yep looking into this. I'be tried using _elem->Attribute("name") but that doesn't give it either-often giving the link or something different. Similarly the sdfVisualName doesn't seem correct. I'm tracing the code to see if there's an easy way to get the refString from parseURDF but it's taking a bit longer because of my unfamiliarity with the codebase. Please let me know if my approach is correct.

azeey · 2024-08-19T22:52:19Z

src/parser_urdf_TEST.cc

+ parser.InitModelString(str, config, &sdfResult);
+
+ EXPECT_PRED2(sdf::testing::contains, buffer.str(),
+ "<link> tag reference[link1] does not exist"


Suggested change

"<link> tag reference[link1] does not exist"

"<link> tag reference[collision1] does not exist"

azeey · 2024-08-19T22:53:08Z

src/parser_urdf_TEST.cc

+ sdf::ParserConfig config;
+ parser.InitModelString(str, config, &sdfResult);
+
+ EXPECT_PRED2(sdf::testing::contains, buffer.str(),


Can you comment why the two joint1s are different?

azeey

Some syntax errors in the URDF.

azeey · 2024-08-21T06:12:03Z

src/parser_urdf_TEST.cc

+ <box>
+ <size>1 1 1</size>
+ </box>


Suggested change

<box>

<size>1 1 1</size>

</box>

<box size="1 1 1"/>

Yep, will add to next commit

azeey · 2024-08-21T06:12:14Z

src/parser_urdf_TEST.cc

+ <size>1 1 1</size>
+ </box>
+ </geometry>
+ <material>


Suggested change

<material>

<material name="mat1">

azeey · 2024-08-21T06:19:42Z

src/parser_urdf_TEST.cc

+ "<joint> tag reference[joint1] does not exist"
+ " in the URDF model. Please ensure that the reference attribute"
+ " matches the name of a joint.");
+ }


I've noticed with this PR that even properly working URDFs print a warning message. Do you mind adding a test with a URDF that has a correct reference and check that there is no warning printed.

Yep ran into this while addressing the previous point. Will do.

aagrawal05 · 2024-09-09T10:12:29Z

Apologies for the delay in resolving this PR. Finding a solution is taking longer than expected and I am currently traveling. I will try to push my changes by the end of this week.

aagrawal05 requested review from azeey and scpeters as code owners April 5, 2024 00:39

github-actions bot added the 🎵 harmonic Gazebo Harmonic label Apr 5, 2024

ahcorde requested changes Apr 5, 2024

View reviewed changes

aagrawal05 force-pushed the parser_ref_warning_message branch from 22e332a to e538eb2 Compare April 5, 2024 08:30

aagrawal05 and others added 5 commits April 6, 2024 11:27

initial draft of warning message functionality

640766f

Signed-off-by: Aditya Agrawal <agraw17107@gapps.uwcsea.edu.sg>

fixed formatting

ff06de3

Signed-off-by: Aditya Agrawal <agraw17107@gapps.uwcsea.edu.sg>

Formatting changes parser_urdf_TEST.cc

bff4f75

Signed-off-by: Aditya Agrawal <58559719+aagrawal05@users.noreply.github.com> Signed-off-by: Aditya Agrawal <agraw17107@gapps.uwcsea.edu.sg>

Formatting changes in parser_urdf.cc

d9f56f5

Signed-off-by: Aditya Agrawal <58559719+aagrawal05@users.noreply.github.com> Signed-off-by: Aditya Agrawal <agraw17107@gapps.uwcsea.edu.sg>

Update parser_urdf_TEST.cc and fixed mistake in test

2d221fe

Signed-off-by: Aditya Agrawal <58559719+aagrawal05@users.noreply.github.com> Signed-off-by: Aditya Agrawal <agraw17107@gapps.uwcsea.edu.sg>

aagrawal05 force-pushed the parser_ref_warning_message branch from ab47435 to 2d221fe Compare April 6, 2024 03:27

aagrawal05 requested a review from ahcorde April 8, 2024 12:44

azeey added the beta Targeting beta release of upcoming collection label Jul 29, 2024

traversaro approved these changes Aug 6, 2024

View reviewed changes

aagrawal05 closed this Aug 15, 2024

aagrawal05 reopened this Aug 16, 2024

added other SDFExtensions

d610aa9

Signed-off-by: Aditya Agrawal <agraw17107@gapps.uwcsea.edu.sg>

aagrawal05 force-pushed the parser_ref_warning_message branch from 58b71c9 to d610aa9 Compare August 16, 2024 11:52

azeey requested changes Aug 16, 2024

View reviewed changes

azeey and others added 2 commits August 16, 2024 15:54

Merge branch 'sdf14' into parser_ref_warning_message

47f9c1f

fixed error messages, tests, and style

8046a01

Signed-off-by: Aditya Agrawal <agraw17107@gapps.uwcsea.edu.sg>

style

7f7b183

Signed-off-by: Aditya Agrawal <agraw17107@gapps.uwcsea.edu.sg>

azeey reviewed Aug 20, 2024

View reviewed changes

azeey reviewed Aug 21, 2024

View reviewed changes

azeey removed the beta Targeting beta release of upcoming collection label Sep 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draft: Parser warning message when <gazebo> reference does not exist in URDF #1392

Draft: Parser warning message when <gazebo> reference does not exist in URDF #1392

aagrawal05 commented Apr 5, 2024 •

edited

Loading

ahcorde left a comment

aagrawal05 commented Apr 5, 2024 •

edited

Loading

aagrawal05 commented Apr 5, 2024 •

edited

Loading

azeey commented Aug 6, 2024

traversaro commented Aug 6, 2024

aagrawal05 commented Aug 15, 2024

azeey commented Aug 15, 2024

aagrawal05 commented Aug 16, 2024

aagrawal05 commented Aug 16, 2024

aagrawal05 commented Aug 17, 2024 •

edited

Loading

azeey Aug 19, 2024

azeey Aug 19, 2024

aagrawal05 Aug 21, 2024

azeey Aug 19, 2024

azeey Aug 19, 2024

azeey left a comment

azeey Aug 21, 2024

aagrawal05 Aug 21, 2024

azeey Aug 21, 2024

azeey Aug 21, 2024

aagrawal05 Aug 21, 2024

aagrawal05 commented Sep 9, 2024

	"<link> tag reference[link1] does not exist"
	"<link> tag reference[visual1] does not exist"

	"<link> tag reference[link1] does not exist"
	"<link> tag reference[collision1] does not exist"

Draft: Parser warning message when <gazebo> reference does not exist in URDF #1392

Are you sure you want to change the base?

Draft: Parser warning message when <gazebo> reference does not exist in URDF #1392

Conversation

aagrawal05 commented Apr 5, 2024 • edited Loading

🎉 New feature

Summary

Test it

Checklist

ahcorde left a comment

Choose a reason for hiding this comment

aagrawal05 commented Apr 5, 2024 • edited Loading

aagrawal05 commented Apr 5, 2024 • edited Loading

azeey commented Aug 6, 2024

traversaro commented Aug 6, 2024

aagrawal05 commented Aug 15, 2024

azeey commented Aug 15, 2024

aagrawal05 commented Aug 16, 2024

aagrawal05 commented Aug 16, 2024

aagrawal05 commented Aug 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

azeey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aagrawal05 commented Sep 9, 2024

aagrawal05 commented Apr 5, 2024 •

edited

Loading

aagrawal05 commented Apr 5, 2024 •

edited

Loading

aagrawal05 commented Apr 5, 2024 •

edited

Loading

aagrawal05 commented Aug 17, 2024 •

edited

Loading