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

Minor cleanup of dartsim #515

Merged
merged 6 commits into from
Aug 8, 2023
Merged

Conversation

shameekganguly
Copy link
Contributor

  • Change remaning ignerr to gzerr
  • Clangtidy fixes
  • Remove unused header

Signed-off-by: Shameek Ganguly <shameekarcanesphinx@gmail.com>
Signed-off-by: Shameek Ganguly <shameekarcanesphinx@gmail.com>
@github-actions github-actions bot added 🌱 garden Ignition Garden 🎵 harmonic Gazebo Harmonic labels Jun 7, 2023
Signed-off-by: Shameek Ganguly <shameekarcanesphinx@gmail.com>
@@ -252,7 +252,7 @@ Vector3d ShapeFeatures::GetEllipsoidShapeRadii(
Identity ShapeFeatures::AttachEllipsoidShape(
const Identity &_linkID,
const std::string &_name,
const Vector3d _radii,
const Vector3d &_radii,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a breaking change, so we wouldn't want to do it in this branch. Instead, it should go into main. Also, to do this, we'd have to change the signatures in the AttachEllipsoidShapeFeature (https://github.com/gazebosim/gz-physics/blob/8ee8028861567303cdba2602c9ba79c34a5ab542/include/gz/physics/EllipsoidShape.hh)

Copy link
Member

Choose a reason for hiding this comment

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

I've applied these API changes in #521, targeting main

Copy link
Member

Choose a reason for hiding this comment

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

and #521 has been merged

Copy link
Member

Choose a reason for hiding this comment

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

I reverted this change in 39e4753

@azeey azeey added the beta Targeting beta release of upcoming collection label Jul 31, 2023
Continue passing Vector3 by value to
AttachEllipsoidShape on garden. This has
already been changed to pass by const ref on main.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@shameekganguly
Copy link
Contributor Author

Thanks Steve, looks good!

@scpeters scpeters merged commit 871bab7 into gazebosim:gz-physics6 Aug 8, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Targeting beta release of upcoming collection 🌱 garden Ignition Garden 🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants