-
Notifications
You must be signed in to change notification settings - Fork 58
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
Add NavSat (GPS) sensor #177
Conversation
Signed-off-by: Dre Westcook <[email protected]>
Signed-off-by: Dre Westcook <[email protected]>
Signed-off-by: Dre Westcook <[email protected]>
Signed-off-by: Dre Westcook <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
409baa1
to
b464cf4
Compare
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
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.
Looks pretty good! I left mostly minor comments.
Signed-off-by: Louise Poubel <[email protected]> Signed-off-by: Ashton Larkin <[email protected]> Co-authored-by: Ashton Larkin <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Thanks for the review, @adlarkin ! I've addressed all comments |
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-sensors6 #177 +/- ##
================================================
+ Coverage 76.04% 76.55% +0.51%
================================================
Files 27 29 +2
Lines 2763 2871 +108
================================================
+ Hits 2101 2198 +97
- Misses 662 673 +11
Continue to review full report at Codecov.
|
This pull request has been mentioned on Gazebo Community. There might be relevant details there: https://community.gazebosim.org/t/new-ignition-releases-2022-01-10/1228/1 |
🎉 New feature
Part of
Supersedes
Requires
Required by
Summary
The fork for the original PR (#72) has been removed, so I can't push changes to that PR. I kept @Dotrar's commits in this branch, that's where most of the work is. The bulk of my changes are related to migrating from GPS to NavSat.
I retargeted this PR from Dome to Fortress because the GPS sensor will need to use spherical coordinates (see gazebosim/gz-sim#981).
Test it
Try the
ign-gazebo
PR: gazebosim/gz-sim#1248Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge
🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸