TC-VALCC-3.1: Allow immediate open of valve #35851
Open
+214
−201
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
If the valve can be opened before the command is complete or before the next read is done, the test will fail because it expects the transitioning phase to happen. In the spec: When the movement is complete, the device SHALL set the CurrentState attribute to the Open value. ^ this can happen before the end of the command.
|
Review changes with Changed Files
|
github-actions
bot
added
tests
matter-1.4-te2-script-change
|
PR #35851: Size comparison from 9c6c365 to fa83b47 Full report (88 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
andy31415
reviewed
Oct 3, 2024
andy31415
approved these changes
Oct 3, 2024
|
This seems out of sync with the test plan. Is that being updated as well? |
|
@fessehaeve - yes, the test plan will need to be updated, I can update it from this script once this is approved. The changes to the cluster are happening in a separate branch. I should have the changes up in a PR by the SDK tomorrow, but they're orthogonal to this change - just an explanation for why I'm only doing a partial clean of the current code. |
ReneJosefsen
reviewed
Oct 9, 2024
ReneJosefsen
approved these changes
Oct 9, 2024
fessehaeve
reviewed
Oct 10, 2024
...p/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.cpp
Outdated
Show resolved
Hide resolved
fessehaeve
reviewed
Oct 10, 2024
...p/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.cpp
Show resolved
Hide resolved
fessehaeve
reviewed
Oct 10, 2024
...p/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.cpp
Show resolved
Hide resolved
cecille
removed
the
matter-1.4-te2-script-change
github-actions
bot
added
the
matter-1.4-te2-script-change
fessehaeve
approved these changes
Oct 23, 2024
|
PR #35851: Size comparison from 84fb78f to 0793ceb Full report (10 builds for cc32xx, nrfconnect, qpg, stm32, 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.
If the valve can be opened before the command is complete or before the next read is done, the test will fail because it expects the transitioning phase to happen AND it expects to be able to read the attribute in the intermediate state.
In the spec:
When the movement is complete, the device SHALL set the CurrentState attribute to the Open value.
^ this can happen before the end of the command.
The current implementation does actually set the current state to open immediately, but the test doesn't catch this because the current implementation also doesn't set the target back to NULL when the change to CurrentState happens.
Changes to the test:
Changes to the current cluster implementation:
Note that these changes are being done as part of an effort to refactor this cluster entirely, so changes to the implementation will be short-lived and are implemented only to allow landing the test changes before the refactor.