fix and restore test script for TC-RVCOPSTATE-2.5 #40002
Merged
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
…hip#39931) * Adding TC_RVCOPSTATE_2_5 automated script * Update TC_RVCOPSTATE_2_5.py * Update TC_RVCOPSTATE_2_5.py * Update TC_RVCOPSTATE_2_5.py * Fixing copilot review comments * Restyled by autopep8 * Restyled by isort * Fixing review comments * Restyled by autopep8 * Restyled by isort * Removing unused import * Update src/python_testing/TC_RVCOPSTATE_2_5.py * update script to match updated test plan: remove forced command to Docked and associated check - state coulde be Charging, for instance * add check for SeekingCharger operational state in step 8 to match test plan * Restyled by autopep8 * fix indent * Restyled by autopep8 * force example app to docked state in CI * Update src/python_testing/TC_RVCOPSTATE_2_5.py Co-authored-by: Petru Lauric <81822411+plauric@users.noreply.github.com> * Update src/python_testing/TC_RVCOPSTATE_2_5.py Co-authored-by: Petru Lauric <81822411+plauric@users.noreply.github.com> * Update src/python_testing/TC_RVCOPSTATE_2_5.py Co-authored-by: Petru Lauric <81822411+plauric@users.noreply.github.com> * reset RVC example app when run in CI * Restyled by autopep8 * use required clean mode number as `cleaning_mode` --------- Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Kiel Oleson <kielo@apple.com> Co-authored-by: Petru Lauric <81822411+plauric@users.noreply.github.com>
![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 restores and fixes a Python test script for TC-RVCOPSTATE-2.5. The main change is the addition of the test file TC_RVCOPSTATE_2_5.py and enabling it for CI by providing the necessary application arguments.
My review of the new test script has identified a few issues that should be addressed:
- A
criticalissue where a variable is used before it's guaranteed to be defined, which would cause the test to fail in CI. - A
highseverity correctness issue in a helper function where an incorrect enum utility function is used, which works only by coincidence. - A
mediumseverity issue with a misleading assertion message that could hinder debugging.
I have provided specific code suggestions to resolve these issues. Overall, the changes are in the right direction to get this test working correctly.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
PR #40002: Size comparison from e560a10 to d83de15 Full report (3 builds for cc32xx, stm32)
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
PR #40002: Size comparison from e560a10 to b13fba0 Full report (59 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #40002: Size comparison from e560a10 to 784e0e3 Full report (59 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
There was a problem hiding this comment.
Pull Request Overview
This PR restores a Python automation test script for TC-RVCOPSTATE-2.5 that was previously reverted due to CI failures. The main purpose is to add proper CI configuration with app-pipe arguments to enable the test to communicate with the example RVC (Robotic Vacuum Cleaner) application.
- Adds complete test implementation for RVC Operational State cluster testing
- Includes CI test arguments configuration with app-pipe support for automated testing
- Implements test steps for mode transitions, GoHome command, and docking verification
woody-apple
approved these changes
Jul 14, 2025
kiel-apple
commented
Jul 14, 2025
andy31415
approved these changes
Jul 15, 2025
soares-sergio
approved these changes
Jul 15, 2025
andy31415
reviewed
Jul 15, 2025
| InvalidInMode = 0x3 | ||
|
|
||
|
|
||
| def error_enum_to_text(error_enum): |
There was a problem hiding this comment.
does an enum class not have a readable % to be used instead? Having this seems harder to maintain.
Or maybe override the str method if we really need to...
andy31415
reviewed
Jul 15, 2025
| operational_state = await self.read_operational_state(endpoint=self.endpoint) | ||
| asserts.assert_true(operational_state == expected_operational_state, | ||
| # TODO: nice conversion of states to strings | ||
| f"Expected OperationalState of {expected_operational_state}, got {operational_state}" |
There was a problem hiding this comment.
Could we use !r and get some goot result?
There was a problem hiding this comment.
https://peps.python.org/pep-0498/#s-r-and-a-are-redundant ... so either !r or repr()
bzbarsky-apple
added a commit
to bzbarsky-apple/connectedhomeip
that referenced
this pull request
Jul 15, 2025
project-chip#39672 renamed ClusterAttributeChangeAccumulator to AttributeSubscriptionHandler after project-chip#40002 ran CI but before it merged, so now we have non-working tests.
cecille
pushed a commit
that referenced
this pull request
Jul 16, 2025
LyudmilaKostanyan
pushed a commit
to LyudmilaKostanyan/connectedhomeip
that referenced
this pull request
Jul 16, 2025
* Adding python automation test script for TC-RVCOPSTATE-2.5 (project-chip#39931) * Adding TC_RVCOPSTATE_2_5 automated script * Update TC_RVCOPSTATE_2_5.py * Update TC_RVCOPSTATE_2_5.py * Update TC_RVCOPSTATE_2_5.py * Fixing copilot review comments * Restyled by autopep8 * Restyled by isort * Fixing review comments * Restyled by autopep8 * Restyled by isort * Removing unused import * Update src/python_testing/TC_RVCOPSTATE_2_5.py * update script to match updated test plan: remove forced command to Docked and associated check - state coulde be Charging, for instance * add check for SeekingCharger operational state in step 8 to match test plan * Restyled by autopep8 * fix indent * Restyled by autopep8 * force example app to docked state in CI * Update src/python_testing/TC_RVCOPSTATE_2_5.py Co-authored-by: Petru Lauric <81822411+plauric@users.noreply.github.com> * Update src/python_testing/TC_RVCOPSTATE_2_5.py Co-authored-by: Petru Lauric <81822411+plauric@users.noreply.github.com> * Update src/python_testing/TC_RVCOPSTATE_2_5.py Co-authored-by: Petru Lauric <81822411+plauric@users.noreply.github.com> * reset RVC example app when run in CI * Restyled by autopep8 * use required clean mode number as `cleaning_mode` --------- Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Kiel Oleson <kielo@apple.com> Co-authored-by: Petru Lauric <81822411+plauric@users.noreply.github.com> * add app pipe arguments for CI * Update src/python_testing/TC_RVCOPSTATE_2_5.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * fix description of step 9 when waiting for user input * use RVC app instead of all-clusters for CI testing * Update src/python_testing/TC_RVCOPSTATE_2_5.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * add command to idle mode so GoHome succeeds * Restyled by autopep8 --------- Co-authored-by: Kishok G <133193761+KishokG@users.noreply.github.com> Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Petru Lauric <81822411+plauric@users.noreply.github.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
LyudmilaKostanyan
pushed a commit
to LyudmilaKostanyan/connectedhomeip
that referenced
this pull request
Jul 16, 2025
project-chip#39672 renamed ClusterAttributeChangeAccumulator to AttributeSubscriptionHandler after project-chip#40002 ran CI but before it merged, so now we have non-working tests.
miduggan24
pushed a commit
to miduggan24/connectedhomeip
that referenced
this pull request
Jul 17, 2025
project-chip#39672 renamed ClusterAttributeChangeAccumulator to AttributeSubscriptionHandler after project-chip#40002 ran CI but before it merged, so now we have non-working tests.
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.
all-clusters-appGoHomecommand #40008)Summary
This PR was reverted because CI was failing on the added test. The added test needed app pipe arguments to work correctly after I modified it to send commands to the example app.
Related issues
PR #39931 is the original change with testing notes and code conversations.
Testing
scripts/examples/gn_build_example.sh examples/rvc-app/linux examples/rvc-app/linux/out/rvc-app./examples/rvc-app/linux/out/rvc-app/chip-rvc-app --app-pipe /tmp/rvc_fifosource scripts/activate.sh && scripts/build_python.sh -m platform -d true -i python_envpython3 src/python_testing/TC_RVCOPSTATE_2_5.py --commissioning-method on-network --passcode 20202021 --discriminator 3840 --storage-path ~/.chip_admin_storage.json --bypass-attestation-verifier --endpoint 1 --PICS examples/rvc-app/rvc-common/pics/rvc-app-pics-values --int-arg runmode_cleanmode:1 --app-pipe /tmp/rvc_fifoReadability 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