Skip to content
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

Add World::ValidateGraphs method #601

Merged
merged 8 commits into from
Jul 1, 2021

Conversation

scpeters
Copy link
Member

🎉 New feature

I noticed a bug in ign-physics that needs a new API in libsdformat to properly fix it.

Summary

I noticed a test in ign-physics in which some models do not have properly loaded graphs, but it's hard to detect since the graph data is private. This creates a World::ValidateGraphs() method to call the hidden validate*Graph functions in FrameSemantics.cc with the private graph data. It also requires an extra check in the validate*Graph functions that the scope points to a valid graph.

Test it

Run the UNIT_World_TEST and INTEGRATION_world_dom tests and see that they pass.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Code check passed (In source directory, run sh tools/code_check.sh)
  • 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

The graph data is private, so expose a method for
checking that the graphs are valid.

Signed-off-by: Steve Peters <[email protected]>
@scpeters scpeters requested review from azeey and chapulina June 25, 2021 07:48
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Jun 25, 2021
@scpeters scpeters requested a review from jennuine June 25, 2021 07:50
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just have the same request as in #602 for more testing.

scpeters added 2 commits June 28, 2021 17:34
Load an sdf::World instead of an sdf::Root to
see the graph errors.

Signed-off-by: Steve Peters <[email protected]>
@scpeters scpeters merged commit fd84096 into gazebosim:sdf11 Jul 1, 2021
@scpeters scpeters deleted the world_validate_graphs branch July 1, 2021 00:12
@scpeters scpeters mentioned this pull request Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏢 edifice Ignition Edifice
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants