sparkfun / SparkFun_RTK_Everywhere_Firmware

Centimeter precision GPS/GNSS using L1/L2/L5 signals broadcast over Bluetooth, WiFi, and Ethernet.
http://docs.sparkfun.com/SparkFun_RTK_Everywhere_Firmware/
Other
10 stars 6 forks source link

RTC clock synchronisation - interrupt vs. rtcUpdate #249

Closed PaulZC closed 1 week ago

PaulZC commented 2 months ago

It looks like we are currently synchronising the ESP32 RTC to GNSS in two places:

rtcUpdate - here if (millis() - lastRTCAttempt > syncRTCInterval)

tpISR - here via interrupt, on the rising edge of PPS if (millisNow - lastRTCSync > syncRTCInterval)

Let's make sure tpISR wins - on platforms that support it - for the most accurate clock synchronisation...

I think we have some redundancy and probable conflict between lastRTCAttempt and lastRTCSync. Let's resolve that...

PaulZC commented 4 weeks ago

I'll take this one. It fits in nicely with NTP #372

PaulZC commented 2 weeks ago

rtcUpdate will only set the RTC once. It can't resync it once online.rtc is true.

https://github.com/sparkfun/SparkFun_RTK_Everywhere_Firmware/blob/d95eefe61b78b07b6af05b492d1c1fb86aa5e43a/Firmware/RTK_Everywhere/RTK_Everywhere.ino#L1412

lastRTCAttempt and lastRTCSync are doing different things. lastRTCAttempt restricts how often rtcUpdate can validate the time and attempt to set the RTC. lastRTCSync controls how often tpISR can resync the RTC. Both are needed.

syncRTCInterval initially limits rtcUpdate to validating the time to once per second. It later limits tpISR to resyncing the RTC once per minute.

rtcSyncd indicates that the RTC has been set accurately by tpISR. It controls if the system state enters STATE_NTPSERVER_SYNC and starts responding to NTP requests.

menuSystem can set online.rtc back to false, but only when the timeZone is changed. That's fine. The user can't expect NTP time to be accurate after changing the time zone. I'm adding code to change the state back to STATE_NTPSERVER_NO_SYNC if rtcSyncd becomes false - i.e. because the user has changed the time zone.

PaulZC commented 1 week ago

Resolved by #404

nseidle commented 1 week ago

This is in release v1.3.