AutoDiscoverRTCClock::begin() adopts an RTC on an I2C ACK alone. Any other device that answers at one of the four RTC addresses becomes the node's clock. Its registers are then read as the time, and written on every time sync. Upstream has already hit this twice, and fixed each board by hand with DISABLE_DS3231_PROBE. The probe itself still has the defect for every other board and device.
Code references are at upstream dev 3e3150c8. The libraries are at the versions MeshCore builds against: Adafruit RTClib 2.1.4 and Melopero RV3028 1.2.0.
The defect
begin() runs i2c_probe() for each RTC address, which only checks for an ACK. It then marks the chip found:
| Chip |
Address |
Adopted at |
Written at begin |
Written on every sync (setCurrentTime()) |
| DS3231 |
0x68 |
L31-L32 (RTClib begin() only opens the device) |
none |
00h-06h via adjust() (L87) |
| RV3028 |
0x52 |
L36-L41 |
35h, 37h, and 10h through set24HourMode() |
00h-06h via setTime() (L91) |
| PCF8563 |
0x51 |
L44-L47 (RTClib begin() only opens the device) |
none |
time registers via adjust() (L93) |
| RX8130CE |
0x32 |
L49-L53 |
30h, 1Ch, 1Dh, 1Eh and 1Fh (RTC_RX8130CE::begin()) |
time registers via adjust() (L96) |
Once a device is adopted:
- The clock is wrong.
getCurrentTime() returns whatever the foreign device's registers decode to.
- The foreign device gets written. Every sync overwrites its registers. Syncs come from the CLI
clock and time commands, from app and peer timestamps, and from GPS every 30 minutes (TIME_SYNC_INTERVAL, L247).
- The order makes it worse. The DS3231 is checked first and wins, so an IMU at 0x68 hides a real RTC elsewhere on the bus.
Address conflicts are ordinary:
- 0x68 is the default address of the common IMUs (MPU-6050, ICM-20948, ICM-42607).
- 0x50-0x57 is the 24-series I2C EEPROM range, which covers both 0x51 (PCF8563) and 0x52 (RV3028).
Already seen upstream
Both opt-outs are per board, so any board not yet found still has the defect, and so does any device at the other three addresses.
Proposed fix
#3544. Adopt an RTC only if its time registers read like that chip's, per its datasheet:
- Bits documented as always 0 must read 0 (DS3231, RV3028, RX8130CE; NXP documents none for the PCF8563).
- Not all seven registers may read 0xFF.
- Seconds, minutes, date and month must be valid BCD in range, unless the chip's power-loss flag is set (not applied to the RX8130CE).
- Two reads must both rule a device out, and a failed read keeps today's behaviour.
The masks and flags are cited from each vendor's datasheet in #3544, which also has a read-only hardware test on a RAK4631. The existing DISABLE_DS3231_PROBE opt-outs stay.
Related: #3545 (RV3028 EEPROM configuration), which touches the same begin() block.
AutoDiscoverRTCClock::begin()adopts an RTC on an I2C ACK alone. Any other device that answers at one of the four RTC addresses becomes the node's clock. Its registers are then read as the time, and written on every time sync. Upstream has already hit this twice, and fixed each board by hand withDISABLE_DS3231_PROBE. The probe itself still has the defect for every other board and device.Code references are at upstream
dev3e3150c8. The libraries are at the versions MeshCore builds against: Adafruit RTClib2.1.4and Melopero RV30281.2.0.The defect
begin()runsi2c_probe()for each RTC address, which only checks for an ACK. It then marks the chip found:setCurrentTime())begin()only opens the device)adjust()(L87)set24HourMode()setTime()(L91)begin()only opens the device)adjust()(L93)RTC_RX8130CE::begin())adjust()(L96)Once a device is adopted:
getCurrentTime()returns whatever the foreign device's registers decode to.clockandtimecommands, from app and peer timestamps, and from GPS every 30 minutes (TIME_SYNC_INTERVAL, L247).Address conflicts are ordinary:
Already seen upstream
e7e97ec4"add option to disable DS3231 probe", andvariants/lilygo_techo_card/variant.hL50: "DS3231 lives at 0x68 but this board has ICM20948 at that address, causing broken clock."0908b034"Fix: Do not misidentify IMU as an RTC on Heltec T1", andvariants/heltec_t1/variant.hL11-L15: "the IMU gets misidentified as a DS3231 and its unrelated registers get read/written as if they were time/status registers, producing a bogus, unfixable clock."Both opt-outs are per board, so any board not yet found still has the defect, and so does any device at the other three addresses.
Proposed fix
#3544. Adopt an RTC only if its time registers read like that chip's, per its datasheet:
The masks and flags are cited from each vendor's datasheet in #3544, which also has a read-only hardware test on a RAK4631. The existing
DISABLE_DS3231_PROBEopt-outs stay.Related: #3545 (RV3028 EEPROM configuration), which touches the same
begin()block.