-
Notifications
You must be signed in to change notification settings - Fork 452
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
AISMessage::toNMEA in ais.cpp not terminating sentences per NMEA spec #1333
Comments
The sentences are already joined with a newline in the last line of the function:
So I think your patch would give \r\n\n Is the problem that your s/w is requiring a carriage return as well (Presumably it's something Windows based?) So you would just need:
But perhaps this should only be for Windows. |
That return statement does stick a \n on the end, but that breaks protocol without the \r and also breaks protocol when toNMEA has parsed more than one sentence at a time. It puts nothing at the end of the first sentence, or any except the very last sentence. There needs to be a "\r\n" at the end of each sentence to follow NMEA protocol specification (over RS485/422); it's not a Windows thing. I'd imagine some parsers are okay with the way it is because they stop pulling in characters at the * and don't start again until it finds a $, but not others. The software I'm using is very commonly used for navigation so it's probably purposefully strict. At any rate, if you'd like it pushed to a merge request, or however that's done, I'm happy to share the fix. I can also give you some packet dumps to show what I'm talking about with multiple sentences being parsed. |
Okay I ran a wireshark. I was not understanding toNMEA. It seems to be only handling one sentence at a time. But oddly there are no trailing \n in the packets (this is from a clean build of master - c6c0d2d. Here's a wireshark dump of two packets, one from master and one from my patch.
|
join adds \r\n between sentences when there are multiple sentences, but doesn't append it at the end of individual sentences. So I think the fix is:
The above patch also fixes reading beyond the end of the array. |
Should be fixed in 7.5.1. |
NMEA sentences require a sequence at the end of each sentence. toNMEA is concatenating sentences together and then terminating the bunch with an . This may work for some plotters, but not others like TimeZero (a professional plotter).
This fixed it (ais.cpp 86-91):
I'm totally a noob to Open Source workflow, otherwise I'd have tried the official way to submit the change.
The text was updated successfully, but these errors were encountered: