Work on TC-IDM-3-2 #35692
Open
Work on TC-IDM-3-2 #35692
Conversation
This file contains 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
|
Review changes with SemanticDiff. Analyzed 1 of 1 files. Overall, the semantic diff is 33% smaller than the GitHub diff.
|
github-actions
bot
added
tests
matter-1.4-te2-script-change
|
PR #35692: Size comparison from 8f7d74d to c8cc8c1 Full report (82 builds for bl602, bl702, bl702l, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #35692: Size comparison from a99bb07 to 9723442 Full report (8 builds for cc32xx, nxp, stm32, tizen)
|
|
PR #35692: Size comparison from 17f25a1 to 405937e Full report (88 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
austina-csa
force-pushed
the
issue_322_temp_8
branch
from
405937e
to
2559dc1
Compare
|
PR #35692: Size comparison from 1597be7 to 2559dc1 Full report (88 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #35692: Size comparison from 2675445 to 49753b8 Full report (88 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #35692: Size comparison from f54e3c8 to 0ee0752 Full report (88 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #35692: Size comparison from c565622 to f8c4bc7 Full report (88 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
cecille
reviewed
Oct 3, 2024
|
|
||
| # Verify that the DUT sends a WriteResponseMessage with any status except UNSUPPORTED_WRITE or DATA_VERSION_MISMATCH. If the Status is SUCCESS, verify the updated | ||
| # value by sending a ReadRequestMessage for all affected paths. If the status is SUCCESS, send a WriteRequestMessage to set the value back to `original`. | ||
| self.user_params["use_pase_only"] = False |
There was a problem hiding this comment.
instead of overwriting this, set allow_pase to false in the setup_class_helper if you want to disallow PASE. use_pase_only was also removed recently, so this won't do anything anyway.
| for cluster_id in self.xml_clusters: | ||
| xml_cluster = self.xml_clusters[cluster_id] | ||
| attributes = xml_cluster.attributes | ||
| for attribute_id in attributes: |
There was a problem hiding this comment.
Suggested change
| for attribute_id in attributes: | |
| for attribute_id, xml_attribute in attributes.items(): | |
| xml_cluster = self.xml_clusters[cluster_id] | ||
| attributes = xml_cluster.attributes | ||
| for attribute_id in attributes: | ||
| xml_attribute = attributes[attribute_id] |
There was a problem hiding this comment.
Suggested change
| xml_attribute = attributes[attribute_id] |
| for attribute_id in attributes: | ||
| xml_attribute = attributes[attribute_id] | ||
| write_access = xml_attribute.write_access | ||
| if write_access: |
There was a problem hiding this comment.
can you flip this around to un-nest this a bit?
ie
if not write_access:
continue
| xml_attribute = attributes[attribute_id] | ||
| write_access = xml_attribute.write_access | ||
| if write_access: | ||
| if cluster_id in Clusters.ClusterObjects.ALL_ATTRIBUTES: |
There was a problem hiding this comment.
Suggested change
| if cluster_id in Clusters.ClusterObjects.ALL_ATTRIBUTES: | |
| if cluster_id in Clusters.ClusterObjects.ALL_ATTRIBUTES and attribute_id in Clusters.ClusterObjects.ALL_ATTRIBUTES[cluster_id]: | |
^ that should let you eliminate the try loop below.
| if cluster_id in Clusters.ClusterObjects.ALL_ATTRIBUTES: | ||
| try: | ||
| if hasattr(Clusters.ClusterObjects.ALL_ATTRIBUTES[cluster_id][attribute_id], 'value'): | ||
| writable_attributes.append((cluster_id, attribute_id, write_access)) |
There was a problem hiding this comment.
do you end up using the write_access for anything? The TH has admin, so if it's writable, it can be written by the TH.
| chosen_writable_attribute = Clusters.ClusterObjects.ALL_ATTRIBUTES[chosen_writable_attribute_tuple[0]][chosen_writable_attribute_tuple[1]] | ||
| chosen_writable_cluster = Clusters.ClusterObjects.ALL_CLUSTERS[chosen_writable_attribute_tuple[0]] | ||
|
|
||
| output_1 = await self.default_controller.ReadAttribute(self.dut_node_id, [chosen_writable_attribute]) |
There was a problem hiding this comment.
It might actually make your life easier to use the Read( function instead of read attribute - that function returns an object that stores the returned values by ID rather than forcing you to pass in the class object.
|
|
||
| output_1 = await self.default_controller.ReadAttribute(self.dut_node_id, [chosen_writable_attribute]) | ||
| endpoint = next(iter(output_1)) | ||
| await self.default_controller.WriteAttribute(self.dut_node_id, [(endpoint, chosen_writable_attribute(value=123456))]) |
There was a problem hiding this comment.
This only works if you happen to hit a writable attribute that takes an int. Here, you want to look at attribute_type and pick a value based on the ClusterObjectFieldDescriptor
| await self.default_controller.WriteAttribute(self.dut_node_id, [(endpoint, chosen_writable_attribute(value=123456))]) | ||
| output_3 = await self.default_controller.ReadAttribute(self.dut_node_id, [chosen_writable_attribute]) | ||
|
|
||
| asserts.assert_not_equal(output_1[endpoint][chosen_writable_cluster][chosen_writable_attribute], |
There was a problem hiding this comment.
will only work if the write returned success.
| asserts.assert_not_equal(output_1[endpoint][chosen_writable_cluster][chosen_writable_attribute], | ||
| output_3[endpoint][chosen_writable_cluster][chosen_writable_attribute], | ||
| "Output did not change") | ||
|
|
There was a problem hiding this comment.
need to add the cleanup step to return the attribute to its original value.
|
PR #35692: Size comparison from 213bf23 to c5e68d7 Full report (88 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Only first test step is implemented for now but more will be added.
This approach is done for efficiency so that errors with implementing steps can be caught quickly.