[Create Test] Creating IDM_3_2 python3 test module #41066
+707
−452
Conversation
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
- Creating test module for Task #[322](project-chip/matter-test-scripts#322) - Created updated test plan to remove test step 1 here: [5539](CHIP-Specifications/chip-test-plans#5539)
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively automates the manual test case TC_IDM_3_2 by creating a new Python test module. The conversion from YAML to a Python script is well-executed and covers the intended test scenarios for write response actions. I've identified one critical issue that would cause a test failure and a couple of medium-severity issues related to code style and a typo. Overall, this is a great step towards automating our test suite.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
PR #41066: Size comparison from b779ea9 to 6c3d0aa Full report (5 builds for cc32xx, realtek, stm32)
|
|
PR #41066: Size comparison from b779ea9 to 3ddd434 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #41066 +/- ##
==========================================
- Coverage 51.06% 51.03% -0.04%
==========================================
Files 1385 1385
Lines 100882 100949 +67
Branches 13054 13066 +12
==========================================
+ Hits 51516 51517 +1
- Misses 49366 49432 +66 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
PR #41066: Size comparison from b779ea9 to dcd0548 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
cecille
reviewed
Sep 20, 2025
src/python_testing/TC_IDM_3_2.py
Outdated
| ''' | ||
|
|
||
| # Read an attribute to get the current DataVersion | ||
| test_cluster = Clusters.BasicInformation |
There was a problem hiding this comment.
I think we could get some odd devices through this test because it's not marked as being for commissionable device, but just for any device with a server. So you might end up with devices that are software components. In that case, they may not have any writable attributes at all. Node label is a really good choice for commissionable nodes. So it totally makes sense to try that one if it exists, but it might not. Realistically, testing data version filters while writing attributes on non-commissionable devices is likely such a corner case that I might argue it's not worth pursuing. Maybe? The challenge here is, of course, we don't really know if the value we're writing is acceptable if it's not node label.
Ok, so proposal for now - just put this into an if / else on the existence of the node label. The else with a fallback can be handled in a follow up.
There was a problem hiding this comment.
Hi Cecille,
I have updated this to continue using the node label as our attribute, but now as you have mentioned above placing it inside of an if exists in attribute_list then do test steps 5 and 6, else it just skips test steps 5 and 6 since we are using the node label for both test steps at this time.
Added the following follow-up task for tracking this: project-chip/matter-test-scripts#693
…e label if check for test steps 5 and 6, and added commissioning test step 0: - Added plumbing for TIMED_REQUEST_MISMATCH validation in test step 7: Following similar logic as noticed in TestOnlySendCommandTimedRequestFlagWithNoTimedInvoke function - Added commissioning test step 0 - Added if check for exist of node label attribute to see if we should run test step 5 and 6, else we currently skip these test steps (Until follow-up is implemented)
There was a problem hiding this comment.
I did not check all the FFI bits in here carefully.... I am assuming @cecille will or has.
There was a problem hiding this comment.
Glanced over this, and seems ok, but did not review carefully.
There was a problem hiding this comment.
Did not review this at all; I assume @cecille is handling this.
… mode changes back to 644
|
PR #41066: Size comparison from bab6726 to b896bb9 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
- Remove redundant WriteClient constructor taking only bool parameter - Merge pychip_WriteClient_TestOnlyWriteAttributesTimedRequestNoTimedAction and pychip_WriteClient_TestOnlyWriteAttributesTimedActionNoTimedRequestFlag into single pychip_WriteClient_TestOnlyWriteAttributesWithMismatchedTimedRequestField - Update Python bindings and tests to use unified API The single test function now takes both timedRequestTimeoutMs and timedRequestFieldValue parameters, allowing callers to specify exactly what combination they need for each test scenario.
|
PR #41066: Size comparison from bab6726 to 610043b Full report (3 builds for realtek, stm32)
|
|
PR #41066: Size comparison from ad60c95 to 7eeb60c Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
5 tasks
bzbarsky-apple
approved these changes
Nov 3, 2025
cecille
approved these changes
Nov 6, 2025
| attrs = [] | ||
| for v in attributes: | ||
| if len(v) == 2: | ||
| attrs.append(ClusterAttribute.AttributeWriteRequest( |
There was a problem hiding this comment.
would you mind adding the parameter names here so it's easier to tell what's going on? Is the attribute parameter list supposed to be a union of possible tuples?
There was a problem hiding this comment.
I've added the parameter names to the AttributeWriteRequest calls and updated the type hint to show it accepts either a 2-tuple or 3-tuple.
A bit clearer now. Thank you!
|
|
||
| namespace { | ||
| // Helper function to process write attributes data - reduces code duplication | ||
| CHIP_ERROR ProcessWriteAttributesData(WriteClient * client, python::PyWriteAttributeData * writeAttributesData, |
There was a problem hiding this comment.
Yeah, if you can get it so the legacy list encoding just calls this function with a parameter, that would be cleaner.
- Add parameter names to AttributeWriteRequest calls in ChipDeviceCtrl.py for clarity - Update type hints to show union of 2-tuple and 3-tuple formats - Refactor ProcessWriteAttributesData in attribute.cpp to accept forceLegacyListEncoding parameter, eliminating code duplication between legacy and normal encoding paths
|
PR #41066: Size comparison from 791f872 to d10727e Full report (5 builds for cc32xx, realtek, stm32)
|
|
PR #41066: Size comparison from 791f872 to 7d3f430 Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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
This PR creates the TC_IDM_3_2 Python test module to validate Write Response Actions from the DUT to the TH.
Includes:
Related issues
Testing
Adds Python Test: TC_IDM_3_2.py
Validated locally using chip-all-clusters-app on WSL Linux using following command:
Notes
Readability checklist
descriptive
“When in Rome…”
rule (coding style)