Repository navigation
Exclude config and diagnostic entities from groups - #976
zigpy-review-bot wants to merge 1 commit into
Conversation
A grouped Sonoff S60 contributed its "Network LED" config switch to the switch group entity: the group state was ORed over the LED switches, so the group showed "on" with all relays off, and HA listed the LED switches as group members. Group commands only target the main cluster, so config and diagnostic entities are not part of the group.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #976 +/- ##
=======================================
Coverage 97.24% 97.24%
=======================================
Files 61 61
Lines 10748 10748
=======================================
Hits 10452 10452
Misses 296 296 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
TheJulianJES
left a comment
There was a problem hiding this comment.
This still isn't an ideal rule. Maybe we should look more at the clusters or whatever. I'll play around a bit more..
There was a problem hiding this comment.
🟢 Approval recommended
The focused filter addresses the reported bug consistently across group consumers, with regression coverage and no unresolved findings.
No open issues 🎉
What changed in this PR
This PR prevents config and diagnostic entities from affecting Zigbee group membership and state, fixing groups that appear on because a device’s Network LED switch is on.
Changes:
- Excludes categorized entities from the shared group-member entity list.
- Adds a Sonoff plug regression test covering membership, LED-only activity, and relay state changes.
| File | Description |
|---|---|
zha/zigbee/group.py |
Filters config and diagnostic entities out of group membership. |
tests/test_switch.py |
Verifies that Network LED switches are excluded and relay states control the group state. |
Review effort: Balanced
|
This is an interesting side effect of splitting them off. Groups being a union of all exposed functionality is kind of nice if they all consist of the same device and you want to change a feature on all of them at once. On the other hand, this is unintuitive if a single device exposes a single feature and adds it to a large group. It also possibly conflicts if we ever decide to have per-group settings exposed as configuration entities... |
|
Yeah.. #977 is IMO the better approach compared to this PR. (Do note that this is completely unrelated to the ZHA group device entry changes in HA. I just noticed it during testing that the group entity's more info dialog showed the "Network LED" config switch for my Sonoff S60 plug and also always stayed on because of it. 😅)
(Though this was only a UI/association issue. Turning on/off the group never wrote the network LED switch here, for example. |
|
Superseded by #977 |
Summary
Alternative: #977 fixes the same bug with a stricter rule. It keeps only the entity classes driven by the clusters that group commands go to (
Switch,Light,Fan), so it also leaves out BinaryOutput switches, quirks v2 attribute switches without a category, and IKEA air purifier fans. Pick one and close the other.When two Sonoff S60ZBTPF plugs are grouped, the switch group entity also picks up each plug's "Network LED" config switch (a
ConfigurableAttributeSwitchon the same endpoint as the relay). The group state is the OR of all available member switch entities, so with both relays off and the LEDs on, the group shows as on. HA also lists the "Network LED" switches as members of the group entity, because Core builds the group's member list fromGroupMember.associated_entitiesas well.Config and diagnostic entities are settings or device-health entities, not the device's main controls. Group commands go to the cluster the group entity controls (OnOff for switch groups), so these entities don't follow the group and should not be part of it.
Changes
GroupMember.associated_entitiesnow skips entities that have anentity_category(config or diagnostic). This one property feeds group entity discovery,Group.get_platform_entities()(group state, availability, polling) andGroupMember.member_info, and it is also what HA Core reads for the group entity's member list, so HA gets the corrected member list from a ZHA version bump alone.new_entity_categoryare on siren (IAS WD) entities, and there are no siren groups.Testing
test_group_ignores_config_switches: two S60ZBTPF devices from the diagnostics snapshot in a group. The "Network LED" switches are not group members, the group is off while only the LEDs are on, and it turns on when a relay turns on. The test fails without the change.