[WIP] Introduce device type requirement test step in IDM-10.5#43674
Draft
AryaHassanli wants to merge 24 commits into
Draft
[WIP] Introduce device type requirement test step in IDM-10.5#43674AryaHassanli wants to merge 24 commits into
AryaHassanli wants to merge 24 commits into
Conversation
github-actions
Bot
added
documentation
![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 introduces validation for composed device types in TC-IDM-10.5, which involves updating numerous XML data model files and implementing new parsing and validation logic in Python. The changes are well-structured and correctly implement the new test logic. I've identified one issue where the XML parser for constraints doesn't handle the <allowed> tag, which would cause some validation checks to be skipped. My review includes a suggestion to fix this.
|
PR #43674: Size comparison from 4428cc9 to c585edf Full report (10 builds for cc13x4_26x4, cc32xx, nrfconnect, realtek, stm32)
|
|
PR #43674: Size comparison from 4428cc9 to c120a26 Full report (12 builds for cc13x4_26x4, cc32xx, nrfconnect, qpg, realtek, stm32)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #43674 +/- ##
==========================================
- Coverage 55.51% 55.51% -0.01%
==========================================
Files 1628 1628
Lines 111080 111083 +3
Branches 13414 13416 +2
==========================================
Hits 61668 61668
- Misses 49412 49415 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
PR #43674: Size comparison from 4428cc9 to 65dfb83 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
AryaHassanli
force-pushed
the
device-type-req-step-idm-10-5
branch
from
5b18d63 to
3e2fd39
Compare
|
PR #43674: Size comparison from 099e624 to 6060661 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #43674: Size comparison from 20fcb76 to a610183 Full report (6 builds for cc32xx, nrfconnect, realtek, stm32)
|
for more information, see https://pre-commit.ci
|
PR #43674: Size comparison from 20fcb76 to fea6963 Increases above 0.2%:
Full report (33 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
cecille
reviewed
May 25, 2026
| except KeyError: | ||
| spec_version = 0 | ||
|
|
||
| if spec_version < 0x01060000: |
Contributor
There was a problem hiding this comment.
Is this a structural problem with the spec, or is the intent to re-pull the DM files for earlier revisions then adjust this?
|
|
||
| count = 0 | ||
| for child_ep_id in parts_list: | ||
| if child_ep_id in self.endpoints: |
Contributor
There was a problem hiding this comment.
It might make sense to reverse the logic here and un-nest a bit - if the child_ep is listed in the parts_list and isn't in self.endpoints, something bad has happened and the test should throw an error and move on.
| for child_ep_id in parts_list: | ||
| if child_ep_id in self.endpoints: | ||
| child_ep = self.endpoints[child_ep_id] | ||
| if Clusters.Descriptor in child_ep: |
Contributor
There was a problem hiding this comment.
Same here - if there's no descriptor, that's an error.
|
|
||
| return success, problems | ||
|
|
||
| def check_composed_device_type_requirements(self, allow_provisional: bool = False) -> tuple[bool, list[ProblemNotice]]: |
Contributor
There was a problem hiding this comment.
Could you create some unit tests for this function?
…requirement check
for more information, see https://pre-commit.ci
AryaHassanli
changed the title
|
PR #43674: Size comparison from 9258186 to 39049b3 Full report (16 builds for cc13x4_26x4, cc32xx, nrfconnect, psoc6, qpg, realtek, stm32)
|
|
PR #43674: Size comparison from 9258186 to c9dd96b Increases above 0.2%:
Full report (35 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
AryaHassanli
force-pushed
the
device-type-req-step-idm-10-5
branch
from
dcd1a34 to
cfffde2
Compare
for more information, see https://pre-commit.ci
…li/connectedhomeip into device-type-req-step-idm-10-5
for more information, see https://pre-commit.ci
|
PR #43674: Size comparison from 4d3283d to 8cf923f Full report (35 builds for bl602, bl616, 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
3 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
Add composed device type requirements check to IDM-10.5
Overview:
This PR adds a new step to the Interaction Model (IDM) 10.5 test to verify composed device type requirements against the Data Model. This ensures that nodes claiming a composed device type actually include all required component device types with their respective cluster overrides.
To support full topology testing as defined in the Matter specification, this PR also introduces support for the
deviceTypeLocationattribute, allowing the test to verify requirements on specific locations (Self, Child, Root, Descendant, Any Endpoint) rather than assuming they are always direct children.Details:
check_composed_device_type_requirementstodevice_conformance_tests.py(reporting as IDM-10.5).spec_parsing.pyto parsecomposedDeviceTypeand its details including thedeviceTypeLocationattribute and composed device types cluster and element requirements from Data Model XML files.TestComposedDeviceTypeMatching.pyto verify the matching algorithm and the new location requirements:anyEndpointanddeviceEndpointlocations.composedDeviceType, theirclusterRequirements, anddeviceTypeLocationattribute (reflecting latest spec changes).Related issues
Alchemy PR: project-chip/alchemy#52
Test Plan PR: https://github.com/CHIP-Specifications/chip-test-plans/pull/5962
Testing
Readability checklist
The checklist below will help the reviewer finish PR review in time and keep the
code readable:
descriptive
“When in Rome…”
rule (coding style)
See: Pull Request Guidelines