From 9d0d5e31b2a3a7a4cdb3491903e2e4b8bcfd0821 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sun, 4 Oct 2026 09:19:15 -0700 Subject: [PATCH] Adopt an RTC only if its time registers read like that chip AutoDiscoverRTCClock::begin() adopted each RTC on an I2C ACK alone, so any device answering at 0x68, 0x52, 0x51 or 0x32 (an IMU, a 24-series EEPROM) became the clock: read as the time, and written on every time sync. Upstream already worked around this per board with DISABLE_DS3231_PROBE for IMUs at 0x68. Read the seven time registers and skip the device if two reads both show it cannot be that chip, per its datasheet: - a bit documented as always 0 is set (DS3231, RV3028, RX8130CE; the PCF8563 documents none), or all seven read 0xFF (an erased EEPROM); - seconds, minutes, date or month is not valid BCD in range while the chip's power-loss flag (OSF, PORF, VL), set at power-up when its time may be undefined, is clear. Not applied to the RX8130CE, whose driver clears VLF in begin() without setting the time. The year and hours are not checked, as MeshCore can write an out-of-range year and 12-hour mode sets the hours' PM bit. A failed read keeps today's behaviour. Reads use a repeated start. Tested read-only on a RAK4631: the RV3028 is adopted, and two environment sensors on the same bus are ruled out under the rules that can tell them apart. Co-Authored-By: Claude Opus 5.5 --- src/helpers/AutoDiscoverRTCClock.cpp | 93 ++++++++++++++++++++++++++-- 1 file changed, 89 insertions(+), 4 deletions(-) diff --git a/src/helpers/AutoDiscoverRTCClock.cpp b/src/helpers/AutoDiscoverRTCClock.cpp index af08fddb03..53e80e15a3 100644 --- a/src/helpers/AutoDiscoverRTCClock.cpp +++ b/src/helpers/AutoDiscoverRTCClock.cpp @@ -26,14 +26,97 @@ bool AutoDiscoverRTCClock::i2c_probe(TwoWire& wire, uint8_t addr) { return (error == 0); } +// An ACK only says that something answers at an RTC's address: an IMU at 0x68 or an EEPROM in +// 0x50-0x57 answers too, and would then be read as the clock and written on every time sync. +// So an RTC is adopted only if its time registers read like that chip's, per its datasheet: +// - bits documented as always 0 must read 0 (the PCF8563 documents none), and the seven +// registers must not all read 0xFF, as an erased EEPROM does; +// - seconds, minutes, date and month must be valid BCD in range, unless the chip's power-loss +// flag is set: each of these chips sets it at power-up, when its time may be undefined +// (not checked for the RX8130CE, see its entry below). +// The year is not checked, as MeshCore can write an out-of-range one from a bad epoch. This +// cannot catch every device: one whose bytes happen to fit is still adopted, as before. +struct RtcId { + uint8_t time_reg; // seconds register; the seven time registers follow it + uint8_t zero[7]; // bits that always read 0 + uint8_t date_idx; // index of the date register within the seven + uint8_t flag_reg; // power-loss flag register and bit; flag_bit 0 skips the field check + uint8_t flag_bit; +}; + +#if !defined(DISABLE_DS3231_PROBE) +// DS3231 (Maxim 19-5170 Rev 10): Figure 1, p. 11; OSF in Status (0Fh) bit 7, p. 14. 00h bit 7 +// is left out: a DS1307 at 0x68, which this code also drives, powers up with its clock-halt bit +// there set (Maxim DS1307 Rev 3/15, Table 2 and text, p. 8). Its other bits match. +static const RtcId DS3231_ID = + { 0x00, { 0x00, 0x80, 0x80, 0xF8, 0xC0, 0x60, 0x00 }, 4, 0x0F, 0x80 }; +#endif +// RV3028 (RV-3028-C7 App Manual Rev 1.4): 3.2, p. 12; PORF in Status (0Eh) bit 0, 3.7, p. 22 +static const RtcId RV3028_ID = + { 0x00, { 0x80, 0x80, 0xC0, 0xF8, 0xC0, 0xE0, 0x00 }, 4, 0x0E, 0x01 }; +// PCF8563 (NXP Rev 11.1): unused bits are "not relevant", not 0, Table 4, p. 10; VL in +// VL_seconds (02h) bit 7, Table 8, p. 13, and set at power-up, Table 27, p. 24 +static const RtcId PCF8563_ID = { 0x02, { 0 }, 3, 0x02, 0x80 }; +// RX8130CE (Epson ETM50E-10): read value always 0, 13.2.1, p. 22. No field check: its power-loss +// flag VLF (1Dh bit 1) cannot vouch for the fields, as RTC_RX8130CE::begin() clears it on every +// boot without setting the time. +static const RtcId RX8130CE_ID = + { 0x10, { 0x80, 0x80, 0xC0, 0x80, 0xC0, 0xE0, 0x00 }, 4, 0x1D, 0x00 }; + +// Reads n registers from reg, with a repeated start as every one of these datasheets documents +static bool rtcRead(TwoWire& wire, uint8_t addr, uint8_t reg, uint8_t* buf, uint8_t n) { + wire.beginTransmission(addr); + wire.write(reg); + if (wire.endTransmission(false) != 0) return false; + if (wire.requestFrom(addr, n) != n) return false; + for (uint8_t i = 0; i < n; i++) buf[i] = wire.read(); + return true; +} + +static bool bcdInRange(uint8_t v, uint8_t lo, uint8_t hi) { + if ((v & 0x0F) > 9 || (v >> 4) > 9) return false; + uint8_t d = (v >> 4) * 10 + (v & 0x0F); + return d >= lo && d <= hi; +} + +// 1 if this read rules the device out, 0 if not, -1 if a read failed +static int rtcCheck(TwoWire& wire, uint8_t addr, const RtcId& id) { + uint8_t t[7]; + if (!rtcRead(wire, addr, id.time_reg, t, 7)) return -1; + bool all_ff = true; + for (int i = 0; i < 7; i++) { + if (t[i] & id.zero[i]) return 1; + all_ff = all_ff && t[i] == 0xFF; + } + if (all_ff) return 1; // an erased EEPROM + if (id.flag_bit == 0) return 0; + if (bcdInRange(t[0] & 0x7F, 0, 59) && bcdInRange(t[1] & 0x7F, 0, 59) + && bcdInRange(t[id.date_idx] & 0x3F, 1, 31) && bcdInRange(t[5] & 0x1F, 1, 12)) { + return 0; + } + uint8_t flag; + if (!rtcRead(wire, addr, id.flag_reg, &flag, 1)) return -1; + return (flag & id.flag_bit) ? 0 : 1; // power was lost, so the time is undefined +} + +// True if two reads both rule the device out. A failed read proves nothing, so the device is +// adopted as before, and one corrupted read cannot hide a real RTC. +static bool rtcRuledOut(TwoWire& wire, uint8_t addr, const RtcId& id) { + if (rtcCheck(wire, addr, id) != 1 || rtcCheck(wire, addr, id) != 1) return false; + MESH_DEBUG_PRINTLN("RTC: device at 0x%02X does not read like the RTC expected there", addr); + return true; +} + void AutoDiscoverRTCClock::begin(TwoWire& wire) { #if !defined(DISABLE_DS3231_PROBE) - if (i2c_probe(wire, DS3231_ADDRESS)) { + if (i2c_probe(wire, DS3231_ADDRESS) + && !rtcRuledOut(wire, DS3231_ADDRESS, DS3231_ID)) { ds3231_success = rtc_3231.begin(&wire); } #endif - if (i2c_probe(wire, RV3028_ADDRESS)) { + if (i2c_probe(wire, RV3028_ADDRESS) + && !rtcRuledOut(wire, RV3028_ADDRESS, RV3028_ID)) { rtc_rv3028.initI2C(wire); rtc_rv3028.writeToRegister(0x35, 0x00); rtc_rv3028.writeToRegister(0x37, 0xB4); // Direct Switching Mode (DSM): when VDD < VBACKUP, switchover occurs from VDD to VBACKUP @@ -41,12 +124,14 @@ void AutoDiscoverRTCClock::begin(TwoWire& wire) { rv3028_success = true; } - if (i2c_probe(wire, PCF8563_ADDRESS)) { + if (i2c_probe(wire, PCF8563_ADDRESS) + && !rtcRuledOut(wire, PCF8563_ADDRESS, PCF8563_ID)) { MESH_DEBUG_PRINTLN("PCF8563: Found"); rtc_8563_success = rtc_8563.begin(&wire); } - if (i2c_probe(wire, RX8130CE_ADDRESS)) { + if (i2c_probe(wire, RX8130CE_ADDRESS) + && !rtcRuledOut(wire, RX8130CE_ADDRESS, RX8130CE_ID)) { MESH_DEBUG_PRINTLN("RX8130CE: Found"); rtc_8130.begin(&wire); rtc_8130_success = true;