Timekeeping and time display accuracy improvements#440
Open
markmentovai wants to merge 2 commits into
Open
Conversation
All watch faces were deciding when to redraw in their `OswAppV2::onLoop`
overrides by calling `time` and comparing the result to the last-drawn
time as stored in a `lastTime` member variable. `time` returns a time on
the ESP32 clock’s timebase, so redraws were triggered whenever the ESP32
clock ticked to a new whole second. The actual source for the displayed
time, however, is the DS3231M clock, which is largely asynchronous with
respect to the ESP32 clock. The two may (and, indeed, often do) tick
over to a new whole second at different instants. This meant that when
the ESP32 clock ticked, triggering a watch face to want to redraw, the
time subsequently taken from the DS3231M clock could have been up to
(but not including) 1s stale (past the instant of the DS3231M clock’s
most recent tick) when it was first drawn. This had the effect of
causing the watch to appear to run up to 1s slow, even with the DS3231M
perfectly set.
The watch faces are improved by using the time reported by the DS3231M
clock as the basis for the decision to redraw, the same time source as
used for the actual time to be displayed. This is achieved by calling
`OswHal::getUTCTime` in place of `time`. `OswHal::getUTCTime` delegates
to OswHal’s default time provider, which is normally the DS3231M device.
This is guaranteed to always be on the same timebase as the
`OswHal::get{Local,Dual,}{Time,Date}` calls used for display purposes,
as these functions also all ultimately source their time and date data
from `OswHal::getUTCTime`.
The NTP synchronization started by OswDevices::NativeESP32::triggerNTPUpdate’s `configTime` call, when successful, sets the ESP32 clock with microsecond precision based on the response packet’s transmit timestamp (ignoring round-trip delay, as currently configured). Normally, the DS3231M clock is the system’s primary time provider, not the ESP32 clock. At the first pass through OswServiceTaskWiFi::loop following a successful NTP synchronization, the new time is pushed to the DS3231M clock. Although the DS3231M features 5ppm accuracy, it is not possible to directly set or obtain the time it keeps with precision any better than 1s, nor is most of OpenSmartWatch equipped with interfaces to work with time with sub-1s-precision. When the NTP-sourced time was pushed from the ESP32 clock to the DS3231M clock, the fractional portion of the timestamp was dropped, truncating the timestamp to the preceding whole second. The truncated timestamp could have been up to (but not including) 1s earlier than the actual time at the moment the DS3231M clock was set. This had the effect of causing the watch to appear to run up to 1s slow, even with a recent and accurate NTP synchronization. To ensure that the DS3231M clock is NTP-synchronized accurately with resolution better than 1s, it will now be set twice: first immediately, as soon as a new NTP synchronization is available (as was previously done), and again as soon as the ESP32 clock is observed ticking to a new whole second. Although the immediate synchronization sets the DS3231M clock to a time up to (but not including) 1s earlier than the correct time, it will be reset to a more accurate time within 1s, so there will be no observable discrepancy. The immediate synchronization is retained to make the new NTP-synchronized time available to the system via the DS3231M clock as soon as possible, while the second synchronization refines the accuracy of the DS3231M clock.
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.
This fixes two accuracy bugs in timekeeping or time display. Each of the bugs could have contributed to a perfectly set NTP-synchronized watch appearing to run up to (but not including) 1s slow. Together, they could have made the watch appear to run up to (but not including) 2s slow.
There’s one more source of error that can cause the watch to run slow on NTP synchronization: as configured, NTP synchronization doesn’t currently consider the round-trip delay, so the time set will be the NTP response packet’s transmit timestamp, which will be slightly stale by the time it’s used (although freshness will usually not be worse than tens of milliseconds, already much tighter than the other accuracy problems corrected here). This could be addressed by building ESP-IDF lwip with
SNTP_COMP_ROUNDTRIP, but lwip is consumed by this project asliblwip.a, which was built as a binary component into arduino-esp32 (here), in turn brought in via the PlatformIO espressif32 platform definition. That makes it difficult to solve this problem without a project-specific NTP implementation (which could just be a rebuild of the ESP-IDF lwip SNTP component with the desired options configured). This pull request doesn’t attempt to resolve this problem, because its expected magnitude is relatively small.All watch faces were deciding when to redraw in their
OswAppV2::onLoopoverrides by callingtimeand comparing the result to the last-drawn time as stored in alastTimemember variable.timereturns a time on the ESP32 clock’s timebase, so redraws were triggered whenever the ESP32 clock ticked to a new whole second. The actual source for the displayed time, however, is the DS3231M clock, which is largely asynchronous with respect to the ESP32 clock. The two may (and, indeed, often do) tick over to a new whole second at different instants. This meant that when the ESP32 clock ticked, triggering a watch face to want to redraw, the time subsequently taken from the DS3231M clock could have been up to (but not including) 1s stale (past the instant of the DS3231M clock’s most recent tick) when it was first drawn. This had the effect of causing the watch to appear to run up to 1s slow, even with the DS3231M perfectly set.The watch faces are improved by using the time reported by the DS3231M clock as the basis for the decision to redraw, the same time source as used for the actual time to be displayed. This is achieved by calling
OswHal::getUTCTimein place oftime.OswHal::getUTCTimedelegates to OswHal’s default time provider, which is normally the DS3231M device. This is guaranteed to always be on the same timebase as theOswHal::get{Local,Dual,}{Time,Date}calls used for display purposes, as these functions also all ultimately source their time and date data fromOswHal::getUTCTime.The NTP synchronization started by
OswDevices::NativeESP32::triggerNTPUpdate’sconfigTimecall, when successful, sets the ESP32 clock with microsecond precision based on the response packet’s transmit timestamp (ignoring round-trip delay, as currently configured). Normally, the DS3231M clock is the system’s primary time provider, not the ESP32 clock. At the first pass throughOswServiceTaskWiFi::loopfollowing a successful NTP synchronization, the new time is pushed to the DS3231M clock.Although the DS3231M features 5ppm accuracy, it is not possible to directly set or obtain the time it keeps with precision any better than 1s, nor is most of OpenSmartWatch equipped with interfaces to work with time with sub-1s-precision. When the NTP-sourced time was pushed from the ESP32 clock to the DS3231M clock, the fractional portion of the timestamp was dropped, truncating the timestamp to the preceding whole second. The truncated timestamp could have been up to (but not including) 1s earlier than the actual time at the moment the DS3231M clock was set. This had the effect of causing the watch to appear to run up to 1s slow, even with a recent and accurate NTP synchronization.
To ensure that the DS3231M clock is NTP-synchronized accurately with resolution better than 1s, it will now be set twice: first immediately, as soon as a new NTP synchronization is available (as was previously done), and again as soon as the ESP32 clock is observed ticking to a new whole second. Although the immediate synchronization sets the DS3231M clock to a time up to (but not including) 1s earlier than the correct time, it will be reset to a more accurate time within 1s, so there will be no observable discrepancy. The immediate synchronization is retained to make the new NTP-synchronized time available to the system via the DS3231M clock as soon as possible, while the second synchronization refines the accuracy of the DS3231M clock.