diff --git a/src/helpers/MQTTConnectionPolicy.h b/src/helpers/MQTTConnectionPolicy.h index b2c57690..096c5d38 100644 --- a/src/helpers/MQTTConnectionPolicy.h +++ b/src/helpers/MQTTConnectionPolicy.h @@ -234,4 +234,25 @@ static inline StaleTokenAction classifyStaleToken(bool minted, bool connected, return broker_enforces_exp ? StaleTokenAction::Bounce : StaleTokenAction::KeepAlive; } +// Which clock to fall back on when no NTP server answered. +enum class ClockSource : uint8_t { + None, // nothing plausible to work from — stay unsynced + System, // libc already holds a usable time + Rtc, // libc does not, but the RTC does +}; + +// System first: a clock SNTP set recently outranks an RTC that may have drifted. +// The RTC matters on a cold boot, where ESP32RTCClock::begin() seeds libc with a 2024 +// placeholder on power-on while a detected chip already holds real time and +// AutoDiscoverRTCClock::begin() never copies one into the other. Never while +// validating a server: that asks whether a specific host answers, and no clock can +// answer it. Pass rtc_time 0 when the board has no clock to consult. +static inline ClockSource chooseFallbackClock(bool validating_server, uint32_t system_time, + uint32_t rtc_time, uint32_t min_valid_epoch) { + if (validating_server) return ClockSource::None; + if (system_time >= min_valid_epoch) return ClockSource::System; + if (rtc_time >= min_valid_epoch) return ClockSource::Rtc; + return ClockSource::None; +} + } // namespace MQTTConnectionPolicy diff --git a/src/helpers/bridges/MQTTBridge.cpp b/src/helpers/bridges/MQTTBridge.cpp index 4da87149..7afb2e28 100644 --- a/src/helpers/bridges/MQTTBridge.cpp +++ b/src/helpers/bridges/MQTTBridge.cpp @@ -4026,12 +4026,21 @@ bool MQTTBridge::syncTimeWithNTP(bool force, bool primary_only) { // all. Keep the behaviour, but as its own decision rather than as a claim about a // server that never replied. Not on the validation path — `set mqtt.ntp` asks // whether that server works, and the clock cannot answer for it. - if (!ntp_ok && !primary_only) { - unsigned long existing = (unsigned long)time(nullptr); - if (existing >= kMinValidEpoch) { - epochTime = existing; + if (!ntp_ok) { + const unsigned long system_time = (unsigned long)time(nullptr); + // On a cold boot with a detected RTC chip these disagree: ESP32RTCClock::begin() + // stamps libc with a 2024 placeholder on power-on, AutoDiscoverRTCClock::begin() + // never copies the chip into it, and getCurrentTime() reads the chip. Asking libc + // alone would reject a board that knows exactly what time it is. + const unsigned long rtc_time = _rtc ? (unsigned long)_rtc->getCurrentTime() : 0; + const MQTTConnectionPolicy::ClockSource source = MQTTConnectionPolicy::chooseFallbackClock( + primary_only, (uint32_t)system_time, (uint32_t)rtc_time, (uint32_t)kMinValidEpoch); + if (source != MQTTConnectionPolicy::ClockSource::None) { + const bool from_rtc = (source == MQTTConnectionPolicy::ClockSource::Rtc); + epochTime = from_rtc ? rtc_time : system_time; ntp_ok = true; - MQTT_DEBUG_PRINTLN("No NTP server answered; continuing on the existing clock: %lu", existing); + MQTT_DEBUG_PRINTLN("No NTP server answered; continuing on the existing %s: %lu", + from_rtc ? "RTC" : "system clock", epochTime); } } @@ -4048,8 +4057,10 @@ bool MQTTBridge::syncTimeWithNTP(bool force, bool primary_only) { accepted.tv_usec = 0; settimeofday(&accepted, nullptr); - // Only when a server actually answered: there is nothing to point SNTP at - // otherwise, and the existing configuration is the best guess available. + // Only when a server supplied the accepted epoch. The fallback above necessarily + // points configTime() at each server before knowing whether it replies; this is + // the post-acceptance call, and there is nothing to re-point it at when the epoch + // came from a local clock. if (ntp_server_used) { configTime(0, 0, ntp_server_used); } diff --git a/test/test_mqtt_connection_policy/test_mqtt_connection_policy.cpp b/test/test_mqtt_connection_policy/test_mqtt_connection_policy.cpp index 7c7bd7de..c3cd469e 100644 --- a/test/test_mqtt_connection_policy/test_mqtt_connection_policy.cpp +++ b/test/test_mqtt_connection_policy/test_mqtt_connection_policy.cpp @@ -290,6 +290,48 @@ TEST(StaleToken, FailedMintNeverReconnects) { EXPECT_EQ(StaleTokenAction::Defer, Policy::classifyStaleToken(false, true, false)); } +using Policy::ClockSource; + +// 2026-01-01, the bridge's plausibility floor, and the 2024 placeholder +// ESP32RTCClock::begin() stamps into libc on a power-on reset. +static const uint32_t kFloor = 1767225600; +static const uint32_t kPowerOnPlaceholder = 1715770351; +static const uint32_t kPlausibleNow = 1786000000; + +TEST(FallbackClock, PrefersTheSystemClockWhenItIsUsable) { + // A clock SNTP set recently outranks an RTC that may have drifted. + EXPECT_EQ(ClockSource::System, + Policy::chooseFallbackClock(false, kPlausibleNow, kPlausibleNow - 900, kFloor)); +} + +TEST(FallbackClock, FallsBackToTheRtcOnAColdBoot) { + // The case the system-clock-only check missed: libc holds the power-on + // placeholder while a detected chip holds real time. + EXPECT_EQ(ClockSource::Rtc, + Policy::chooseFallbackClock(false, kPowerOnPlaceholder, kPlausibleNow, kFloor)); +} + +TEST(FallbackClock, NothingUsableStaysUnsynced) { + EXPECT_EQ(ClockSource::None, + Policy::chooseFallbackClock(false, kPowerOnPlaceholder, kPowerOnPlaceholder, kFloor)); + // rtc_time 0 is how a board with no clock to consult is passed in. + EXPECT_EQ(ClockSource::None, + Policy::chooseFallbackClock(false, kPowerOnPlaceholder, 0, kFloor)); +} + +TEST(FallbackClock, ServerValidationNeverAcceptsALocalClock) { + // `set mqtt.ntp` asks whether that host answers. No clock can answer for it, + // however plausible — this is the path where a typo has to fail. + EXPECT_EQ(ClockSource::None, + Policy::chooseFallbackClock(true, kPlausibleNow, kPlausibleNow, kFloor)); +} + +TEST(FallbackClock, TheFloorItselfIsAccepted) { + EXPECT_EQ(ClockSource::System, Policy::chooseFallbackClock(false, kFloor, 0, kFloor)); + EXPECT_EQ(ClockSource::Rtc, Policy::chooseFallbackClock(false, kFloor - 1, kFloor, kFloor)); + EXPECT_EQ(ClockSource::None, Policy::chooseFallbackClock(false, kFloor - 1, kFloor - 1, kFloor)); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS();