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 ModelSDF serializer #851

Merged
merged 9 commits into from
Aug 16, 2021
Merged

Add ModelSDF serializer #851

merged 9 commits into from
Aug 16, 2021

Conversation

caguero
Copy link
Contributor

@caguero caguero commented Jun 7, 2021

🎉 New feature

Summary

Adds a serializer to the ModelSDF component.

Test it

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • 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

Signed-off-by: Carlos Agüero <[email protected]>
@caguero caguero requested a review from chapulina as a code owner June 7, 2021 15:51
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label Jun 7, 2021
@caguero caguero requested a review from azeey June 7, 2021 15:52
include/ignition/gazebo/components/Model.hh Outdated Show resolved Hide resolved
test/integration/components.cc Outdated Show resolved Hide resolved
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.

LGTM 👍

@caguero
Copy link
Contributor Author

caguero commented Jun 24, 2021

I'm considering closing this PR in favor of pull request #881. That way all the changes needed for "Scaling" will be contained in the scale branch.

@chapulina
Copy link
Contributor

I'm considering closing this PR in favor of pull request #881. That way all the changes needed for "Scaling" will be contained in the scale branch.

The changes here are pretty self-contained and independent of scaling, so I think it's ok to get it into main, then rebase the scale branch on top of main. But if you want to move it there I think it's ok too.

std::string sdf(std::istreambuf_iterator<char>(_in), {});

sdf::Errors errors = root.LoadSdfString(sdf);
if (!errors.empty())
Copy link
Contributor

@jennuine jennuine Aug 2, 2021

Choose a reason for hiding this comment

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

Should this check !root.Element()->HasElement("model") instead? #881 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed! c428d20

Copy link
Contributor

Choose a reason for hiding this comment

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

Could also do !root.Model()

@caguero caguero merged commit 1215bf8 into main Aug 16, 2021
@caguero caguero deleted the caguero/model_serializer branch August 16, 2021 13:37
WilliamLewww pushed a commit to WilliamLewww/ign-gazebo that referenced this pull request Dec 7, 2021
* Add ModelSDF serializer.

Signed-off-by: Carlos Agüero <[email protected]>
Signed-off-by: William Lew <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants