Skip to content

Commit a3b7ee1

Browse files
committed
Address code review feedback
1 parent 0273f65 commit a3b7ee1

1 file changed

Lines changed: 12 additions & 18 deletions

File tree

rs-matter/src/dm/clusters/binding.rs

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -170,17 +170,20 @@ impl<const N: usize> Bindings<N> {
170170
let endpoint = t.endpoint()?;
171171
let cluster = t.cluster()?;
172172

173-
// Spec §9.6.5.1:
174-
// - Group and Endpoint MUST NOT both be present.
175-
// - Node SHALL be present when Endpoint is present.
176-
// - At least one of (Node+Endpoint) or Group must identify a target.
173+
// Spec §9.6.5.1 — the conformance columns say Group is
174+
// `!Endpoint` and Endpoint is `!Group`, so **exactly one** of
175+
// them must identify the target. Node's conformance is
176+
// `Endpoint`, i.e. Node is mandatory whenever Endpoint is
177+
// present (it names the remote node for the unicast target).
177178
if group.is_some() && endpoint.is_some() {
178179
return Err(ErrorCode::ConstraintError.into());
179180
}
180-
if endpoint.is_some() && node.is_none() {
181+
if group.is_none() && endpoint.is_none() {
182+
// Rejects both "all fields missing" and "only Node
183+
// present" (no destination at all).
181184
return Err(ErrorCode::ConstraintError.into());
182185
}
183-
if group.is_none() && node.is_none() && endpoint.is_none() {
186+
if endpoint.is_some() && node.is_none() {
184187
return Err(ErrorCode::ConstraintError.into());
185188
}
186189

@@ -215,18 +218,9 @@ impl<const N: usize> Bindings<N> {
215218

216219
self.state.lock(|cell| {
217220
let mut state = cell.borrow_mut();
218-
// Drop every existing entry on this (endpoint, fabric).
219-
// Iterate-by-index because `retain` doesn't compose with
220-
// our bounded Vec API the same way.
221-
let mut i = 0;
222-
while i < state.len() {
223-
let e = &state[i];
224-
if e.endpoint_id == endpoint_id && e.fab_idx == fab_idx {
225-
state.remove(i);
226-
} else {
227-
i += 1;
228-
}
229-
}
221+
// Drop every existing entry on this (endpoint, fabric)
222+
// in O(N) — one pass, in-place shift.
223+
state.retain(|e| !(e.endpoint_id == endpoint_id && e.fab_idx == fab_idx));
230224
// Now bulk-insert the validated list. Capacity-exhausted
231225
// here means the *combined* fabric counts exceeded `N`.
232226
for sb in parsed {

0 commit comments

Comments
 (0)