TC-ICDB-2.2/2.4: fix check-in resume wait#72728
Open
raul-marquez-csa wants to merge 5 commits into
Open
Conversation
for more information, see https://pre-commit.ci
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors ICD (Intermittent Connected Device) test cases to register ICD clients during commissioning rather than explicitly sending RegisterClient commands during the test steps. It also updates the verification logic to wait for and assert actual check-in messages received from the DUT using a new helper method assert_checkin_received and a dynamically calculated timeout checkin_resume_wait_s. Feedback is provided to cast the float timeout to an integer when calling WaitForCheckIn to prevent potential type or truncation issues in the underlying bindings.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #72728 +/- ##
==========================================
- Coverage 56.86% 56.79% -0.08%
==========================================
Files 1639 1642 +3
Lines 112757 112757
Branches 13123 13139 +16
==========================================
- Hits 64119 64040 -79
- Misses 48638 48717 +79 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
PR #72728: Size comparison from b13372f to 7c39724 Full report (30 builds for cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
Apollon77
reviewed
Jun 25, 2026
Apollon77
left a comment
Apollon77
left a comment
Contributor
There was a problem hiding this comment.
Thanks for tackling this — the direction is right and matches the issue diagnosis: assert each fabric's check-in directly, derive the wait from the DUT's own timing values, and keep the wait silent so the device can actually go idle. APIs all check out and the refactor reads cleanly.
Two things from the issue thread didn't make it into the implementation, and one of them looks like it can re-introduce the flake we were trying to remove:
MaximumCheckInBackoffis missing from the wait formula (the thread converged onMaxInterval + MRP + max(FullCycle, MaximumCheckInBackoff)). This matters in 2.4 step 7 — see inline comment. The step-3 comment was also updated to say the attribute is read, but it isn't.- MRP is modelled as a second
FullCyclerather than a bounded retransmission value. It over-waits, so probably not flaky, but the docstring describes it as something it isn't — see inline.
Also: the gemini-code-assist suggestion to cast the timeout to int should not be applied — WaitForCheckIn(timeoutSeconds: float) feeds asyncio.wait_for(timeout=...), which takes a float; an int would truncate sub-second precision.
Details inline.
…/raul-marquez-csa/connectedhomeip into tc-icdb-2.2-2.4-checkin-wait-fix
|
PR #72728: Size comparison from b13372f to f89223a Increases above 0.2%:
Full report (33 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
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
2 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.
Summary
Addresses:
[Test] TC-ICDB-2.4 / TC-ICDB-2.2: post-subscription-shutdown check-in wait is too short (ignores MaxInterval + MRP), flaky vs. spec-compliant DUT
#72668
TC-ICDB-2.4
Before
After a client's subscription was turned off, the test waited a fixed single cycle and read one device-wide counter, expecting it to tick up: once for the first dropped client, at least twice to conclude both were checked in. So "each client got a check-in" was inferred from a single shared number, within one cycle.
This caused spec-compliant devices to wrongly fail: the spec lets a device space out check-ins and only detect a dropped subscription after its next report fails, so a compliant device often didn't tick the shared counter enough within that one cycle.
After
Now the test confirms each client actually receives its own check-in, on its own fabric, instead of reading a shared counter. It also waits long enough to cover the time the device needs to detect the dropped subscription plus one resume cycle, rather than a fixed single cycle, so a compliant device that spaces out its check-ins still passes.
TC-ICDB-2.2
Before
After the client's subscription was turned off, the test waited a fixed single cycle and then read the counter once, expecting it to have ticked up to show a check-in had resumed.
This caused spec-compliant devices to wrongly fail: the device only detects a dropped subscription after its next report fails, then resumes check-ins a cycle later, so within that one fixed cycle the counter often had not ticked yet.
After
Now the test waits long enough to cover the time the device needs to detect the dropped subscription plus one resume cycle, then reads the counter. It waits quietly the whole time, because repeatedly reading the device would keep it awake and stop it from ever sending the check-in.
Updates performed
support_modules/icd_support.py
TC_ICDB_2_4.py
TC_ICDB_2_1_2_2.py
Testing