Empty fields spam rosout with warnings

[mikepurvis]

I think I reported this previously, but I can't find the old ticket to wake it up. In any case, here's an example bag with some data from a ublox receiver that has no sky view.

ESR's gpsd docs offer the following:

According to UNMEA, the NMEA standard requires that a field (such as
altitude, latitude, or longitude) must be left empty when the GPS has
no valid data for it. However, many receivers violate this. It's
common, for example, to see latitude/longitude/altitude figures filled
with zeros when the GPS has no valid data.

Unfortunately, the link for the citation is broken, but it's a start. Seems nmea_navsat_driver shouldn't try to cast empty strings to numbers, or the try-block should be at the field level rather than external to add_sentence.

[ericperko]

Okay, I believe I have this fixed properly. The new behavior can be summarized as follows:

• For any float fields that fail to parse, replace them with a NaN (so e.g. a missing longitude will be output as NaN)
• For missing time info, don't output a TimeReference message
• I haven't done anything to handle missing int fields, as the only ones we currently parse shouldn't ever have invalid data (fix type and number of satellites)

This removes the warning spam when I run it on your bag file (thanks for uploading one - it's much appreciated). Can you pull the 5-handle_empty_fields branch and verify that it fixes the issue on the live receiver?

[mikepurvis]

Looks great, thanks Eric!

I should have a chance to try this out this coming week, with two different receivers. The only note I have about the code is that convert_float_with_nan_for_empty isn't entirely accurate—it's more of a safe_float function, since it returns NaN for anything that throws a ValueError, including problems which could arise from serial bitflips or whatever else.

[ericperko]

Ya, I couldn't think of a better name for the function. I like safe_float much better, so it has been changed.

[ericperko]

Mike, did you get a chance to test this yet?

[mikepurvis]

Sorry, fell off my radar. I can definitely check at least one device in short order—will try to get to that soon.

[mikepurvis]

Okay, done! The modifications in the branch work great. I gave it a try with a cheap GlobalTop PA6H, and switching to the branch caused the following fix messages to be published with no sky view:

header:
  seq: 24
  stamp:
    secs: 1398174070
    nsecs: 594047000
  frame_id: navsat
status:
  status: -1
  service: 1
latitude: nan
longitude: nan
altitude: nan
position_covariance: [nan, 0.0, 0.0, 0.0, nan, 0.0, 0.0, 0.0, nan]
position_covariance_type: 1

Publishing NaN messages like this is definitely a departure from previous behaviour, but I think it's good—it allows the processing node to distinguish between "GPS is off or malfunctioning" and "GPS has no fix". OTOH, processing nodes will have to be updated to be NaN-aware (though they always should have been checking the status field).

The one thing I would note is that the fix topic is now being published upon receipt of GGA whether the fix is valid or not, whereas the RMC-based vel topic is only published when a fix is valid. I think this is okay behaviour, but something to perhaps clarify in documentation.

[ericperko]

I've updated the documentation a bit to point out that /vel will only be published if valid info was received (because TwistStamped has no status field) and that /fix "will be published with whatever positional and status data was available even if the device doesn't have a valid fix. Invalid fields may contain NaNs."

Since this does change the existing messages that the driver outputs, I'm not going to release this into Hydro to avoid breaking existing systems. It is in the Indigo release that should be available shortly.

[mikepurvis]

👍

[mikepurvis]

Blah, I've got another receiver that's doing this. Sample output:

$GPGGA,,,,,,0,,,,,,,,*66
$GPRMC,,V,,,,,,,,,,N*53
$GPGSA,,1,,,,,,,,,,,,,,,*5F
$GPGLL,,,,,,V,N*64
$GPVTG,,,,,,,,,N*30

Recorded session: https://www.dropbox.com/s/1hzfzln8brwmbm0/novatel-smart6.bag?dl=0

[ericperko]

Wow. Was that indoors or with no antenna connected? I'm surprised it even bothers to output sentences that are so lacking in information.

I'll take a look at this bag either this weekend or next weekend and make a fix.

Could you include your rosout warnings to make sure that I get the same errors when playing back the bag file?

[mikepurvis]

Yes, this is indoors. It's a Novatel Smart6, cold start right out of the box.

It's still helpful to see that the electronics are alive, but yes, these sentences are completely void of information— basically just enough to trigger warnings:

^[[0m^[[33m[WARN] [WallTime: 1422980871.646909] Value error, likely due to missing fields in the NMEA message. Error was: invalid literal for int() with base 10: ''. Please report this issue at github.com/ros-drivers/nmea_navsat_driver, including a bag file with the NMEA sentences that caused it.
^[[0m^[[33m[WARN] [WallTime: 1422980871.696196] Value error, likely due to missing fields in the NMEA message. Error was: invalid literal for int() with base 10: ''. Please report this issue at github.com/ros-drivers/nmea_navsat_driver, including a bag file with the NMEA sentences that caused it.
^[[0m^[[33m[WARN] [WallTime: 1422980871.746766] Value error, likely due to missing fields in the NMEA message. Error was: invalid literal for int() with base 10: ''. Please report this issue at github.com/ros-drivers/nmea_navsat_driver, including a bag file with the NMEA sentences that caused it.