Merge info_object into state and emit state change diffs#804
Conversation
Clean up typing a little
This reverts commit eca348c.
83e45b4 to
c9f046c
Compare
There was a problem hiding this comment.
Pull request overview
This PR migrates ZHA entity and group “state” from untyped dicts (and info_object) to strongly typed dataclass-based state objects, and adds per-change diffs (state_diff) to EntityStateChangedEvent to support more efficient downstream consumers (e.g., Home Assistant Core).
Changes:
- Replace
BaseEntityInfo/info_objectusage withBaseEntityStateand typed per-platform*Statedataclasses across platforms and diagnostics. - Emit
EntityStateChangedEvent.state_diffcomputed from previous vs current state, and add event emission suppression for new-entity bootstrap. - Update tests and mypy configuration to match the new typed-state API.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| zha/zigbee/group.py | Switch group member/group “info” payloads from info_object to typed state. |
| zha/zigbee/device.py | Emit state-changed events after entity recomputation and adjust diagnostics output to state dataclasses. |
| zha/event.py | Add suppress_events() context manager and make EventBase.emit() suppression-aware. |
| zha/application/platforms/update.py | Replace dict state + info_object with UpdateState dataclass state. |
| zha/application/platforms/switch.py | Replace dict state + info_object with SwitchState/ConfigurableAttributeSwitchState. |
| zha/application/platforms/siren.py | Replace dict state + info_object with SirenState and adjust squawk typing. |
| zha/application/platforms/sensor/init.py | Replace dict state + info_object with typed sensor state dataclasses; adjust attribute handling. |
| zha/application/platforms/select.py | Replace dict state + info_object with SelectState/EnumSelectState. |
| zha/application/platforms/number/init.py | Replace dict state + info_object with NumberState. |
| zha/application/platforms/lock/init.py | Replace dict state with typed LockState dataclass and refine lock-state typing. |
| zha/application/platforms/light/init.py | Replace dict state + info_object with LightState dataclass state. |
| zha/application/platforms/helpers.py | Update aggregation helpers to work with BaseEntityState dataclasses. |
| zha/application/platforms/fan/init.py | Replace dict state + info_object with FanState dataclass; update group aggregation. |
| zha/application/platforms/device_tracker.py | Replace dict state with typed DeviceTrackerState. |
| zha/application/platforms/cover/init.py | Replace dict state with typed CoverEntityState. |
| zha/application/platforms/climate/init.py | Replace dict state + info_object with ClimateState/ThermostatState. |
| zha/application/platforms/button/init.py | Replace info_object with typed button state dataclasses. |
| zha/application/platforms/binary_sensor/init.py | Replace dict state + info_object with BinarySensorState. |
| zha/application/platforms/alarm_control_panel/init.py | Replace dict state + info_object with AlarmControlPanelState. |
| zha/application/platforms/init.py | Introduce BaseEntityState, add state_diff + compute_state_diff(), and switch base/platform/group state to dataclasses. |
| zha/application/gateway.py | Emit group gateway messages using zha_group.state. |
| tests/test_update.py | Update update-entity assertions to typed state accessors. |
| tests/test_switch.py | Update switch assertions to typed state accessors. |
| tests/test_siren.py | Update siren assertions to typed state accessors. |
| tests/test_sensor.py | Update sensor assertions to typed state accessors and extra-state handling. |
| tests/test_select.py | Update select assertions to typed state accessors. |
| tests/test_number.py | Update number assertions to typed state accessors. |
| tests/test_lock.py | Update lock assertions to typed state accessors. |
| tests/test_light.py | Update light assertions to typed state accessors. |
| tests/test_gateway.py | Update gateway assertions to typed state accessors. |
| tests/test_fan.py | Update fan assertions to typed state accessors. |
| tests/test_discover.py | Update discover assertions to typed state accessors. |
| tests/test_device.py | Update device assertions to typed state accessors. |
| tests/test_device_tracker.py | Update device tracker assertions to typed state accessors. |
| tests/test_cover.py | Update cover assertions to derive cover state from typed boolean fields. |
| tests/test_climate.py | Update climate assertions to typed state accessors. |
| tests/test_alarm_control_panel.py | Update alarm control panel assertions to typed state accessors. |
| tests/common.py | Update helper selection logic from info_object.unique_id to state.unique_id. |
| pyproject.toml | Update mypy overrides to reflect the refactor and reduced ignore needs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
zigpy-review-bot
left a comment
There was a problem hiding this comment.
Comment. I reviewed the substantive parts of this change (the info_object→state merge, compute_state_diff, suppress_events, and the _add_pending_entities emit changes), ran the non-snapshot tests, and checked types. The approach looks sound — there's one real test failure hiding in the otherwise-expected red CI, plus a verdict on the Copilot bot's three threads.
Must-fix: one leftover info_object fails CI on its own
tests/test_sensor.py:911 (in test_analog_input_bogus_resolution) still reads entity.info_object.suggested_display_precision, which this PR removes → AttributeError: 'AnalogInputSensor' object has no attribute 'info_object' for both the [0.0] and [inf] params. It came in via #838 after your rename pass over this file, so it slipped through — the sibling asserts at lines 811/886 already use entity.state.…. Same fix:
assert entity.state.suggested_display_precision is NoneThis is not one of the reverted-diagnostics failures — it fails independently of the snapshot regen and stays red after the un-revert. It also isn't in the current CI run (which predates the #838 merge), so current CI doesn't show it yet. (The Copilot review flagged this one as well.)
Expected red (no action)
The ~851 test_discover.py::test_devices_from_files[…] failures and test_device_override_entities are all the intentionally-reverted diagnostics — the old-format snapshots still carry the nested info_object/state blocks — matching your PR note. Nothing to do there beyond the planned un-revert.
On the Copilot review's other two threads
compute_state_diffKeyError(platforms/__init__.py): not triggerable in practice —oldandneware always the same entity'sstate, so they're the same dataclass type with identical keys, and theold is Nonefirst-emit case is already handled. A symmetric.get()would be defense-in-depth only, not required.siren.py# type:ignoremissing a space: non-issue — mypy honors the no-space form. Verified locally (# type:ignore[assignment]does suppress the error), and CI's mypy hook is green.
Otherwise looks good
suppress_events()— theContextVar+ sync-onlywithblock in_add_pending_entitiesis task-safe: there's noawaitinside the suppressed region, so the flag can't leak into concurrently-scheduled tasks. The baseline-emit-under-suppression for new entities, plus the follow-up emit for existing entities whoseprimary/capabilities changed, reads correctly.mypy zha/is clean on the changed source files. The# type: ignore[misc]on theSwitch/SwitchGroupMRO overrides is legitimate — it's return-type variance only;dataclasses.replace(super().state, …)preserves the concreteSwitchState, sois_onsurvives at runtime (confirmed via the producedSwitchState(…, is_on=…)).
Not approving since it's pre-merge WIP with the diagnostics reverted, but the design is solid — just fold in the test fix.
zigpy-review-bot
left a comment
There was a problem hiding this comment.
Comment. Re-review at 1ccfbffc. The tests/test_sensor.py leftover from the last round is fixed. Locally: mypy zha/ is clean — 0 errors against the ~76-error dev baseline, ruff is clean, 467/467 non-snapshot tests pass, and the 849 test_discover failures are exactly the reverted regen you flagged in the description. Nothing fails independently of the snapshot revert.
I also walked every _zha_state.<field> your Core branch reads against the corresponding new dataclass. Two of them don't resolve.
Must-address
1. DeviceCounterSensor.state.available is always None. It's the only concrete entity subclassing BaseEntity directly, so it never passes through PlatformEntity.state/GroupEntity.state, which are what fill available. Inline below — measured values and impact there.
2. BitmapSensorState advertises extra_state_attribute_names that aren't fields on it. Raises AttributeError in the Core half for every Danfoss bitmap sensor. Inline below.
3. Bump DIAGNOSTICS_JSON_VERSION. zha/zigbee/device.py:107 is still 2, but get_diagnostics_json now emits a different zha_lib_entities shape. #657 bumped 1 → 2 for exactly this kind of restructure.
4. The new machinery has no tests. compute_state_diff, state_diff and suppress_events have zero direct coverage — grepping tests/ for any of the three returns nothing. Given the premise is that a consumer can reconstruct state from diffs alone, worth asserting a diff payload's contents, that a newly-added entity's baseline is not emitted, and that dataclasses.replace(cached, **diff) round-trips back to entity.state.
Design note, mostly for the Core half
The diff protocol assumes a consumer's baseline read and its listener attach are atomic. In the Core branch they aren't: ZHAEntity.__init__ captures self._zha_state = entity.state, but the STATE_CHANGED listener is only attached in async_added_to_hass. ZHA advances __previous_state on every emit whether or not anyone is listening, so anything emitted in that window is lost permanently rather than merely late — under the old re-read-everything model a missed event was self-healing. The re-sync at the end of async_added_to_hass doesn't cover it either, since it sits below if (state := await self.async_get_last_state()) is None: return. Moving that assignment above the early return closes the window.
The ZHA side of this reads correctly, for what it's worth: the suppress_events() baseline and the follow-up emit for existing entities whose primary/capabilities changed are both right, the suppressed block contains no await so the ContextVar can't leak into concurrent tasks, and every runtime recompute_capabilities() call site is either inside __init__ or immediately followed by maybe_emit_state_changed_event().
Nits
EntityStateChangedEventis now unhashable (frozen dataclass with adictfield). Nothing hashes it today, but it's a silent behaviour change on a public event type.BaseBinarySensorandBaseSelectEntitydeclareis_on/optionsbut theirstateno longer carries them — those moved down toBinarySensor/EnumSelectState. Every concrete subclass in tree is fine, so this is only about a base class no longer describing itself; compareBaseSwitch/BaseLock/BaseSiren, which kept theirs.BitMapSensor.formatterrecomputes the same per-bit dict thatstatenow builds — could readself.state.bit_states.
Otherwise
The typing payoff is real: zha.application.platforms.{button,update,siren,lock} all dropped out of the pyproject.toml mypy override list, and the remaining zha/ tree type-checks clean. The # type: ignore[misc] on the Switch/SwitchGroup MRO overrides is legitimate (return-type variance only — I traced the cooperative super() chain and the concrete SwitchState/FanState/LightState survive with available correctly filled by PlatformEntity/GroupEntity).
I ran a second-opinion pass with GPT-5.6 Sol; it independently flagged the DeviceCounterSensor placeholders below. Its only other finding was the system_mode → sys_mode rename, which I checked and is fine — your Core branch maps it explicitly.
Not approving while the diagnostics regen is reverted, but the shape of this is good.
| def state(self) -> DeviceCounterSensorState: | ||
| """Return the state for this sensor.""" | ||
| return DeviceCounterSensorState( | ||
| **super().state.__dict__, |
There was a problem hiding this comment.
DeviceCounterSensor is the only concrete entity that subclasses BaseEntity directly, so super().state here is BaseEntity.state, which hardcodes available=None and device_ieee=None — neither PlatformEntity.state nor GroupEntity.state is in the chain to fill them in. Measured on a coordinator counter sensor at this commit:
entity.available (live property) : True
state.available (HA reads this) : None # bool(None) is False
entity.identifiers.device_ieee : 00:15:8d:00:02:32:4f:32
state.device_ieee : None
On dev this was masked — DeviceCounterEntityInfo declared device_ieee: str and available: bool, annotations that were already None at runtime, and nothing read them. This PR drops those declarations without filling the values, and the Core branch changes ZHAEntity.available from entity.available to self._zha_state.available. Net effect: coordinator counter sensors come up unavailable and never recover, because available never changes and so no diff is ever emitted for it.
Impact is bounded — DeviceCounterSensor is entity_registry_enabled_default = False, so only users who explicitly enabled the coordinator counters see it. Fix is to mirror what PlatformEntity.state does:
return DeviceCounterSensorState(
**super().state.__dict__,
available=self.available,
device_ieee=self._device.ieee,
...
)(dataclasses.replace on the base state would work too.) GPT-5.6 Sol flagged this same spot independently.
| bit_states[bit.name] = bit in self._bitmap(value) | ||
| return BitmapSensorState( | ||
| **super().state.__dict__, | ||
| bit_states=bit_states, |
There was a problem hiding this comment.
The per-bit values moved into this nested bit_states dict, but _attr_extra_state_attribute_names a few lines up (lines 3646-3648) still advertises the flat bit names. extra_state_attribute_names is now a field on the state dataclass, and the Core-side sensor.py resolves each advertised name with getattr(self._zha_state, name) — so none of them resolve. Running that exact access against DanfossSoftwareErrorCode at this commit:
ADVERTISED : ['Critical_low_battery', 'Encoder_jammed', ..., 'Unknown_hw_error'] (11 names)
STATE FIELDS : [..., 'bit_states', ...] (0 of the 11)
-> AttributeError: 'BitmapSensorState' object has no attribute 'Unknown_hw_error'
The whole extra_state_attributes property raises, not just the missing key. This bites harder than it looks: BitMapSensor.formatter only ever returns "something" / "nothing", so these attributes are the only place a user can see which error bit is set.
You already added an ElectricalMeasurementState branch in the Core extra_state_attributes for max_attribute_name, and handled the system_mode → sys_mode rename in climate.py, so this looks like the one that slipped rather than a deliberate choice. Either flatten the bits into real dataclass fields, or add a matching BitmapSensorState branch that reads bit_states.
| state_dict = dataclasses.asdict(platform_entity.state) | ||
| state_dict["migrate_unique_ids"] = list(state_dict["migrate_unique_ids"]) | ||
| state_dict["device_ieee"] = str(state_dict["device_ieee"]) | ||
| state_dict["extra_state_attribute_names"] = sorted( |
There was a problem hiding this comment.
This changes the on-disk shape of zha_lib_entities — the info_object / state nesting is gone and extra_state_attributes is replaced by extra_state_attribute_names — but DIAGNOSTICS_JSON_VERSION at line 107 is still 2. #657 bumped it 1 → 2 for exactly this kind of restructure; please bump to 3 here too.
| state_diff: dict[str, Any] | ||
|
|
||
|
|
||
| def compute_state_diff( |
There was a problem hiding this comment.
compute_state_diff, the state_diff field, and suppress_events are the three new mechanisms in this PR and none of them has direct test coverage — grepping tests/ for any of the three names returns nothing.
The correctness property the Core half depends on is that dataclasses.replace(cached_state, **state_diff) reproduces entity.state exactly, for a consumer that only ever sees diffs. Three cheap tests would pin that down: a diff payload's contents after a single attribute change; that a newly-added entity emits no baseline event (the suppress_events() path in device.py); and a round-trip asserting the replayed state equals entity.state.
|
|
||
| # New entities have no listener yet (consumers capture their initial state when | ||
| # the add event registers them), so silence their changes | ||
| with suppress_events(): |
There was a problem hiding this comment.
This is right as far as ZHA is concerned — no await inside the block, so the ContextVar can't leak into concurrently-scheduled tasks, and the baseline is still recorded because maybe_emit_state_changed_event assigns __previous_state outside the emit() call.
The assumption in the comment is the part worth a second look, though: "consumers capture their initial state when the add event registers them". In the Core branch the capture and the listener attach aren't atomic — ZHAEntity.__init__ does self._zha_state = entity.state, but the STATE_CHANGED subscription only happens later in async_added_to_hass. Since ZHA advances __previous_state on every emit regardless of listeners, anything emitted in that gap is lost permanently rather than merely late (under the old re-read-the-whole-state model a missed event was self-healing). The re-sync at the end of async_added_to_hass doesn't rescue it either — it sits below if (state := await self.async_get_last_state()) is None: return, so a freshly-added entity skips it. Hoisting that assignment above the early return closes the window.
| def native_value(self) -> date | datetime | str | int | float | None: | ||
| """Return the state of the entity.""" | ||
| assert self._attribute_name is not None | ||
| if self._attribute_name not in self._cluster.attributes_by_name: |
There was a problem hiding this comment.
Nit: this guard is unrelated to the state migration and isn't mentioned in the description — it turns what used to raise into a silent None. Worth a short comment saying which case it covers (a quirks v2 entity naming an attribute the cluster doesn't define, presumably), so it doesn't read as unexplained defensive code later.
| class ButtonState(BaseEntityState): | ||
| """State for button entities.""" | ||
|
|
||
| pass |
There was a problem hiding this comment.
Nit: the pass is redundant — the docstring already forms the body.
This PR replaces the
statedict with a strongly typed dataclass that absorbsinfo_object. We now also emit state change diffs inEntityStateChangedEvent's newstate_diffattribute.For stronger typing and to support the client/server architecture split in the future, we benefit from having more centralized state for entities. This will allow us to send batched state updates to the client instead of the client being notified of a general "update" and then reaching for the
statedict of the ZHA entity.The HA Core half is here: home-assistant/core@dev...puddly:core:puddly/zha-state-migration. It uses the
state_diffvalues and also adds dynamic capability recomputation.Note: I intentionally regenerated diagnostics and reverted the regeneration commit so that this diff is minimal. Otherwise, it'll be a hundred of thousand lines of code. This will be un-reverted before merge.
Part of OpenHomeFoundation/roadmap#93 and a continuation of #663.