AutoDiscoverRTCClock adopts any device that ACKs at an RTC address, then reads it as the clock and writes it on every sync
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 48/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Active
- Tech stack
- c
- Domain
- embedded-iot
Research direction
Start with src/helpers/AutoDiscoverRTCClock.cpp, especially begin(), getCurrentTime() and setCurrentTime(), then review the cited RTC libraries and datasheet checks in related issue #3544. Verify that detection rules out incompatible devices at each RTC address and preserves the stated behavior for failed reads. Done means the probe no longer adopts devices based on an I2C ACK alone; #3544 describes a read-only hardware test on a RAK4631.
Written by the indexing model from the issue text.
Description
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
clockandtimecommands, 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
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:
- 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.
- Dominant language
- C
- Stars
- 3.9k
- Forks
- 1.5k
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 26
Getting set up
This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from meshcore-dev/MeshCore
-
Difficulty 2/5 1-3 hours Newbie friendliness 63/100
meshcore-dev/MeshCore#3622 ·
Maintainers usually reply within 1 day
-
[Feature Request] Set Australia (Mid) Preset to automatically enable 2 byte default path hash sizeOpen
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
meshcore-dev/MeshCore#3571 · 5 reactions ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
meshcore-dev/MeshCore#3567 · 1 comment ·
Maintainers usually reply within 1 day
-
RTC_RX8130CE writes and reads the one-hot WEEK register as a numberPossibly taken A pull request linked to this issue is open or already merged. Open
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
meshcore-dev/MeshCore#3549 ·
Maintainers usually reply within 1 day
-
Repeater: CLI reply to 'setperm <key> 0' is lost when the removed ACL entry sits before the requester (stale ClientInfo* after array shift)Possibly taken A pull request linked to this issue is open or already merged. Open
Difficulty 2/5 1-3 hours Newbie friendliness 77/100
meshcore-dev/MeshCore#3513 ·
Maintainers usually reply within 1 day
All issues in meshcore-dev/MeshCore
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 66/100
Maintainers usually reply within 1 day
-
Bug Status: Needs Triage
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
category:port-update
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
Maintainers usually reply within 2 days
-
Update OpenBSD CI to OpenBSD 8.0 and vmactions/openbsd-vm v1.4.9Possibly taken @hyoklee claimed this today. Open
Difficulty 1/5 1-3 hours Newbie friendliness 85/100
Maintainers usually reply within 2 days
-
v9.6.0: CMake incorrectly enforces presence of libevdevPossibly taken A pull request linked to this issue is open or already merged. Open
Difficulty 2/5 1-3 hours Newbie friendliness 67/100
Maintainers usually reply within 1 day