[J-Turtle] Fix uninitialized values in NavSatFix and add missing NavSatStatus UNKNOWN

[Ryanf55]

• Fixes status field in NavSatStatus message has bad default value #196

I would love to get this in IRON by the freeze on Mon. April 17, 2023

[tfoote]

There's a few syntax suggestions but overall I think that this change makes sense. Could you do a few fixups and we can circulate this further. It would be great to get some input from other GPS users/maintainers. And also to look at where in the public codebase this is used to make sure that there doesn't seem to be any major incompatibilities/assumptions about this value that we'll break.

[tfoote]

This makes sense to me to have an uninitialized state as the default.

[Ryanf55]

I view it as a safety problem that the default state is "healthy". Because ROSIDL supports a default constructor in C++, users of NavSatFix are not required to fill this out. Thus, if they take shortcuts, only fit out the lat-long fields, and lose GPS signal,they now report the status as having a fix even though the data is invalid and might not realize it.

[dagar]

Doesn't defaulting to STATUS_NO_FIX fix the problem?

[dagar]

You could also default the latitude and longitude to NAN to avoid accidental visits to Null Island. https://en.wikipedia.org/wiki/Null_Island

[Ryanf55]

Doesn't defaulting to STATUS_NO_FIX fix the problem?

Yes, but adding a new value makes it exactly clear between these two situations:

1. The ROS driver has started but it has not received a fix status message from the GPS =>STATUS_UNKNOWN
2. The ROS driver has communicated with the GPS, and the GPS said it has no fix => STATUS_NO_FIX

I'd vote for keeping these as distinct as one means you might have a wiring or baud issue, and the other means that you just need to wait for it to warm up or get a better view of the sky.

[Ryanf55]

I am a fan of setting the latitude and longitude to NAN as default (assuming the ROS IDL lets us do that).

[Ryanf55]

You could also default the latitude and longitude to NAN to avoid accidental visits to Null Island. https://en.wikipedia.org/wiki/Null_Island

This is not currently possible. See my docs PR which explains what's currently possible with NaN.
ros2/ros2_documentation#4210

I am working on adding support for NaN defaults and NaN constants, but it's not clear whether it will make the Jazzy release.

[tfoote]

Adding in the NaN logic is reasonable, but I think that should be a sanity check of the data, more than the canonical way to know what the status is. Checking all field values for NaN can be very expensive versus being able to quickly check an enumeration and stop processing data early.

[Ryanf55]

I would be more of a NaN fan if tooling supported it better, but there's currently no way to to set a NaN value in DDS IDL, nor is it supported in JSON. See here for implementation notes. I'd prefer not hold up this PR which was intended as a bugfix with other things like NaN support.
ros2/rosidl#789

[tfoote]

For reference default value syntax is defined here: https://design.ros2.org/articles/legacy_interface_definition.html

[Ryanf55]

All fix-ups are complete.
The addition of unknown would make sense here: https://github.com/ros-drivers/nmea_navsat_driver/blob/b168f499349e5f26e3aed9982deb44ed2f41f8e2/src/libnmea_navsat_driver/driver.py#L110

When you are happy, please share/tag some stakeholders.

Cheers!

[tfoote]

@evenator @mikepurvis @danthony06 @kmhallen @hortovanyi

I've seen your info on GPS packages. Do you have any concerns?

[tfoote]

The changes look good to me. I've reached out for others thoughts.

[danthony06]

@tfoote I think the changes look good, and mirror some experimental changes I've made for the gps_common package in ROS 2.

One thing we've seen, and have gotten some open issues with, on the gps_common package is better real time kinematic GPS support. If we're making changes to this message, is it worth thinking about adding some additional definitions to indicate it's an RTK capable system, and whether or not it's actively using RTK corrections?

[hortovanyi]

If the device is uninitialised it shouldn't be publishing a NavSatFix message IMHO.

NavSatFix is based on the older NMEA standard ..

Will be releasing an update soon that will publish NavSatFix message from the UBX data, as a separate node with a high precision location.

The UBLOX UBX values are different to the older NavSatFix values https://github.com/aussierobots/ublox_dgnss/blob/main/ublox_ubx_msgs/msg/UBXNavStatus.msg
https://github.com/aussierobots/ublox_dgnss/blob/main/ublox_ubx_msgs/msg/GpsFix.msg
https://github.com/aussierobots/ublox_dgnss/blob/main/ublox_ubx_msgs/msg/CarrSoln.msg

Of most concern to us is if the differential correction has been applied such that high precision (ie centimetre level accuracy) location is possible

[hortovanyi]

Heres a link to the proposed code for translation the status message for NavSatFix from UBX data. Havent merged and published this branch yet https://github.com/aussierobots/ublox_dgnss/blob/1e10c8562ad79a8fca1540a9c78968c117920980/ublox_nav_sat_fix_hp_node/src/ublox_nav_sat_fix_hp_node.cpp#L174

[danthony06]

I don't think we're necessarily tied to NMEA 0183, and aren't directly representing that information in this message. There's plenty of non-NMEA 0183 based GPS receivers that can indicate things like a hardware fault that could be encapsulated by this unknown status that are separate from a normally functioning GPS receiver that just hasn't acquired a fix yet, and we still send out a NavSatFix message for them.

[hortovanyi]

@danthony06 in that scenario from a UBX perspective there is no fix https://github.com/aussierobots/ublox_dgnss/blob/main/ublox_ubx_msgs/msg/GpsFix.msg
ublox_ubx_msgs::msg::GpsFix::GPS_NO_FIX

which translate through to
sensor_msgs::msg::NavSatStatus::STATUS_NO_FIX

[Ryanf55]

I don't think we're necessarily tied to NMEA 0183, and aren't directly representing that information in this message. There's plenty of non-NMEA 0183 based GPS receivers that can indicate things like a hardware fault that could be encapsulated by this unknown status that are separate from a normally functioning GPS receiver that just hasn't acquired a fix yet, and we still send out a NavSatFix message for them.

I've used this message with many non-NMEA message standards. If there was a new ROS message for every GPS protocol, it would be unusable and couple two interfaces together.

[Ryanf55]

If the device is uninitialised it shouldn't be publishing a NavSatFix message IMHO.

NavSatFix is based on the older NMEA standard ..

Will be releasing an update soon that will publish NavSatFix message from the UBX data, as a separate node with a high precision location.

The UBLOX UBX values are different to the older NavSatFix values https://github.com/aussierobots/ublox_dgnss/blob/main/ublox_ubx_msgs/msg/UBXNavStatus.msg https://github.com/aussierobots/ublox_dgnss/blob/main/ublox_ubx_msgs/msg/GpsFix.msg https://github.com/aussierobots/ublox_dgnss/blob/main/ublox_ubx_msgs/msg/CarrSoln.msg

Of most concern to us is if the differential correction has been applied such that high precision (ie centimetre level accuracy) location is possible

What is the effect of a subscribing application knowing it's RTK capable? We just populate the covariance and RTk happens to have a really low covariance when it goes into the state estimation.

[danthony06]

I don't think we're necessarily tied to NMEA 0183, and aren't directly representing that information in this message. There's plenty of non-NMEA 0183 based GPS receivers that can indicate things like a hardware fault that could be encapsulated by this unknown status that are separate from a normally functioning GPS receiver that just hasn't acquired a fix yet, and we still send out a NavSatFix message for them.

I've used this message with many non-NMEA message standards. If there was a new ROS message for every GPS protocol, it would be unusable and couple two interfaces together.

Yes, we also do this with the open source Novatel GPS driver to publish NavSatFix messages. I don't think we're disagreeing here.

The benefit of knowing if the receiver is RTK receiver is it helps remove ambiguity if the receiver is not functioning properly, or is in degraded conditions. I'm specifically thinking of times where I've been trying to diagnose a faulty RTK receiver in the field, and it's nice to know if the system believes it's applying RTK corrections and the fix is still bad, or if it's RTK capable, but is not using them for some other reason. Looking at the covariances doesn't necessarily give me that level of insight into the system.

[hortovanyi]

The main reason to publish the NavSatFix message is for compatibility with existing tools and visualisation of location with co-variance matrices. It really lacks a lot of other detail. Such as which constellations have been used, spoofing, dead reckoning, power state to name a few. If there was a hardware issue wouldn’t it be better to publish a diagnostic message? https://github.com/ros/diagnostics/blob/ros2/README.md diagnostics/README.md at ros2 · ros/diagnostics github.com

…

I don't think we're necessarily tied to NMEA 0183, and aren't directly representing that information in this message. There's plenty of non-NMEA 0183 based GPS receivers that can indicate things like a hardware fault that could be encapsulated by this unknown status that are separate from a normally functioning GPS receiver that just hasn't acquired a fix yet, and we still send out a NavSatFix message for them. I've used this message with many non-NMEA message standards. If there was a new ROS message for every GPS protocol, it would be unusable and couple two interfaces together. Yes, we also do this with the open source Novatel GPS driver to publish NavSatFix messages. I don't think we're disagreeing here. The benefit of knowing if the receiver is RTK receiver is it helps remove ambiguity if the receiver is not functioning properly, or is in degraded conditions. I'm specifically thinking of times where I've been trying to diagnose a faulty RTK receiver in the field, and it's nice to know if the system believes it's applying RTK corrections and the fix is still bad, or if it's RTK capable, but is not using them for some other reason. Looking at the covariances doesn't necessarily give me that level of insight into the system.

[Ryanf55]

I don't think we're necessarily tied to NMEA 0183, and aren't directly representing that information in this message. There's plenty of non-NMEA 0183 based GPS receivers that can indicate things like a hardware fault that could be encapsulated by this unknown status that are separate from a normally functioning GPS receiver that just hasn't acquired a fix yet, and we still send out a NavSatFix message for them.

I've used this message with many non-NMEA message standards. If there was a new ROS message for every GPS protocol, it would be unusable and couple two interfaces together.

Yes, we also do this with the open source Novatel GPS driver to publish NavSatFix messages. I don't think we're disagreeing here.

The benefit of knowing if the receiver is RTK receiver is it helps remove ambiguity if the receiver is not functioning properly, or is in degraded conditions. I'm specifically thinking of times where I've been trying to diagnose a faulty RTK receiver in the field, and it's nice to know if the system believes it's applying RTK corrections and the fix is still bad, or if it's RTK capable, but is not using them for some other reason. Looking at the covariances doesn't necessarily give me that level of insight into the system.

Yes, I was agreeing. Happy to add that fix type, a STATUS_RTK_FIX

[Ryanf55]

The main reason to publish the NavSatFix message is for compatibility with existing tools and visualisation of location with co-variance matrices. It really lacks a lot of other detail. Such as which constellations have been used, spoofing, dead reckoning, power state to name a few. If there was a hardware issue wouldn’t it be better to publish a diagnostic message? https://github.com/ros/diagnostics/blob/ros2/README.md diagnostics/README.md at ros2 · ros/diagnostics github.com
…
On 11 Apr 2023, at 7:15 am, David Anthony @.***> wrote: I don't think we're necessarily tied to NMEA 0183, and aren't directly representing that information in this message. There's plenty of non-NMEA 0183 based GPS receivers that can indicate things like a hardware fault that could be encapsulated by this unknown status that are separate from a normally functioning GPS receiver that just hasn't acquired a fix yet, and we still send out a NavSatFix message for them. I've used this message with many non-NMEA message standards. If there was a new ROS message for every GPS protocol, it would be unusable and couple two interfaces together. Yes, we also do this with the open source Novatel GPS driver to publish NavSatFix messages. I don't think we're disagreeing here. The benefit of knowing if the receiver is RTK receiver is it helps remove ambiguity if the receiver is not functioning properly, or is in degraded conditions. I'm specifically thinking of times where I've been trying to diagnose a faulty RTK receiver in the field, and it's nice to know if the system believes it's applying RTK corrections and the fix is still bad, or if it's RTK capable, but is not using them for some other reason. Looking at the covariances doesn't necessarily give me that level of insight into the system. — Reply to this email directly, view it on GitHub <#220 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAVTAVBJ77472ILIZ74PITDXARZ7NANCNFSM6AAAAAAWSIZ4WY. You are receiving this because you were mentioned.

Agree, this is approach I've used too to use the diagnostics topic, for GPS diagnostics.

The PR here is scoped for fixing the missing enumerations. Given the timeline for Iron freeze, I'd like to keep this discussion focused on small changes here to those specific enums, rather than adding in N number of other fields that may push back the merge beyond Iron. For Iron users, until then, diagnostics can be useful, and they can always use the GPSFix message in gps_common which has a lot more detail.

I'm happy to work with the stakeholders to define the GPS message to support all the kinematics information.. but I don't think it needs to be done in this PR.

[hortovanyi]

This is what the UBX nav status message has in it

navigation fix status information

bool diff_corr # differential corrections available
bool carr_soln_valid # valid carrSoln

It is separate from the gps fix value

[danthony06]

@Ryanf55 sounds good to me. Sorry to derail the conversation about the RTK status.

[hortovanyi]

my last comment - STATUS_UNKNOWN is STATUS_NO_FIX. There is no need to differentiate. If the device is not initialised or in fault or spoofed (compromised) it shouldn't publish a NavSatFix message

[Ryanf55]

This is what the UBX nav status message has in it

navigation fix status information

bool diff_corr # differential corrections available bool carr_soln_valid # valid carrSoln

It is separate from the gps fix value

Can you provide the reference?

If the device is not initialised or in fault or spoofed (compromised) it shouldn't publish a NavSatFix message

This puts the burden on the subscribing application to rate-check the NavSatFix messages, and then handle timeout appropriately. Currently, in Foxglove map panel for example, it colors the icon based on the message. If the position fails, and it stops publishing, their UI will just show the last message received, but it just looks like the vehicle has stopped moving. When you publish status known, it colors the icon red.
I do not agree with your assertion there; that's not how it's used in all cases. If you think the above use case is abusing the message, then perhaps you can refer to some documentation on usage that would disagree with Foxglove's implementation?

[hortovanyi]

This is what the UBX nav status message has in it

navigation fix status information

bool diff_corr # differential corrections available bool carr_soln_valid # valid carrSoln
It is separate from the gps fix value

Can you provide the reference?
Pages 155 and 156 3.15.20 UBX-NAV-STATUS from https://content.u-blox.com/sites/default/files/documents/u-blox-F9-HPG-1.32_InterfaceDescription_UBX-22008968.pdf
https://content.u-blox.com/sites/default/files/documents/u-blox-F9-HPG-1.32_InterfaceDescription_UBX-22008968.pdf

If the device is not initialised or in fault or spoofed (compromised) it shouldn't publish a NavSatFix message

This puts the burden on the subscribing application to rate-check the NavSatFix messages, and then handle timeout appropriately. Currently, in Foxglove map panel for example, it colors the icon based on the message. If the position fails, and it stops publishing, their UI will just show the last message received, but it just looks like the vehicle has stopped moving. When you publish status known, it colors the icon red. I do not agree with your assertion there; that's not how it's used in all cases. If you think the above use case is abusing the message, then perhaps you can refer to some documentation on usage that would disagree with Foxglove's implementation?

If the message has not started to be published or stops being published it is not reliable and should not be displayed. In the later case, it is no longer valid. What decay rate does the Foxglove map panel have for that type of message before it removes it??

NavSatFix is a message from a simpler time for GPS devices where accuracy was really 10s of meters - suitable for navigation by boats on water! Hence the NMEA standard.

If I was asked to vote to add that new status item, I would vote no - it will break code for little gain IMHO.

In my projects, I don't use NavSatFix, the publishing of it is really being added so that the projects can visualise the lat,lon,alt in high precision with existing tools.

[danthony06]

If we're successfully communicating with the GPS, I expect to see ROS messages, even if everything is invalid. Otherwise, I don't know if the device has malfunctioned, lost communications, or my node is simply misconfigured.

This is also consistent with other sensors. For example, laser rangefinders typically still publish NaNs if they can't measure a target's distance, lidars will still publish pointclouds with all NaNs, and IMUs will publish even if they're unable to measure state in the devices I've used.

[evenator]

No objections from me.

[hortovanyi]

If we're successfully communicating with the GPS, I expect to see ROS messages, even if everything is invalid. Otherwise, I don't know if the device has malfunctioned, lost communications, or my node is simply misconfigured.

This is also consistent with other sensors. For example, laser rangefinders typically still publish NaNs if they can't measure a target's distance, lidars will still publish pointclouds with all NaNs, and IMUs will publish even if they're unable to measure state in the devices I've used.

I've written a few drivers now for lidars, computer vision cameras, can bus, IMUs etc - the most complicated are the GPS drivers - I don't agree with the above :) ... check the log files

[Ryanf55]

@tfoote If I remove the unknown status, and just have uint16 SERVICE_UNKNOWN = 0 defined for this PR, can this make Iron? I can't seem to get agreement on anything else.

[clalancette]

@tfoote If I remove the unknown status, and just have uint16 SERVICE_UNKNOWN = 0 defined for this PR, can this make Iron? I can't seem to get agreement on anything else.

Unfortunately, no. The freeze was earlier this week. But if we get agreement on this, we can merge it into Rolling and have it ready for J-Turtle.

[StepTurtle]

Hey, I agree with your opinion about adding additional field for STATUS_RTK, STATUS_DGPS. Also, if it is possible, i think more information will be good. For example, my custom GPS message have these fields:

GPS_POSITION_TYPE_NO_SOLUTION = 0
GPS_POSITION_TYPE_UNKNOWN = 1
GPS_POSITION_TYPE_SINGLE = 2
GPS_POSITION_TYPE_PSRDIFF = 3
GPS_POSITION_TYPE_SBAS = 4
GPS_POSITION_TYPE_OMNISTAR = 5
GPS_POSITION_TYPE_RTK_FLOAT = 6
GPS_POSITION_TYPE_RTK_INT = 7
GPS_POSITION_TYPE_PPP_FLOAT = 8
GPS_POSITION_TYPE_PPP_INT = 9
GPS_POSITION_TYPE_FIXED = 10

So, Are you thinking adding new fields or just add STUTUS_RTK?

[danthony06]

For gps_common, we are wrapping the gpsd client, so we mostly use the statuses they define here: https://gitlab.com/gpsd/gpsd/-/blob/master/include/gps.h#L193, which I think is pretty close to what you are proposing. I would not mention vendor specific solutions, like what I assume the _OMNISTAR member is, just because there's so many possible vendors. Instead, I would replace it with something more generic.

[Ryanf55]

For gps_common, we are wrapping the gpsd client, so we mostly use the statuses they define here: https://gitlab.com/gpsd/gpsd/-/blob/master/include/gps.h#L193, which I think is pretty close to what you are proposing. I would not mention vendor specific solutions, like what I assume the _OMNISTAR member is, just because there's so many possible vendors. Instead, I would replace it with something more generic.

Since ROS messages and API's already try to align with Linux conventions, this seems like a great idea. Agree on not adding vendor-specific messages. I don't see why a ROS consumers of GPS data would care which vendor their GPS sensor is using for corrections; I haven't seen other messages in this repo that have vendor specifics.

[danthony06]

I'd suggest adding a DGPS definition, because that's relatively common. Is PSRDIFF specific to Novatel receivers? I've only seen it used there, but I'm not sure if it's something proprietary to them.

[StepTurtle]

To the best of my knowledge, PSRDIFF is not specific to Novatel receivers and can be used for all GPS receivers. Additionally, I highly support the idea of adding a DGPS field.

Moreover, I believe it would be beneficial to have more detailed fields for corrections, as it often requires checking the GPS interface. To address this issue, I have started using the Rviz plugin, which displays various GPS features, including corrections. Consequently, I currently utilize a custom message to send GPS status. Therefore, having more status fields is important to me. However, if other ROS users do not share this preference, I understand and respect their perspective.

Regarding the OMNISTAR field, you are correct that it is very specific. I included all the fields from my example usage to demonstrate this to you.

Furthermore, I have examined the gpsd repository you mentioned, @danthony06, and I am considering using a similar solution. Thank you for bringing it to my attention, and I appreciate your interest in this matter.

[peci1]

I'd like to get this moving again.

First of all, I have to say stability of interfaces is a great part of ROS. However, if the current definition is based on a 15-year-old technical state and even such simple things as adding a few (fake) enum values takes years (it was requested for ROS 1 in 2020), it really hits hard on the usability of ROS and leads to big fragmentation where everyone implements his own message type because the available ones do not fit.

Let's first review existing ROS interfaces. Except NavSatStatus, there is gps_common/GPSStatus. It defines the following constants:

int16 STATUS_NO_FIX=-1   # Unable to fix position
int16 STATUS_FIX=0       # Normal fix
int16 STATUS_SBAS_FIX=1  # Fixed using a satellite-based augmentation system
int16 STATUS_GBAS_FIX=2  #          or a ground-based augmentation system
int16 STATUS_DGPS_FIX=18 # Fixed with DGPS
int16 STATUS_WAAS_FIX=33 # Fixed with WAAS

Don't ask me where 18 and 33 originated. I don't know and neither does git blame. WAAS is a special case of SBAS, so that can be ignored as there already is agreement that vendor-specific things should not be a part of this message.

As this is the only other widely used generic ROS interface, I would suggest to keep the values of constants in NavSatStatus compatible. There's plainly no reason to redefine them differently (except for having a nice increasing number sequence).

So my suggestion is:

# See https://gssc.esa.int/navipedia/index.php/GNSS_Augmentation for explanations
int8 STATUS_UNKNOWN = -2        # status is not yet set
int8 STATUS_NO_FIX =  -1        # unable to fix position
int8 STATUS_FIX =      0        # unaugmented fix (refered to as 'single' in RTK receivers)
int8 STATUS_SBAS_FIX = 1        # fix with satellite-based augmentation
int8 STATUS_GBAS_FIX = 2        # fix with ground-based augmentation, i.e. radio beacons near airports
# value of the following constant is chosen to correspond to the value used in gps_common/GPSStatus
int8 STATUS_DGPS_FIX=18     # fix with DGPS (pseudorange corrections, sometimes referred to as 'differential fix')
int8 STATUS_RTK_FLOAT=3     # RTK with float ambiguities
int8 STATUS_RTK_FIX=4     # RTK with fixed ambiguities
int8 STATUS_PPP_FLOAT=5     # PPP with float ambiguities
int8 STATUS_PPP_FIX=6     # PPP with fixed ambiguities
int8 STATUS_REGULATED_FIX=7     # fix using restricted or regulated channels (only if no other augmentation is used)

This should cover most of the generally used algorithms for now and close future.

Regarding statuses with dead-reckoning - I don't think they need special treatment. If the DR is short, the precision of the last valid mode will mostly hold, so the driver can keep e.g. RTK fix. If the DR is longer, the driver should downgrade to FIX and record the growing variance. But maybe I'm wrong on this one, I've never worked with a DR receiver.

Regarding diff_soln, I think that it is already covered by the RTK fix constants. I.e. there can't be a RTK float or fix solution unless differential corrections were used (AFAIK). And if the user wants to know the receiver is RTK capable but did not use the corrections, this should really go into diagnostics. This message should really only serve as the common denominator for many types of receivers. The same holds e.g. for age of corrections. Either they're fresh and the status is RTK float/fix, or they're stale and the status is only FIX without augmentation. Details about age of corrections etc. should go into vendor-specific or diagnostic messages.

ROS drivers should be dumb passthrus as much as possible— so basically, a packet of data comes in from the device -> a ROS message gets published

I always treated ROS drivers as a bit more clever pieces. Each manufacturer has different SDK with different approaches. But ROS has some standard not only regarding data in the messages, but also regarding when the messages should be published. The role of the ROS driver is to adapt the SDK to the ROS API. Sometimes it's just passthrough, sometimes it requires more sophisticated transformations.

Regarding STATUS_UNKNOWN, I think it doesn't hurt to add it, but it would have to have a clearly differentiated meaning from NO_FIX (currently, both seem very similar to me). Receivers could e.g. use it to signal that the rest of the message is invalid (for whatever reason) (but doesn't NO_FIX mean the same?). It would be great to agree that drivers should periodically publish this unknown status (or NO_FIX) as soon as they establish communication with the receiver. E.g. UNKNOWN would be the output before first fix, while NO_FIX would be the output when an already acquired fix is lost. I don't think it would be too difficult for ublox to add a simple timer to their driver that would publish these UNKNOWN/NO_FIX messages until a fix is acquired. It's actually the same problem with the already existing NO_FIX constant. Some drivers will never use it because they just don't publish when fix is unavailable. But I think that's wrong from the point of view of clients. As said earlier in this thread, it would be great if clients would not need to implement the timeouts, but the drivers did that for them.

Regarding future extensibility, it might also be good to consider moving the constant definitions to their own .msg files. That would have the great benefit that adding a new constant would not change the signature/hash of the status message, so adding new constants in the future would not be such a pain for downstream packages and inter-release interoperability. This approach can actually be used right away as NavSatStatus needs to be changed anyway to incorporate the default value (be it NO_FIX or UNKNOWN). For legacy reasons, the few constants defined in NavSatStatus could also be left there, with a comment that more constants are available in another .msg file.

[ros-discourse]

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/future-of-ros-2-gps-support/33297/37

[Ryanf55]

As said earlier in this thread, it would be great if clients would not need to implement the timeouts, but the drivers did that for them.

I like your statement: As said earlier in this thread, it would be great if clients would not need to implement the timeouts, but the drivers did that for them.

The part about great to agree that drivers should periodically publish this unknown status (or NO_FIX) as soon as they establish communication with the receiver was exactly my intent. Or, you start a ROS driver expecting serial data at a certain baud rate, but your GPS is using the wrong baud, so the driver needs to report that it has no clue what the fix state is.

Publishing NO_FIX makes it clear to consuming libraries that the ROS driver is up and running, but that it does not have connection to the GPS sensor device.

Would you be ok if I took the minimum changes in the PR to fix the known issues, and then you do a follow up PR for adding the new constants as you propose above?

Now is the time to do message work for J-turtle.

[peci1]

If that was aimed at me, then sure, go ahead ;)

[Ryanf55]

If that was aimed at me, then sure, go ahead ;)

Yup. If you agree with the current PR, then can you approve?

I would like to document this context somewhere, but the message definition does not seem like the right place. Does anyone know where a good spot to document advanced usage and intent of messages is?

[hortovanyi]

Here are the standard fix code for NMEA that I believe NavSatFix is based on - this change doesnt make sense to me. However the UBX codes we use are different from the old NMEA

https://www.sparkfun.com/datasheets/GPS/NMEA%20Reference%20Manual-Rev2.1-Dec07.pdf

Table 1-4 Position Fix Indicator
Value Description
0 Fix not available or invalid
1 GPS SPS Mode, fix valid
2 Differential GPS, SPS Mode, fix valid
3-5 Not supported
6 Dead Reckoning Mode, fix valid

[tfoote]

I would suggest that we not complicate this by doing the NaN change now too. There was a bunch of discussion in June last year.

And there's a concrete proposal to extend the enumeration of fix types here: #220 (comment) from @peci1

ANd a thread from the end of the year proposing a new message. https://discourse.ros.org/t/future-of-ros-2-gps-support/33297/54 that would seem like a good idea too. @SteveMacenski

But as I haven't seen progress on that I think it would make sense to do at least this iteration for now while the next generation is being designed.

[tfoote]

Adding in the NaN logic is reasonable, but I think that should be a sanity check of the data, more than the canonical way to know what the status is. Checking all field values for NaN can be very expensive versus being able to quickly check an enumeration and stop processing data early.

[SteveMacenski]

@tfoote I would just like to mention that that next generation isn't a fast moving freight train to the future and very well might not be something we ultimately spearhead if the right partners and collaborators don't assemble. I would not take the discussions in that thread as a indicator that this is the immediate priority - because frankly it doesn't seem to be from anyone involved thus far (but very hopeful that we'll get started with some parts of it sooner than later).

Its looking increasingly like it will be scaled back to improvements over the handling of GPS data with new frameworks (i.e. replace navsattransform; use 'new' & standard GPS processing libraries) within ROS with new coordinate systems (optional UTM, ECEF, etc) rather than addressing the core message interface. Core interface message changes for RTK and other subtle details I think should be spearheaded by more active users of that data and experience deploying some of the more advanced systems available. Just like the vision_msgs or radar_msgs, I'm not completely blind to activities happening in those spaces, but I view my role in those efforts as:

• A activation function for the discussion to create the conditions for an open discussion about needs and recruit those with vested interests and expertise to join in
• When things stall out, prodding folks to get it over the finish line
• Rectify when competing interests exist a solution that seems rational and meeting the intent of the most applications / user's needs
• being available to iterate over the eventual issues over the next 1-2 years that inevitably come up to handle the edge cases we missed

I'd welcome someone like @Ryanf55 or others to come and propose what that new future looks like on the interfaces side of things for replacing or augmenting NavSatFix and NavSatStatus in the modern age of GNSS / RTK solutions. I wasn't / aren't doing it because I have some unique insight, but rather that I see a problem. I'm more than happy to pass that flag onto someone else with more energy or immediate need to wrangle the cats, since its frankly not the top priority or top 5 priorities for me right now -- at least until the broader GPS handling/coordinate system work is at a testing prototype level. That thread took an unexpected turn to talk about interfaces, that wasn't what I proposed in the initial thread prompt (if you read my initial message) and I didn't commit that group or myself to the new interface design work on any prioritized timeline.

Tl;dr: I wouldn't block 'good' here in expectation of 'great' coming along any day now. Some effort that I'm related to might come along before long that touches alot of GPS capabilities, but interface design isn't currently in that imminent scope. I encourage others with a vested interest and expertise to take that on and I'll be an early adopter of it. A group of 3-4 expert end users and 3-4 GNSS/INS manufacturers with ROS knowledge could get alot done in a few calls over a quarter, I think. Happy to help support assembling that group if someone wants to take the lead on it and bring it over the line

[tfoote]

+1 @SteveMacenski Yes I'd definitely like to land this incremental change. There was the one worry from @peci1 about the bitfield logic that I wanted to resolve. I've accepted his documentation suggestion to make that less of a worry.

As we're approaching the cutoff for Jazzy feature freeze I'd like to land this as is without increasing scope. With the current scope of only adding enumerations it's backwards compatilble.

Does anyone have any objections to doing this minimal change as proposed now? @Ryanf55 @peci1 @dagar

[peci1]

Thanks Tully, I'm happy with the PR now!

[Ryanf55]

+1 @SteveMacenski Yes I'd definitely like to land this incremental change. There was the one worry from @peci1 about the bitfield logic that I wanted to resolve. I've accepted his documentation suggestion to make that less of a worry.

As we're approaching the cutoff for Jazzy feature freeze I'd like to land this as is without increasing scope. With the current scope of only adding enumerations it's backwards compatilble.

Does anyone have any objections to doing this minimal change as proposed now? @Ryanf55 @peci1 @dagar

Approved!

[Ryanf55]

I'd welcome someone like @Ryanf55 or others to come and propose what that new future looks like on the interfaces side of things for replacing or augmenting NavSatFix and NavSatStatus in the modern age of GNSS / RTK solutions. I wasn't / aren't doing it because I have some unique insight, but rather that I see a problem. I'm more than happy to pass that flag onto someone else with more energy or immediate need to wrangle the cats, since its frankly not the top priority or top 5 priorities for me right now -- at least until the broader GPS handling/coordinate system work is at a testing prototype level. That thread took an unexpected turn to talk about interfaces, that wasn't what I proposed in the initial thread prompt (if you read my initial message) and I didn't commit that group or myself to the new interface design work on any prioritized timeline.

Tl;dr: I wouldn't block 'good' here in expectation of 'great' coming along any day now. Some effort that I'm related to might come along before long that touches alot of GPS capabilities, but interface design isn't currently in that imminent scope. I encourage others with a vested interest and expertise to take that on and I'll be an early adopter of it. A group of 3-4 expert end users and 3-4 GNSS/INS manufacturers with ROS knowledge could get alot done in a few calls over a quarter, I think. Happy to help support assembling that group if someone wants to take the lead on it and bring it over the line

I'm happy to take this up in May. MicroStrain is a GNSS-INS manufacturer who understands ROS and also is very good at API design; I think we can leverage their expertise in designing messages for GNSS-INS. I also have a good relationship with Trimble who make the PX-1. It's pretty clear from the discussions here that NavSatFix is not appropriate for many use cases with GNSS-INS, and there is also a need for vendor-specific messages anyways.

Is anyone interested in able to plan on a small working group in May to focus on interface design for GNSS-INS to try to cover some of the items here:

• Support 3D orientation
• dual-antenna systems
• Have clearer enums for the different types of corrections
• Clear states for when the device is not connected vs has no GPS fix
• Support RTK, RTX, and other types of correction services
• Guidelines for vendor-specific messing and perhaps some examples with existing UBX driver
• Add dedicated 3D velocity data
• Clear ways to represent accuracy of the data

I don't consider myself an expert, but I do have time and access to multiple vendor's hardware to test on

[tfoote]

Thanks for all the effort to iterate on this change. It's small but will hopefully be helpful. I'm going to merge it as is and thanks @Ryanf55 for planning to kick off the planning for the next cycle after the release.

[clalancette]

I didn't see a CI run for this change. Given that it is pretty minimal, I don't expect a lot of fallout, but that long line might run afoul of our linters. And it is just our best practice to run CI on all changes.

Here is some CI:

• Linux
• Linux-aarch64
• Windows