rtc: identify RTCs by documented always-zero bits; stop skipping real ones by ptr727 · Pull Request #99 · liquidraver/ZephCore

ptr727 · 2026-10-04T17:37:30Z

The MeshCore counterpart is meshcore-dev/MeshCore#3544 (open).

Code links are permalinks: dev at f9d01cf, this PR's commit 03f306b. Data sheets are cited by document revision, section and page:

Part	Document
DS3231	Maxim 19-5170 Rev 10
DS3232	Maxim 19-5337 Rev 5
DS1307	Maxim, REV 3/15
PCF8563	NXP Product data sheet Rev. 11.1, 19 January 2026
RV3028	Micro Crystal RV-3028-C7 Application Manual Rev. 1.4, November 2021
RX8130CE	Epson Application Manual ETM50E-10

The defect

rtc_probe() decides from one read whether the device at a declared address is the RTC its descriptor expects: a valid BCD block including hours and year (#L137-L141), or the power-loss flag set or unreadable.

Real RTCs are skipped. A chip whose year byte is not BCD 00-99, or whose hours are not 00-23 after masking 0x3F, is passed over while its flag is clear. Other firmware leaves both: MeshCore's RTClib writes bin2bcd(year - 2000), so a bad epoch puts A0 or more in the year, and a DS3231 or RV3028 in 12-hour mode reads PM hours with bit 5 set.
Nothing rules a device out by identity, and one read decides.
An erased EEPROM is adopted. Every byte 0xFF reads as a lost-power RTC through its 0xFF "status", and the EEPROM then gets time writes on every sync.

The fix

The rules of meshcore-dev/MeshCore#3544, with the differences listed under Notes:

Identity masks. A new optional zero-mask descriptor property (binding) holds the time-block bits each part's data sheet shows as 0, set in rtc-i2c.dtsi:

Descriptor	Mask (00h-06h of the block)	Source
0x68 DS3231 / DS3232 / DS1307	`00 80 80 F8 C0 60 00`	DS3231 Figure 1, p. 11; DS3232 Figure 1, p. 11; DS1307 Table 2, p. 8 ("0 = Always reads back as 0"). 05h bit 7 is Century on the DS3231/DS3232. 00h bit 7 is left out: it is the DS1307's clock-halt bit, set on a new chip (p. 8).
0x52 RV3028	`80 80 C0 F8 C0 E0 00`	§3.2, p. 12 ("Read only. Always 0")
0x32 RX8130CE	`80 80 C0 80 C0 E0 00`	§13.2.1 Table 12, p. 22 ("read value is always 0")
0x51 PCF8563	none	Table 4, p. 10 marks unused bits "not relevant"

Rule-out (rtc_ruled_out()). One read rules a device out if a masked bit is set, or if seconds, minutes, date or month are out of range (rtc_fields_ok()) while the power-loss flag does not read as set. An unreadable flag counts as not set. The year and hours are no longer identity checks.
Two reads (rtc_identify()). A device is passed over only when two reads each rule it out. A failed first read means nothing is there; a failed second read does not count, and an all-0xFF second read rules the device out. With no clean read the chip is adopted for write-back, but no time is taken from it.
Erased parts. An all-0xFF block is skipped, and the first save probes once more (zephcore_rtc_save()), since a real RTC can power up that way.
Restore (rtc_probe()) takes a time only from a clean read whose fields, hours and year are all valid. Otherwise the chip waits for the next sync.

Notes for review

Differences from Adopt an RTC only if its time registers read like that chip 🤖🤖 meshcore-dev/MeshCore#3544:
- With no clean read, MeshCore still reads a time. ZephCore takes none.
- MeshCore drops the RX8130CE field check, because its driver clears VLF on every boot without setting the time. ZephCore keeps the check, since its save clears the flag only after writing the time. The cost: an RX8130CE that lost power and then booted MeshCore once before any sync has VLF clear and undefined fields (ETM50E-10 Table 13, p. 23), and ZephCore passes it over (as dev does).
- An unreadable power-loss flag counts as not set here, so out-of-range fields still rule a device out, as rtc: do not adopt a chip whose status read failed (iteration branch) ptr727/liquidraver-ZephCore#30 does. MeshCore adopts in that case.
- All-0xFF is a single-read skip with one re-probe on the first save here; in MeshCore it is a two-read rule-out.
What the two reads buy. One passing read still adopts; the second read only gives a device ruled out by the first read another chance. Identity is strengthened by the masks. For a descriptor with no mask (the PCF8563, meshtracker_x1's node, a board override) it is slightly weaker than on dev, since the hours and year no longer count.
Interaction with rak4631: use a fitted RAK12002 RTC, and keep its backup switchover enabled #98. rak4631: use a fitted RAK12002 RTC, and keep its backup switchover enabled #98 (the RAK4631's RAK12002 support) adds an RV3028-only identity check on that board's descriptor. Both PRs edit rtc_probe(), so whichever merges second will be rebased on the other; once both are in, rak4631: use a fitted RAK12002 RTC, and keep its backup switchover enabled #98's RV3028 check becomes this PR's zero-mask on its descriptor. A third fork change, rtc: do not adopt a chip whose status read failed (iteration branch) ptr727/liquidraver-ZephCore#30 (a failed status read), touches the same function and is not upstream yet.
meshtracker_x1 declares its own 0x32 descriptor (RX8900 layout) without a mask, so only the field rule applies there. Its comment now says so.

Testing

Builds at 03f306b, with no new diagnostics (the only one is the existing ZephyrFsFormat.c:52 deprecation warning that plain dev also has):

rak4631 repeater
thinknode_m1
lilygo_techo
meshtracker_x1/nrf52840

Hardware, read-only, on a RAK4631 bus with three real I2C parts: a RAK12002 (RV3028) at 0x52, a RAK1902 (ST LPS22HB, WHO_AM_I = B1) at 0x5C, and a RAK1906 (Bosch BME680) at 0x76. A throwaway standalone Zephyr app ("T38", not on this branch) compiled this branch's ZephyrRTCDiscover.c unchanged at a7b43f6. It declared every chip layout at each of those addresses, plus 0x68, 0x51 and 0x32. It printed rtc_identify()'s verdict for each every 15 s; three passes were identical. It made I2C reads only, and had no filesystem, settings or radio. It ran on the MeshCore session's board, by arrangement, because that board carries the two extra modules.

Device	Address	DS3231 layout	PCF8563	RV3028	RX8130CE
RAK12002 RV3028	0x52	FOUND*	NOT_THIS	FOUND, flag clear	NOT_THIS
RAK1902 LPS22HB	0x5C	FOUND, flag set†	NOT_THIS	NOT_THIS	NOT_THIS
RAK1906 BME680	0x76	NOT_THIS	NOT_THIS	NOT_THIS	NOT_THIS
none	0x68 / 0x51 / 0x32	ABSENT	ABSENT	—	ABSENT

* An RV3028's registers also satisfy the DS3231 layout. No board declares a DS3231 at 0x52.
† The LPS22HB's WHO_AM_I (B1) sits where a DS3231's OSF bit is, so it reads as "power lost" and is not ruled out. meshcore-dev/MeshCore#3544 records the same limit. No board declares a DS3231 at 0x5C.

These match meshcore-dev/MeshCore#3544's table except the RX8130CE layout at 0x5C, which ZephCore rules out by fields and MeshCore does not (the agreed difference above). The later commits change the verdicts only by dropping the DS3231 mask's 00h bit 7. None of these verdicts depended on that bit: at 0x76 the BME680 is still ruled out by 01h bit 7 (AA).

Raw log: comment on the fork iteration PR. The boards sit indoors with no GPS fix, so GPS never set the clock. Behaviour with a real fix was covered by outdoor testing for the earlier upstream RTC and GNSS changes.

Not tested on hardware: a DS3231, DS3232, DS1307, PCF8563 or RX8130CE (none is fitted here); an erased EEPROM; a part in 12-hour mode.

Known limitations

A set power-loss flag does not waive the zero-mask, matching Adopt an RTC only if its time registers read like that chip 🤖🤖 meshcore-dev/MeshCore#3544. The DS3231 and DS3232 sheets leave the power-up state undefined, so a new part could in principle show a 1 in a masked bit and be refused. The sheets show those bits as 0 and give no way to write them.
12-hour mode is neither decoded nor cleared: fork issue RTC probe assumes 24-hour mode: an RV3028 left in 12-hour mode is misread or never adopted ptr727/liquidraver-ZephCore#34.
A DS1307 with clock-halt set and a recent time is restored even though stopped. On a DS1307, 0Fh is RAM, so the flag test reads a user byte. Both are as on dev.
The PCF8563 has no mask, so any device at 0x51 whose register 02h has bit 7 set reads as "power lost".
A non-RTC part whose first read is ruled out and whose second read fails is adopted (MeshCore parity).
The RX8130CE's weekday is written in binary, not one-hot: fork issue RTC save writes the weekday in binary; the RX8130CE's WEEK register is one-hot ptr727/liquidraver-ZephCore#39, pre-existing; MeshCore's fix is Write and read the RX8130CE WEEK register as one-hot 🤖🤖 meshcore-dev/MeshCore#3550.
No host test exercises rtc_identify() or rtc_probe(); the hardware run above is the evidence.

Fork issue: ptr727#38. Iteration history and review rounds: ptr727#40. MeshCore counterpart: meshcore-dev/MeshCore#3544.

🤖 Generated with Claude Code

Copilot

Copilot review overview

🔵 Needs a closer look

The RTC probing changes are hardware-sensitive and retain documentation gaps around retry behavior, warranting final human review.

Review effort: Lite
Findings: 1

Open (1)

Document second all-0xFF read and missing retry behavior · New

What changed in this PR

This PR strengthens RTC auto-discovery using documented register masks, repeated reads, and erased-device handling.

Changes:

Adds optional zero-mask RTC descriptors.
Refines RTC validation and probing behavior.
Updates board descriptors and API documentation.

File	Summary
`zephcore/dts/bindings/rtc/zephcore,rtc-i2c.yaml`	Defines `zero-mask`; documentation should mention the second all-`0xFF` rule-out and retry behavior.
`zephcore/boards/nrf52840/meshtracker_x1/meshtracker_x1_nrf52840.dts`	Documents board-specific RTC discovery behavior.
`zephcore/boards/common/rtc-i2c.dtsi`	Adds documented masks for supported RTCs.
`zephcore/adapters/clock/ZephyrRTCDiscover.h`	Updates discovery and save contracts; the second all-`0xFF` case needs documentation.
`zephcore/adapters/clock/ZephyrRTCDiscover.c`	Implements strengthened identity checks, retries, and erased-device handling.

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

From Copilot's review of liquidraver#99: the header's zephcore_rtc_restore() contract did not say that an all-0xFF second read rules a device out, which rtc_identify() has done since c947e26, nor that, unlike a first-read all-0xFF skip, it does not itself cause the first save to probe again. The restore paragraph now lists the cases as the code checks them: a failed first read (nothing there), an all-0xFF first read (skipped), then the two-read rule-out, whose list now includes the all-0xFF second read. The save paragraph now says a first-read all-0xFF skip makes the first save probe again when nothing was adopted, and that a device ruled out on its second read is not probed again for that reason (it is re-identified only if another candidate's skip triggers the re-probe). The same paragraphs, and two comments in the .c, also now match the code where they over-claimed: probing runs in order and stops at the first sane time; masked bits are ones the data sheet shows as 0, as the binding and dtsi say; a time needs its fields, hours and year each in range, since restore re-checks them (87b86d6), with 12-hour mode not decoded (fork issue #34). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

… ones rtc_probe() decided from one read whether the device at a declared address was the RTC its descriptor expects: a valid BCD block including hours and year, or the power-loss flag set or unreadable. That skipped real RTCs whose year byte or hours another firmware left out of range (a year byte of A0 or more; 12-hour mode reads PM hours with bit 5 set), ruled nothing out by identity, and adopted an erased EEPROM through its 0xFF "status". - An optional zero-mask descriptor property holds the time-block bits a part's data sheet shows as 0. rtc-i2c.dtsi sets it for 0x68 (DS3231 19-5170 Rev 10 and DS3232 19-5337 Rev 5, Figure 1, p. 11; DS1307 Rev 3/15, Table 2, p. 8; 00h bit 7 left out, the DS1307's clock-halt bit), the RV3028 (App Manual Rev 1.4, 3.2, p. 12) and the RX8130CE (ETM50E-10, 13.2.1 Table 12, p. 22). The PCF8563 gets none: its data sheet (Rev 11.1, Table 4, p. 10) marks unused bits "not relevant". - One read rules a device out if a masked bit is set, or if seconds, minutes, date or month are out of range while the power-loss flag does not read as set. The year and hours are no longer identity checks. - A device is passed over only when two reads each rule it out. A failed first read means nothing is there; a failed second read does not count, and with no clean read the chip is adopted for write-back but gives no time. An all-0xFF second read rules the device out. - An all-0xFF first read is skipped, and the first save probes once more, since a real RTC can power up that way. - A time is restored only from a clean read whose fields, hours and year are all valid. The decision is one function, rtc_identify(). The same rules, with a few agreed differences, are in meshcore-dev/MeshCore#3544. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…idraver#98's rebase The owner's review of liquidraver#98 asked for a rebase onto dev dropping what liquidraver#99 provides, plus four changes. This merge carries all of them: - liquidraver#99's identify rules replace this branch's all-0xFF skip and re-probe, its hardcoded RV3028 always-zero check, and its "time unreadable" path. The RAK4631 node now carries zero-mask, as rtc-i2c.dtsi does. - The EEPROM store runs only on a clean identification (RTC_FOUND). A device adopted on a failed second read (RTC_FOUND_GARBLED) is the time write-back target but is never configured. - At an rv3028-eeprom-config descriptor the time is read once more with BSF cleared before and checked after (manual 4.2, p. 45: a switchover disables the I2C interface, so a read's tail comes back 0xFF, and a tail inside the year byte can still be valid BCD). Known limitation 1. - All RV3028 code is gated on DT_ANY_COMPAT_HAS_PROP_STATUS_OKAY, so a board without rv3028-eeprom-config carries none of it. - The RV3028 defines no longer split the BCD2BIN/BIN2BCD pair. - The overlay comment says any other RV3028 at 0x52 gets the same settings, trickle charging included. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Third review round, authorized by Pieter, plus two rules agreed across the MeshCore, ZephCore and Zephyr RV3028 work: - Time write order: year 00h first, 00h-05h, then the real year, inside the BSF bracket. A write cut by a power loss, which takes the MCU down and so is never repeated, leaves year 2000: the next boot reads "time not yet set" and still adopts the chip, since identity ignores the year. Month 00h was rejected as the marker: liquidraver#99's identity rules would then rule a real RV3028 out. Writing seconds restarts the prescaler (4.5.1), so no tick lands between the writes. MeshCore #3421's work uses the same order. - EEbusy: after a Refresh that did not complete, and for the RAM-only fallback, 37h is written with switchover on only once EEbusy reads 0; otherwise switchover stays off until a retry or the daily refresh (3.15.6). Same rule as Zephyr #121252's work and MeshCore #3545's. - A store skipped at boot counts as the first try, so it gets 3 retries like a failed one, not 4. - The time-write repeat stops after 12 attempts (about a minute) and logs once, so a locked or silent chip is not written every 5 s for the rest of uptime. - The header states that zephcore_rtc_save() must run on the system work queue, which its repeat shares state with. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…r lines Comment and documentation only; the RTC object's code is byte-identical in release and debug builds. From the fourth local review pass: - A save is "confirmed" by acknowledgements and a clear BSF, not a read back, so a password-locked chip that ignores the writes passes; the comment no longer says it never confirms. - After a failed store, switchover is off only if the store had already disabled it; header and binding now say so. - The write-order comment credits the final year write, not the burst, with overwriting a tick that lands before it. - The Kconfig help names the "chip is the one adopted" condition. The header's RV3028 text is now its own paragraph after liquidraver#99's, so liquidraver#99's lines are no longer reflowed, and over-long lines are rewrapped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

dev brought liquidraver#99's RTC identification (rtc_identify() and its verdicts), liquidraver#98's RAK12002 and RV3028 EEPROM configuration, liquidraver#100, and the rename of docs/Repeater_CLI_commands.md to docs/CLI_commands.md. The hw docs moved with the rename without conflict. ZephyrRTCDiscover.c conflicted where hw recorded a report state at the inline reads liquidraver#99 replaced. dev's logic is taken whole, and each verdict now maps to a report state: an address NACK or RTC_NOT_THIS is absent, RTC_FOUND and RTC_FOUND_GARBLED are present. rtc_identify() gains RTC_UNREAD for a bus that is not ready or a read failing other than with -EIO, which keeps hw's "unprobed" for those; discovery skips it exactly as it skips RTC_ABSENT. liquidraver#99's all-0xFF skip is neither absent nor present, so it gets its own state, rendered "all 0xff". It counts with unprobed toward the summary, now worded "unsettled", since neither settles whether an RTC is fitted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI lite review requested due to automatic review settings October 4, 2026 17:37

This was referenced Oct 4, 2026

rak4631: use a fitted RAK12002 RTC, and keep its backup switchover enabled #98

Merged

RTC probe: identify RTCs by documented always-zero bits, stop skipping real ones (iteration branch) ptr727/liquidraver-ZephCore#40

Closed

Copilot started reviewing on behalf of ptr727 October 4, 2026 17:37 View session

Copilot AI reviewed Oct 4, 2026

View reviewed changes

Comment thread zephcore/adapters/clock/ZephyrRTCDiscover.h Outdated

ptr727 mentioned this pull request Oct 4, 2026

Session Handoff [hw-cli]: liquidraver/ZephCore #98 and #99 upstream; monitor them, reconnect with MeshCore and Zephyr sessions ptr727/liquidraver-ZephCore#41

Closed

ptr727 force-pushed the rtc-probe-identity branch from 41d6f76 to 03f306b Compare October 4, 2026 18:49

liquidraver merged commit 158df92 into liquidraver:dev Oct 5, 2026
1 check passed

ptr727 mentioned this pull request Oct 5, 2026

Session Handoff [hw-cli]: liquidraver/ZephCore#98 rebased onto #99 and pushed; fork #44/#45 ready for upstream on Pieter's go ptr727/liquidraver-ZephCore#46

Closed

ptr727 deleted the rtc-probe-identity branch October 5, 2026 15:52

ptr727 mentioned this pull request Oct 5, 2026

Session Handoff [rv3028-eeprom]: Two RTC PRs re-pushed with cross-firmware fixes; triage Copilot headlines ptr727/meshcore-dev-MeshCore#19

Closed

Navigation Menu

Repository navigation

rtc: identify RTCs by documented always-zero bits; stop skipping real ones - #99

rtc: identify RTCs by documented always-zero bits; stop skipping real ones#99
liquidraver merged 1 commit into
liquidraver:devfrom
ptr727:rtc-probe-identity

ptr727 commented Oct 4, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ptr727 commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The defect

The fix

Notes for review

Testing

Known limitations

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ptr727 commented Oct 4, 2026 •

edited

Loading