Type events by name (discriminated Event subtypes)#2127
Merged
Conversation
The FailureEvent subtype existed but no event names mapped to it, leaving it dead code. Derive failure-event mappings from the EventName enum by suffix so all *FailedEvent payloads gain failure_type/failure_type_code, with new failure events covered automatically.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the flat Event model into a discriminated union of typed subtypes, keyed on the name field. This enables consumers to use isinstance checks or pattern matching instead of defensive None-guards when accessing event-specific fields. Unknown/unmodeled event names gracefully fall back to the base Event class for forward compatibility.
Changes:
- Splits the monolithic
Eventclass into a base class (with only universal fields) plus concrete subtypes (DeviceStateChangedEvent,ExecutionStateChangedEvent,FailureEvent, device lifecycle events, zone events), each with precisely-typed required fields. - Adds a cattrs discriminator hook in
converter.pythat inspects the"name"field to dispatch structuring to the appropriate subtype, with pre-built hooks to avoid infinite recursion. - Comprehensive test coverage for subtype construction, converter dispatch, fallback behavior, and integration with both cloud and local API event fixtures.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
pyoverkiz/models.py |
Splits Event into base + typed subtypes; adds EVENT_TYPE_BY_NAME discriminator map with auto-registration of *FailedEvent names. |
pyoverkiz/converter.py |
Registers a discriminated-union structure hook for Event that resolves subtypes by name and delegates to pre-built rename-aware hooks. |
tests/test_models.py |
Adds extensive tests: subtype construction, converter dispatch per event family, base fallback, and local fixture integration. |
tests/test_client.py |
Guards device_states access behind isinstance(event, DeviceStateChangedEvent) since the field no longer exists on the base class. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Model all Gateway* events via a single GatewayEvent carrying gateway_id, enrich FailureEvent with gateway_id/device_url/protocol_type so failure payloads keep their scope, and drop the never-sent failure_type_code from failures (it stays on ExecutionStateChangedEvent, where the API sends it). Make identity fields the API reliably sends required (device_url, gateway_id, zone_oid, exec_id/new_state/old_state) so consumers get non-None guarantees after isinstance narrowing. To keep strictness from making batches fragile, the discriminator now degrades a single malformed event to the base Event with a logged warning instead of failing the whole fetch_events() list. Also: tolerate null deviceStates, share _DeviceMetadataEvent/_ZoneEvent bases, and document the event changes in the v2 migration guide.
Gateway events now mirror device events: a private _GatewayEvent base plus one public class per name (GatewayDownEvent, GatewayAliveEvent, ...), with the three documented extra-field events carrying their payload (timeout, firmware_type, function_type/enabled). This drops the GatewayEvent grab-bag that collapsed 22 distinct events a consumer would want to tell apart into one type. Failures stay as the single shared FailureEvent by design — consumers branch on "did it fail, and why" (failure_type), not on which operation failed; the docstring and migration guide now state this explicitly. The event model is three rules: each modeled event is its own class, all *FailedEvent -> FailureEvent, anything unmodeled -> base Event.
Promote the per-category event bases to public (DeviceEvent, GatewayEvent, ZoneEvent) and add ExecutionEvent, each carrying that category's identity field (device_url / gateway_id / zone_oid / exec_id). Consumers can now narrow broadly (any gateway event) or to a leaf (GatewayDownEvent) for full payload. The Created/Updated DRY helpers stay private (_DeviceMetadataEvent, _ZoneMutationEvent) — they are not a category anyone narrows to. Fix a latent camelize bug surfaced by the now-required zone_oid: the API sends zoneOID / deviceURLs / placeOIDs, but camelize produced zoneOid / deviceUrls / placeOids, so zone events silently lost those fields (and now degraded to base Event). Added the missing acronym overrides.
Lift per-item resilience to the list[Event] level so a single unstructurable event (missing name, non-dict) is dropped and logged instead of failing the whole fetch. Remove the speculative deviceStates null coercion, which never occurs in real data and is now redundant with subtype-degradation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Turns the flat
Eventmodel into a discriminated union keyed on thenamefield. A slim baseEventcarries only the fields common to every event; concrete events structure into typed subtypes with their documented, precisely-typed fields. Unknown / unmodeled event names fall back to the baseEvent, so forward-compatibility is preserved.This replaces the previous single ~24-field
Eventwhere every field was optional, which forced consumers (notably Home Assistant) to write defensiveif not event.device_url/if event.exec_idguards even when the API guarantees those fields for a given event type.Warning
Breaking change (intended for the v2 release). Fields like
device_url,device_states,new_state,exec_id, etc. are no longer on the baseEvent— they live on the subtypes. Consumers must narrow withisinstance/matchbefore accessing them.client.fetch_events()still returnslist[Event].What's added
Event:name,timestamp,setup_oid,owning_partners(the last is newly surfaced; it appears on ~95 documented event types and was previously dropped).DeviceStateChangedEvent—device_url,device_states(non-optional)ExecutionRegisteredEvent,ExecutionStateChangedEventFailureEvent— base for the*FailedEventnames; carriesfailure_type+failure_type_codetogetherDeviceAvailableEvent,DeviceUnavailableEvent,DeviceDisabledEvent,DeviceCreatedEvent,DeviceUpdatedEvent,DeviceRemovedEvent(created/updated/removed addcontrollable_name, previously dropped)ZoneCreatedEvent,ZoneUpdatedEvent,ZoneDeletedEvent(zone_oid,device_urls,place_oids— previously dropped)EVENT_TYPE_BY_NAMEinmodels.py+ a cattrs structure hook onEventinconverter.py. The hook resolves the subtype fromnameand delegates to each class's camelCase-rename-aware hook. Subtype hooks are pre-built and called directly to avoid infinite recursion (cattrs would otherwise re-dispatch a subclass back through the baseEventhook). All*FailedEventnames are mapped toFailureEvent, derived from theEventNameenum by suffix so new failure events are covered automatically.Testing
TestEventcoverage: direct subtype construction, converter dispatch per family, base fallback for known-unmodeled names, fallback for genuinely-unknown names, and*FailedEvent→FailureEvent.tests/fixtures/event/local_events.jsonfixture end-to-end (local-API typed values incl. a nested object value).ruff check,ruff format,mypy, andtyall clean.Follow-up (separate PR)
The Home Assistant
overkizcoordinator can drop itsEventNamehandler registry and the redundantNone-guards in favour of amatch event:block over the subtypes — cleaner and statically checked. Not included here; pyoverkiz only exposes the subtypes + discriminator.