Clarify and de-duplicate the event system (#102)

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
+  <https://github.com/eman/nwp500-python/issues/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
   <https://github.com/eman/nwp500-python/issues/101>`_): the library no
   longer exposes ``awscrt`` SDK types on its own public/semi-public API

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
 =========================
 

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

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,

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
                     ),

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
                     ),

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

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]