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

Fix SphericalCoordinate deprecation warning in DVL sensor #460

Merged
merged 2 commits into from
Aug 23, 2024

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Aug 23, 2024

🦟 Bug fix

Summary

Fixed deprecation warning by updating the gz::math::SphericalCoordinate::PositionTransform call to use the new gz::math::CoordinateVector3 arg introduced in gazebosim/gz-math#616.

cc @peci1

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • 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.

Signed-off-by: Ian Chen <[email protected]>
@github-actions github-actions bot added the 🏛️ ionic Gazebo Ionic label Aug 23, 2024
@iche033 iche033 requested a review from arjo129 August 23, 2024 00:43
src/DopplerVelocityLog.cc Outdated Show resolved Hide resolved
Signed-off-by: Ian Chen <[email protected]>
@iche033 iche033 merged commit 40d966b into main Aug 23, 2024
9 checks passed
@iche033 iche033 deleted the spherical_deprecation branch August 23, 2024 17:16
this->worldState->origin.PositionTransform(
samplePointInWorldFrame,
gz::math::CoordinateVector3::Metric(samplePointInWorldFrame),
Copy link
Contributor

Choose a reason for hiding this comment

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

@iche033 Technically, if somebody used LOCAL for reference, this will probably silently change the behavior. It is possible this would go completely without a build warning if the value is read from SDF. But I haven't worked with DVL, so I don't know if setting LOCAL would make sense...

Copy link
Contributor Author

@iche033 iche033 Aug 23, 2024

Choose a reason for hiding this comment

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

I'm not very familiar with it as well. I looked at the possible frames that gz-sim eventually passes to DVL, and I think they are: GLOBAL, SPHERICAL, and ECEF (from the EnvironmentPreload system, and EnvironmentLoader GUI plugin). Maybe @arjo129 knows more about this and can chime in?

Copy link
Contributor

Choose a reason for hiding this comment

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

@arjo129 what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏛️ ionic Gazebo Ionic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants