Remove non-ZCL siren select entities for basic sirens#666
Remove non-ZCL siren select entities for basic sirens#666TheJulianJES merged 4 commits intozigpy:devfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #666 +/- ##
=======================================
Coverage 97.62% 97.62%
=======================================
Files 62 62
Lines 10761 10764 +3
=======================================
+ Hits 10505 10508 +3
Misses 256 256 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9e4f067 to
42b5a35
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates ZHA’s registry-based entity discovery to support excluding entities based on quirk “exposed features”, and uses that to prevent creating the non-ZCL siren-related select entities for devices marked as siren_basic.
Changes:
- Add
not_exposed_featurestoClusterHandlerMatchas a negative filter for entity discovery. - Update discovery to skip entity classes when a device exposes any excluded features.
- Mark the non-ZCL IAS WD select entities as excluded for
SIREN_BASICdevices and update device snapshot JSONs accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
zha/application/platforms/select.py |
Excludes non-ZCL IAS WD select entities when SIREN_BASIC is exposed. |
zha/application/platforms/__init__.py |
Extends ClusterHandlerMatch with not_exposed_features. |
zha/application/discovery.py |
Applies the new negative exposed-feature filter during endpoint entity discovery. |
tests/data/devices/frient-a-s-smszb-120.json |
Updates expected discovered entities by removing select entities for basic siren device. |
tests/data/devices/frient-a-s-scazb-141.json |
Same as above for another basic siren device snapshot. |
tests/data/devices/frient-a-s-flszb-110.json |
Same as above for another basic siren device snapshot. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| manufacturers: frozenset[str] | None = None | ||
| models: frozenset[str] | None = None | ||
| exposed_features: frozenset[str] | None = None | ||
| not_exposed_features: frozenset[str] | None = None | ||
|
|
There was a problem hiding this comment.
ClusterHandlerMatch is not kw_only, so adding not_exposed_features in the middle of the dataclass field list is a backwards-incompatible change for any positional instantiations (they’d start binding later positional args to the new field). To minimize API breakage risk, either append not_exposed_features to the end of the dataclass fields (after feature_priority) or make the dataclass kw_only=True so only keyword construction is allowed.
There was a problem hiding this comment.
We should likely just make this kw_only. We're already using it as such, so this isn't breaking anything.
|
In the future, we likely want these non-ZCL entities to be bound to the presence of the |
Proposed change
This removes the non-ZCL select entities created for sirens IF they hint
"siren_basic"as an "exposed feature" from quirks.This is done by adding
not_exposed_featuresto discovery. This might also come in handy for future entities. This was one of the suggested options to achieve this from the PR linked below.Other options would have included overriding
is_supportedin all three classes to ignore based on the exposed features OR creating three more classes and making use offeature_priorityto always returnFalseforis_supportedin the basic siren ZCL select entities. IMO, thenot_exposed_featuresis cleaner and "fine".Additional information
Related (and see for more description):