fix: device protocol correctness

CHANGELOG.rst

@@ -7,6 +7,48 @@ Unreleased
 
 Bug Fixes
 ---------
+- **Fix malformed weekly reservation and recirculation schedule
+  payloads**: ``update_weekly_reservation()`` and
+  ``configure_recirculation_schedule()`` dumped the schedule models
+  as-is, double-nesting the request (``request.reservation.reservation``
+  / ``request.schedule.schedule``) and leaking pydantic computed display
+  fields — including a unit-converted ``temperature`` alongside the raw
+  half-Celsius ``param`` — into device commands. Both now send the flat,
+  raw protocol shape used by ``update_reservations()``. A new
+  ``NavienBaseModel.to_protocol_dict()`` dumps only declared protocol
+  fields.
+- **Preserve command order when a queued flush fails**: a command that
+  failed mid-flush was re-queued at the tail, behind commands queued
+  after it, inverting order-sensitive sequences (e.g. ``set_temp``
+  replayed before ``power_on``). The queue is now a deque and failed
+  commands are re-inserted at the front.
+- **Expire stale queued commands**: queued commands stored a timestamp
+  that was never checked, so a multi-hour outage replayed hours-old
+  control commands (e.g. ``set_power``) to the appliance on reconnect.
+  Commands older than ``MqttConnectionConfig.max_queued_command_age``
+  (default 300 s, ``None`` to disable) are now discarded at send time.
+- **Fix Fahrenheit conversion for sub-zero temperatures** (ASYMMETRIC
+  formula): the rounding used Python's floored ``%``, which is always
+  non-negative, while the firmware/app uses a truncated remainder. Raw
+  ``-11`` (-5.5 °C) decoded to 22 °F instead of the app's 23 °F. Now
+  uses ``math.fmod`` semantics.
+- **Fix freeze protection default limits**: the defaults (43/65) were
+  Fahrenheit display values stored in raw half-Celsius fields, decoding
+  to 70.7 °F / 90.5 °F when the device omitted them. Corrected to raw
+  12/20 (43 °F / 50 °F), matching the documented fixed limits.
+- **Emit error_detected when the error code changes**: a transition
+  between two non-zero error codes (e.g. E799 → E407) emitted no event,
+  so consumers kept displaying the stale error.
+- **Fix TOU price encoding**: ``encode_price()`` used banker's rounding,
+  under-encoding exact half values at even boundaries (0.125 at
+  ``decimal_point=2`` encoded to 12 instead of 13) — now uses
+  ``Decimal`` half-up rounding. ``build_tou_period()`` also treated
+  ``bool`` prices as pre-encoded integers (``True`` sent as price 1).
+- **Fix protocol documentation errors**: ``decode_reservation_hex()``
+  documented the enable flag inverted (1=enabled instead of 2=enabled);
+  the ``build_reservation_entry()`` example showed ``week: 158`` for
+  Mon/Wed/Fri instead of the correct 84; temperature doctest examples
+  showed ``int`` raw values where ``float`` is returned.
 - **Run MQTT message dispatch on the event loop**: JSON parsing, pydantic
   model validation, and user callbacks all executed directly on the AWS
   CRT network thread. A slow or blocking callback (e.g. the CLI monitor's

src/nwp500/_base.py

@@ -39,6 +39,17 @@ def model_dump(self, **kwargs: Any) -> dict[str, Any]:
         converted: dict[str, Any] = self._convert_enums_to_names(result)
         return converted
 
+    def to_protocol_dict(self) -> dict[str, Any]:
+        """Dump only the declared protocol fields, by alias.
+
+        Excludes pydantic ``computed_field`` properties, which are
+        display-oriented (formatted times, weekday names, unit-converted
+        temperatures) and must never be sent to the device.
+        """
+        return self.model_dump(
+            by_alias=True, include=set(type(self).model_fields)
+        )
+
     @staticmethod
     def _convert_enums_to_names(
         data: Any, visited: set[int] | None = None

src/nwp500/encoding.py

@@ -10,6 +10,7 @@
 
 import logging
 from collections.abc import Iterable
+from decimal import ROUND_HALF_UP, Decimal
 from numbers import Real
 
 from .exceptions import ParameterValidationError, RangeValidationError
@@ -238,7 +239,7 @@ def decode_season_bitfield(bitfield: int) -> list[int]:
 # ============================================================================
 
 
-def encode_price(value: Real, decimal_point: int) -> int:
+def encode_price(value: Real | float | Decimal, decimal_point: int) -> int:
     """
     Encode a price into the integer representation expected by the device.
 
@@ -274,8 +275,25 @@ def encode_price(value: Real, decimal_point: int) -> int:
             min_value=0,
             max_value=10,
         )
-    scale = 10**decimal_point
-    return int(round(float(value) * scale))
+    # Use Decimal with HALF_UP rounding: round() applies banker's
+    # rounding, which under-encodes exact half values at even
+    # boundaries (e.g. 0.125 at decimal_point=2 -> 12 instead of 13).
+    # Build the Decimal from the original value where possible —
+    # round-tripping through float would lose precision for Decimal
+    # inputs and introduce binary floating-point artifacts.
+    decimal_value: Decimal
+    if isinstance(value, Decimal):
+        decimal_value = value
+    elif isinstance(value, bool | int):
+        decimal_value = Decimal(int(value))
+    elif isinstance(value, float):
+        decimal_value = Decimal(str(value))
+    else:
+        # Other Real implementations (e.g. Fraction) may not have a
+        # Decimal-parseable str(); go through float as a last resort.
+        decimal_value = Decimal(str(float(value)))
+    scaled = decimal_value * (Decimal(10) ** decimal_point)
+    return int(scaled.quantize(Decimal("1"), rounding=ROUND_HALF_UP))
 
 
 def decode_price(value: int, decimal_point: int) -> float:
@@ -324,7 +342,7 @@ def decode_reservation_hex(hex_string: str) -> list[dict[str, int]]:
     Decode a hex-encoded reservation string into structured reservation entries.
 
     The reservation data is encoded as 6 bytes per entry:
-    - Byte 0: enable (1=enabled, 2=disabled)
+    - Byte 0: enable (2=enabled, 1=disabled, device boolean convention)
     - Byte 1: week bitfield (days of week)
     - Byte 2: hour (0-23)
     - Byte 3: minute (0-59)
@@ -423,7 +441,7 @@ def build_reservation_entry(
         ... )
         {
             'enable': 2,
-            'week': 158,
+            'week': 84,
             'hour': 6,
             'min': 30,
             'mode': 3,
@@ -592,16 +610,26 @@ def build_tou_period(
     encoded_min: int
     encoded_max: int
 
+    # bool is an int subclass and would otherwise silently pass through
+    # the "already encoded" branch below as 1/0; treat it as a numeric
+    # price instead.
+    price_min_num: Real | float = (
+        float(price_min) if isinstance(price_min, bool) else price_min
+    )
+    price_max_num: Real | float = (
+        float(price_max) if isinstance(price_max, bool) else price_max
+    )
+
     # Encode prices if they're Real numbers (not already encoded integers)
-    if not isinstance(price_min, int):
-        encoded_min = encode_price(price_min, decimal_point)
+    if not isinstance(price_min_num, int):
+        encoded_min = encode_price(price_min_num, decimal_point)
     else:
-        encoded_min = price_min
+        encoded_min = price_min_num
 
-    if not isinstance(price_max, int):
-        encoded_max = encode_price(price_max, decimal_point)
+    if not isinstance(price_max_num, int):
+        encoded_max = encode_price(price_max_num, decimal_point)
     else:
-        encoded_max = price_max
+        encoded_max = price_max_num
 
     return {
         "season": season_bitfield,

src/nwp500/models/status.py

@@ -575,12 +575,16 @@ class DeviceStatus(NavienBaseModel):
     freeze_protection_temp_min_raw: int = temperature_field(
         "Active freeze protection lower limit",
         alias="freezeProtectionTempMin",
-        default=43,
+        # Raw half-degrees Celsius: 12 = 6 degC = 43 degF (documented
+        # fixed lower limit in data_conversions.rst)
+        default=12,
     )
     freeze_protection_temp_max_raw: int = temperature_field(
         "Active freeze protection upper limit",
         alias="freezeProtectionTempMax",
-        default=65,
+        # Raw half-degrees Celsius: 20 = 10 degC = 50 degF (documented
+        # fixed upper limit in data_conversions.rst)
+        default=20,
     )
 
     def _is_celsius(self) -> bool:

src/nwp500/mqtt/command_queue.py

@@ -7,8 +7,8 @@
 
 from __future__ import annotations
 
-import asyncio
 import logging
+from collections import deque
 from collections.abc import Callable
 from datetime import UTC, datetime
 from typing import TYPE_CHECKING, Any
@@ -35,9 +35,11 @@ class MqttCommandQueue:
     when the connection is restored. This ensures commands are not lost
     during temporary network interruptions.
 
-    The queue uses asyncio.Queue with a fixed maximum size. When the queue
-    is full, the oldest command is automatically dropped to make room for
-    new commands (FIFO with overflow dropping).
+    The queue uses a deque with a fixed maximum size. When the queue is
+    full, the oldest command is automatically dropped to make room for
+    new commands (FIFO with overflow dropping). Commands older than
+    ``config.max_queued_command_age`` seconds are discarded at send time
+    rather than replayed to the device.
     """
 
     def __init__(self, config: MqttConnectionConfig):
@@ -48,9 +50,10 @@ def __init__(self, config: MqttConnectionConfig):
             config: MQTT connection configuration with queue settings
         """
         self.config = config
-        # Python 3.10+ handles asyncio.Queue initialization without running loop
-        self._queue: asyncio.Queue[QueuedCommand] = asyncio.Queue(
-            maxsize=config.max_queued_commands
+        # A deque (not asyncio.Queue) so a command that fails mid-flush
+        # can be re-inserted at the FRONT, preserving command order.
+        self._queue: deque[QueuedCommand] = deque(
+            maxlen=config.max_queued_commands
         )
 
     def enqueue(
@@ -82,26 +85,17 @@ def enqueue(
             timestamp=datetime.now(UTC),
         )
 
-        # If queue is full, drop oldest command first
-        if self._queue.full():
-            try:
-                # Remove oldest command (non-blocking)
-                dropped = self._queue.get_nowait()
-                _logger.warning(
-                    f"Command queue full ({self.config.max_queued_commands}), "
-                    f"dropped oldest command to '{redact_topic(dropped.topic)}'"
-                )
-            except asyncio.QueueEmpty:
-                # Race condition - queue was emptied between check and get
-                pass
-
-        # Add new command (should never block since we just made room if needed)
-        try:
-            self._queue.put_nowait(command)
-            _logger.info(f"Queued command (queue size: {self._queue.qsize()})")
-        except asyncio.QueueFull:
-            _logger.error("Failed to enqueue command - queue unexpectedly full")
-            raise
+        # deque(maxlen=N) drops from the opposite end automatically, but
+        # log the overflow explicitly for visibility.
+        if len(self._queue) == self._queue.maxlen:
+            dropped = self._queue.popleft()
+            _logger.warning(
+                f"Command queue full ({self.config.max_queued_commands}), "
+                f"dropped oldest command to '{redact_topic(dropped.topic)}'"
+            )
+
+        self._queue.append(command)
+        _logger.info(f"Queued command (queue size: {len(self._queue)})")
 
     async def send_all(
         self,
@@ -121,22 +115,30 @@ async def send_all(
         Returns:
             Tuple of (sent_count, failed_count)
         """
-        if self._queue.empty():
+        if not self._queue:
             return (0, 0)
 
-        queue_size = self._queue.qsize()
+        queue_size = len(self._queue)
         _logger.info(f"Sending {queue_size} queued command(s)...")
 
         sent_count = 0
         failed_count = 0
+        max_age = self.config.max_queued_command_age
+        now = datetime.now(UTC)
 
-        while not self._queue.empty() and is_connected_func():
-            try:
-                # Get command from queue (non-blocking)
-                command = self._queue.get_nowait()
-            except asyncio.QueueEmpty:
-                # Queue was emptied by another task
-                break
+        while self._queue and is_connected_func():
+            command = self._queue.popleft()
+
+            # Don't replay stale control commands (e.g. an hours-old
+            # set_power) to a physical appliance after a long outage.
+            age = (now - command.timestamp).total_seconds()
+            if max_age is not None and age > max_age:
+                _logger.warning(
+                    f"Discarding expired queued command to "
+                    f"'{redact_topic(command.topic)}' "
+                    f"(age {age:.0f}s > max {max_age:.0f}s)"
+                )
+                continue
 
             try:
                 # Publish the queued command
@@ -156,15 +158,10 @@ async def send_all(
                     f"Failed to send queued command to "
                     f"'{redact_topic(command.topic)}': {e}"
                 )
-                # Re-queue if there's room
-                if not self._queue.full():
-                    try:
-                        self._queue.put_nowait(command)
-                        _logger.warning("Re-queued failed command")
-                    except asyncio.QueueFull:
-                        _logger.error(
-                            "Failed to re-queue command - queue is full"
-                        )
+                # Re-queue at the FRONT so command order is preserved on
+                # the next flush (re-appending would invert order-
+                # sensitive sequences like power_on -> set_temp).
+                self._queue.appendleft(command)
                 break  # Stop processing on error to avoid cascade failures
 
         if sent_count > 0:
@@ -182,14 +179,8 @@ def clear(self) -> int:
         Returns:
             Number of commands cleared
         """
-        # Drain the queue
-        cleared = 0
-        while not self._queue.empty():
-            try:
-                self._queue.get_nowait()
-                cleared += 1
-            except asyncio.QueueEmpty:
-                break
+        cleared = len(self._queue)
+        self._queue.clear()
 
         if cleared > 0:
             _logger.info(f"Cleared {cleared} queued command(s)")
@@ -198,14 +189,14 @@ def clear(self) -> int:
     @property
     def count(self) -> int:
         """Get the number of queued commands."""
-        return self._queue.qsize()
+        return len(self._queue)
 
     @property
     def is_empty(self) -> bool:
         """Check if the queue is empty."""
-        return self._queue.empty()
+        return not self._queue
 
     @property
     def is_full(self) -> bool:
         """Check if the queue is full."""
-        return self._queue.full()
+        return len(self._queue) == self._queue.maxlen

src/nwp500/mqtt/control.py

@@ -680,10 +680,18 @@ async def update_weekly_reservation(
         Returns:
             Publish packet ID
         """
+        # Match the documented request shape used by update_reservations():
+        # reservationUse at the request level and reservation as a flat
+        # list of raw protocol entries. Dumping the schedule model as-is
+        # would double-nest the payload and leak computed display fields
+        # (formatted times, unit-converted temperatures) to the device.
         return await self._send_command(
             device=device,
             command_code=CommandCode.RESERVATION_WEEKLY,
-            reservation=schedule.model_dump(by_alias=True),
+            reservationUse=schedule.reservation_use,
+            reservation=[
+                entry.to_protocol_dict() for entry in schedule.reservation
+            ],
         )
 
     @requires_capability("program_reservation_use")
@@ -709,10 +717,13 @@ async def configure_recirculation_schedule(
         Returns:
             Publish packet ID
         """
+        # Send a flat list of raw protocol entries; dumping the schedule
+        # model as-is would nest the payload as schedule.schedule and
+        # leak computed display fields to the device.
         return await self._send_command(
             device=device,
             command_code=CommandCode.RECIR_RESERVATION,
-            schedule=schedule.model_dump(by_alias=True),
+            schedule=[entry.to_protocol_dict() for entry in schedule.schedule],
         )
 
     @requires_capability("recirculation_use")