Repository navigation
GPS: set the clock only from the RMC sentence that was validated - #3625
Open
nullrouten0 wants to merge 2 commits into
Open
nullrouten0 wants to merge 2 commits into
nullrouten0 wants to merge 2 commits into
Conversation
Add a native_gps PlatformIO environment that compiles the real MicroNMEALocationProvider header against the real MicroNMEA 2.0.6 library, with the address and undefined-behaviour sanitizers on, and run it in the unit-test workflow. The mocks gain what the provider header needs to compile on a host: yield(), the C character classification functions, minimal digital I/O, mesh::RTCClock and a DateTime stand-in. The suite has a shared helper header and starts with a parsing smoke test on well-formed GGA and RMC sentences; later changes add their own test files to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MicroNMEALocationProvider counted seconds while the position flag was true and then wrote getTimestamp(), which reads MicroNMEA's current fields. Nothing tied those fields to the sentence that had just passed its checksum: before any date is parsed the date fields are zero; a missing or merged field makes parseDate() keep the previous date; and a sentence whose checksum covers only "GPRMC" parses nothing at all. On dev the new tests show it writing a 2099 date from one bad sentence, the previous day at the 30-minute resync after midnight, and a 30-minute-old time when sentences stop parsing. Read the clock sample from the text of each RMC that passed its checksum: hhmmss, status 'A' and ddmmyy, all before the first '*', and a real calendar date. Set the RTC only after three consecutive samples advance with millis() within 2 s, and only if the newest is at most 2 s old. Repeats of the same second are ignored so faster receivers work. When syncs happen, the position path and getTimestamp() are unchanged. Receivers that send hhmmss without fractional seconds now set the clock (MicroNMEA misreads their position, so they never did); receivers that send GGA but no RMC no longer do (GGA has no date). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
mikecarper
added a commit
to mikecarper/MeshCore
that referenced
this pull request
Oct 18, 2026
Port the guarded NMEA buffer, validated RMC clock samples and coordinate bounds from meshcore-dev#3624, meshcore-dev#3625 and meshcore-dev#3626. Adapt clock admission to retain this branch's signed date limits, bounded loop work, tracker acquisitions, sample consumption, diagnostics and periodic settings. Upstream-PR: meshcore-dev#3624 Upstream-Commit: 1ac389b Upstream-PR: meshcore-dev#3625 Upstream-Commit: 72ed4dc Upstream-PR: meshcore-dev#3626 Upstream-Commit: 499727a Co-authored-by: nullrouten0 <201262769+nullrouten0@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes
hhmmssfrom the time field, statusA, andddmmyyfrom the date field. All of them must come before the first*, so the checksum covers them, and the date must be a real calendar date. Any other sentence gives no sample.millis(), within 2 s, and only if the newest sample is at most 2 s old. The value written is that sample. Repeats of the same second are ignored, so receivers faster than 1 Hz still work.gps sync), the position path,getTimestamp(), and the other location providers.Why
MeshCore compares timestamps per sender. A repeater or room rejects a login whose timestamp is not newer than the last one from that client, and companion nodes drop an advert that is not newer than the last one from that key. So a node's clock mostly needs to move forward. The costly failure is a clock that jumps ahead and is later corrected: peers have recorded the future value and refuse the node's correct adverts and logins until real time catches up. #3050 shows how hard that is to recover from.
Today the provider counts seconds while
nmea.isValid(), a position flag, is true, then writesgetTimestamp(), which reads MicroNMEA's current fields. Nothing ties those fields to the sentence that was just checked:DateTimeturns into a date in 2043, as Do not feed an invalid GPS time into the RTC 🤖🤖 #3422 shows on hardware. Do not feed an invalid GPS time into the RTC 🤖🤖 #3422 covers this case.parseDate()lands on a different field, and if that field is empty it keeps the previous sentence's date.*.$GPRMC*4B,...passes on the five charactersGPRMC, nothing is parsed, and the previous sentence's fields are written.The new tests reproduce each of these on
dev:devsets the clock to 2099-10-17 12:00:37.Case 1 needs only a receiver that reports a position before it has a date. Cases 2 and 3 need a damaged sentence whose checksum still matches. The 8-bit XOR checksum always catches a single changed character, but a sentence that lost a run of bytes, for example to a UART receive overflow during a long loop stall, still matches about 1 time in 73. That figure comes from 200,000 random dropped runs of 2 to 20 bytes in a real RMC sentence.
What I checked
pio test -e native -e native_kiss_modem -e native_gps: 77 of 77 pass, the 54 existing tests plus 23 innative_gps.dev's provider: 15 of the 21 new clock tests fail. In 9 of them,devwrites a time it should not have: no date yet, a date the checksum does not cover, impossible fields, the previous day after midnight, a frozen time, the 2099 outlier, a stuck time, and stale samples at a resync, twice. 5 fail only on timing; they pin the agreement rule and say so. 1 is the deliberate change described below. The other 6 pass on both and guard against regressions, such as a clean receiver, a receiver faster than 1 Hz, and a leap day.devand this branch:Heltec_v3_repeaterRAK_4631_repeaterBehaviour changes to know about
hhmmss, no.ss) now set the clock. MicroNMEA skips a fixed nine characters for the time, so it misreads these sentences' position fields andisValid()stays false; ondevthey never set the clock. Their position is still invalid, as before.devthey wrote whatever date MicroNMEA held, which is 2043 until an RMC arrives.devit happened on the fourth one-second check.Not checked
loop(). This PR does not touchgetTimestamp(), so the overlap is a few lines and whichever merges second needs a small rebase. Do not feed an invalid GPS time into the RTC 🤖🤖 #3422's validation ingetTimestamp()is still useful for any other caller.Related
The first commit adds the
native_gpstest environment and is the same commit as in #3624, the guard-tail PR. The tests for this change are in their own file,test_clock.cpp, so the GPS PRs merge in any order without conflicts.🤖 Generated with Claude Code