[HVAC] Implement TC-TSTAT-2.2 #36023
Open
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 2 files.
|
github-actions
bot
added
app
tests
matter-1.4-te2-script-change
|
PR #36023: Size comparison from 6a29ee5 to 83be40a Full report (81 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
fessehaeve
reviewed
Oct 14, 2024
fessehaeve
reviewed
Oct 14, 2024
| def steps_TC_TSTAT_2_2(self) -> list[TestStep]: | ||
| steps = [ | ||
| TestStep("1", "Commission DUT to TH"), | ||
| TestStep("2a", "Test Harness Client reads attribute OccupiedCoolingSetpoint from the DUT"), |
There was a problem hiding this comment.
Can we store all initial attribute values (mainly min and max values) so that we can restore them at the end of the test? I wasn't able to run this test back to back since by the end of the test some of the attribute values don't make sense anymore.
There was a problem hiding this comment.
The exact issue I saw here is that, by the end of the test, the heatingsetpoint is very close to the abs/maxheatingsetpoint and some tests increment fixed values to heatingsetpoint which would be out of range and fail a test.
There was a problem hiding this comment.
This turned out to be harder than you might expect, as the default values for the thermostat cluster violate its own constraints. Once PR 36023 is merged, we can update the code to match, and then this will work.
There was a problem hiding this comment.
@hasty are you referencing the correct PR in your comment? It is referring to this PR itself.
Co-authored-by: fesseha-eve <[email protected]>
bauerschwan
approved these changes
Oct 23, 2024
bauerschwan
reviewed
Oct 23, 2024
|
|
||
| def steps_TC_TSTAT_2_2(self) -> list[TestStep]: | ||
| steps = [ | ||
| TestStep("1", "Commission DUT to TH"), |
There was a problem hiding this comment.
Can you please change this to
TestStep("1", "Commission DUT to TH",
is_commissioning=True),
so that the TH commissions the DUT if necessary?
|
@hasty We have to add steps 19 to 22c as per the test plan. Here is the test plan reference: https://github.com/CHIP-Specifications/chip-test-plans/blob/master/src/cluster/thermostat.adoc#tc-tstat-2-2-setpoint-test-cases-with-server-as-dut |
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.
This provides a Python implementation of TC-TSTAT-2.2 to replace the unused TC-TSTAT-2.2 YAML test.
Fixes #35748