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

Add Gps Sensor to IGN-Sensors library #72

Closed
wants to merge 4 commits into from
Closed

Add Gps Sensor to IGN-Sensors library #72

wants to merge 4 commits into from

Conversation

Dotrar
Copy link

@Dotrar Dotrar commented Dec 30, 2020

Related issues:
#23
gazebosim/sdformat#453

very similar to the imu and altimeter sensor objects, just a gps sensor, hopefully as one would expect.

Dre Westcook added 2 commits December 30, 2020 19:19
Signed-off-by: Dre Westcook <dre.west@outlook.com>
Signed-off-by: Dre Westcook <dre.west@outlook.com>
@Dotrar Dotrar requested a review from iche033 as a code owner December 30, 2020 08:22
@github-actions github-actions bot added the 🔮 dome Ignition Dome label Dec 30, 2020
@osrf-triage osrf-triage added this to Inbox in Core development Dec 30, 2020
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

Please review the style comments and check the CI you have some linters errors.

Do you mind to add some tests too ? (like these ones https://github.com/ignitionrobotics/ign-sensors/tree/ign-sensors4/test/integration)

include/ignition/sensors/GpsSensor.hh Outdated Show resolved Hide resolved
include/ignition/sensors/GpsSensor.hh Show resolved Hide resolved
include/ignition/sensors/GpsSensor.hh Show resolved Hide resolved
include/ignition/sensors/GpsSensor.hh Show resolved Hide resolved
include/ignition/sensors/GpsSensor.hh Show resolved Hide resolved
src/GpsSensor.cc Outdated Show resolved Hide resolved
src/GpsSensor.cc Outdated Show resolved Hide resolved
src/GpsSensor.cc Outdated Show resolved Hide resolved
src/GpsSensor.cc Outdated Show resolved Hide resolved
src/GpsSensor.cc Outdated Show resolved Hide resolved
Core development automation moved this from Inbox to In review Dec 30, 2020
Dre Westcook added 2 commits December 31, 2020 13:51
Signed-off-by: Dre Westcook <dre.west@outlook.com>
Signed-off-by: Dre Westcook <dre.west@outlook.com>
@Dotrar
Copy link
Author

Dotrar commented Dec 31, 2020

Added some tests and I fixed up the linting errors from the ./tools/code_check.sh script.

this is going to need gazebosim/sdformat#453 to be accepted before it willl compile here, just for the fyi.

I think most of the help I might need would be on the gazebo sim side. gazebosim/gz-sim#519

I'll fix up the lint on there soon.

@chapulina chapulina added the needs upstream release Blocked by a release of an upstream library label Dec 31, 2020
@chapulina chapulina moved this from In review to In progress in Core development Jan 27, 2021
@chapulina
Copy link
Contributor

@Dotrar , the SDFormat PR has been merged. Are you interested in iterating on this PR and the one in ign-gazebo?

@chapulina chapulina mentioned this pull request Dec 14, 2021
9 tasks
@chapulina
Copy link
Contributor

Thank you again for all the work, @Dotrar . Since the fork has been deleted, I opened a new PR in #177.

@chapulina chapulina closed this Dec 14, 2021
Core development automation moved this from In progress to Done Dec 14, 2021
@j-rivero j-rivero removed this from Done in Core development May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔮 dome Ignition Dome needs upstream release Blocked by a release of an upstream library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants