Polyline geometry DOM

[chapulina]

🎉 New feature

• Closes Load polylines into SDF DOM #296

Summary

Expose polylines through the C++ API.

Polyline is different from other geometries because the user can specify more than one of them. So I'm proposing here an API that's slightly different and returns an std::vector<> as opposed to the raw pointers returned for the other geometries. The API will look slightly different from other geometries, but I think it would need to be different anyway.

Test it

See the added tests.

There's also this Gazebo PR:

• Extruded 2D polyline geometries gz-sim#1456

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 and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

[codecov-commenter]

Codecov Report

Merging #1000 (51c7ead) into sdf9 (0f9b771) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             sdf9    #1000   +/-   ##
=======================================
  Coverage   43.47%   43.47%           
=======================================
  Files           1        1           
  Lines          23       23           
=======================================
  Hits           10       10           
  Misses         13       13           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f9b771...51c7ead. Read the comment docs.

[scpeters]

Polyline is different from other geometries because the user can specify more than one of them.

This is correct, but I noticed that the required attribute is 0 instead of * in the spec where the poly line is included:

• https://github.com/ignitionrobotics/sdformat/blob/sdf12/sdf/1.9/geometry.sdf#L17

I guess this shows that we aren't following that attribute very closely, but perhaps we can update the spec to match the actual usage? (ie. I'm suggesting we change the required value from 0 to * on that line and in the other sdf/1.* folders)

[chapulina]

This is correct, but I noticed that the required attribute is 0 instead of * in the spec where the poly line is included:

I noticed that too. I think the required symbols can be a bit confusing, because * means "zero or more elements are required". I guess it would be clearer if it didn't have the word "required" 🤔

https://github.com/ignitionrobotics/sdformat/blob/12e77821af7c2810c4d9d532ee8cb833cbb8e318/include/sdf/Element.hh#L109-L113

[chapulina]

I think the required symbols can be a bit confusing,

• Ticketed an issue here: Enforce required elements #1002

I'm suggesting we change the required value from 0 to * on that line

@scpeters , after you read that issue, I'd appreciate some feedback on whether you still think we should go ahead with this or if we should improve those required symbols before making any large changes like that.

[scpeters]

I think the required symbols can be a bit confusing,

• Ticketed an issue here: Enforce required elements #1002

I'm suggesting we change the required value from 0 to * on that line

@scpeters , after you read that issue, I'd appreciate some feedback on whether you still think we should go ahead with this or if we should improve those required symbols before making any large changes like that.

that's a fair point; let's not make any changes here to the spec

[scpeters]

fixed minor spelling error in 7a34fab; thanks for iterating, looks good