Try-fix: L76K spamming bad times can crash nodes#7261
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR aims to prevent node crashes caused by persistent bad GPS times by clearing the GPS buffer on invalid timestamps, and also cleans up compiler warnings around loop variable types and RTC-setting return values.
- Switched loop counters from signed to unsigned types to resolve warnings.
- Added an explicit
notificationTypeEnum::nonecase indrawBannerCallback. - Converted
perhapsSetRTCfrom a boolean to anRTCSetResultenum and clear the GPS buffer on invalid time. - Introduced the
RTCSetResultenum to better represent RTC-set outcomes.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/graphics/draw/NotificationRenderer.cpp | Updated loop indices to unsigned and added none case in switch |
| src/gps/RTC.h | Added RTCSetResult enum and updated perhapsSetRTC signatures |
| src/gps/RTC.cpp | Changed perhapsSetRTC implementations to return RTCSetResult |
| src/gps/GPS.cpp | Clear GPS buffer when perhapsSetRTC returns InvalidTime |
Comments suppressed due to low confidence (3)
src/gps/RTC.h:28
- The comment for
RTCSetResultNotSetis misleading—it indicates success. It should describe that the RTC was not updated (e.g., "RTC not set because a higher-quality time was already in place").
RTCSetResultNotSet = 0, ///< RTC was set successfully
src/gps/RTC.h:28
- [nitpick]
RTCSetResultNotSetis ambiguous given its current semantic; consider renaming it to something likeRTCSetResultSkippedorRTCSetResultAlreadySetto better convey that the RTC wasn’t updated.
RTCSetResultNotSet = 0, ///< RTC was set successfully
src/gps/GPS.cpp:1539
- Add or extend unit tests to verify that
clearBuffer()is indeed called whenperhapsSetRTCyieldsInvalidTime, ensuring this branch is exercised.
if (perhapsSetRTC(RTCQualityGPS, t) == RTCSetResultInvalidTime) {
oscgonfer
pushed a commit
to fablabbcn/smartcitizen-meshtastic
that referenced
this pull request
Jul 21, 2025
* Try-fix: Clear GPS buffer when we encounter a bad time in NMEA * Fix signed int warnings
fifieldt
added a commit
to fifieldt/meshtastic-firmware
that referenced
this pull request
Aug 27, 2025
There are certain GPS chips that have a hard-coded time in firmware that they will return before lock. We set our own hard-coded time, BUILD_EPOCH, that should be newer and use the comparison to not set a bad time. In meshtastic#7261 we introduced the RTCSetResult and improved it in meshtastic#7375 . However, the original try-fix left logic in GPS.cpp that could still result in broadcasting the bad time. Further, as part of our fix we cleared the GPS buffer if we didn't get a good time. The mesh was hurting at the time, so this was a reasonable approach. However, given time tends to come in when we're trying to get early lock, this had the potential side effect of throwing away valuable information to get position lock. This change reverses the clearBuffer and changes the logic so if time is not set it will not be broadcast. Fixes meshtastic#7771 Fixes meshtastic#7750
fifieldt
added a commit
to fifieldt/meshtastic-firmware
that referenced
this pull request
Aug 27, 2025
There are certain GPS chips that have a hard-coded time in firmware that they will return before lock. We set our own hard-coded time, BUILD_EPOCH, that should be newer and use the comparison to not set a bad time. In meshtastic#7261 we introduced the RTCSetResult and improved it in meshtastic#7375 . However, the original try-fix left logic in GPS.cpp that could still result in broadcasting the bad time. Further, as part of our fix we cleared the GPS buffer if we didn't get a good time. The mesh was hurting at the time, so this was a reasonable approach. However, given time tends to come in when we're trying to get early lock, this had the potential side effect of throwing away valuable information to get position lock. This change reverses the clearBuffer and changes the logic so if time is not set it will not be broadcast. Fixes meshtastic#7771 Fixes meshtastic#7750
thebentern
pushed a commit
that referenced
this pull request
Aug 29, 2025
There are certain GPS chips that have a hard-coded time in firmware that they will return before lock. We set our own hard-coded time, BUILD_EPOCH, that should be newer and use the comparison to not set a bad time. In #7261 we introduced the RTCSetResult and improved it in #7375 . However, the original try-fix left logic in GPS.cpp that could still result in broadcasting the bad time. Further, as part of our fix we cleared the GPS buffer if we didn't get a good time. The mesh was hurting at the time, so this was a reasonable approach. However, given time tends to come in when we're trying to get early lock, this had the potential side effect of throwing away valuable information to get position lock. This change reverses the clearBuffer and changes the logic so if time is not set it will not be broadcast. Fixes #7771 Fixes #7750
thebentern
pushed a commit
that referenced
this pull request
Aug 29, 2025
There are certain GPS chips that have a hard-coded time in firmware that they will return before lock. We set our own hard-coded time, BUILD_EPOCH, that should be newer and use the comparison to not set a bad time. In #7261 we introduced the RTCSetResult and improved it in #7375 . However, the original try-fix left logic in GPS.cpp that could still result in broadcasting the bad time. Further, as part of our fix we cleared the GPS buffer if we didn't get a good time. The mesh was hurting at the time, so this was a reasonable approach. However, given time tends to come in when we're trying to get early lock, this had the potential side effect of throwing away valuable information to get position lock. This change reverses the clearBuffer and changes the logic so if time is not set it will not be broadcast. Fixes #7771 Fixes #7750
thebentern
added a commit
that referenced
this pull request
Aug 29, 2025
* Merge pull request #7777 from meshtastic/create-pull-request/bump-version Bump release version * Only send Neighbours if we have some to send. (#7493) * Only send Neighbours if we have some to send. The original intent of NeighborInfo was that when a NeighbourInfo was sent all of the nodes that saw it would reply with NeighbourInfo. So, NeighbourInfo was sent even if there were no hop-zero nodes in the NodeDB. Since 2023, when this was implemented, our understanding of running city-wide meshes has improved substantially. We have taken steps to reduce the impact of NeighborInfo over LoRa. This change aligns with those ideas: we will now only send NeighborInfo if we have some neighbors to contribute. The impact of this change is that a node must first see another directly connected node in another packet type before NeighborInfo is sent. This means that a node with no neighbors is no longer able to trigger other nodes to broadcast NeighborInfo. It will, however, receive the regular periodic broadcast of NeighborInfo, and will be able to send NeighborInfo if it has at least 1 neighbor. * Include all the things * AvOid memleak * We don't gotTime if time is 2019. (#7772) There are certain GPS chips that have a hard-coded time in firmware that they will return before lock. We set our own hard-coded time, BUILD_EPOCH, that should be newer and use the comparison to not set a bad time. In #7261 we introduced the RTCSetResult and improved it in #7375 . However, the original try-fix left logic in GPS.cpp that could still result in broadcasting the bad time. Further, as part of our fix we cleared the GPS buffer if we didn't get a good time. The mesh was hurting at the time, so this was a reasonable approach. However, given time tends to come in when we're trying to get early lock, this had the potential side effect of throwing away valuable information to get position lock. This change reverses the clearBuffer and changes the logic so if time is not set it will not be broadcast. Fixes #7771 Fixes #7750 --------- Co-authored-by: Tom Fifield <tom@tomfifield.net>
alexl83
pushed a commit
to alexl83/firmware
that referenced
this pull request
Sep 2, 2025
There are certain GPS chips that have a hard-coded time in firmware that they will return before lock. We set our own hard-coded time, BUILD_EPOCH, that should be newer and use the comparison to not set a bad time. In meshtastic#7261 we introduced the RTCSetResult and improved it in meshtastic#7375 . However, the original try-fix left logic in GPS.cpp that could still result in broadcasting the bad time. Further, as part of our fix we cleared the GPS buffer if we didn't get a good time. The mesh was hurting at the time, so this was a reasonable approach. However, given time tends to come in when we're trying to get early lock, this had the potential side effect of throwing away valuable information to get position lock. This change reverses the clearBuffer and changes the logic so if time is not set it will not be broadcast. Fixes meshtastic#7771 Fixes meshtastic#7750
mehow
pushed a commit
to mehow/meshtastic-firmware
that referenced
this pull request
Sep 5, 2025
There are certain GPS chips that have a hard-coded time in firmware that they will return before lock. We set our own hard-coded time, BUILD_EPOCH, that should be newer and use the comparison to not set a bad time. In meshtastic#7261 we introduced the RTCSetResult and improved it in meshtastic#7375 . However, the original try-fix left logic in GPS.cpp that could still result in broadcasting the bad time. Further, as part of our fix we cleared the GPS buffer if we didn't get a good time. The mesh was hurting at the time, so this was a reasonable approach. However, given time tends to come in when we're trying to get early lock, this had the potential side effect of throwing away valuable information to get position lock. This change reverses the clearBuffer and changes the logic so if time is not set it will not be broadcast. Fixes meshtastic#7771 Fixes meshtastic#7750
pkoryzna
pushed a commit
to pkoryzna/meshtastic-firmware-why2025-carrier
that referenced
this pull request
Sep 13, 2025
There are certain GPS chips that have a hard-coded time in firmware that they will return before lock. We set our own hard-coded time, BUILD_EPOCH, that should be newer and use the comparison to not set a bad time. In meshtastic#7261 we introduced the RTCSetResult and improved it in meshtastic#7375 . However, the original try-fix left logic in GPS.cpp that could still result in broadcasting the bad time. Further, as part of our fix we cleared the GPS buffer if we didn't get a good time. The mesh was hurting at the time, so this was a reasonable approach. However, given time tends to come in when we're trying to get early lock, this had the potential side effect of throwing away valuable information to get position lock. This change reverses the clearBuffer and changes the logic so if time is not set it will not be broadcast. Fixes meshtastic#7771 Fixes meshtastic#7750
pkoryzna
pushed a commit
to pkoryzna/meshtastic-firmware-why2025-carrier
that referenced
this pull request
Sep 13, 2025
* Merge pull request meshtastic#7777 from meshtastic/create-pull-request/bump-version Bump release version * Only send Neighbours if we have some to send. (meshtastic#7493) * Only send Neighbours if we have some to send. The original intent of NeighborInfo was that when a NeighbourInfo was sent all of the nodes that saw it would reply with NeighbourInfo. So, NeighbourInfo was sent even if there were no hop-zero nodes in the NodeDB. Since 2023, when this was implemented, our understanding of running city-wide meshes has improved substantially. We have taken steps to reduce the impact of NeighborInfo over LoRa. This change aligns with those ideas: we will now only send NeighborInfo if we have some neighbors to contribute. The impact of this change is that a node must first see another directly connected node in another packet type before NeighborInfo is sent. This means that a node with no neighbors is no longer able to trigger other nodes to broadcast NeighborInfo. It will, however, receive the regular periodic broadcast of NeighborInfo, and will be able to send NeighborInfo if it has at least 1 neighbor. * Include all the things * AvOid memleak * We don't gotTime if time is 2019. (meshtastic#7772) There are certain GPS chips that have a hard-coded time in firmware that they will return before lock. We set our own hard-coded time, BUILD_EPOCH, that should be newer and use the comparison to not set a bad time. In meshtastic#7261 we introduced the RTCSetResult and improved it in meshtastic#7375 . However, the original try-fix left logic in GPS.cpp that could still result in broadcasting the bad time. Further, as part of our fix we cleared the GPS buffer if we didn't get a good time. The mesh was hurting at the time, so this was a reasonable approach. However, given time tends to come in when we're trying to get early lock, this had the potential side effect of throwing away valuable information to get position lock. This change reverses the clearBuffer and changes the logic so if time is not set it will not be broadcast. Fixes meshtastic#7771 Fixes meshtastic#7750 --------- Co-authored-by: Tom Fifield <tom@tomfifield.net>
jeek
pushed a commit
to jeek/Meshtastic-Exploiteers-Hacker-Pager
that referenced
this pull request
Jun 30, 2026
* Try-fix: Clear GPS buffer when we encounter a bad time in NMEA * Fix signed int warnings
jeek
pushed a commit
to jeek/Meshtastic-Exploiteers-Hacker-Pager
that referenced
this pull request
Jun 30, 2026
There are certain GPS chips that have a hard-coded time in firmware that they will return before lock. We set our own hard-coded time, BUILD_EPOCH, that should be newer and use the comparison to not set a bad time. In meshtastic#7261 we introduced the RTCSetResult and improved it in meshtastic#7375 . However, the original try-fix left logic in GPS.cpp that could still result in broadcasting the bad time. Further, as part of our fix we cleared the GPS buffer if we didn't get a good time. The mesh was hurting at the time, so this was a reasonable approach. However, given time tends to come in when we're trying to get early lock, this had the potential side effect of throwing away valuable information to get position lock. This change reverses the clearBuffer and changes the logic so if time is not set it will not be broadcast. Fixes meshtastic#7771 Fixes meshtastic#7750
jeek
pushed a commit
to jeek/Meshtastic-Exploiteers-Hacker-Pager
that referenced
this pull request
Jun 30, 2026
* Merge pull request meshtastic#7777 from meshtastic/create-pull-request/bump-version Bump release version * Only send Neighbours if we have some to send. (meshtastic#7493) * Only send Neighbours if we have some to send. The original intent of NeighborInfo was that when a NeighbourInfo was sent all of the nodes that saw it would reply with NeighbourInfo. So, NeighbourInfo was sent even if there were no hop-zero nodes in the NodeDB. Since 2023, when this was implemented, our understanding of running city-wide meshes has improved substantially. We have taken steps to reduce the impact of NeighborInfo over LoRa. This change aligns with those ideas: we will now only send NeighborInfo if we have some neighbors to contribute. The impact of this change is that a node must first see another directly connected node in another packet type before NeighborInfo is sent. This means that a node with no neighbors is no longer able to trigger other nodes to broadcast NeighborInfo. It will, however, receive the regular periodic broadcast of NeighborInfo, and will be able to send NeighborInfo if it has at least 1 neighbor. * Include all the things * AvOid memleak * We don't gotTime if time is 2019. (meshtastic#7772) There are certain GPS chips that have a hard-coded time in firmware that they will return before lock. We set our own hard-coded time, BUILD_EPOCH, that should be newer and use the comparison to not set a bad time. In meshtastic#7261 we introduced the RTCSetResult and improved it in meshtastic#7375 . However, the original try-fix left logic in GPS.cpp that could still result in broadcasting the bad time. Further, as part of our fix we cleared the GPS buffer if we didn't get a good time. The mesh was hurting at the time, so this was a reasonable approach. However, given time tends to come in when we're trying to get early lock, this had the potential side effect of throwing away valuable information to get position lock. This change reverses the clearBuffer and changes the logic so if time is not set it will not be broadcast. Fixes meshtastic#7771 Fixes meshtastic#7750 --------- Co-authored-by: Tom Fifield <tom@tomfifield.net>
jeek
pushed a commit
to jeek/Meshtastic-Exploiteers-Hacker-Pager
that referenced
this pull request
Jun 30, 2026
There are certain GPS chips that have a hard-coded time in firmware that they will return before lock. We set our own hard-coded time, BUILD_EPOCH, that should be newer and use the comparison to not set a bad time. In meshtastic#7261 we introduced the RTCSetResult and improved it in meshtastic#7375 . However, the original try-fix left logic in GPS.cpp that could still result in broadcasting the bad time. Further, as part of our fix we cleared the GPS buffer if we didn't get a good time. The mesh was hurting at the time, so this was a reasonable approach. However, given time tends to come in when we're trying to get early lock, this had the potential side effect of throwing away valuable information to get position lock. This change reverses the clearBuffer and changes the logic so if time is not set it will not be broadcast. Fixes meshtastic#7771 Fixes meshtastic#7750
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.
I have seen this bug in the wild for a while now, but it's sporadic. This is an attempt to fix it by clearing the GPS buffer when we see it occur.
Fixed a couple of warnings in there as well