sdformat icon indicating copy to clipboard operation
sdformat copied to clipboard

Published 20 hours ago • verified publisher icon gazebosim

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

Open aagrawal05 opened this issue 10 months ago • 10 comments

🎉 New feature

Closes #1372

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
  • [x] 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)
  • [x] 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.

aagrawal05 avatar Apr 05 '24 00:04 aagrawal05

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 avatar Apr 05 '24 08:04 aagrawal05

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.

aagrawal05 avatar Apr 05 '24 09:04 aagrawal05

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

azeey avatar Aug 06 '24 03:08 azeey

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.

traversaro avatar Aug 06 '24 08:08 traversaro

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.

aagrawal05 avatar Aug 15 '24 08:08 aagrawal05

@aagrawal05 did you mean to close this PR?

azeey avatar Aug 15 '24 14:08 azeey

@aagrawal05 did you mean to close this PR?

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

aagrawal05 avatar Aug 16 '24 00:08 aagrawal05

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

aagrawal05 avatar Aug 16 '24 11:08 aagrawal05

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.

aagrawal05 avatar Aug 17 '24 09:08 aagrawal05

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 avatar Sep 09 '24 10:09 aagrawal05