Skip to content

Base event platform - #864

Merged
puddly merged 2 commits into
zigpy:devfrom
puddly:puddly/base-event-platform
Aug 20, 2026
Merged

Base event platform#864
puddly merged 2 commits into
zigpy:devfrom
puddly:puddly/base-event-platform

Conversation

@puddly

@puddly puddly commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Replaces #419 and implements the base platforms from #594, without adding it to any devices via discovery. This is a strictly opt-in platform for now and will be consumed initially by Green Power devices.


Part of OpenHomeFoundation/roadmap#195

@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.17%. Comparing base (438760c) to head (48fe745).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #864      +/-   ##
==========================================
+ Coverage   97.15%   97.17%   +0.01%     
==========================================
  Files          55       57       +2     
  Lines       10481    10538      +57     
==========================================
+ Hits        10183    10240      +57     
  Misses        298      298              

☔ View full report in Codecov by Harness.
📢 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.

@puddly
puddly marked this pull request as ready for review August 14, 2026 14:37
Copilot AI lite review requested due to automatic review settings August 14, 2026 14:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Introduces a new, opt-in ZHA event platform foundation intended to support event-style entities (no state, runtime-triggered events) for future consumers like Green Power devices.

Changes:

  • Added core event-platform entity types (BaseEvent), event/state dataclasses, and event emission plumbing.
  • Added shared event constants and standard device classes/event types for doorbells and buttons.
  • Wired the event platform module into platform discovery imports (without enabling device discovery) and added unit tests for event triggering/state behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
zha/application/platforms/event/const.py Defines event platform constants, device classes, and standard event-type enums.
zha/application/platforms/event/init.py Implements base event entity, emitted event payload types, and capability-only state.
zha/application/discovery.py Imports the new event platform module into the platform discovery import set (still not in PLATFORMS).
tests/test_platform_event.py Adds tests validating event state contents, event emission behavior, and doorbell ring requirements.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread zha/application/platforms/event/__init__.py
Comment thread zha/application/platforms/event/__init__.py
@puddly puddly mentioned this pull request Aug 14, 2026
@puddly
puddly merged commit 3da3ed9 into zigpy:dev Aug 20, 2026
10 checks passed
@puddly
puddly deleted the puddly/base-event-platform branch August 20, 2026 18:46

@zigpy-review-bot zigpy-review-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed post-merge at 48fe745 (review requested for the record), in a fresh worktree at that head.

Nothing blocking. The platform is well-scoped and the plumbing follows the existing prior art closely: EntityEventTriggeredEvent mirrors EntityStateChangedEvent field-for-field, and EventState(**super().state.__dict__, event_types=...) is the same composition idiom siren/switch/select already use. Keeping event delivery off the state channel (state carries capabilities only) is the right call, and test_trigger_event_does_not_change_state pins it.

Verified locally rather than taken on trust:

  • The 5 new tests pass; full CI is green on all three Python versions.
  • mypy zha/ inside the worktree venv (i.e. with zigpy/zhaquirks actually resolvable, not the dependency-free pre-commit env) reports 0 errors across 59 source files — no new type debt.
  • The ValueError raised in BaseEvent.__init__ is safe under discovery: entity construction is already wrapped in a try/except Exception at zha/application/discovery.py:351-358, so a misdeclared doorbell subclass would be logged and skipped rather than breaking device init.
  • A second-opinion pass with an independent model over the same diff returned no findings.

Three optional follow-ups below, all forward-looking for the Green Power consumer rather than defects in this PR. Details are in the inline comments.

  1. PLATFORMS in zha/application/discovery.py has no consumer anywhere — Home Assistant keeps its own copy, which does not yet list Platform.EVENT. So the second commit is inert today and the real enablement is still a core-side change.
  2. _attr_event_types has no class-level default and isn't validated at construction, so a subclass that forgets it fails late, outside discovery's guard.
  3. "event_triggered" is an inline literal where the equivalent STATE_CHANGED lives in zha/const.py.

I did not re-raise the two inline threads already resolved with the author (event_types returning the raw list, and event_attributes or {}).

Platform.CLIMATE,
Platform.COVER,
Platform.DEVICE_TRACKER,
Platform.EVENT,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Optional, and more a heads-up than a request: this line has no effect today.

grep -rn '\bPLATFORMS\b' --include='*.py' across this repo returns only the definition at zha/application/discovery.py:50 — nothing in the library reads the tuple. Home Assistant doesn't import it either: homeassistant/components/zha/__init__.py defines its own PLATFORMS (currently identical to this one minus Platform.EVENT) and passes that to async_forward_entry_setups / async_unload_platforms.

That's harmless here — nothing registers an event entity via register_entity yet, so the platform stays inert either way — but it does mean the enablement work is entirely core-side: adding Platform.EVENT to core's tuple plus a homeassistant/components/zha/event.py. Worth tracking alongside the Green Power work so it doesn't get lost, since this line makes it look already handled.

Longer term, the two lists have now genuinely diverged for the first time. Either dropping the library-side copy or having core import this one would stop them drifting silently.

PLATFORM = Platform.EVENT

_attr_device_class: EventDeviceClass | None = None
_attr_event_types: list[str]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Optional: _attr_event_types is annotation-only with no class-level default, and __init__ only reads it on the doorbell branch — which and short-circuits for every other device class.

I confirmed the resulting failure mode in a worktree: a subclass that sets _attr_device_class = EventDeviceClass.BUTTON but forgets _attr_event_types constructs successfully, and then raises AttributeError: '<cls>' object has no attribute '_attr_event_types' the first time .state or maybe_emit_state_changed_event() is reached.

The reason that's worth a guard: discovery wraps entity construction in try/except Exception (discovery.py:351-358, logging "Failed to create %s entity"), so a construction-time error is contained and the rest of the device still comes up. State emission runs outside that guard, so this particular mistake escapes it and surfaces well away from its cause.

Since __init__ already validates the doorbell/ring invariant, checking that _attr_event_types is set in the same place is cheap and moves the failure back inside the guarded path. A _attr_event_types: list[str] = [] default plus a non-empty check would do it.

"""Event for when an event entity fires."""

event_type: Final[str] = "entity"
event: Final[str] = "event_triggered"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit, purely for consistency: the parallel EntityStateChangedEvent.event uses the STATE_CHANGED constant from zha.const rather than an inline string, and zha/const.py already carries EVENT and EVENT_TYPE.

An EVENT_TRIGGERED: Final[str] = "event_triggered" next to STATE_CHANGED would keep the pattern uniform, and gives Home Assistant a named symbol to subscribe against. Not functionally important — EntityEventTriggeredEvent.event is importable and the tests already use it that way.

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.

3 participants