Fix missing config entity attribute reads - #825
Conversation
PR #657 ("Replace cluster handlers with entity attributes") moved startup attribute reads from cluster handlers' `ZCL_INIT_ATTRS` onto each entity's `_server_cluster_config`. Several cache-gated config entities never got one, so their backing attribute is never read on a fresh pair. Because these entities skip their own creation when `cluster.get(attribute_name) is None`, they are silently culled on newly-paired devices (devices paired under an older ZHA keep working only because the value persisted in the cache from a past read). Restore the reads by giving each affected entity a `_server_cluster_config` that reads its backing attribute on startup (mirroring the pre-#657 `ZCL_INIT_ATTRS` cache/fresh semantics): - Sonoff SNZB-06P/03P: presence timeout (`ultrasonic_o_to_u_delay`) and detection sensitivity (`ultrasonic_u_to_o_threshold`) - Hue SML00x: motion sensitivity (`sensitivity`) - Danfoss Ally thermostat: exercise day of week, exercise trigger time, orientation, adaptation run control, control algorithm scale factor, external measured room sensor, load room mean, regulation setpoint offset Add a discovery invariant test asserting every cache-gated config entity reads its backing attribute on startup (with a documented allowlist for Tuya EF00 attributes whose cache is seeded by the quirk), plus functional tests that the Sonoff entities are created from the startup read.
Drop the Sonoff read-on-join tests and the cache-gate/startup-read invariant test; a check like the invariant may live in the diagnostics tooling instead in the future.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #825 +/- ##
=======================================
Coverage 97.27% 97.27%
=======================================
Files 55 55
Lines 10930 10940 +10
=======================================
+ Hits 10632 10642 +10
Misses 298 298 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| OccupancySensing.AttributeDefs.pir_o_to_u_delay: AttrConfig( | ||
| read_on_startup=False, | ||
| ), | ||
| OccupancySensing.AttributeDefs.pir_u_to_o_delay: AttrConfig( | ||
| read_on_startup=False, | ||
| ), |
There was a problem hiding this comment.
Huh, I'm not sure how these attributes ended up in the cluster config of this Hue-specific select entity? These are used by a number entity. (Side note: Why does that number entity also include the occupancy attribute for the binary sensor again?)
I'll check to see if other unintentional changes were introduced by that refactor and then remove these in a follow-up PR (to keep this PR as just the fix).
There was a problem hiding this comment.
And the binary sensor also re-adds all the same attributes yet again? Why do all three platforms configure the exact same attributes in the cluster config? We merge these, right..?
zha/zha/application/platforms/binary_sensor/__init__.py
Lines 249 to 260 in c38e03e
There was a problem hiding this comment.
This is a wholesale copy of the old OccupancySensingClusterHandler config: #657 translated each pre-refactor cluster handler by copying its full REPORT_CONFIG/ZCL_INIT_ATTRS block onto every entity class that used that handler — the Occupancy binary sensor, the PIR occupied→unoccupied delay number entity, and this Hue select all got the identical occupancy + pir_*_delay block. The handler's dynamic __init__ additions (Hue sensitivity, Sonoff ultrasonic_*) were dropped in that translation, which is exactly the regression this PR fixes.
And yes, they merge: aggregate_cluster_configs() merges all discovered entities' configs per (endpoint, cluster, direction) — read_on_startup is OR-ed, the tightest reporting wins, bind is OR-ed — so the duplicates are runtime-harmless, but they hide gaps like this one. Two more side effects of the same copy-paste, for the follow-up:
PIRUnoccupiedToOccupiedDelayConfigurationEntityhas no cluster config of its own and only works because its sibling's copy happens to includepir_u_to_o_delay.ThermostatLocalTempCalibration(+ Sonoff/Bosch subclasses) andDanfossExerciseDayOfTheWeekcarry the entire 22-attribute thermostat handler block (includingsetpoint_change_source*), andStartupOnOffSelectEntity/OnOffTransitionTimeConfigurationEntity/BegaColorTemperatureChannelSelectcarry the OnOff/LevelControl state blocks.
On the entity count: before this PR exactly 12 entity classes are broken on fresh pairing — per device that's 2 missing entities on SNZB-06P/SNZB-03P, 1 on each Hue SML001–SML004, and 8 on a Danfoss Ally. After this PR the only cache-gated entities without a ZHA-side read are the two Tuya 0xEF00 ones, which are quirk-seeded by design. (Full audit in my review.)
I've pushed the dedup as a follow-up basis on top of this branch: zigpy-bot/cluster-config-dedup — 5 commits (OccupancySensing / Thermostat / LevelControl / OnOff dedup, each entity now only declaring the attributes it owns, verified by diffing the aggregated per-(cluster, attribute) config view before/after — byte-identical — plus a full test run; and one restore commit for two more #657 losses: Philips SOC001 0xFC06 bind+reporting and the Legrand cable outlet 0xFC40 bind, details in the follow-up comment). Happy to open it as a PR once this one lands.
zigpy-review-bot
left a comment
zigpy-review-bot
left a comment
There was a problem hiding this comment.
Approve — the fix is correct, and I can confirm the affected-entity list in the description is complete.
I audited this programmatically: enumerate every registered entity class whose _is_supported() gates on cluster.get(<attr>) is None (126 classes across select/number/switch), then check which backing attributes are read at startup by any co-matching entity's _server_cluster_config:
- Before this PR, exactly the 12 entity classes listed in the description have no startup read (2× Sonoff SNZB-06P/03P,
HueV1MotionSensitivity/HueV2MotionSensitivity, 8× Danfoss Ally) — nothing else is missing. - After this PR, only the two Tuya
0xEF00entities remain unread (timer_duration,window_detection_function), matching the description: those were never read by ZHA pre-#657 and rely on the quirk seeding the attribute cache. - The
read_on_startupchoices mirror the pre-#657ZCL_INIT_ATTRSmarkings exactly: the oldattr: True(allow-cache) entries →read_on_startup=False, and the two Danfoss# Can changeentries (external_measured_room_sensor,load_room_mean) →read_on_startup=True(checked againstDanfossThermostatClusterHandler/OccupancySensingClusterHandlerate399b13a^). - Ordering is right:
async_initialize()discovers prospective entities, aggregates and reads the configs, and only then runs theis_supported()gating (zha/zigbee/device.py,async_initialize→_add_pending_entities), so the restored reads land before entity creation is decided.
Root-cause note for the record: the pre-#657 OccupancySensingClusterHandler added the Hue sensitivity / Sonoff ultrasonic_* reads dynamically in __init__, and DanfossThermostatClusterHandler extended the base dict — the refactor translated the static handler configs onto entities but dropped these model-conditional additions.
Tests pass locally and mypy (run with project dependencies installed) is clean. A second-opinion Copilot (GPT-5.6 Sol) review of this PR returned no findings.
Pre-existing gap in the same area, not part of this PR: Inovelli VZM36 / VZM32-SN (click to expand)
The same audit shows the Inovelli config entities get no startup read on models without an InovelliVzm3*Init virtual entity: InovelliDimmingModeEntity explicitly matches VZM36, and the unfiltered Inovelli entities match any device exposing 0xFC31 (e.g. VZM32-SN), but Init entities exist only for VZM30-SN/VZM31-SN/VZM35-SN. This is not a #657 regression — the old InovelliConfigEntityClusterHandler.__init__ also only had ZCL_INIT_ATTRS branches for those three models — so these cache-gated entities have plausibly never been created on freshly-paired VZM36/VZM32-SN devices. Worth a follow-up Init entity (or model additions) if those models should expose the config entities.
|
As a follow-up to the review: I re-checked the original refactor (#657) — both its review threads and a fresh pass over the (non-snapshot) diff against the deleted Two more lost-in-translation regressions of the same kind as this PR (verified against
Update: the mechanical items from the list below are now fixed on Still open from the original #657 review threads (verified still present on
|
|
The gaps mentioned above should be addressed, probably with something like the below branches: In the future, these manufacturer-specific entities will be moved to zha-quirks anyway. This PR is only for a HA patch release, so it's a more minimal fix. We should also really expand diagnostics here for what we (try to) bind/init/report. |
| OccupancySensing.AttributeDefs.pir_u_to_o_delay: AttrConfig( | ||
| read_on_startup=False, | ||
| ), | ||
| "sensitivity": AttrConfig(read_on_startup=False), |
There was a problem hiding this comment.
Yeah, planned to do basically.
Proposed change
This PR fixes a regression introduced with the below PR where attributes for several config entities were not read when initially pairing the device, leading to missing entities. This is fixed by adding a cluster config to read the required attributes for those entities.
ZHA refactor PR that caused the issue (note: not the recent ZHA 2.0.0 refactor):
Addresses HA Core issue:
This should also fix a reported issue where the Danfoss entities are missing:
Future
In the future, we may want to move some of the entities to ZHA quirks. I'm not sure about the
ultrasonic_*-based entities used by the Sonoff though. Maybe we should make them more generic instead? Similar to thepir_*attributes?Either way, an initial branch moving all the Sonoff entities to quirks v2 (keeping the same unique ID) can be found here:
I guess we can always move them back to ZHA as generic entities if quirks keeps the cluster ID in the unique ID, so the format is the same as used by ZHA. And we should also move the other entities, like Danfoss and Hue.
AI summary
Problem
LLM-generated text describing the problem with more detail (click to expand)
ZCLEnumSelectEntity,NumberConfigurationEntityandConfigurableAttributeSwitchskip entity creation whencluster.get(attribute_name)returnsNonein_is_supported(). Before #657, the cluster handlers read these attributes during device initialization (ZCL_INIT_ATTRS/REPORT_CONFIG), so a value was cached and the entity was created. #657 moved the startup reads to per-entity_server_cluster_configdeclarations, but several entities never got one. Their backing attribute is now never read, so the entities are silently skipped on freshly-paired devices. Devices paired before the regression keep their entities only because a value is still in the database from a read performed by an older version.Reported for the Sonoff SNZB-06P in home-assistant/core#173224: the "Detection sensitivity" and "Presence detection timeout" entities are missing for newly-paired sensors, while identical sensors paired earlier still have them. Debug logs from that issue confirm the interview never sends a read for
ultrasonic_o_to_u_delay/ultrasonic_u_to_o_threshold, after which both entities are skipped with "… is not supported - skipping … entity creation".Affected entities
All confirmed against the pre-#657 handlers (each attribute was read at startup via
ZCL_INIT_ATTRS):SonoffPresenceSenorTimeout(ultrasonic_o_to_u_delay),SonoffPresenceDetectionSensitivity(ultrasonic_u_to_o_threshold)HueV1MotionSensitivityandHueV2MotionSensitivity(sensitivity) — the V1 entity has a_server_cluster_configthat readsoccupancyand the PIR delays, but not its own backing attributeDanfossExerciseDayOfTheWeek,DanfossExerciseTriggerTime,DanfossOrientation,DanfossAdaptationRunControl,DanfossControlAlgorithmScaleFactor,DanfossExternalMeasuredRoomSensor,DanfossLoadRoomMean,DanfossRegulationSetpointOffsetOther cache-gated entities are unaffected: the IKEA air purifier attributes are read via the fan entity's cluster config, and the Tuya
0xEF00attributes (timer_duration,window_detection_function) are seeded into the attribute cache by the quirk and were never read by ZHA itself.Fix
LLM-generated text describing the fix in detail (click to expand)
Give each affected entity a
_server_cluster_configthat reads its backing attribute on startup.read_on_startupmirrors the pre-#657 semantics: attributes marked cached inZCL_INIT_ATTRSuseread_on_startup=False(allow-cache read, still hits the device when nothing is cached), and the two Danfoss attributes previously marked "can change" (external_measured_room_sensor,load_room_mean) useread_on_startup=True.Device snapshots are unchanged: the recorded devices already carry cached values for these attributes, so they don't exercise the fresh-pair path.
Upgrade note
LLM-generated upgrade note (click to expand)
Affected mains-powered devices (e.g. the SNZB-06P) pick the values up on the next ZHA restart via startup polling, restoring the missing entities. Battery-powered devices (SNZB-03P, Hue motion sensors, Danfoss TRVs) are initialized from cache on restart, so they may additionally need a re-interview (Reconfigure with the device awake) or an unavailable/available cycle.