diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7303a35e..3f6cd80a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -25,6 +25,18 @@ Changed is byte-for-byte unchanged (verified by golden capture and the existing CLI tests); energy output now renders a summary table plus a breakdown table entirely in Rich rather than printing plain text and a Rich table. +- **Event system relationship clarified and de-duplicated** (`#102 + `_): ``events.py`` and + ``mqtt_events.py`` are not two competing event mechanisms. + ``EventEmitter`` (``events.py``) is the sole delivery mechanism, while + ``mqtt_events.py`` provides the event-name registry + (``MqttClientEvents``) and the typed dataclass payloads it carries. + Internal ``emit`` call sites in ``mqtt/state_tracker.py``, + ``mqtt/client.py`` and ``mqtt/subscriptions.py`` now reference the + ``MqttClientEvents`` constants instead of duplicating the raw event-name + strings, so each event name is defined in exactly one place. Module + docstrings and the event-system reference docs were updated to document + the relationship, and typed-payload delivery is now covered by tests. - **awscrt types wrapped out of public MQTT signatures** (`#101 `_): the library no longer exposes ``awscrt`` SDK types on its own public/semi-public API diff --git a/docs/reference/python_api/events.rst b/docs/reference/python_api/events.rst index ca2cabe0..9cc0b3c4 100644 --- a/docs/reference/python_api/events.rst +++ b/docs/reference/python_api/events.rst @@ -17,6 +17,23 @@ Use the event system when you want to react to connection changes, status transitions, or derived state changes such as temperature deltas and error conditions. +How the pieces fit together +--------------------------- + +The event system is split across two complementary modules: + +* :mod:`nwp500.events` provides the delivery **mechanism** — the generic + :class:`~nwp500.events.EventEmitter` (multiple listeners, async handlers, + one-time listeners, priority ordering). ``NavienMqttClient`` extends it. +* :mod:`nwp500.mqtt_events` provides the **vocabulary** — the + :class:`~nwp500.mqtt_events.MqttClientEvents` name registry and the typed, + frozen dataclass payloads carried by each event. + +These are not two competing systems. Internal ``emit`` call sites reference the +``MqttClientEvents`` constants and emit the matching payload dataclass, and you +subscribe with the same constants, so an event name is defined in exactly one +place. + Two Subscription Patterns ========================= diff --git a/src/nwp500/events.py b/src/nwp500/events.py index b4d0ed43..6184f212 100644 --- a/src/nwp500/events.py +++ b/src/nwp500/events.py @@ -1,9 +1,26 @@ """ Event Emitter for Navien device state changes. -This module provides an event-driven architecture for handling device state -changes, allowing multiple listeners per event and automatic state change -detection. +This module provides the event-driven *mechanism* used throughout the library: +a generic, string-keyed :class:`EventEmitter` supporting multiple listeners, +async handlers, one-time listeners, and priority ordering. + +Relationship to :mod:`nwp500.mqtt_events` +----------------------------------------- +:class:`EventEmitter` is only the delivery mechanism. It does not define which +events exist or what payloads they carry. Those are declared in +:mod:`nwp500.mqtt_events`, which provides: + +- the canonical event-name registry + (:class:`~nwp500.mqtt_events.MqttClientEvents` string constants), and +- the typed, frozen dataclass payloads + (e.g. :class:`~nwp500.mqtt_events.StatusReceivedEvent`) passed as the + event argument. + +The two modules are complementary, not competing: internal ``emit`` call +sites reference the ``MqttClientEvents`` constants and emit the matching +typed payload, while consumers subscribe with the same constants via +:meth:`EventEmitter.on`. """ import asyncio diff --git a/src/nwp500/mqtt/client.py b/src/nwp500/mqtt/client.py index 9c47f5f0..2335ffeb 100644 --- a/src/nwp500/mqtt/client.py +++ b/src/nwp500/mqtt/client.py @@ -31,6 +31,7 @@ from ..mqtt_events import ( ConnectionInterruptedEvent, ConnectionResumedEvent, + MqttClientEvents, ) from ..unit_system import UnitSystemType from .command_queue import MqttCommandQueue @@ -308,7 +309,7 @@ def _on_connection_interrupted_internal( # Emit event self._schedule_coroutine( self.emit( - "connection_interrupted", + MqttClientEvents.CONNECTION_INTERRUPTED, ConnectionInterruptedEvent(error=error), ) ) @@ -372,7 +373,7 @@ def _on_connection_resumed_internal( # Emit event self._schedule_coroutine( self.emit( - "connection_resumed", + MqttClientEvents.CONNECTION_RESUMED, ConnectionResumedEvent( return_code=return_code, session_present=session_present, diff --git a/src/nwp500/mqtt/state_tracker.py b/src/nwp500/mqtt/state_tracker.py index f0446d3f..0ed98d89 100644 --- a/src/nwp500/mqtt/state_tracker.py +++ b/src/nwp500/mqtt/state_tracker.py @@ -15,6 +15,7 @@ HeatingStartedEvent, HeatingStoppedEvent, ModeChangedEvent, + MqttClientEvents, PowerChangedEvent, TemperatureChangedEvent, ) @@ -67,7 +68,7 @@ async def process(self, device_mac: str, status: DeviceStatus) -> None: # Temperature change (compare raw values) if status.dhw_temperature_raw != prev.dhw_temperature_raw: await self._event_emitter.emit( - "temperature_changed", + MqttClientEvents.TEMPERATURE_CHANGED, TemperatureChangedEvent( device_mac=device_mac, old_temperature=prev.dhw_temperature, @@ -86,7 +87,7 @@ async def process(self, device_mac: str, status: DeviceStatus) -> None: # Operation mode change (compare raw values) if status.operation_mode != prev.operation_mode: await self._event_emitter.emit( - "mode_changed", + MqttClientEvents.MODE_CHANGED, ModeChangedEvent( device_mac=device_mac, old_mode=prev.operation_mode, @@ -102,7 +103,7 @@ async def process(self, device_mac: str, status: DeviceStatus) -> None: # Power consumption change (compare raw values) if status.current_inst_power != prev.current_inst_power: await self._event_emitter.emit( - "power_changed", + MqttClientEvents.POWER_CHANGED, PowerChangedEvent( device_mac=device_mac, old_power=prev.current_inst_power, @@ -121,14 +122,14 @@ async def process(self, device_mac: str, status: DeviceStatus) -> None: if curr_heating and not prev_heating: await self._event_emitter.emit( - "heating_started", + MqttClientEvents.HEATING_STARTED, HeatingStartedEvent(device_mac=device_mac, status=status), ) _logger.debug("Heating started") if not curr_heating and prev_heating: await self._event_emitter.emit( - "heating_stopped", + MqttClientEvents.HEATING_STOPPED, HeatingStoppedEvent(device_mac=device_mac, status=status), ) _logger.debug("Heating stopped") @@ -138,7 +139,7 @@ async def process(self, device_mac: str, status: DeviceStatus) -> None: # consumers never keep displaying a stale error. if status.error_code and status.error_code != prev.error_code: await self._event_emitter.emit( - "error_detected", + MqttClientEvents.ERROR_DETECTED, ErrorDetectedEvent( device_mac=device_mac, error_code=status.error_code, @@ -149,7 +150,7 @@ async def process(self, device_mac: str, status: DeviceStatus) -> None: if not status.error_code and prev.error_code: await self._event_emitter.emit( - "error_cleared", + MqttClientEvents.ERROR_CLEARED, ErrorClearedEvent( device_mac=device_mac, error_code=prev.error_code ), diff --git a/src/nwp500/mqtt/subscriptions.py b/src/nwp500/mqtt/subscriptions.py index a6578572..0d97e3de 100644 --- a/src/nwp500/mqtt/subscriptions.py +++ b/src/nwp500/mqtt/subscriptions.py @@ -31,7 +31,11 @@ TOUReservationSchedule, WeeklyReservationSchedule, ) -from ..mqtt_events import FeatureReceivedEvent, StatusReceivedEvent +from ..mqtt_events import ( + FeatureReceivedEvent, + MqttClientEvents, + StatusReceivedEvent, +) from ..topic_builder import MqttTopicBuilder from .state_tracker import DeviceStateTracker from .types import QoS, to_awscrt_qos @@ -502,7 +506,7 @@ async def subscribe_device_status( def post_parse(status: DeviceStatus) -> None: self._schedule_coroutine( self._event_emitter.emit( - "status_received", + MqttClientEvents.STATUS_RECEIVED, StatusReceivedEvent(device_mac=device_mac, status=status), ) ) @@ -583,7 +587,7 @@ def post_parse(feature: DeviceFeature) -> None: ) self._schedule_coroutine( self._event_emitter.emit( - "feature_received", + MqttClientEvents.FEATURE_RECEIVED, FeatureReceivedEvent( device_mac=device_mac, feature=feature ), diff --git a/src/nwp500/mqtt_events.py b/src/nwp500/mqtt_events.py index 53c3e735..3ab6ac00 100644 --- a/src/nwp500/mqtt_events.py +++ b/src/nwp500/mqtt_events.py @@ -1,13 +1,25 @@ """Typed event definitions for NavienMqttClient. -This module provides a centralized registry of all events emitted by the -NavienMqttClient, with full type information and documentation. This enables: +This module is the companion to :mod:`nwp500.events`. It does **not** implement +a second event system; delivery is handled solely by the +:class:`~nwp500.events.EventEmitter` mechanism. What this module provides is the +*vocabulary* for that mechanism: + +- a centralized registry of event names + (:class:`MqttClientEvents` string constants), and +- the typed, frozen dataclass payloads carried by each event. + +Together with the emitter, this enables: - IDE autocomplete for event names - Type-safe event handlers - Clear contracts for event data - Programmatic event discovery +Internal ``emit`` call sites use the :class:`MqttClientEvents` constants (rather +than raw strings) and emit the matching payload dataclass, so the event name is +defined in exactly one place. + Example:: from nwp500.mqtt_events import MqttClientEvents diff --git a/tests/test_mqtt_events.py b/tests/test_mqtt_events.py new file mode 100644 index 00000000..899a40a3 --- /dev/null +++ b/tests/test_mqtt_events.py @@ -0,0 +1,120 @@ +"""Tests for typed MQTT event payloads and the event-name registry. + +These tests lock in the contract between :mod:`nwp500.events` (the +:class:`EventEmitter` delivery mechanism) and :mod:`nwp500.mqtt_events` (the +event-name registry and typed dataclass payloads): subscribing with a +``MqttClientEvents`` constant delivers the matching typed payload. +""" + +import dataclasses + +import pytest + +from nwp500.events import EventEmitter +from nwp500.mqtt_events import ( + ConnectionInterruptedEvent, + ConnectionResumedEvent, + ErrorClearedEvent, + ModeChangedEvent, + MqttClientEvents, + PowerChangedEvent, + TemperatureChangedEvent, +) + + +def test_registry_lists_all_event_names(): + """get_all_events returns every declared event constant name.""" + events = MqttClientEvents.get_all_events() + + expected = { + "CONNECTION_INTERRUPTED", + "CONNECTION_RESUMED", + "STATUS_RECEIVED", + "TEMPERATURE_CHANGED", + "MODE_CHANGED", + "POWER_CHANGED", + "HEATING_STARTED", + "HEATING_STOPPED", + "ERROR_DETECTED", + "ERROR_CLEARED", + "FEATURE_RECEIVED", + } + assert set(events) == expected + + +def test_get_event_value_returns_string_constant(): + """get_event_value resolves a constant name to its wire string.""" + assert ( + MqttClientEvents.get_event_value("TEMPERATURE_CHANGED") + == "temperature_changed" + ) + assert MqttClientEvents.TEMPERATURE_CHANGED == "temperature_changed" + + +def test_get_event_value_unknown_raises(): + """Unknown event names raise AttributeError.""" + with pytest.raises(AttributeError): + MqttClientEvents.get_event_value("DOES_NOT_EXIST") + + +@pytest.mark.parametrize( + "event_name, payload", + [ + ( + MqttClientEvents.CONNECTION_INTERRUPTED, + ConnectionInterruptedEvent(error=RuntimeError("boom")), + ), + ( + MqttClientEvents.CONNECTION_RESUMED, + ConnectionResumedEvent(return_code=0, session_present=True), + ), + ( + MqttClientEvents.TEMPERATURE_CHANGED, + TemperatureChangedEvent( + device_mac="aabbccddeeff", + old_temperature=48.0, + new_temperature=50.0, + ), + ), + ( + MqttClientEvents.MODE_CHANGED, + ModeChangedEvent(device_mac="aabbccddeeff", old_mode=1, new_mode=2), + ), + ( + MqttClientEvents.POWER_CHANGED, + PowerChangedEvent( + device_mac="aabbccddeeff", old_power=0, new_power=1200 + ), + ), + ( + MqttClientEvents.ERROR_CLEARED, + ErrorClearedEvent(device_mac="aabbccddeeff", error_code=799), + ), + ], +) +@pytest.mark.asyncio +async def test_typed_payload_delivered_via_registry_constant( + event_name, payload +): + """Subscribing with a registry constant delivers the typed payload.""" + emitter = EventEmitter() + received: list[object] = [] + + emitter.on(event_name, received.append) + + delivered = await emitter.emit(event_name, payload) + + assert delivered == 1 + assert received == [payload] + assert received[0] is payload + + +def test_event_payloads_are_frozen(): + """Typed event payloads are immutable (frozen dataclasses).""" + event = TemperatureChangedEvent( + device_mac="aabbccddeeff", + old_temperature=48.0, + new_temperature=50.0, + ) + with pytest.raises(dataclasses.FrozenInstanceError): + event.new_temperature = 60.0 # type: ignore[misc]