Add TimeSync support for nodes with TimeSynchronization cluster#440
Add TimeSync support for nodes with TimeSynchronization cluster#440markvp wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds proactive UTC time synchronization for Matter nodes that implement the TimeSynchronization cluster, ensuring devices that lose time after power loss are resynced on reconnect, on timeFailure, and periodically.
Changes:
- Introduce
TimeSyncManagerwith cluster detection and periodic resync scheduling - Integrate
TimeSyncManagerinto controller lifecycle (connect, events, removal, close) - Add ws-controller test compilation config and unit tests for sync triggers
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ws-controller/tsconfig.json | Adds test project reference so ws-controller tests are typechecked/compiled. |
| packages/ws-controller/test/tsconfig.json | New tsconfig for ws-controller tests, wiring in shared test settings and references. |
| packages/ws-controller/test/controller/TimeSyncManagerTest.ts | New unit tests for cluster detection + immediate/reactive sync behavior. |
| packages/ws-controller/src/controller/TimeSyncManager.ts | Implements the TimeSyncManager (register/unregister, event-driven sync, periodic resync). |
| packages/ws-controller/src/controller/ControllerCommandHandler.ts | Wires TimeSyncManager into node lifecycle/events and adds setUtcTime command sender. |
|
How will it work when multiple matter controller wants to sync time. For example, if I connected my IKEA ALPSTUGA to 2 matters server with different time. I know it's an edge case but we have concurrency issue because each control may have different logic to sync time. |
|
@piitaya I completely agree :-) Thats why also the real official solution is a bit more effort :-) ... and I need to real all spec on it |
In Home Assistant the Supervisor exposes if the time go synchronized through NTP (see Currently we don't have an event when synchronization state changes 😢 . So we'd probably have to poll the flag for a while if it is false on startup. |
|
Ok, so from what I get ... the interestung question is how to determine IF a host time is synced with any relevant source and so a valid source for it ... and when that happened ... that's maybe a tough one |
|
Sooo, we had a discussion yesterday because we should know if the host is synced time wise:
|
|
@Apollon77 do you want me to work on this or do you want to close it. IF you want me to work on it, please let me know what you would like me to do / not do. |
|
Give all my comments to your AI and I guess he can work with it. If you like feel free to continue. If now I will take it over somewhen ;-) |
|
Would be great to get this one finally over the line. FWIW, having an ENV var to enable the time sync would work for me. |
Review addressedAll feedback from @Apollon77 and @copilot-pull-request-reviewer has been incorporated in the latest commit. Summary of changes: New:
New:
Tests rewritten
|
|
@Apollon77 please can you confirm if my updates address all your concerns or if there are any other changes required? thanks for your patience while I try to get this right :) |
|
@markvp Yes I have seen the updates and did not had time so far to review. |
04564d7 to
5f36f5b
Compare
|
This has been open for 2 months now. I've addressed all your requests and changes. What is the issue? |
|
My last comment is 3 weeks ago and in that time I had a week vacation and the current priority is to have a stable server baseline for the official release. New features like this I will focus on once everything is stable for the official release. Just be patient please |
|
Will continue once the new matter server is released and stable, I want to keep changes low currently to be able to do bugfixes without bigger changes |
|
@markvp Ok now we can continue. Could you please rebase on current changes? Thanks a lot |
…n cluster Introduces opt-in time synchronization via --enable-time-sync / ENABLE_TIME_SYNC. Adds NodeProcessor abstract base class for timer-driven periodic node processing, and TimeSyncManager which extends it to sync UTC time using PeerAddress (matching the CustomClusterPoller upstream migration). Syncs on reconnect after startup window and periodically every 24 hours. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rClientByIdFor Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Eliminates duplicated timer/loop boilerplate (~50 lines) by extending the shared NodeProcessor abstraction. Per-node attribute paths stay in the subclass; shouldProcess, processNode, and onCycleComplete implement the polling logic. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tests cover: registration (startup window guard, connectivity check, cluster detection), syncNode (immediate fire, deduplication, in-flight tracking), unregisterNode, periodic resync via NodeProcessor timer (startup delay, 24h cycle, skip disconnected/in-flight peers), and stop() awaiting in-flight syncs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Math.random() produces a float; @matter/testing's MockTimer requires integer milliseconds and throws on fractional values. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
21d497c to
a9f9697
Compare
done. tests are being blocked by your "request changes". |
Apollon77
left a comment
There was a problem hiding this comment.
makes sense ... I proposed some smaller changes ...
the only detail we need to find out is how we get this parameter set by the integration and how we handle it when fresh-starting the server where the time might not be synced already
Cache #peerOf(nodeId) in a local variable at all three call sites where both customClusterPoller and timeSyncManager receive the same peer, avoiding a redundant call. Simplify hasTimeSyncCluster to a direct attribute key lookup on the Granularity attribute (0/56/1) rather than iterating all keys. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ute 0 The hasTimeSyncCluster implementation now checks specifically for the mandatory Granularity attribute (ID 1) on endpoint 0, rather than any attribute from the cluster. Update test fixtures and assertions to match: makeTimeSyncAttrs uses attribute 1, and the 'any attribute index' test is replaced with a test that confirms non-Granularity attributes return false. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Companion add-on PR submitted to wire up the That PR adds an This way the responsibility for ensuring that the time is actually in sync, or waiting until the time is in sync, lies with the "user" - for HA this is the HA-Addon. If installing this independently, it's the environment in which you install it. |
Add an enable_time_sync boolean option (default false). When enabled, the startup script passes --enable-time-sync to the Matter Server so it pushes the current time to commissioned Matter devices. Before passing the flag, the script checks host NTP sync state via the Supervisor API (GET /supervisor/info, field ntp_synchronized) and logs an advisory warning if the host clock is not synchronised. The warning does not gate the feature, since the time sent to devices is only as accurate as the host clock. The --enable-time-sync flag is only passed when the running Matter Server is new enough to support it (>= 1.2.0). The server uses commander, which exits on unknown options, so on older builds the flag is skipped with a warning rather than breaking startup. Companion to the server-side flag in matter-js/matterjs-server#440. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a time_sync option (auto/on/off, default auto) controlling whether the Matter Server pushes the current time to nodes with the TimeSynchronization cluster, via the server's --enable-time-sync flag. - auto (default): enable only when the host clock is NTP synchronised, as reported by the Supervisor API (GET /supervisor/info, ntp_synchronized). - on: always enable, logging a warning if the host clock is not NTP synchronised, since the time sent to devices is only as accurate as the host clock. - off: leave disabled. The --enable-time-sync flag is only passed when the running Matter Server is new enough to support it (>= 1.2.0). The server uses commander, which exits on unknown options, so on older builds the flag is skipped with a warning rather than breaking startup. Companion to the server-side flag in matter-js/matterjs-server#440. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
TimeSyncManagerthat proactively syncs UTC time on nodes with the TimeSynchronization cluster (0x38)timeFailureevent (reactive), and periodic resync every 12 hoursCustomClusterPollerpattern for consistencyContext
Devices like IKEA ALPSTUGA lack battery backup and lose their time after power loss. The controller previously only set time during commissioning, causing
timeSynchronization.timeFailureevents after reconnection.The
setUtcTimecommand is sent withMillisecondsGranularityandTimeSource.Admin.TlvEpochUshandles automatic conversion from Unix epoch to Matter epoch (2000-01-01).Changes
packages/ws-controller/src/controller/TimeSyncManager.ts—TimeSyncManagerclass andhasTimeSyncCluster()utilitypackages/ws-controller/test/controller/TimeSyncManagerTest.ts— 12 unit testspackages/ws-controller/test/tsconfig.json— enables test compilation for ws-controllerpackages/ws-controller/src/controller/ControllerCommandHandler.ts— integrates TimeSyncManager at all lifecycle pointspackages/ws-controller/tsconfig.json— adds test referenceTest plan
npm run formatpassesnpm run lintpasses (0 warnings, 0 errors)npm run buildpasses (including test type validation)npm test— all 12 ws-controller unit tests passCloses #245
🤖 Generated with Claude Code