Skip to content

Comments

Experiment (DO NOT MERGE) - Collapse ClusterHandler objects into PlatformEntity objects#653

Closed
dmulcahey wants to merge 20 commits intodevfrom
dm/remove-cluster-handlers-2
Closed

Experiment (DO NOT MERGE) - Collapse ClusterHandler objects into PlatformEntity objects#653
dmulcahey wants to merge 20 commits intodevfrom
dm/remove-cluster-handlers-2

Conversation

@dmulcahey
Copy link
Contributor

High-Level Summary
This is a major architectural migration from cluster-handler-centric runtime to a direct cluster-native runtime.
It is a systemic refactor that moves ownership of bind/report/init behavior and event wiring away from zha/zigbee/cluster_handlers/* into endpoint/device/entity flows.


What Changed (Core Architecture)

  1. Cluster handlers removed as a runtime abstraction
  • zha/zigbee/cluster_handlers/ is deleted.
  • Cluster policies/constants moved to new locations:
    • zha/zigbee/cluster_policies.py
    • zha/zigbee/const.py
  • Event constant naming cleaned from ZHA_CLUSTER_HANDLER_* to ZHA_CLUSTER_* in zha/application/const.py (compat string payloads preserved where needed).
  1. Entity model switched from handlers to clusters
  • ClusterHandlerMatch replaced by ClusterMatch in zha/application/platforms/__init__.py.
  • PlatformEntity now takes clusters and builds maps directly from in/out cluster collections.
  • Entity metadata now exposes clusters (not cluster_handlers) through diagnostics info objects.
  • Entities now declare/report lifecycle requirements directly:
    • REPORT_CONFIG
    • ZCL_INIT_ATTRS
  • New merge logic added to combine per-entity requirements with cluster-level constraints, including quirk-direct precedence handling.
  1. Discovery rewritten to cluster-native matching
  • zha/application/discovery.py now matches entity classes via endpoint cluster names/IDs, not handler registries.
  • Endpoint-level cluster claiming and lifecycle setup introduced (claim_clusters(...) path).
  • Quirks v2 metadata now feeds entity/endpoint report-init requirements directly.
  1. Endpoint lifecycle is now the operational center
  • zha/zigbee/endpoint.py gained cluster-native bind/configure/init orchestration:
    • cluster claiming
    • bind decisions
    • reporting setup
    • attribute init reads
  • Several side effects previously implemented in handlers now happen in endpoint/cluster listeners:
    • on/off client command state effects
    • OTA version state tracking
    • IAS Zone updates/enroll behavior
    • Identify trigger effects
    • DoorLock operation notification translation
  • Compatibility payload fields were retained for some events to avoid breaking consumers during transition.
  1. Device orchestration changed materially
  • zha/zigbee/device.py now aggregates pending entity requirements and applies them before configure/init.
  • Pending entity queue semantics changed (ordered map + key strategy for same-entity-id class variants).
  • ZDO handler abstraction removed and replaced with direct endpoint-0/ZDO status handling.
  • Diagnostics schema bumped to version 2 and field shape changed from handlers to clusters.
  1. Gateway/group robustness hardening added
  • zha/application/gateway.py introduces initialization generation tokens to prevent stale async completions from mutating current state.
  • Startup polling task lifecycle and exception handling improved.
  • Group reconcile operations now debounced.
  • zha/zigbee/group.py adds lock-guarded reconciliation and more defensive subscription/entity cleanup.
  1. Helper utilities relocated
  • Exception wrapping/retry and safe command/write helpers moved into zha/application/helpers.py.
  • Bindable policy checks now use cluster policy module, not handler registries.

Platform Layer Migration Pattern
Across platform implementations (examples: zha/application/platforms/fan/__init__.py, zha/application/platforms/climate/__init__.py, zha/application/platforms/binary_sensor/__init__.py, zha/application/platforms/alarm_control_panel/__init__.py, zha/application/platforms/lock/__init__.py, zha/application/platforms/siren.py):

  • Constructor and internal state switched from cluster_handlers to clusters.
  • Match declarations switched from _cluster_handler_match to _cluster_match.
  • Event subscriptions now use direct cluster event types (AttributeReadEvent, AttributeReportedEvent, AttributeUpdatedEvent, AttributeWrittenEvent).
  • Entity command/read/write code paths now call cluster methods/safe wrappers directly.
  • Many entities gained explicit REPORT_CONFIG / ZCL_INIT_ATTRS.

This is mostly mechanical in simpler platforms, but in alarm/siren/lock it includes behavioral logic migration that is review-critical.


Tests and Fixture Churn

  • tests/test_cluster_handlers.py removed.
  • tests/test_cluster_lifecycle.py added for new endpoint/entity requirement aggregation and compatibility semantics.
  • Large updates in tests/test_gateway.py and tests/test_device.py for async race hardening, pending entity handling, and cleanup behavior.
  • Massive fixture migration in tests/data/devices/:
    • diagnostics schema version moved to 2
    • cluster_handlers fields replaced by clusters
    • 739 fixture JSONs touched

This explains the very large deletion count: fixture reshaping dominates diff volume.


Commit Intent by Sequence (Review Narrative)

  • Early commits establish migration strategy/docs/scaffolding and naming cleanup.
  • Mid commits perform systemic rename: handlers -> clusters, plus ZDO handling shift.
  • Then platform-by-platform refactors migrate entity implementations.
  • Final commits harden edge cases: async init races, queue behavior, listener cleanup, and add targeted tests.

Review Focus Areas (Most Risk)

  1. zha/zigbee/endpoint.py
    Risk: lifecycle ordering and parity with old handler side effects.
  2. zha/zigbee/device.py
    Risk: pending requirement merge semantics, ZDO transition, reconfigure flows.
  3. zha/application/discovery.py
    Risk: entity matching/claiming differences and quirks v2 metadata application.
  4. zha/application/gateway.py + zha/zigbee/group.py
    Risk: init/reconcile timing behavior and delayed convergence from debouncing.
  5. Non-mechanical platform migrations (alarm_control_panel, siren, lock)
    Risk: command semantics/state translation regressions.

Suggested Review Plan

  1. Review the architectural pivot files first: zha/zigbee/endpoint.py, zha/zigbee/device.py, zha/application/discovery.py.
  2. Review async hardening behavior: zha/application/gateway.py, zha/zigbee/group.py.
  3. Review platform migrations by risk tier: alarm_control_panel, siren, lock, then broad mechanical platform files.
  4. Review new lifecycle tests and fixture schema migration to confirm expected observability output shape.

@codecov
Copy link

codecov bot commented Feb 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.74%. Comparing base (558bc6f) to head (92cbce9).
⚠️ Report is 5 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #653      +/-   ##
==========================================
- Coverage   97.41%   96.74%   -0.67%     
==========================================
  Files          62       49      -13     
  Lines       10724    10553     -171     
==========================================
- Hits        10447    10210     -237     
- Misses        277      343      +66     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ZHA_CLUSTER_HANDLER_CFG_DONE,
ClusterHandlerConfigurationComplete(
ZHA_CLUSTER_CFG_DONE,
ClusterConfigurationComplete(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hrm may break HA

Comment on lines +301 to +302
super().__init__(clusters, endpoint, device, **kwargs)
self._cluster = clusters[0]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

order flipped

Comment on lines 70 to +73
if ENTITY_METADATA in kwargs:
self._init_from_quirks_metadata(kwargs[ENTITY_METADATA])
super().__init__(cluster_handlers, endpoint, device, **kwargs)
super().__init__(clusters, endpoint, device, **kwargs)
self._cluster = clusters[0]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

different sequencing

Comment on lines +92 to +93
super().__init__(clusters, endpoint, device, **kwargs)
self._cluster = clusters[0]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

order different

# this is a cached read to get the value for state mgt so there is no double read
attr_value = await self._cluster_handler.get_attribute_value(attribute)
attribute = self._attribute_name
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hrm

@@ -342,18 +423,38 @@ def parse(value: bool | int) -> bool:
async def async_update(self) -> None:
"""Attempt to retrieve on off state from the IAS Zone sensor."""
await PlatformEntity.async_update(self)
attribute = self._attribute_name
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder why it kept creating vars for this...



def discover_entities_for_endpoint(endpoint: Endpoint) -> Iterator[PlatformEntity]:
def discover_entities_for_endpoint(endpoint: Endpoint) -> Iterator[PlatformEntity]: # noqa: C901
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hrm

@@ -173,13 +199,13 @@ class DeviceFirmwareInfoUpdatedEvent:


@dataclass(kw_only=True, frozen=True)
class ClusterHandlerConfigurationComplete:
"""Event generated when all cluster handlers are configured."""
class ClusterConfigurationComplete:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder if this changes event shapes for HA

def cluster_handlers_by_name(self) -> dict[str, ClusterHandler]:
"""Return cluster handlers indexed by name."""
return {ch.name: ch for ch in self._all_cluster_handlers.values()}
@property
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hrm

def client_cluster_handlers_by_name(self) -> dict[str, ClientClusterHandler]:
"""Return client cluster handlers indexed by name."""
return {ch.name: ch for ch in self._client_cluster_handlers.values()}
@property
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hrm


return f"0x{command_id:02X}"

def handle_cluster_command(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh this is kinda ugly... wonder why these were all grouped together...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we may need to implement event entities to avoid remote logic from having to be handled like this... 🤔


async def _execute_handler_tasks(
self, func_name: str, *args: Any, max_concurrency: int | None = None
def _handle_smartthings_attribute_event(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hrm

@dmulcahey dmulcahey closed this Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant