NavSat (GPS) map plugin #342
Conversation
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
chapulina
added
enhancement
There was a problem hiding this comment.
LGTM, left some minor comments
TODO: Fix warnings [GUI] [Wrn] [Application.cc:674] [QT] file::/NavSatMap/NavSatMap.qml:97:3: QML Map: Binding loop detected for property "center"
I don't get this warning on focal 🤔 Although I do get it for:
[GUI] [Wrn] [Application.cc:674] [QT] file::/Gazebo/GazeboDrawer.qml:147:3: QML Dialog: Binding loop detected for property "implicitHeight"Signed-off-by: Louise Poubel <[email protected]>
I can't reproduce it anymore either 😄
Ah yeah that's a pre-existing one. We should look into this when working on OOBE soon |
| this->newMessage(this->dataPtr->navSatMsg.latitude_deg(), | ||
| this->dataPtr->navSatMsg.longitude_deg()); |
There was a problem hiding this comment.
What if latitude/longitude_deg is not set in the new message?
There was a problem hiding this comment.
Ah that's a tough one, protobuf doesn't make a distinction between doubles that are not set or set to zero. I think it's not unreasonable to assume that these values must be set here.
We could determine a convention like passing NaN when the fields are unknown. I see that the ROS message specifies NaNs for a missing altitude, but not for missing lat/lon 🤔 https://github.com/ros2/common_interfaces/blob/master/sensor_msgs/msg/NavSatFix.msg
There was a problem hiding this comment.
I think it's not unreasonable to assume that these values must be set here
👍 Sounds good to me
chapulina
removed
the
needs upstream release
Signed-off-by: Louise Poubel <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-gui6 #342 +/- ##
=========================================
Coverage 64.48% 64.48%
=========================================
Files 36 36
Lines 4953 4953
=========================================
Hits 3194 3194
Misses 1759 1759 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
Summary
Ported @Sarath18 's GPS plugin from
ign-rviztoign-gui, seeI had to make a few changes to the QML for it to work for me on Bionic:
mapboxplugin to work, so I went withosmsupportedMapTypeswasn't working for me so I removed it⌖won't render for me when I load the GUI, mysterious... So I went with a different one for now which is not as nice:✲I can think of a few different features to add, like showing multiple vehicles on the same map, marking the world origin, clicking on the map to choose the location to spawn a model... The possibilities are endless! This is just the first MVP.
TODO
[GUI] [Wrn] [Application.cc:674] [QT] file::/NavSatMap/NavSatMap.qml:97:3: QML Map: Binding loop detected for property "center"Test it
It works with gazebosim/gz-sim#1248:
Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-bymessages.🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸