Skip to content
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

Update NMEA parser to handle bad chars #1014

Merged
merged 5 commits into from
May 31, 2022

Conversation

Ezward
Copy link
Contributor

@Ezward Ezward commented May 15, 2022

  • it turns out that it is common for the first set of
    data returned by a gps module to include incorrectly
    framed characters that break the unicode conversion.
  • This change ignores exceptions thrown by the
    unicode decoder and keeps on going until it
    can read a correctly framed line.

- it turns out that it is common for the first set of
  data returned by a gps module to include incorrectly
  framed characters that break the unicode conversion.
- This change ignores exceptions thrown by the
  unicode decoder and keeps on going until it
  can read a correctly framed line.
@Ezward Ezward requested a review from DocGarbanzo May 15, 2022 04:39
@Ezward Ezward self-assigned this May 15, 2022
- prior to this change it just kept going,
  but it should not try to parse a linve
  with a back checksum.
if (message == "GPRMC") or (message == "GNRMC"):

if (message == "GPRMC") or (message == "GNRMC"):
if True or debug:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is always True - why do we need the if. Also we are printing, not logging into the shell, is this intended. I would expect to just have:

logger.debug(line)

so if we set global logging to Debug, it will log, otherwise is won't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I deleted it; it was left over from finding the problem.

@@ -57,6 +57,9 @@ def _readline(self) -> str:
return self.gps.readline().decode()
except serial.serialutil.SerialException:
pass
except UnicodeDecodeError:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the real fix, this looks good.

@DocGarbanzo
Copy link
Contributor

@Ezward - can you please also update the version in setup.py, you might need to pull if @Heavy02011's PR #1013 goes in first.

@Ezward Ezward requested a review from DocGarbanzo May 23, 2022 20:07
@DocGarbanzo
Copy link
Contributor

@Ezward - can you please pull from main and update the version here, too? The change looks correct now.

Copy link
Contributor

@DocGarbanzo DocGarbanzo left a comment

Choose a reason for hiding this comment

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

LGTM

@DocGarbanzo DocGarbanzo merged commit 9ac86d4 into main May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants