Skip to content

sdf 1.8: add capsule geometry type#389

Merged
scpeters merged 14 commits intogazebosim:masterfrom
scpeters:capsule
Dec 15, 2020
Merged

sdf 1.8: add capsule geometry type#389
scpeters merged 14 commits intogazebosim:masterfrom
scpeters:capsule

Conversation

@scpeters
Copy link
Copy Markdown
Member

@scpeters scpeters commented Oct 13, 2020

This adds a capsule geometry type to the SDFormat 1.8 spec. This shape is a cylinder with hemispheres capping each end. Due to the similarity to a cylinder, the same parameters length and radius are used. The Cylinder DOM class was copied and modified mainly with search and replace s/ylinder/apsule/ to support the new shape. This requires gazebosim/gz-math#163, so this will be a Draft PR until that is merged and released.

Part of #376.

Update: ignition-math 6.7.0 has been released, so this is now ready for review.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Copy Cylinder class and unit tests to create Capsule.
Add Capsule to Geometry enum, logic, and integration
test.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@codecov-io
Copy link
Copy Markdown

codecov-io commented Nov 24, 2020

Codecov Report

Merging #389 (d9f11c4) into master (28e16c8) will increase coverage by 0.07%.
The diff coverage is 95.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #389      +/-   ##
==========================================
+ Coverage   87.52%   87.60%   +0.07%     
==========================================
  Files          62       63       +1     
  Lines        9542     9616      +74     
==========================================
+ Hits         8352     8424      +72     
- Misses       1190     1192       +2     
Impacted Files Coverage Δ
src/Geometry.cc 92.03% <84.61%> (-0.97%) ⬇️
src/Capsule.cc 96.61% <96.61%> (ø)
src/Cylinder.cc 96.66% <100.00%> (+3.56%) ⬆️

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 28e16c8...d9f11c4. Read the comment docs.

@scpeters scpeters marked this pull request as ready for review November 24, 2020 06:22
Copy link
Copy Markdown
Collaborator

@azeey azeey left a comment

Choose a reason for hiding this comment

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

LGTM. Just left some minor comments

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
public: Capsule(Capsule &&_capsule) noexcept;

/// \brief Destructor
public: virtual ~Capsule();
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

is there any point to having a virtual destructor? I copied it from the other classes, but would it be better without the virtual?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I was going to leave a comment about that but didn't because the other classes use virtual destructors. IMO, these classes are not meant to be used polymorphically (they don't have any virtual functions), so there is no need for a virtual destructor.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

made non-virtual in 1c09156

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Copy link
Copy Markdown
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, I just have some minor comments and questions.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
The parser will populate them with default values.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@scpeters scpeters merged commit fc4dc8c into gazebosim:master Dec 15, 2020
@scpeters scpeters deleted the capsule branch December 15, 2020 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants