-
Notifications
You must be signed in to change notification settings - Fork 40
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
Conversation
shameekganguly
commented
Jun 7, 2023
- 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>
Signed-off-by: Shameek Ganguly <shameekarcanesphinx@gmail.com>
dartsim/src/ShapeFeatures.cc
Outdated
@@ -252,7 +252,7 @@ Vector3d ShapeFeatures::GetEllipsoidShapeRadii( | |||
Identity ShapeFeatures::AttachEllipsoidShape( | |||
const Identity &_linkID, | |||
const std::string &_name, | |||
const Vector3d _radii, | |||
const Vector3d &_radii, |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
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>
Thanks Steve, looks good! |