Skip to content

Better parsing and merging of GSA messages #149

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Better parsing and merging of GSA messages #149

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c9671d6
extend GSA parsing with GNSS system id
jonasrudloff Jun 11, 2025
1f0a39f
Merge multiple GSA messages from diffrent GNSS systems
jonasrudloff Jun 11, 2025
80118a0
remove unnessary reset of fix_satellites_prns
jonasrudloff Jun 13, 2025
d99759a
Collect mode fields of GSA messages
jonasrudloff Jun 25, 2025
3d6fe7a
GSV: parse elevation, azimuth, snr as float
jonasrudloff Jul 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,14 @@ pub struct Nmea {
pub speed_over_ground: Option<f32>,
pub true_course: Option<f32>,
pub num_of_fix_satellites: Option<u32>,
pub gsa_mode1: Option<gsa::GsaMode1>,
pub gsa_mode2: Option<gsa::GsaMode2>,
pub hdop: Option<f32>,
pub vdop: Option<f32>,
pub pdop: Option<f32>,
/// Geoid separation in meters
pub geoid_separation: Option<f32>,
pub fix_satellites_prns: Option<Vec<u32, 18>>,
pub fix_satellites_prns: Vec<(GnssType, u32), 18>,
satellites_scan: [SatsPack; GnssType::COUNT],
required_sentences_for_nav: SentenceMask,
#[cfg_attr(feature = "defmt-03", defmt(Debug2Format))]
Expand Down Expand Up @@ -208,7 +210,14 @@ impl<'a> Nmea {
}

fn merge_gsa_data(&mut self, gsa: GsaData) {
self.fix_satellites_prns = Some(gsa.fix_sats_prn);
for prn in gsa.fix_sats_prn {
let sat = (gsa.gnss_type, prn);
if !self.fix_satellites_prns.contains(&sat) {
let _ = self.fix_satellites_prns.push(sat);
}
}
self.gsa_mode1 = Some(gsa.mode1);
self.gsa_mode2 = Some(gsa.mode2);
self.hdop = gsa.hdop;
self.vdop = gsa.vdop;
self.pdop = gsa.pdop;
Expand Down
82 changes: 75 additions & 7 deletions src/sentences/gsa.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ use nom::{
Err, IResult, InputLength, Parser,
};

use crate::{parse::NmeaSentence, sentences::utils::number, Error, SentenceType};
use crate::{
parse::NmeaSentence,
sentences::{utils::number, GnssType},
Error, SentenceType,
};

#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "defmt-03", derive(defmt::Format))]
Expand Down Expand Up @@ -48,6 +52,7 @@ pub struct GsaData {
pub pdop: Option<f32>,
pub hdop: Option<f32>,
pub vdop: Option<f32>,
pub gnss_type: GnssType,
}

/// This function is take from `nom`, see `nom::multi::many0`
Expand Down Expand Up @@ -85,7 +90,35 @@ fn gsa_prn_fields_parse(i: &str) -> IResult<&str, Vec<Option<u32>, 18>> {
many0(terminated(opt(number::<u32>), char(',')))(i)
}

type GsaTail = (Vec<Option<u32>, 18>, Option<f32>, Option<f32>, Option<f32>);
type GsaTail = (
Vec<Option<u32>, 18>,
Option<f32>,
Option<f32>,
Option<f32>,
Option<GnssType>,
);

fn do_parse_gsa_tail_with_gnss_type(i: &str) -> IResult<&str, GsaTail> {
let (i, prns) = gsa_prn_fields_parse(i)?;
let (i, pdop) = float(i)?;
let (i, _) = char(',')(i)?;
let (i, hdop) = float(i)?;
let (i, _) = char(',')(i)?;
let (i, vdop) = float(i)?;
let (i, _) = char(',')(i)?;
let (i, gnss_type) = number::<u8>(i)?;
Copy link
Member

@elpiel elpiel Sep 1, 2025

Choose a reason for hiding this comment

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

If I understand correctly this is the System ID:
https://gpsd.gitlab.io/gpsd/NMEA.html#_gsa_gps_dop_and_active_satellites

At least from this documentation I don't see 5 and 6 but if your receiver uses these numbers I'm ok with including them.

Copy link
Author

@jonasrudloff jonasrudloff Sep 1, 2025

Choose a reason for hiding this comment

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

gpsd is describing system id 5 and 6 here: https://gpsd.gitlab.io/gpsd/NMEA.html#_nmea_4_11_system_id_and_signal_id

Our receiver does not, afaik, produce these numbers


let gnss_type = match gnss_type {
1 => Some(GnssType::Gps),
2 => Some(GnssType::Glonass),
3 => Some(GnssType::Galileo),
4 => Some(GnssType::Beidou),
5 => Some(GnssType::Qzss),
6 => Some(GnssType::NavIC),
_ => None,
};
Ok((i, (prns, Some(pdop), Some(hdop), Some(vdop), gnss_type)))
}

fn do_parse_gsa_tail(i: &str) -> IResult<&str, GsaTail> {
let (i, prns) = gsa_prn_fields_parse(i)?;
Expand All @@ -94,7 +127,7 @@ fn do_parse_gsa_tail(i: &str) -> IResult<&str, GsaTail> {
let (i, hdop) = float(i)?;
let (i, _) = char(',')(i)?;
let (i, vdop) = float(i)?;
Ok((i, (prns, Some(pdop), Some(hdop), Some(vdop))))
Ok((i, (prns, Some(pdop), Some(hdop), Some(vdop), None)))
}

fn is_comma(x: char) -> bool {
Expand All @@ -103,17 +136,33 @@ fn is_comma(x: char) -> bool {

fn do_parse_empty_gsa_tail(i: &str) -> IResult<&str, GsaTail> {
value(
(Vec::new(), None, None, None),
(Vec::new(), None, None, None, None),
all_consuming(take_while1(is_comma)),
)(i)
}

fn do_parse_gsa(i: &str) -> IResult<&str, GsaData> {
fn do_parse_gsa<'a>(talker_id: &str, i: &'a str) -> IResult<&'a str, GsaData> {
let (i, mode1) = one_of("MA")(i)?;
let (i, _) = char(',')(i)?;
let (i, mode2) = one_of("123")(i)?;
let (i, _) = char(',')(i)?;
let (i, mut tail) = alt((do_parse_empty_gsa_tail, do_parse_gsa_tail))(i)?;
let (i, mut tail) = alt((
do_parse_empty_gsa_tail,
do_parse_gsa_tail_with_gnss_type,
do_parse_gsa_tail,
))(i)?;

let gnss_type = match talker_id {
"GA" => GnssType::Galileo,
"GP" => GnssType::Gps,
"GL" => GnssType::Glonass,
"BD" | "GB" => GnssType::Beidou,
"GI" => GnssType::NavIC,
"GQ" | "PQ" | "QZ" => GnssType::Qzss,
"GN" => tail.4.unwrap_or(GnssType::Gps),
_ => tail.4.unwrap_or(GnssType::Gps),
Comment on lines +155 to +163
Copy link
Member

@elpiel elpiel Sep 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

  • Maybe an Optional field makes more sense, as this system id was introduced in NMEA 4.11?

  • Orrr... return an error in GN talker with no tail. I also think we can match the talker to the parsed System Id exists in the sentence (sort of a second validation)

  • Also, could you please document this feature in the struct itself including the NMEA 4.11 version and how the system id is handled by us.

Not sure how it's handled by receivers in previous versions and multi-constelation view.
Here's what I found (https://gpsd.gitlab.io/gpsd/NMEA.html#_satellite_ids):

GLONASS satellite numbers come in two flavors. If a sentence has a GL talker ID, expect the skyviews to be GLONASS-only and in the range 1-32; you must add 64 to get a globally-unique NMEA ID. If the sentence has a GN talker ID, the device emits a multi-constellation skyview with GLONASS IDs already in the 65-96 range.

Copy link
Author

@jonasrudloff jonasrudloff Sep 1, 2025

Choose a reason for hiding this comment

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

So the trouble here, according to gpsd, is that before NMEA 4.11 there was no standard way of knowing which satellite id corresponded to which system. So the ids used in GSA messages with talker GN is product dependent, and thus we have no way of distinguishing between satellites from different systems. NMEA 4.11 added the system id(last field of GSA messages) which is optional. If the system id is present all the satellites in a GSA message have the system assigned ids.

gpsd does this whole thing to decide which number each satellite has: https://gitlab.com/gpsd/gpsd/-/blob/master/drivers/driver_nmea0183.c#L549

Its been some time since I made this pull request and my memory is no longer clear about exactly what is going on.
But this seems like a hard problem to solve due to lack of accurate information.

Copy link
Member

@elpiel elpiel Sep 1, 2025

Choose a reason for hiding this comment

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

If I understand the issue correctly, I'd suggest to use a None in the case of GN with no System id/gnss type.
Seems the safest approach instead of providing false information to the user or their hardware.
E.g. if they know their hardware only outputs X or Y talkers but doesn't support NMEA 4.11 system id, they will see Gps which is incorrect.

};

Ok((
i,
GsaData {
Expand Down Expand Up @@ -141,6 +190,7 @@ fn do_parse_gsa(i: &str) -> IResult<&str, GsaData> {
pdop: tail.1,
hdop: tail.2,
vdop: tail.3,
gnss_type,
},
))
}
Expand Down Expand Up @@ -199,7 +249,7 @@ pub fn parse_gsa(sentence: NmeaSentence) -> Result<GsaData, Error> {
found: sentence.message_id,
})
} else {
Ok(do_parse_gsa(sentence.data)?.1)
Ok(do_parse_gsa(sentence.talker_id, sentence.data)?.1)
}
}

Expand Down Expand Up @@ -232,6 +282,22 @@ mod tests {
pdop: Some(3.6),
hdop: Some(2.1),
vdop: Some(2.2),
gnss_type: GnssType::Gps
},
gsa
);
let s =
parse_nmea_sentence("$GNGSA,A,3,23,02,27,10,08,,,,,,,,3.45,1.87,2.89,1*01").unwrap();
let gsa = parse_gsa(s).unwrap();
assert_eq!(
GsaData {
mode1: GsaMode1::Automatic,
mode2: GsaMode2::Fix3D,
fix_sats_prn: Vec::from_slice(&[23, 2, 27, 10, 8]).unwrap(),
pdop: Some(3.45),
hdop: Some(1.87),
vdop: Some(2.89),
gnss_type: GnssType::Gps
},
gsa
);
Expand All @@ -241,6 +307,8 @@ mod tests {
"$BDGSA,A,3,214,,,,,,,,,,,,1.8,1.1,1.4*18",
"$GNGSA,A,3,31,26,21,,,,,,,,,,3.77,2.55,2.77*1A",
"$GNGSA,A,3,75,86,87,,,,,,,,,,3.77,2.55,2.77*1C",
"$GNGSA,A,3,23,02,27,10,08,,,,,,,,3.45,1.87,2.89,1*01",
"$GNGSA,A,3,,,,,,,,,,,,,3.45,1.87,2.89,4*0B",
"$GPGSA,A,1,,,,*32",
];
for line in &gsa_examples {
Expand Down
13 changes: 7 additions & 6 deletions src/sentences/gsv.rs
View file
Open in desktop
Copy link
Member

@elpiel elpiel Sep 1, 2025

Choose a reason for hiding this comment

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

Could you explain the changes in Gsv?
At least in the example here: https://gpsd.gitlab.io/gpsd/NMEA.html#_gsv_satellites_in_view

I don't see the use of floating numbers in the string, could you verify that a float can be returned and include a test for it?
I'd prefer an actual messages being used rather than a fake one to make sure that a receiver is actually returning a float.

Copy link
Author

@jonasrudloff jonasrudloff Sep 1, 2025

Choose a reason for hiding this comment

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

Yes, we are seeing floats(or at least fixedpoint numbers with 2 decimals), and we had parse failures due to this. Note that it does not change the type of the Satellite struct as it already had f32, now we are just also parsing those floats.

I can get our GNSS to produce those messages for testing.

Copy link
Member

@elpiel elpiel Sep 1, 2025

Choose a reason for hiding this comment

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

Yes, please! Thank you!

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use heapless::Vec;
use nom::{
character::complete::char,
combinator::{cond, opt, rest_len},
number::complete::float,
IResult,
};

Expand Down Expand Up @@ -68,20 +69,20 @@ pub struct GsvData {
fn parse_gsv_sat_info(i: &str) -> IResult<&str, Satellite> {
let (i, prn) = number::<u32>(i)?;
let (i, _) = char(',')(i)?;
let (i, elevation) = opt(number::<i32>)(i)?;
let (i, elevation) = opt(float)(i)?;
let (i, _) = char(',')(i)?;
let (i, azimuth) = opt(number::<i32>)(i)?;
let (i, azimuth) = opt(float)(i)?;
let (i, _) = char(',')(i)?;
let (i, snr) = opt(number::<i32>)(i)?;
let (i, snr) = opt(float)(i)?;
let (i, _) = cond(rest_len(i)?.1 > 0, char(','))(i)?;
Ok((
i,
Satellite {
gnss_type: GnssType::Galileo,
prn,
elevation: elevation.map(|v| v as f32),
azimuth: azimuth.map(|v| v as f32),
snr: snr.map(|v| v as f32),
elevation: elevation,
azimuth: azimuth,
snr: snr,
},
))
}
Expand Down
Loading