Repository navigation
Sync csa branch with main - #1273
Merged
Merged
Conversation
… platform-bot) (#74682)
* Add support for BRD4343C * Update is_si917y_mcu block * Update is_si917y_mcu block * Update matter_support submodule
…ed by platform-bot) (#74683)
…Auto-merged by platform-bot) (#74464) When the debug console is configured with non-blocking transfer via SerialManager, log output is buffered in a ring buffer and flushed asynchronously by a lower-priority SerialManager task. After RunAllTests() returns, test_task exits and FreeRTOS enters prvTaskExitError(), which spins at the task's high priority. This starves the SerialManager task, preventing the final test results (including "Done running all tests") from being flushed to UART, causing the CI harness to time out. Add an infinite loop after RunAllTests() to keep test_task alive, matching the pattern already used in the FSL_OSA_MAIN_FUNC_ENABLE path.
…gen integration) (#74318)
* [MFM] Regenerate Media File Management XML with Alchemy v1.7.7 Regenerate media-file-management-cluster.xml from current spec master. Fixes #74557: SupportedMimeTypes read privilege is View per spec (was Manage), which failed TC-ACE-2.1 / TC-ACE-2.4. The regen also brings in other spec changes: - ImageUri -> ImageURI (FileDescriptionStruct, AddFile, OfferFile) - RequestSharedFiles.SupportedMimeTypes is no longer nullable - GetSharedFileResponse.FileDescription is mandatory (still nullable) - TotalStorage/AvailableStorage drop the default="0" Update the MFM server, delegate, unit tests, tv-app, tv-casting-app and Python test base for the renamed and re-typed fields. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Rename imageUri to imageURI in tv-casting Darwin bridge structs Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…tch entries by value (#74429) * fix(jfds): compare CommitFailed failureCode as an IM Status Code statusEntry.failureCode holds an Interaction Model Status Code. The RefreshNode triage for CommitFailed group, binding, node key set and ACL entries converted it with CHIP_ERROR(uint8_t), which builds an SdkPart::kCore error, and compared that against CHIP_IM_GLOBAL_STATUS(), which builds an SdkPart::kIMGlobalStatus error. The two never compared equal, so ConstraintError and ResourceExhausted entries were classified as recoverable and retried instead of removed. The triage now casts failureCode to Protocols::InteractionModel::Status and compares it against the Status values, as RefreshNode specifies (Matter Core R1.4.2, 11.24.7.11). The group entry branch still skips the entry in both cases; only its comparison changes here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(jfds): add value-identity matching for ACL and binding entries The Access Control Acl attribute and the Binding attribute carry no per-entry identity on the wire: the datastore's listID and statusEntry are not written to the node, so an entry fetched from a node can only be matched to a datastore entry by its value. Add pure functions in chip::app::detail for that: - AclEntryValueEquals compares privilege, authMode, subjects and targets. Subjects and targets are compared as multisets. A null list equals an empty one: AclStorage reports an empty subject or target list as null, while the datastore sends an empty non-null list. - BindingEntryValueEquals compares endpointID and the binding target. - ApplyAclEdit and ApplyBindingEdit return the list that results from applying one add, replace or DeletePending removal to a node's fetched list. Removal erases every equal entry and treats an absent entry as already removed. An ACL removal or replacement also erases the superseded value, if one is given. The bodies of BindingMatches and ACLTargetMatches move into detail::BindingTargetValueEquals and detail::AclTargetValueEquals; the private members forward to them, so existing callers are unchanged. Also add datastore::ACLEntryStruct::supersededValue and a private EncodeAclEntryForSync, both unused until later changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): record failed syncs as CommitFailed Every Delegate::SyncNode completion receives the result of the operation, but the datastore's completions returned on error without recording anything: a failed add stayed Pending and a failed removal stayed DeletePending. When SyncNode failed to start, most callers returned the error and left the entry unchanged. Completions and start failures now record CommitFailed, with the IM status of the error as FailureCode, as the specification describes for each operation. Removal completions erase the entry only on success. CommitFailed is the same state for a failed add and a failed removal, and RefreshNode re-writes recoverable CommitFailed entries. Entries being removed are therefore also marked internally: a pendingRemoval flag on ACL entries, and sets keyed by entry identity for bindings, endpoint groups and node key sets. RefreshNode leaves those ACL and binding entries out of its full-list writes and erases them once the write succeeds, and does not re-write node key sets being removed. Add paths clear any removal mark left for their key, since list IDs and group assignments can recur. The Delegate interface now documents its contract: an accepted call completes exactly once, argument spans are valid only during the call, fetched vectors only during the callback, and delegates do not modify the vectors they are given. In the Linux jf-admin-app delegate: - Connection failures, GroupKeyMap write and reconnect failures, KeySet read failures, AddGroup start failures and start failures of the write, invoke or read in OnDeviceConnectedFn now complete the operation with the error. A key set write for an unknown key set ID completes with CHIP_ERROR_NOT_FOUND. Previously these paths released the command without calling the completion. - CallbackContext::Complete clears the stored callbacks after invoking them, so each operation completes once. - The binding single-entry sync completes with the fetch error when the binding list cannot be read, instead of writing a list that holds only the new entry. - OnWriteFailureResponse no longer writes CommitFailed and a truncated error code into the vector passed to SyncNode. - When GetConnectedDevice fails, SyncNode and Fetch* release the in-flight command before returning the error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): tear down RefreshNode in one place RefreshNode reset its state at about a dozen separate sites, and some paths reset nothing: a RefreshNode call that failed synchronously, for example for an unknown node, left mRefreshingNodeId set and every later refresh returned CHIP_ERROR_INCORRECT_STATE. Errors from ContinueRefresh inside callbacks were ignored. FinishRefresh now ends a refresh and resets all of its state, including the per-pass lists and indices. It is the only place that clears mRefreshingNodeId. RefreshNode calls it when ContinueRefresh fails, and every callback calls it when the next step fails. A failed binding write, group entry write or key set write no longer ends the refresh: the failure is recorded on the entries and the remaining stages still run, including the ACL write. The node is marked Committed only if every stage succeeded; otherwise it stays Pending. On a successful binding write, every written Pending or CommitFailed binding is now marked Committed with FailureCode 0. Previously the completion copied the state of the written copy back, so a binding that was being retried after CommitFailed stayed CommitFailed. The binding and ACL completions key on the node ID captured when the write was issued. On success they also erase entries being removed, and the ACL completion clears supersededValue on the entries it commits. On failure they record CommitFailed on the written entries and on entries being removed, which keep their removal intent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): match refreshed ACL entries by value The Access Control Acl attribute has no per-entry identity: the ACL entries RefreshNode reads from a node carry no listID, so every fetched entry had listID 0. The merge matched fetched entries to datastore entries by listID, so it imported only the first fetched entry, and treated datastore entries with other listIDs as absent from the node. The merge now compares values with detail::AclEntryValueEquals. For each fetched entry, in order: - If it equals a datastore entry for the node, in any state, that entry is kept with its listID and state. - If it equals the supersededValue of an entry for the node, it is not adopted. The node still holds the value from before a Pending update, and the refresh write replaces it. - Otherwise it was added outside the datastore and is adopted as Committed with a new listID. RefreshNode says the remaining entries should be replaced by the entries on the node; this is a "should", and adopting a superseded value would undo the pending update. Datastore entries the node does not hold are erased if they are Committed or being removed (the removal has taken effect). Fixes #74377. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): send the real entry when removing groups and key sets RefreshNode's endpoint-group and key-set removal steps, and RemoveNodeKeySetEntry, passed a zero-initialized entry to SyncNode. Its state is Pending and its IDs are 0, so the Linux delegate sent AddGroup(0) or KeySetWrite for key set 0 instead of RemoveGroup or KeySetRemove. They now send a copy of the entry being removed, in the DeletePending state. The endpoint-group step now handles CommitFailed entries like the other lists. Previously it skipped them whether or not the failure was recoverable. Unrecoverable failures are erased; recoverable ones are retried as an add, or as a removal for an entry being removed. It also collects its work before issuing any SyncNode: a completion that ran synchronously erased the entry the loop's iterator pointed at. The key-set removal step now also covers entries whose earlier removal failed, and RemoveNodeKeySetEntry records the removal intent, erases the entry only on success, and records CommitFailed on failure. The endpoint-group and binding fetch merges now erase entries the node no longer holds if they are Committed or being removed, whatever their state. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): limit RefreshNode's binding merge to the fetched endpoint RefreshNode fetches the Binding attribute one endpoint at a time and then erased every Committed or DeletePending binding of the node that was not in that endpoint's list, including bindings on other endpoints. Because the root endpoint is always fetched, a refresh erased the node's bindings on every other endpoint, or re-imported them with new list IDs. The merge now only erases bindings on the endpoint whose list it fetched, as the endpoint-group merge already does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): keep removing entries whose removal failed unrecoverably RefreshNode must drop an entry whose CommitFailure is unrecoverable (CONSTRAINT_ERROR or RESOURCE_EXHAUSTED). When that entry was being removed, the node still holds it, and the next refresh's merge adopted the node's copy as a new Committed entry, or added it back as a missing entry for bindings and endpoint groups. The refresh triage now drops such entries first, as specified, and records a tombstone for the value of any dropped entry that was being removed (for ACLs, also its superseded value). When a later refresh finds a tombstoned value on the node, it adds the value back as DeletePending with removal intent, and that refresh removes it. For bindings and endpoint groups this is the "add any missing entries" step; for ACLs and key sets the specification says the remaining entries "should" be replaced by the node's, which this refines. A tombstone is dropped when a refresh finds the node no longer holds its value, and each tombstone list is capped at the size of the list it shadows. Adding an entry equal to one that is being removed now cancels the removal: AddACLToNode, AddBindingToEndpointForNode, AddGroupIDToEndpointForNode and AddNodeKeySetEntry clear the removal intent, mark the entry Pending and sync it as an add. Previously they returned early and the removal went ahead. An add also drops any tombstone for its value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): serialize read-modify-write syncs per node A single-entry ACL or binding sync reads the node's whole Acl or Binding list, edits it and writes it back. Two such syncs for the same node could run at the same time, and the second write replaced the first. A sync could also run while RefreshNode was writing the same list. The datastore now runs at most one of these operations per node at a time. AddACLToNode, RemoveACLFromNode, the ACL updates in UpdateGroup, AddBindingToEndpointForNode and RemoveBindingFromEndpointForNode record their change, then start the sync, or queue it behind the node's operation in flight. A queued operation looks its entry up by node and list ID when it starts: it syncs a removal if the entry is being removed, an add of the current value if the entry is not Committed, and nothing otherwise. A completion applies only if the entry is still being added or removed as when its sync started, so an add that cancels an in-flight removal is kept. RefreshNode returns BUSY while the node has operations in flight or queued, and holds the node until it finishes; operations requested in the meantime run afterwards. The refresh no longer erases an entry whose removal was requested while its write was in flight, since that write still contained the entry. The queue lives in the datastore rather than in the delegate so that it applies to every delegate and can be unit tested. It is bounded at kMaxACLs waiting operations per node; beyond that the operation returns RESOURCE_EXHAUSTED. Like the rest of the datastore it uses std::map and std::deque, which allocate on the heap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jf-admin-app): replace and remove ACL and binding entries by value The Acl and Binding attributes have no per-entry identity: entries read from a node carry no nodeID or listID. The Linux delegate's single-entry syncs still matched on them. An ACL removal compared nodeID and listID against the fetched entries, which are always 0, and an update appended the new value next to the old one. A binding add appended the entry even if the node already held it. The single-entry ACL SyncNode overload now also takes the entry's superseded value, and the datastore sends the full stored value for removals instead of an entry carrying only nodeID and listID. The delegate applies the edit with detail::ApplyAclEdit, and bindings with detail::ApplyBindingEdit, then writes the resulting list: - a removal removes every entry equal to the value, and to the superseded value if set; - an update removes the superseded value and adds the new one; - an add is skipped if an equal entry is present. Removing a value the node does not hold, or several equal copies of it, is not an error: identical entries are the same grant, and an absent entry is already removed. UpdateGroup records an ACL entry's previous value as supersededValue before changing its privilege, keeping the oldest value across back-to-back updates, and every commit clears it. When an update succeeds after the entry was updated again, the sent value becomes the superseded value for the next sync. This changes the signature of the single-entry ACL overload of JointFabricDatastore::Delegate::SyncNode; the three-argument form is removed. Joint Fabric is provisional, and the jf-admin-app delegate is the only implementation in the tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(jfds): update RefreshNode comment in TC_JFDS_BindingSync RefreshNode now keeps all of a node's ACL entries, so the comment no longer says it keeps only part of them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): log RefreshNode stage failures A failed group, key set or binding write during RefreshNode leaves the node Pending, but only the binding write logged its error. The refresh now logs every failed write with its error. A failed fetch of an endpoint's groups or bindings, which also happens for endpoints without the Groups or Binding cluster, is logged, and the merge for that endpoint is skipped as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * test(jfds): pass MakeEndpointGroupEntry's failure code as uint8_t DatastoreStatusEntryStruct's failureCode is a uint8_t. The helper took a uint32_t, and Darwin builds, which enable -Wimplicit-int-conversion, rejected the narrowing assignment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): do not commit ACL updates made during RefreshNode's write RefreshNode's ACL write holds the node's sync slot, so a permission change made by UpdateGroup while it is in flight is queued behind it. On success, the write's completion marked every Pending entry it had written as Committed and cleared its superseded value, although the write carried the value from before the change. The queued sync then found a Committed entry and did nothing: the node kept the old privilege while the datastore reported the new one as Committed. The refresh now keeps owned copies of the values it writes. The completion commits an entry only if its value still equals the written one. Otherwise the written value becomes the entry's superseded value, and the queued sync replaces it, as single-entry ACL syncs already do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): ignore group and key set removals cancelled by an add Group and key set syncs are not serialized per node, so an add can cancel a removal while the removal is in flight. The removal's completion erased the entry by key alone, including the re-added entry, whose own completion then found nothing to commit. The node held the group while the datastore no longer listed it. A failed removal likewise marked the re-added entry CommitFailed. The removal completions in RemoveGroupIDFromEndpointForNode, RemoveNodeKeySetEntry and RefreshNode's group and key set stages now apply only to entries that are still being removed, as ACL and binding completions already do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): match ACL entries by value in AddACLToNode AddACLToNode looked for an existing entry with ACLMatches, which compares subjects and targets in order, while the node's edits and RefreshNode's merge compare them as multisets. Adding an entry that differed only in subject or target order created a second datastore entry that was the same entry on the node. Removing either one then removed the node's only copy, and the next refresh dropped the other from the datastore. AddACLToNode now decodes the entry first and matches it with detail::AclEntryValueEquals. ACLMatches, and ACLTargetMatches, which only it used, are removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * refactor(jfds): store removal intent on binding, group and key set entries Removal intent for bindings, endpoint groups and node key sets lived in three sets keyed by each entry's identity, beside the entries themselves. Every erase and add had to clear the matching key, and RemoveNode, which erases only the node record, left keys behind that would mark a later entry with the same identity as being removed. The entries are now stored as datastore::EndpointBindingEntryStruct, datastore::EndpointGroupIDEntryStruct and datastore::NodeKeySetEntryStruct. Each extends the generated cluster type with a pendingRemoval flag, as datastore::ACLEntryStruct already does, so the intent is created and destroyed with its entry. MarkRemovalRequested and HasRemovalIntent become one template each, and ClearRemovalIntent is removed. The flag is not on the wire: the EndpointGroupIDList, EndpointBindingList and NodeKeySetList attribute reads encode the generated base type, so they are unchanged. This changes the return types of GetEndpointGroupIDList, GetEndpointBindingList and GetNodeKeySetList. Their element types derive from the generated types, so code that reads the entries keeps working; code that inserts entries constructs the new types. Joint Fabric is provisional, and the cluster server and the unit tests are the only users in the tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): queue group and key set syncs per node, and answer BUSY when full Only single-entry ACL and binding syncs went through the per-node queue. Group and key set syncs started at once, so an add and a removal of the same group or key set could reach a node out of order. Every single-entry sync outside RefreshNode now goes through RunOrQueueNodeSync: AddGroupIDToEndpointForNode, RemoveGroupIDFromEndpointForNode, UpdateGroup (endpoint groups, node key sets and ACL entries) and UpdateKeySet. As for ACLs and bindings, each queued operation looks its entry up when it starts, and bindings, groups and key sets share one StartEntrySync. RefreshNode's own syncs do not queue: the refresh already holds the node's slot. Queued entries stay Pending until their sync runs. A full queue now fails the command with BUSY instead of RESOURCE_EXHAUSTED. The specification ties RESOURCE_EXHAUSTED to the datastore's storage capacity, with duties to notify the user and offer a purge, while BUSY is a temporary condition that commissioners retry. Commands check the queue before changing any entry, so a rejected command leaves the datastore unchanged and the retry repeats it. Previously an ACL or binding add stored the entry first, and the retry found it Pending, succeeded and queued nothing. UpdateGroup and UpdateKeySet check every affected node before changing anything. Behavior changes that follow the specification: - AddGroupIDToEndpointForNode marks the node's key set entry Committed once the key set is written ("If this succeeds, update the new KeySet entry in the Datastore to Committed"); it was left Pending. - UpdateKeySet stores the key set, then marks each node entry for it Pending and syncs the stored value, Committing it on success. It sent the command's value before storing it and left the entries' state unchanged. The group sync in AddGroupIDToEndpointForNode is queued behind the key set sync, and is not attempted if the key set failed to reach the node, as before: the node would reject the group's key map with an unrecoverable CONSTRAINT_ERROR, and RefreshNode would drop the entry. RemoveNodeKeySetEntry, used by UpdateGroup when a group's key set changes, now removes the old key set from every node in the group, not only the first one found. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): discard syncs in flight when the joint fabric is removed ClearAllRecords, which OnFabricRemoved runs for the anchor fabric, emptied the per-node sync queues but left the refresh state and the operations in flight alone. Their completions still ran afterwards. A stale single-entry completion called FinishNodeSync for its node, which started an operation queued after the removal while another was already running, so two read-modify-write syncs could overlap. A refresh in flight kept its node ID set, and RefreshNode returned INCORRECT_STATE until the stale refresh called back, and its completion could then end a later refresh. ClearAllRecords now resets all refresh state along with the queues, and increments a generation counter. Every refresh and queued-sync completion captures the counter when its operation starts and returns without touching the datastore if it has changed. Queued operations are dropped without starting. Nothing changes for clients: the specification has no requirements for syncs in flight when the fabric is removed, and its CommitFailed and retry rules apply only to entries that still exist. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(jfds): share duplicated sync bookkeeping - CancelRemoval replaces five copies of the block that re-adds an entry being removed. - MarkRefreshWriteFailed replaces MarkRefreshBindingsSyncFailed and MarkRefreshAclsSyncFailed, which differed only in their lists. - InRefreshWrite replaces four lambdas that looked an entry up in the active refresh's ACL or binding write. - AclTombstoneMatches replaces the tombstone predicate written out in RefreshNode's ACL merge and in AddACLToNode. - QueueNodeKeySetSyncs replaces three copies of the loop that queues key set syncs. No behavior change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): propagate CAT changes to encoded ACL subjects UpdateGroup selected ACL entries by comparing each subject with the raw GroupID, whatever the entry's auth mode, and then set the privilege of every selected entry to the group's permission. A CASE entry that grants a group through its CASE Authenticated Tag holds the subject kMinCASEAuthTag | (GroupCAT << 16) | GroupCATVersion, so it was never selected, and changing GroupCAT or GroupCATVersion did not change the ACL entries on the nodes. A CASE subject equal to the GroupID, which is a node ID, was selected instead. A version-only change also overwrote the privilege of every selected entry. UpdateGroup now selects subjects by auth mode: - in Group entries, subjects equal to the GroupID; - in CASE entries, CAT subjects whose identifier is the group's previous GroupCAT, at any version, so an entry left at an older version is also updated. When GroupCAT or GroupCATVersion changes, the selected CASE subjects are rewritten to the new CAT and version. The privilege changes only when GroupPermission changes. If an entry also names other subjects, it is split: the original keeps the other subjects and its privilege, and a new entry with a new list ID holds the group's subjects with the new privilege. The original is replaced on the node before the new entry is added. Each modified entry records its previous value, so the node's entry is replaced rather than appended, and is synced through the per-node queue. A GroupCATVersion of 0 is rejected with CONSTRAINT_ERROR before anything changes: it is not a valid CAT, and the field's range is 1 to 65535. A permission change that would need more ACL entries than kMaxACLs is rejected with RESOURCE_EXHAUSTED. The per-node queue check counts two syncs for each entry that is split. Matter Core R1.4.2 11.24.4.5 (Removing a Node from a Group) and 11.24.7.5 (UpdateGroup). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * test(jfds): read recorded ACL syncs through checked accessors clang-tidy's bugprone-unchecked-optional-access flagged every `delegate.lastAclSyncOwned->` and `delegate.lastAclSuperseded->` in TestJointFabricDatastore, including those after an ASSERT_TRUE(... .has_value()), and fails the Darwin build. TrackingDelegate gains LastAclSync() and LastAclSuperseded(), which read through value_or, and the tests use them. The has_value() assertions stay, so a missing sync still fails the test directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): reject reserved and zero GroupCAT values UpdateGroup checked the GroupCAT the group already had, but not the one in the command. UpdateGroup(G, GroupCAT=0xFFFF) rewrote every CASE ACL subject naming G's CAT to the Administrator CAT and synced the change to the nodes. G then carried the Administrator CAT, so later UpdateGroup and RemoveGroup commands on it failed. GroupCAT 0 and the Anchor CAT were accepted the same way. A CAT change without a GroupCATVersion keeps the stored version, which ForceAddGroup does not check, so a stored version of 0 was written into the subjects as an invalid CAT. AddGroup accepted GroupCAT 0 and any GroupCATVersion. UpdateGroup and AddGroup now reject a GroupCAT of 0, the Anchor CAT or the Administrator CAT with CONSTRAINT_ERROR before anything changes (GroupCAT is 1 to 65534, and neither command may set a reserved CAT). UpdateGroup also rejects a CAT change whose effective version is 0, and AddGroup rejects a GroupCATVersion outside 1 to 65534. ForceAddGroup, which JFAManager uses to create the Administrator and Anchor groups, is unchanged. Matter Core R1.4.2 11.24.7.4 (AddGroup) and 11.24.7.5 (UpdateGroup). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): leave ACL entries being removed out of UpdateGroup UpdateGroup rewrote and split ACL entries without checking whether they were being removed. With E = {CASE, Operate, [CAT(G,v1), 0x1111]} queued, in flight or failed for removal (CommitFailed keeps the removal intent while the node is offline), UpdateGroup(G, GroupPermission=Manage) split E: E kept 0x1111 and its removal, and a new entry {CASE, Manage, [CAT(G,v1)]} was added and synced as an add, so the node held an entry naming G's CAT after E's removal. A refresh of an offline node did the same. On the non-split path, the rewrite overwrote DeletePending or CommitFailed with Pending, so the attribute misreported the entry while its removal was still pending. aclChangeFor now returns no change for an entry with removal intent, so the split pre-count, the BUSY pre-count and the mutation loop all leave it untouched: its value, privilege, state and removal intent are kept, and its removal goes ahead with the value the node holds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): record the value an ACL update left on a node being removed Each ACL entry keeps one superseded value, the value the node is believed to hold while an update is pending. When an update succeeded after its entry had been marked for removal, the completion looked the entry up as an add, found nothing, and did not record that the node now held the value sent, although the write had erased the superseded value. With E Committed on N with CAT(C,1), UpdateGroup(v2) sets E to v2 and syncs it. While that sync is in flight, UpdateGroup(v3) sets E to v3, keeping v1 as superseded, and RemoveACLFromNode(E) marks it for removal. The v2 write succeeds, and the removal then sends v3 with superseded v1. N holds only v2, so nothing is erased, E is dropped from the datastore, and the next RefreshNode adopts v2 as a new Committed entry. The completion now looks the entry up by key. If its value still equals the value sent, the superseded value is cleared, since the node no longer holds it, and the entry is marked Committed unless it is being removed. Otherwise the value sent becomes the superseded value, which the queued sync replaces or removes. The state and removal intent of an entry being removed are left unchanged. A write that the node applied but that was reported as failed, for example because the response was lost, can still leave a value on the node that the datastore does not record. Covering it needs a list of superseded values through ApplyAclEdit and the Delegate, which is out of scope here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): reject epoch keys that are not 16 bytes AddKeySet and UpdateKeySet accepted a non-null epoch key of 0 to 15 bytes: the datastore only checked that a key fits its 16-byte storage, and neither the cluster handler nor the decoder checks the length. Nodes reject such a key in KeySetWrite with CONSTRAINT_ERROR, which is recorded on every node's entry for the key set, and the next RefreshNode drops those entries as unrecoverable. Nothing re-creates them, so a corrected UpdateKeySet reached no node, and removing a node from the group never sent it KeySetRemove. An epoch key must now be null or exactly 16 bytes, the length the Group Key Management cluster requires. Any other length fails AddKeySet and UpdateKeySet with CONSTRAINT_ERROR before the key set is stored or synced. Tests that used shorter keys, including the key set the test delegate returns from FetchGroupKeySet, now use 16-byte keys. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): do not replace a node's ACL when its entries cannot be adopted RefreshNode adopts the ACL entries a node holds that the datastore does not track. When the datastore's ACL list was full (kMaxACLs entries across all nodes), the merge skipped such an entry with a log, and the refresh then wrote the node's ACL list from the datastore anyway. The write replaces the node's whole list, so it deleted the skipped entries, and the refresh marked the node Committed. Adding back a tombstoned value to be removed did not check the limit at all. The merge now counts the fetched entries it cannot store, in both paths. If there are any, the refresh ends with RESOURCE_EXHAUSTED before the ACL write, logs the count once, and the node stays Pending. The tombstones are kept, so a value that could not be added back is removed by a later refresh instead of being adopted. Entries the node no longer holds are still erased first, which frees room for the next refresh. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): keep the ACL value a node holds while its update is unapplied RefreshNode's ACL merge skipped a fetched entry equal to some entry's superseded value, the value the entry had before an update the node has not applied. It did not mark that entry as seen or look at its state: - When the node rejected the update for good (CONSTRAINT_ERROR, as for a Group-auth entry raised to Administer), the triage dropped the entry as an unrecoverable failure, and the ACL write left out both values, deleting the entry the node still held, and the refresh reported Committed. - When the entry's removal had failed as well, the merge took the unseen entry's removal as done and erased it before the write. If the write then failed, nothing remembered the removal, and the next refresh adopted the old value as Committed. Every entry whose superseded value the node holds is now seen, so its removal is retried until the node no longer holds the value. An entry whose update failed with an unrecoverable status, and that is not being removed, goes back to the superseded value as Committed, keeping its list ID, so the write keeps the node's entry. Pending updates and recoverable failures are unchanged: the write replaces the old value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): sync each new entry alone when a refresh list write fails RefreshNode writes a node's whole ACL list, and its binding list, in one write, and the node answers with one status. MarkRefreshWriteFailed recorded that status on every entry in the write that was not Committed. An unrecoverable status (CONSTRAINT_ERROR or RESOURCE_EXHAUSTED) makes the next refresh drop the entry, so one invalid pending entry, or a list one entry over the node's per-fabric capacity, dropped every pending entry, including those the node would have accepted on their own. Entries being removed, which the write leaves out, recorded the status too. An unrecoverable write status is now recorded only when the write held a single entry that was not Committed. With several, each records a recoverable FAILURE, and a single-entry sync is queued for each. The syncs start once the refresh releases the node and record the node's own status for each entry. If the node's sync queue fills up, the remaining entries stay recoverable and the next refresh retries them. Entries being removed record FAILURE instead of an unrecoverable write status and keep their removal intent. Recoverable statuses are recorded as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): record the ACL value a refresh wrote for an entry being removed The earlier fix that records the value an ACL update left on a node being removed covered single-entry syncs only. RefreshNode's ACL write completion still updated only entries that were Pending or CommitFailed when it ran, so an entry marked for removal while the write was in flight kept a superseded value that the node no longer held. E is Committed on N with CAT(C,1), and UpdateGroup(v2) times out on N, leaving E CommitFailed with v2 and superseded v1. RefreshNode(N) fetches v1 and writes N's ACL list with v2. While the write is in flight, UpdateGroup(v3) sets E to v3, keeping v1 as superseded, and RemoveACLFromNode(E) marks it for removal. The write succeeds, so N holds v2, but E still had superseded v1. The queued removal then sent v3 with superseded v1, erased nothing on N, and E was dropped from the datastore; the next refresh adopted v2 as a new Committed entry. The completion now handles every entry of the node that was in the write as StartAclEntrySync does, whatever its state: if the entry still has the written value, its superseded value is cleared and it is marked Committed unless it is being removed; otherwise the written value becomes its superseded value, which the queued sync replaces or removes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): keep a full ACL list from blocking refreshes and removals The earlier fix that ends a refresh with RESOURCE_EXHAUSTED when a fetched ACL entry cannot be adopted compared the whole ACL list with kMaxACLs. That count included the node's own entries that the triage before the ACL write drops, those whose sync failed for good. With the list full, a node holding one entry the datastore does not track could then never be refreshed again: every refresh ended before the triage that would have made room. A tombstoned value that the node still held also ended the refresh while the list was full, so a value whose removal had been requested stayed on the node until room was freed elsewhere. The merge now counts the room of the node's entries that the triage drops as free: those with an unrecoverable failure, other than updates whose old value the node still holds, which the merge restores. A tombstoned value that cannot be added back as DeletePending for lack of room no longer ends the refresh. Its tombstone is kept, and the ACL write leaves the value out, which removes it from the node, since the write replaces the node's whole list. The tombstone is dropped once the node no longer holds the value, as before. RefreshWithFullAclListKeepsTombstone expected a full list to end the refresh without a write. It is now RefreshWithFullAclListRemovesTombstonedValue, and expects a write without the value, and the kept tombstone to remove the value again at the next refresh. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(jfds): queue a refresh's per-entry re-syncs by key The earlier fix that syncs each new entry alone when a refresh list write fails collected full copies of the entries to sync, including an ACL entry's subject and target vectors and its superseded value, only to read their keys when it queued the syncs. MarkRefreshWriteFailed now collects the start function of each sync, which captures only the entry's key, and hands it to the node's queue as is. RunOrQueueEntrySync becomes EntrySyncStart, which returns that function instead of queuing it. No behavior change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): reject an unknown key set security policy in AddKeySet The earlier fix that rejects epoch keys that are not 16 bytes closed one way for AddKeySet to store a key set that every node refuses. UpdateKeySet also rejects a GroupKeySecurityPolicy of kUnknownEnumValue, but AddKeySet stored such a key set. Nodes answer its KeySetWrite with CONSTRAINT_ERROR, which is recorded on every node's entry for the key set, and the next RefreshNode drops those entries as unrecoverable, so nothing would write the key set to the nodes or remove it from them again. AddKeySet now rejects an unknown security policy with CONSTRAINT_ERROR before the key set is stored, as UpdateKeySet does. The key set 0 placeholder that JFAManager adds for the IPK keeps the default policy, TrustFirst, and is still accepted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): sync a key set updated while its write is in flight A key-set entry's sync completion matched the entry by node, key set and removal intent only, and committed it. With a CASE session to N, UpdateKeySet(K, v1) marks (N,K) Pending and sends KeySetWrite(v1). UpdateKeySet(K, v2) before N answers stores v2, marks (N,K) Pending again and queues a second sync. The v1 completion then marked (N,K) Committed, and the queued sync, which skips Committed entries, never sent v2. N kept v1, and RefreshNode does not rewrite Committed entries. RefreshNode's own key-set writes, which the refresh does not wait for, matched on node and key set alone, with the same result. A late failure of one of them could also mark an entry whose newer write had already succeeded. Key-set, endpoint-group and binding entries now carry a sync revision, which is not on the wire. UpdateKeySet changes it on the entries it marks for a new write, and so does UpdateGroup on the entries of a renamed group; nothing changes it on binding entries, which carry it because StartEntrySync is shared. A queued sync captures the revision when it starts, and its success commits the entry only if the revision is unchanged; otherwise the entry stays Pending for the queued sync. RefreshNode's key-set writes apply their result, success or failure, only to an entry that is still being added at the revision written. A failed queued sync is still recorded when the revision has changed. The queued sync of the new value runs anyway, since only Committed entries are skipped, and StartEndpointGroupEntrySync relies on the recorded failure to hold back a group add while the node lacks the group's key set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): limit a refresh's key-set syncs to the refreshed node RefreshNode's key-set stages handled every node's entries. They collected all nodes' key-set removals once and sent each one later, outside that node's sync queue and without reading the entry again. They also rewrote other nodes' Pending and CommitFailed entries, and dropped every node's entries with an unrecoverable failure. With (X,K) and (M,K) both being removed, RefreshNode(N) could wait on the removal from an offline X while AddGroupIDToEndpointForNode(M, ep, G), for a group G using K, cancelled the removal of (M,K) and wrote K and G to M. The refresh then sent the collected KeySetRemove(K) to M, which deleted K and its GroupKeyMap entries, while (M,K) and the group stayed Committed, and no refresh repaired it. The key-set stages now handle only the refreshed node's entries. Other nodes' entries are synced by their own refreshes and sync queues, which keep a node's key-set writes and removals in order. Each collected removal is read again before it is sent, and skipped if the entry is gone or no longer being removed: an add that cancelled it is queued behind the refresh. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): don't commit a refreshed group entry changed during its sync RefreshNode syncs the node's group entries that are Pending, CommitFailed or being removed without waiting for the syncs, and each completion matched its entry by node, endpoint, group and removal intent only. UpdateGroup marks a group's endpoint entries for another sync, and changes their sync revision, when the group is renamed. If it did so during a refresh, its sync of the entry waited behind the refresh, and a successful completion of the refresh's sync marked the entry Committed, so the queued sync, which skips Committed entries, never ran, and the new name never reached the node. A refresh sync that failed after a later sync of the entry had succeeded also marked the entry CommitFailed. The refresh's group syncs now capture the entry's sync revision. Their completions commit the entry, record a failure on it or erase it after a removal only if it is still being added or removed as when the sync started and its revision is unchanged. This extends the revision check that "sync a key set updated while its write is in flight" added for queued syncs and the refresh's key-set writes to the refresh's group syncs. As in the key-set stage, a failure still marks the refresh failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): change the sync revision when a removal is cancelled The earlier fix that gave key-set, endpoint-group and binding entries a sync revision changed it only where UpdateKeySet and UpdateGroup mark an entry for a new write. Both skip entries that are being removed, and cancelling a removal left the revision as it was. An add in flight from before the removal request therefore committed the entry once a re-add cancelled the removal, although the stored value had changed in between, and the queued syncs then skipped the Committed entry. With a CASE session to N, AddGroupIDToEndpointForNode(N, 1, G), for a group G using key set K stored as A, sends KeySetWrite(A). RemoveGroupIDFromEndpointForNode(N, 1, G) marks (N,K) for removal, and UpdateKeySet(K, B) skips it. A second AddGroupIDToEndpointForNode(N, 1, G) cancels the removal, and the success of the write of A then marked (N,K) Committed: N kept A while the datastore read Committed. CancelRemoval now changes the sync revision of the entry types that carry one, so an add in flight from before the removal request does not commit the entry, and the queued sync writes the current value. When nothing changed in between, the entry is written once more. ACL entries have no sync revision: their syncs compare the value sent with the entry's value, which UpdateGroup leaves unchanged while the entry is being removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): classify transport timeouts and busy errors A failed sync records the IM status of its error as the entry's FailureCode. Errors that carry no IM status were all recorded as FAILURE, including a transport timeout (CHIP_ERROR_TIMEOUT) and a busy CASE session (CHIP_ERROR_BUSY). An ACL add sent to a node that does not answer, for example, recorded CommitFailed with FAILURE. CHIP_ERROR_TIMEOUT is now recorded as TIMEOUT and CHIP_ERROR_BUSY as BUSY, for single-entry syncs and for a refresh's list writes. Other errors are recorded as before. RefreshNode still retries entries with either code: it drops only entries with CONSTRAINT_ERROR or RESOURCE_EXHAUSTED. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): drop a key-set removal the node no longer needs RefreshNode's key-set removal stage sent KeySetRemove for each of the node's key-set entries being removed, without checking the key-set list it had fetched from the node. If the node did not hold the key set, it answered NOT_FOUND, which is recorded as recoverable: the entry stayed CommitFailed with its removal intent, and every refresh sent the removal again and left the node Pending. This happened when the key set's write failed while the node was offline and its group was then removed from the node's endpoint. The stage also considered only key sets still in the datastore, so a removal of a key set that had since been removed from the datastore, and that the node no longer held, was never sent nor erased. The stage now goes through the node's key-set entries being removed directly, and erases an entry whose key set is not in the fetched list without sending KeySetRemove. A removal of a key set the node holds is sent as before. A failed fetch still ends the refresh before this stage. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(jfds): expect Committed in TC-JFDS-2.5 step 33 Step 33 read the node's NodeKeySetList entry for the key set that AddGroupIDToEndpointForNode wrote in step 29, and expected it to be Pending. The datastore marks the entry Committed once the key set is written, as the specification requires ("If this succeeds, update the new KeySet entry in the Datastore to Committed"). Step 30 already waits for the group entry to be Committed, which follows the key set's write. The step now expects Committed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(jfds): tighten tests that pass without the behavior they check AddOfTombstonedAclValueDropsTombstone passed whether or not AddACLToNode dropped the tombstone of the added value: the refresh matched the node's value to the added entry, so the tombstone was never consulted. The test now removes the added entry before the refresh, with the node holding the value again. The refresh adopts the value only if the tombstone is gone, and removes it otherwise. ReAddedBindingIsNotTreatedAsRemoval checked that a removal intent left in a set keyed by list ID did not apply to a re-added binding with the same list ID. Removal intent is now stored on the entry and erased with it, so the test checked nothing specific. It is removed; AddCancelsPendingBindingRemoval checks that an add cancels a failed binding removal and that RefreshNode then writes the binding. The checks that an added or adopted ACL entry got a new list ID also reject 0. FindAcl results are checked for nullptr before they are dereferenced, and RunDeferred fails the test instead of reading past the end of the deferred completions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(jfds): move the fabric removal comment to its test The comment describing OnFabricRemovedWipesDatastoreOnlyForAnchorFabric sat above RefreshDropsUnrecoverableCommitFailedAcl, an unrelated test. It now sits above the test it describes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(jfds): define ResetRefreshState in the header Server.h calls JointFabricDatastore::OnFabricRemoved when CHIP_DEVICE_CONFIG_ENABLE_JOINT_FABRIC is set, and OnFabricRemoved and ClearAllRecords are defined in the header. Linux and Darwin test builds set the define, but link JointFabricDatastore.cpp only when chip_device_config_enable_joint_fabric is set. ClearAllRecords now calls ResetRefreshState, which was defined in JointFabricDatastore.cpp, so apps such as chip-shell failed to link. ResetRefreshState is now defined in the header. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…haracters (#74674)
* [AccountLogin] Return ConstraintError for a SetupPIN shorter than 8 characters
The Login command's SetupPIN field has a spec minimum length of 8
characters, but the server never checked it, so a short PIN reached the
delegate and the failure was reported as UnsupportedAccess. Check the
length before calling the delegate and return ConstraintError. The check
is on string length only, so values with leading zeros ("00001234") are
accepted. Fixes TC-IDM-9.1 (#74571).
Also zero-pad the passcode to 8 digits when ContentAppPlatform builds the
SetupPIN for its post-commissioning Login, so a passcode such as 00001234
is no longer sent as "1234".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…tion and PlayPreset (#74671) * [ContentLauncher] Spec-compliant delegate defaults for ContentReplication and PlayPreset The default ContentReplicationRequest handler returned Success with no ReplicationInfo, which the spec does not allow. It now returns ReplicationNotSupported with a null ReplicationInfo. The default PlayPreset handler returned Success for any PresetID even though the default Presets list is empty; it now returns PresetNotFound. Add unit tests for both defaults. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix TestContentLaunchServer on Tizen and clang-tidy Initialize chip::Platform memory for the suite, since encoding responses into MockCommandHandler allocates through it (the test aborted on Tizen). Compare the whole ClusterStatusCode instead of calling .value() on the optional cluster code, which clang-tidy flags as unchecked. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…bot) (#73536) * Updated wifi scan type to extended * Updated sl_wifi_set_scan_callback_v2 and added NetworkScanCallbackContext * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Minor fix * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Applied coderabbitai suggestions * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Addressed comments * Dropped semaphore for advance and extended scans --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
… (#74700) * [Chef] Add delegates for code-driven Thermostat cluster * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
silabs-matter-ci-bot
Bot
added
changing-git-submodules-on-purpose
sl-require-admin-action
Contributor
|
The check for |
Michael Duggan (miduggan24)
approved these changes
Oct 13, 2026
sarthak shaha (Sarthak-Shaha)
approved these changes
Oct 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR syncs the csa branch with the main branch.
PR MUST BE MERGED WITH MERGE COMMIT - ADMIN MUST ENABLE THE OPTION