Adopt an RTC only if its time registers read like that chip (iteration branch)

[ptr727]

Iteration branch for #13. Keep it open; never merge it. A clean single-commit branch for upstream is cut from it once it has baked. Upstream takes PRs on dev.

References used throughout. Each "§", table and "p." number below is the cited document's own, and page numbers are the printed ones.

• DS3231 at 0x68: DS3231 Extremely Accurate I2C-Integrated RTC/TCXO/Crystal, Maxim Integrated (Analog Devices), 19-5170 Rev 10, 3/15: https://www.analog.com/media/en/technical-documentation/data-sheets/DS3231.pdf
• RV3028 at 0x52: RV-3028-C7 Application Manual, Micro Crystal, Rev. 1.4, November 2021, from the product page: https://www.microcrystal.com/en/products/real-time-clock-rtc-modules/rv-3028-c7
• PCF8563 at 0x51: PCF8563 Real-time clock/calendar, Product data sheet, NXP, Rev. 11.1, 19 January 2026: https://www.nxp.com/docs/en/data-sheet/PCF8563.pdf
• DS1307 at 0x68, which the DS3231 code also drives: DS1307 64 x 8, Serial, I2C Real-Time Clock, Maxim Integrated (Analog Devices), REV 3/15: https://www.analog.com/media/en/technical-documentation/data-sheets/DS1307.pdf
• DS3232 at 0x68, the same layout as the DS3231: DS3232 Extremely Accurate I2C RTC with Integrated Crystal and SRAM, Maxim Integrated (Analog Devices), 19-5337 Rev 5, 7/10: https://www.analog.com/media/en/technical-documentation/data-sheets/DS3232.pdf
• RX8130CE at 0x32: RX8130CE Application Manual, Seiko Epson, ETM50E-10, from the product page: https://www5.epsondevice.com/en/products/rtc/rx8130ce.html
• Code:
  • before: upstream dev 3e3150c8
  • after: c36cb9ee
  • Adafruit RTClib at 2.1.4, the version MeshCore builds

The defect

#13 has the full write-up. In short, begin() adopts each RTC on an I2C ACK alone. Any device answering at 0x68, 0x52, 0x51 or 0x32 then becomes the clock:

• getCurrentTime() reads its registers as the time.
• setCurrentTime() writes its registers on every sync, which includes GPS every 30 minutes.
• An RV3028 or RX8130CE impostor also gets written during begin().

Upstream has already worked around this board by board for IMUs at 0x68:

• e7e97ec4, with lilygo_techo_card
• 0908b034, with heltec_t1: "producing a bogus, unfixable clock"

The fix

Each RTC is adopted only if its time registers read like that chip's. Each rule below is taken from that chip's datasheet (L29-L38, table L39-L65). rtcCheck() rules a device out if any of these hold:

1. Always-zero bit set. A bit the datasheet documents as always 0, in the seven time registers, reads 1:

   Chip	Time registers	Always-zero bits	Source
   DS3231	00h-06h	00 80 80 F8 C0 60 00 (05h bit 7 is Century, so it is allowed; 00h bit 7 is left out for the DS1307, see below)	Figure 1, p. 11
   RV3028	00h-06h	80 80 C0 F8 C0 E0 00	§3.2, p. 12
   PCF8563	02h-08h	none: unused bits are "x = not relevant", not 0	Table 4, p. 10
   RX8130CE	10h-16h	80 80 C0 80 C0 E0 00, where "'0' means … the read value is always 0"	§13.2.1 Table 12, p. 22

2. All 0xFF. All seven bytes read 0xFF, as an erased 24-series EEPROM does.

3. Invalid time with the power-loss flag clear. Seconds, minutes, date or month is not valid BCD in range, and the chip's power-loss flag is clear. Each chip sets that flag at power-up, when its time may be undefined:

   • DS3231: OSF, 0Fh bit 7 ("set to logic 1 … the first time power is applied", p. 14)
   • RV3028: PORF, 0Eh bit 0 (§3.7, p. 22)
   • PCF8563: VL, 02h bit 7 (Table 8, p. 13; startup value 1, Table 27, p. 24)

   The RX8130CE is exempt. Its flag VLF (1Dh bit 1) cannot vouch for the fields, because RTC_RX8130CE::begin() clears it on every boot without setting the time.

How the decision is made:

• rtcRuledOut() rejects a device only if two reads both rule it out. A failed read never does, so it keeps today's behaviour.
• rtcRead() uses a repeated start. That is the only read form the RX8130CE manual documents (§19.6), and all four documents allow it.
• The year, hours and weekday are never checked. MeshCore itself can write an out-of-range year: RTClib writes year - 2000 as BCD, so a bad epoch of 2100 or later gives A0+. A chip left in 12-hour mode sets the hours' PM bit. Neither may rule out a real chip.
• The checks gate all four probes in begin(). The existing DISABLE_DS3231_PROBE opt-outs stay, and compile the DS3231 table out.

Notes for review

• This cannot catch every foreign device. One whose bytes happen to fit is still adopted, as before. The hardware test shows one: under the DS3231 rule, an LPS22HB's WHO_AM_I (B1 at 0Fh) sits where the DS3231 keeps OSF, so it reads as "power lost". The per-board opt-outs remain the answer for a known conflict.
• DS1307 at 0x68, still adopted. A DS1307 (hobby RTC modules) answers at 0x68 and works through the same DS3231 code: the time layout matches, and RTClib's adjust() clears its clock-halt bit. It powers up with that bit (00h bit 7, CH) set (DS1307 Table 2 and text, p. 8), so the 0x68 mask leaves 00h bit 7 out. Every other mask bit is documented 0 on the DS3231, the DS3232 (Figure 1, p. 11) and the DS1307. A new DS1307 (seconds 80, date and month 01) then passes the field check. The trade-off: an ICM-20948's WHO_AM_I (EA at 00h) no longer trips the mask. It fails the field check instead, so it is ruled out unless its 0Fh has bit 7 set. Boards with that IMU keep DISABLE_DS3231_PROBE.
• Interaction with Store RV3028 backup switchover and trickle charger config in EEPROM (iteration branch) #11: Store RV3028 backup switchover and trickle charger config in EEPROM (iteration branch) #11 rewrites the RV3028 branch of begin(), and gates its own EEPROM writes with an RV3028-only always-zero check. Whichever lands second takes the other's begin() hunk.

Testing

Builds: release builds of RAK_4631_repeater (nRF52840), heltec_v4_repeater (ESP32-S3), RAK_11310_repeater (RP2040), Heltec_t1_repeater (with DISABLE_DS3231_PROBE) and R1Neo_repeater (the RX8130CE board), plus MESH_DEBUG=1 on the first three. No new warnings.

Hardware (read-only). A RAK4631 with a RAK12002 (RV3028), a RAK1902 (ST LPS22HB) and a RAK1906 (Bosch BME680) on the I2C bus, and nothing else, per an ACK scan. Its RAK12501 GNSS (Quectel L76K) is on the UART, not the I2C bus. The build is test/rtc-probe-diag (cc4bf1fa): 0aacafa4 plus a throwaway serial CLI. The later change that leaves 00h bit 7 out of the 0x68 mask was recomputed against the raw blocks below and changes no verdict. It ACK-scans the bus, then runs each chip's check against each address found, using register reads only.

Address	Device	Registers 00h-06h	DS3231 rule	RV3028 rule	PCF8563 rule	RX8130CE rule
0x52	RAK12002 RV3028 (the real RTC)	06 09 12 01 08 01 00	ok	ok, adopted	ruled out	ok
0x5C	RAK1902 LPS22HB (0Fh = WHO_AM_I B1)	00 00 00 00 00 00 00	ok (0Fh reads as OSF)	ruled out	ruled out	ok (no field check)
0x76	RAK1906 BME680 (D0h chip ID = 61)	33 AA 16 49 13 01 39	ruled out	ruled out	ruled out	ruled out

ZephCore, a separate firmware, has an independent implementation of the same identity rules: ptr727/liquidraver-ZephCore#40, for its issue #38. Run read-only on this same board at a7b43f6 (raw log), it gave the same verdicts at every address, with one recorded exception. ZephCore keeps a field check for the RX8130CE, so it also rules out the LPS22HB under that rule. MeshCore cannot keep that check, because its RX8130CE driver clears VLF on every boot.

The two implementations agree on the rules and the masks. Where they differ, the difference is deliberate and has a recorded reason:

• No clean read: ZephCore adopts the chip but takes no time from it. MeshCore keeps today's getCurrentTime() behaviour.
• RX8130CE field check: ZephCore keeps it. MeshCore drops it, for the driver reason above.
• Unreadable power-loss flag: ZephCore treats it as clear, following its existing convention. MeshCore adopts the device, because a failed read proves nothing.
• All 0xFF: ZephCore skips the device on one read and re-probes once, on its first save. MeshCore rules it out only on two agreeing reads.

An earlier build that checked only the always-zero bits let the LPS22HB's all-zero block pass three of the four rules. That result is why the field check with the power-loss exception was added.

Refs #13

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 20fb5048-b3b6-4a06-993b-8835e7b62b21

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Copilot review overview

🔵 Needs a closer look

Hardware-specific RTC detection changes warrant final human validation.

Review effort: Lite
Findings: None

What changed in this PR

Adds chip-specific RTC register validation to prevent false I2C device detection during automatic RTC discovery.

Changes:

• Validates register masks, BCD fields, and power-loss conditions.
• Requires two consecutive rejection reads.
• Applies checks to all four RTC probes while preserving opt-outs.

File	Description
src/​helpers/​AutoDiscoverRTCClock.cpp	Adds RTC identity validation and gates device adoption.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[ptr727]

Upstream: PR meshcore-dev#3544 (from fix/rtc-probe-identity at e2ad35cc, byte-identical to this branch's 0aacafa4) and issue meshcore-dev#3546.