Skip to content

Conversation

fifieldt
Copy link
Member

@fifieldt fifieldt commented Sep 21, 2025

After the recent patch, we hold lock for a bit before updating the position. The positions that come in after the hold are genuinely better positions
than what we've been doing before. However, they only come 20 seconds
after we've got lock.

Previously, we would update the local position as soon as we got a lock as well as at the end of the hold, since a hold was not always possible. With this patch, if the settings allow, we should skip that first local position update.

Fixes #8029

🤝 Attestations

  • I have tested that my proposed changes behave as described.
  • I have tested that my proposed changes do not cause any obvious regressions on the following devices:
    • Heltec (Lora32) V3
    • LilyGo T-Deck
    • LilyGo T-Beam
    • RAK WisBlock 4631
    • Seeed Studio T-1000E tracker card
    • Other (please specify below)

@fifieldt
Copy link
Member Author

(untested, and some code can be optimised, back on this in a couple days)

@fifieldt fifieldt added the enhancement New feature or request label Sep 21, 2025
@fifieldt fifieldt requested a review from Copilot September 22, 2025 00:39
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes GPS position updates by implementing a smart hold mechanism that waits for better GPS data before updating the local position, based on the configured update interval. When the GPS update interval is greater than 10 seconds, the system now holds for up to 20 seconds after acquiring a GPS lock to allow for ephemeris data download, resulting in more accurate position fixes.

Key changes:

  • Skip immediate position updates on GPS lock when update interval > 10 seconds
  • Implement conditional hold logic based on GPS update interval settings
  • Restructure GPS state machine logic with clearer documentation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

fifieldt and others added 4 commits September 22, 2025 10:56
After the recent patch, we hold lock for a bit before updating the position.
The positions that come in after the hold are genuinely better positions
 than what we've been doing before. However, they only come 20 seconds
 after we've got lock.

Previously, we would update the local position as soon as we got a lock as well
as at the end of the hold, since a hold was not always possible.
With this patch, if the settings allow, we should skip that first local position update.

Fixes meshtastic#8029
Co-authored-by: Copilot <[email protected]>
@fifieldt fifieldt requested a review from Copilot September 22, 2025 01:07
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

fifieldt and others added 2 commits September 22, 2025 11:13
@fifieldt fifieldt changed the title [WIP] Wait until after GPS lock hold before updating position, if we can. Wait until after GPS lock hold before updating position, if we can. Sep 27, 2025
@fifieldt
Copy link
Member Author

fifieldt commented Oct 3, 2025

Definitely bugs in this one ...

INFO | ??:??:?? 91 [GPS] GPS power state move from HARDSLEEP to HARDSLEEP
DEBUG | ??:??:?? 96 [GPS] Took 10s to get lock
DEBUG | ??:??:?? 96 [GPS] Predict 31s to get next lock
DEBUG | ??:??:?? 96 [GPS] 28s until next search

@fifieldt
Copy link
Member Author

fifieldt commented Oct 4, 2025

Testing with T1000E

gps_update_interval Behaviour Test
1-10 secs No hold logic. Always update the local position. Accuracy comes from the fact the GPS is always on (see GPS::down() ) OK
11-19 secs Hold for (gps_update_interval – 10). Update local position after the hold. This gives the best possible chance for a high quality lock, given our limited hold time. OK
20 secs As per 11-19 seconds (so hold for 10 secs) OK
21-40 secs Hold for 20 seconds, and then update the local position. OK
41+ secs As per 21-40 seconds. OK

Ended up changing the hold time for lower gps_update_intervals to gps_update_intervals - 10sec to avoid GPS being stuck on all the time for those.

@fifieldt fifieldt marked this pull request as ready for review October 4, 2025 01:56
@fifieldt fifieldt requested a review from Copilot October 4, 2025 02:05
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@fifieldt
Copy link
Member Author

fifieldt commented Oct 4, 2025

Confirmed tooLong still works correctly as before:

WARN | 02:46:54 908 [GPS] Couldn't publish a valid location: didn't get a GPS lock in time

DEBUG | 02:46:54 908 [GPS] Took 900s to get lock

DEBUG | 02:46:54 908 [GPS] Predict 0s to get next lock

DEBUG | 02:46:54 908 [GPS] 29s until next search

INFO | 02:46:54 908 [GPS] GPS power state move from ACTIVE to HARDSLEEP

@b8b8
Copy link

b8b8 commented Oct 7, 2025

GPS update: 8sec hold no hold - confirmed
GPS update: 25sec hold for 15sec - confirmed
GPS update: 35sec hold for 20 sec - confirmed
GPS update: 120sec hold for 20 sec - confirmed (hmm i thought that should be higher)

@fifieldt
Copy link
Member Author

fifieldt commented Oct 7, 2025

Thanks, @b8b8 . 20s is the max hold time.

@fifieldt
Copy link
Member Author

fifieldt commented Oct 7, 2025

Confirmed with b8b8's testing that the map view is now a lot smoother, positions are more accurate etc.

@fifieldt
Copy link
Member Author

fifieldt commented Oct 7, 2025

Ran a loop inside another loop to see if the parallel paths worked out OK. It did, the paths were clear and distinct, with no overlap. This is good position data :)

Screenshot_20251007-152134

@thebentern thebentern merged commit 468b40e into meshtastic:develop Oct 7, 2025
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wait until after GPS lock hold before updating position, if we can.
3 participants