Repository navigation
Add child devices for multi-channel devices - #947
zigpy-review-bot wants to merge 12 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #947 +/- ##
==========================================
+ Coverage 97.22% 97.24% +0.02%
==========================================
Files 61 62 +1
Lines 10676 10836 +160
==========================================
+ Hits 10380 10538 +158
- Misses 296 298 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| # `compute_output_child_devices`. Splitting changes how existing entities are | ||
| # presented, so devices are opted in one by one, after checking their diagnostics. | ||
| # Quirks can define child devices themselves instead, which takes precedence. | ||
| CHILD_DEVICE_ALLOWLIST: Final[tuple[ChildDeviceMatch, ...]] = ( |
There was a problem hiding this comment.
Given there are so few of these (so far), should we even keep this in ZHA?
There was a problem hiding this comment.
I'm also not a big fan of bringing (new) model/manufacturer information back into ZHA, but right now, this feels like a ZHA feature to me, for which you wouldn't even need quirks (though it can provide optional info). I'm not sure I'd expect turning off quirks in the HA YAML config to suddenly disabling all child device support.
But other than that, we'd then also need a v1 quirk API for defining child devices (or at least opting in to automatically creating them), as these Tuya devices are wildcard v1 quirks and actually match a ton of different (but similar) products.
But yeah, I don't know... I think this opt-in list will be in a weird place no matter what. I guess we could maybe just move it to a central list in the quirks repo? And not have the opt-in part of individual quirks for now? (But yeah, you're right in that it's not many devices at the moment but they are v1 quirks.. Though we could probably make that work.)
There was a problem hiding this comment.
Some data on this (the counts are in the PR body as well): of the 39 fixtures that get split, 20 are unquirked (Aurora, Sunricher, eWeLink, AEOTEC, several LUMI switches, and plain Tuya TS011F/TS0013/TS0115/TS110E/TS0002 variants) and 19 use v1 quirks. None uses a v2 quirk. So if the list moved into quirks, about half of these devices would first need a quirk just to opt in.
I'd keep it in ZHA for now, mostly for the reason already mentioned: it's a ZHA feature on otherwise standard-compliant devices, and enable_quirks: false shouldn't silently turn it off. But the v1 part may be easier than it looks, so the ZHA list could shrink to just the unquirked devices:
Details
A central list in zha-quirks would follow enable_quirks. ZHA never imports zhaquirks: zhaquirks.setup() pushes the quirks into ZHA's registry, and Home Assistant only calls it when enable_quirks is on (homeassistant/components/zha/helpers.py, setup_function=zhaquirks.setup). A central list in zha-quirks would have to be registered the same way, so turning quirks off would turn child devices off too. Workable, but not what people would expect.
v1 quirks don't need a new API to opt in. v1 quirks already reach ZHA through quirk_id, which ends up in Device.exposes_features (zha/zigbee/device.py, the ATTR_QUIRK_ID lookup in _init_from_zigpy_device), and quirk_id can be a set. The Tuya ts000x quirks already set quirk_id = TUYA_PLUG_ONOFF. So ChildDeviceMatch could also match exposed features, the same way ClusterMatch does. A v1 quirk would opt in by adding a feature ID to its quirk_id, and a v2 quirk with .exposes_feature(...). The entries for quirked devices (the Tuya ts000x/ts001x/ts011f_plug/ts130f quirks, LUMI relay.c2acn01 and switch.acn047) could then move to their quirks, and only the unquirked models would stay in ZHA, or get quirks later.
"So few" undercounts a bit. The 18 models in the fixtures are a lower bound: the Tuya entries match every _TZ… manufacturer ID, and the v1 Tuya quirks are wildcards too, so real installs will see many more split devices. The collision rule keeps single-output variants of those models unsplit.
If you'd rather get the model list out of ZHA now, I'd do it as the exposed-feature opt-in above (no new v1 API) for the quirked devices. For the unquirked ones, there are two options: either add a central list in zha-quirks and document that disabling quirks disables child devices, or keep a small list in ZHA until they have quirks.
|
@zigpy-review-bot Explain the (HA) issue more with why moving entities, including the re-interview, and disabled part. (What would happen if Also, if there's HA issue(s) or API limitations, create issue(s) in zigpy/backlog repo for those. |
|
On the HA side, two things move entities: the entity platform, which does it whenever an entity is added or re-added, and What the entity platform already does
What breaks without
|
| with reconcile | without | |
|---|---|---|
| Entities move to the child devices and back | yes | yes (platform) |
| Child devices after the split goes away | removed | both stay, empty, "Channel 1" still disabled |
| Switch that was on the disabled "Channel 1" | main device, enabled | main device, still disabled_by: device |
| ZHA config entry reloads | 1, 30 s later | 0 |
So the reconcile does two things the platform can't:
- It removes child devices ZHA no longer defines. Nothing in core removes a child device on its own, so without it they stay as empty "Channel N" devices under the main device. Removing a device also deletes every entity still on it, so the reconcile first moves whatever is left on a stale child device to the main device: disabled entries, and entities ZHA no longer creates. Only then does it remove the child device.
- It fixes the
devicedisabler. Disabling a child device disables its entities withdisabled_by: device. When such an entity moves to an enabled device, the entity registry doesn't re-check that disabler. It only does so when the device the entity is on gets updated. The frontend locks the "Enabled" switch for this disabler, so the entity would stay disabled for no visible reason, until the user changes the main device (e.g. disables and re-enables it)._async_move_entityclears it, and sets it when an enabled entity lands on a disabled device. Clearing it re-enables the entity, and like any re-enabled entity that makes HA reload the ZHA config entry 30 s later, once. That's the reload mentioned under "Known gaps" in the PR body.
A disabled main device needs nothing extra: core creates its child devices disabled as well and re-enables them, with their entities, together with the main device.
Why the re-interview call
_create_entity_metadata already runs the reconcile on HA start, on a config entry reload, on a new join, and when a re-interview swaps the device object because a different quirk matched. A normal re-interview keeps the same object: ZHA tears down and re-adds the entities through per-entity events, and _create_entity_metadata never runs. Without the extra call in handle_device_fully_initialized, a re-interview that changes the split (e.g. a quirk that now opts out) would leave the stale child devices and the stuck disabler until the next HA restart.
One small inaccuracy on the branch: the comment in test_reconcile_child_devices ("A disabled entity is never added, so only an explicit move puts it on its child") doesn't match what the probe shows: the platform moves disabled entities too. The move loop in the reconcile is still needed for the disabler fix. Only its device_id change overlaps with what the platform does.
HA issues
- [Core] Moving an entity to another device keeps a stale
disabled_by: devicebacklog#81: moving an entity to another device keeps a staledisabled_by: device(core bug, ZHA, Z-Wave and Matter all move entities this way). - [Core] No core support for removing child devices an integration no longer provides backlog#82: core has no support for removing child devices an integration no longer provides, so each integration has to move the entities back and remove the child device itself.
I didn't file the 30 s config entry reload after re-enabling an entity, because that's long-standing core behaviour for every integration and is already described in zigpy/backlog#80. Changes to the child devices at runtime (recompute_entities()) are a ZHA-side gap, already listed under "Known gaps".
How this was checked
- A throwaway test in the HA branch worktree, run with
ZHAGatewayProxy._async_reconcile_child_devicesas is and patched to a no-op. It drivesGateway._async_device_reinterviewedon the same device object, toggles the allowlist and the child device'sdisabled_by, then countsasync_reloadcalls after advancing the clock 31 s. A second run covered a disabled main device. The branch'stests/components/zha/test_child_devices.pypasses (9 tests). - Core code:
EntityPlatform._async_add_entity(the registry update comes before the disabled check, and it only ever sets thedevicedisabler),EntityRegistry.async_device_modified(removing a device removes its entities, anddevice-disabled entities are only re-enabled on an update of their device),_async_update_entity(only adjustsconfig_entrydisablers), anddevice_registry.async_cleanup(never removes a child device whose main device and config entry still exist). Unchanged on current HAdev.
|
@zigpy-review-bot zigpy/backlog#79 and zigpy/backlog#81 should be fixed. There's also a merge conflict now, so update this PR, including PR body. |
|
Merged dev into this PR (c73ea4e) and resolved the conflict, which came from #919 making PlatformEntity device-bound. |
Summary
This lets ZHA split a device into child devices: logical parts of one Zigbee device, such as the gangs of a wall switch or the sockets of a power strip, that Home Assistant shows as sub-devices of the main device (home-assistant/architecture#1414). It is the ZHA library side of zigpy/backlog#73 and home-assistant/core#172450.
Today all entities of a multi-channel device sit on one device and end up with identical names: a 3-gang Tuya switch has three entities all called "Light", told apart only by the
_2/_3entity ID suffix. With this PR, each output gets its own child device, named "Channel 1", "Channel 2", ... in endpoint order, and its output entity becomes that child device's primary entity.Child devices come from one of two places:
CHILD_DEVICE_ALLOWLIST) of devices whose outputs are split automatically. The first version opts in specific models only, all checked against their diagnostics.Device._quirk_child_devices()hook. TheQuirkBuilderAPI for this is added in Add child devices to quirks v2 zha-device-handlers#5456. A quirk definition replaces the automatic split, and an empty definition disables it.How the automatic split works
The rule is computed from the device's entities, not from endpoint device types. I ran both kinds of rule over all 926 diagnostics files in
tests/data/devices/. A Matter-style device-type rule (an endpoint is an output if its device type is a light/plug/cover type and repeats on a sibling) misfires on Xiaomi devices: they stampON_OFF_LIGHT/SMART_PLUGon endpoints that only carry Analog/Multistate Input clusters, and some useON_OFF_SWITCHfor the real relays. So for an allowlisted device:backlight_mode,power_on_stateandchild_lock.The flip side of rule 2: an entity that belongs to one output but exists only once stays on the main device, such as the window covering type sensor that only the second cover of the
_TZ3000_kmsbwdolTS130Freports. A quirk can assign such entities explicitly.Explicitly assigned entities override both rules (
PlatformEntity._attr_child_device_id, set through the quirk entity keywordchild_device_id). A definition that puts one endpoint into two child devices, or refers to an unknown child device, is logged and ignored rather than breaking the device's initialization.The primary entity is now elected per child device and for the main device. On a device with child devices, the main device only elects an output entity (or an explicitly primary one) as its primary entity. Otherwise the leftover energy sensor of a power strip would become its primary entity and lose its name. Several explicitly primary entities on the same device or child device now log a warning, and none of them is reported as primary, instead of failing an assertion.
Allowlist and diagnostics
All 926 diagnostics files gain the new fields, and the 46 files of allowlisted multi-output devices also gain their child devices, so the regenerated files show the result for each of them. That is 41 distinct devices across 24 models, with 2 to 5 child devices each. Another 28 files match an allowlisted model but have one output or none, so they are not split.
To pick the devices, I took every fixture where an output repeats on several endpoints, forced the split on, and checked the result. Only devices with diagnostics are allowlisted, and only where the result is right.
Allowlisted
TS0002,TS0003,TS0004,TS000F,TS0012,TS0013(multi-gang switches and relays),TS011F,TS0115(power strips),TS130F(dual cover) andTS110E(dual dimmer), for any manufacturer. Single-output variants of these models are not split, because they have no sibling output (e.g. the single-relayTS000Ffixtures).lumi.relay.c2acn01,lumi.switch.acn047,lumi.switch.agl006,lumi.switch.agl010,lumi.switch.b2naus01,lumi.switch.b2nc01andlumi.switch.l2aeu1.ZGA004, AuroraDoubleSocket50AU, EcoDimEco-Dim.05 ZigbeeandEcoDim-Zigbee 3.0(double dimmers), eWeLinkZB-SW02, frientIOMZB-110(its two relay outputs; the four inputs stay on the main device) and SunricherON/OFF(2CH).For example, the Tuya
TS011F4-socket strip (tz3000-lqb7lcq9-ts011f-0x00000050.json) gets four child devices with one switch each, while its meter, LQI/RSSI, identify and firmware entities stay on the main device. Numbering follows the output endpoints, so the USB port of the TuyaTS0115strip (endpoint 7) becomes "Channel 5". The Tuya_TZ3000_uwkja6z1TS011Fmeters each socket, so its current/power/energy/voltage sensors move to their sockets' child devices.Not allowlisted
TS0601multi-gang switches and dimmers (TuyaQuintupleSwitchGP,TuyaQuadrupleSwitchGP,TuyaTripleSwitchVar03,TuyaTripleSwitchDimmerGP,TuyaDoubleSwitchDimmer(GP), and the unquirked 8-gang_TZE200_wktrysab). They split cleanly, butTS0601is Tuya's generic model, so the allowlist would need an entry per manufacturer ID. Letting their quirks opt in fits better (see the inline thread on where the allowlist should live).SLR2b(heating and hot water receiver). It splits well (only one diagnostic sensor that exists on a single channel stays on the main device), but "Heating" and "Hot water" names from a quirk would read better than "Channel 1/2".VZM36, where the automatic split gets it wrong: the fan on endpoint 2 stays on the main device while the light on endpoint 2 moves.RBSH-MMS-ZB-EU(BMCT-SLZ), whose outputs depend on its mode (cover or lights).lumi.ctrl_neutral2(2-gang) andlumi.switch.b1lacn02(1-gang) would get 4 and 3 child devices. Their extra On/Off endpoints are almost certainly buttons or inputs, not relays.TS0726scene switches: it is unclear whether every On/Off endpoint is a relay.UIOT-WindowCovring2[04,02]: three identical cover endpoints, and the diagnostics don't show whether all three are real motors.DONGLE-E_R(not a multi-channel device), DigiXBee3and ptvoptvo.switch(DIY firmware with user-configured endpoints).Changes
zha/zigbee/child_device.py(new): theChildDevicedataclass, the allowlist, the automatic split (compute_output_child_devices) and the assignment for quirk-defined child devices.zha/zigbee/device.py: the_quirk_child_devices()hook, child device assignment in_add_pending_entities, per child device primary entity election (is_primary_entity),Device.child_devices, andchild_devicesinExtendedDeviceInfoand the diagnostics.zha/application/platforms/__init__.py:PlatformEntity.child_device, thechild_device_idquirk entity keyword, andchild_device_idinBaseEntityState.tests/data/devices/*.json: regenerated. Every file gainschild_devicesand achild_device_idper entity; the 46 allowlisted multi-output files also gain their child devices and the primary entity flags.Breaking changes
None in the library API: the new
BaseEntityStateandExtendedDeviceInfofields have defaults.For Home Assistant users, it is a breaking change once the integration uses this. Entities of allowlisted devices move to child devices on the next start: entity IDs stay the same, but displayed names change. An output entity takes the name of its child device, for example
Channel 2instead of<device> Light, and a renamed entity shows the child device name in front of its name (Channel 1 Kettle). Targeting the main device still includes its child devices.Home Assistant
The integration side is prepared on a Home Assistant Core branch: dev...TheJulianJES:core:zigpy-bot/zha-child-devices. The PR will be opened once this is released.
ChildDeviceInfowith the identifier(zha, "<ieee>-<child id>"), named through a newchanneldevice translation (Channel {index}), or the quirk's fallback name when its translation key isn't translated in Home Assistant.devicedisabler is cleared (Home Assistant core now does this when an entity moves to another device or is restored, Fix derived entity disabled_by handling home-assistant/core#186750, fixing [Core] Re-created entity restoresdisabled_by: deviceon a device that is not disabled backlog#79 and [Core] Moving an entity to another device keeps a staledisabled_by: devicebacklog#81), and child devices that no longer exist are removed after their entities moved back to the main device, so entity IDs and customizations survive.Z-Wave (home-assistant/core#185985) and Matter (home-assistant/core#186033) are adding child devices at the same time, with similar opt-in scoping.
Testing
tests/test_child_device.pycovers the allowlisted split, per-output meters, unlisted and single-output devices, outputs disabled by default, device-level entities repeated on every output, virtual entities, quirk-defined child devices (including device-level clusters on a child device's endpoint), disabling the split, overlapping endpoints, explicit assignments with runtime changes, conflicting explicit primary entities, and primary entity re-election.zha/zigbee/child_device.pyhas 100% coverage.test_quirks_v2_child_devicesruns against the quirks PR and is skipped until a zha-quirks release withQuirkBuilder.child_deviceis pinned. It passes locally with that branch installed.Decisions to review
TS0002,TS0003,TS0004,TS000F,TS0012,TS0013,TS011F,TS0115,TS130F,TS110E) apply to every_TZ…manufacturer ID, so real installs will see more split devices than the fixtures show, including variants without a fixture. The collision rule keeps single-output variants unsplit.TS0115USB port becoming "Channel 5".Known gaps
recompute_entities()) reach Home Assistant on the next restart or re-interview, not immediately.test_quirks_v2_child_devicesis skipped until a zha-quirks release with Add child devices to quirks v2 zha-device-handlers#5456 is pinned.