Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[TestEventTrigger] Update all platforms to register at least a default TestEventTriggerDelegate #31723

Open
tcarmelveilleux opened this issue Jan 27, 2024 · 0 comments
Open

[TestEventTrigger] Update all platforms to register at least a default TestEventTriggerDelegate #31723

tcarmelveilleux opened this issue Jan 27, 2024 · 0 comments
Labels
cert platform
Milestone
1.3 SVE

Comments

@tcarmelveilleux
Copy link
Contributor

  • TestEventTrigger delegates are implemented differently per platform and per cluster. This makes it very difficult to make a product that implements test event triggers for > 1 cluster concurrently.
  • TestEventTrigger handling sometimes relies on global methods on some platforms, like emberAfHandleEventTrigger on Silabs and Ameba platforms.
    • This makes it appear like emberAfHandleEventTrigger is a proper standard, but it should not be.
  • Most platforms do not correctly handle runtime selection of test event trigger key. The test event trigger key should NOT be enabled by default.
@tcarmelveilleux tcarmelveilleux added this to the 1.3 SVE milestone Jan 27, 2024
@github-project-automation github-project-automation bot moved this to Open Cert Blockers in [Certification] Blockers Jan 27, 2024
tcarmelveilleux added a commit to tcarmelveilleux/connectedhomeip that referenced this issue Jan 28, 2024
@tcarmelveilleux
Make TestEventTrigger delegate testable and extensible
2201e85 
- Make TestEventTriggerDelegate base class manage multiple sub-handlers.
- Add unit tests for TestEventTriggerDelegate
- Introduce an interface for generic TestEventTrigger handlers
- Add Clear() method to Intrusive list to remove everything.
- Document the fact that `emberAfHandleEventTrigger` should NOT be used
- Update all examples as needed.
  - Renumber OTA and SmokeCO testevent trigger prefix to match new Matter 1.3
    standard (first 16 bits == cluster ID being tested).
  - Clean-up dead code
  - Register OTA test event handler as intended.
- Add TODOs for platforms to clean-up TestEventTriggerDelegate registration.
- Updated BOOLCFG tests to use `--hex-arg` for PIXIT since `--int-arg` is too
  confusing with complex keys.

Issue project-chip#31723

Testing done:
- EEVSE Python tests using event triggers pass
- SMOKECO YAML tests using event triggers pass
- BOOLCFG Python tests using event triggers pass
- TC_TestEventTrigger passes
- New unit tests pass
@tcarmelveilleux tcarmelveilleux mentioned this issue Jan 28, 2024
Merged
mergify bot pushed a commit that referenced this issue Jan 29, 2024
@tcarmelveilleux @restyled-commits
Make TestEventTrigger delegate testable and extensible (#31724)
4cd1825 
* Make TestEventTrigger delegate testable and extensible

- Make TestEventTriggerDelegate base class manage multiple sub-handlers.
- Add unit tests for TestEventTriggerDelegate
- Introduce an interface for generic TestEventTrigger handlers
- Add Clear() method to Intrusive list to remove everything.
- Document the fact that `emberAfHandleEventTrigger` should NOT be used
- Update all examples as needed.
  - Renumber OTA and SmokeCO testevent trigger prefix to match new Matter 1.3
    standard (first 16 bits == cluster ID being tested).
  - Clean-up dead code
  - Register OTA test event handler as intended.
- Add TODOs for platforms to clean-up TestEventTriggerDelegate registration.
- Updated BOOLCFG tests to use `--hex-arg` for PIXIT since `--int-arg` is too
  confusing with complex keys.

Issue #31723

Testing done:
- EEVSE Python tests using event triggers pass
- SMOKECO YAML tests using event triggers pass
- BOOLCFG Python tests using event triggers pass
- TC_TestEventTrigger passes
- New unit tests pass

* Fix merge issue

* Restyled by clang-format

* Fix uninitialized variable access

* Fix Ameba/Silabs build

* Fix CopyString bound

* More fixes of Ameba/Silabs build

---------

Co-authored-by: Restyled.io <[email protected]>
@pankore pankore mentioned this issue Feb 20, 2024
Merged
maciejbaczmanski pushed a commit to maciejbaczmanski/connectedhomeip that referenced this issue Jul 15, 2024
@tcarmelveilleux @ArekBalysNordic
[nrf fromtree] Make TestEventTrigger delegate testable and extensible (…
70be330 
…project-chip#31724)

* Make TestEventTrigger delegate testable and extensible

- Make TestEventTriggerDelegate base class manage multiple sub-handlers.
- Add unit tests for TestEventTriggerDelegate
- Introduce an interface for generic TestEventTrigger handlers
- Add Clear() method to Intrusive list to remove everything.
- Document the fact that `emberAfHandleEventTrigger` should NOT be used
- Update all examples as needed.
  - Renumber OTA and SmokeCO testevent trigger prefix to match new Matter 1.3
    standard (first 16 bits == cluster ID being tested).
  - Clean-up dead code
  - Register OTA test event handler as intended.
- Add TODOs for platforms to clean-up TestEventTriggerDelegate registration.
- Updated BOOLCFG tests to use `--hex-arg` for PIXIT since `--int-arg` is too
  confusing with complex keys.

Issue project-chip#31723

Testing done:
- EEVSE Python tests using event triggers pass
- SMOKECO YAML tests using event triggers pass
- BOOLCFG Python tests using event triggers pass
- TC_TestEventTrigger passes
- New unit tests pass

* Fix merge issue

* Restyled by clang-format

* Fix uninitialized variable access

* Fix Ameba/Silabs build

* Fix CopyString bound

* More fixes of Ameba/Silabs build

---------

Co-authored-by: Restyled.io <[email protected]>

Cherry-picked from: 4cd1825
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
cert platform
Projects
[Certification] Blockers
Status: Open Cert Blockers
Milestone
1.3 SVE
Development

No branches or pull requests
1 participant
@tcarmelveilleux