diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 27090d7e..fc454098 100644 --- a/CHANGELOG.rst +++ b/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 diff --git a/src/nwp500/_base.py b/src/nwp500/_base.py index 12d88787..faa06742 100644 --- a/src/nwp500/_base.py +++ b/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 diff --git a/src/nwp500/encoding.py b/src/nwp500/encoding.py index 2566c215..85203270 100644 --- a/src/nwp500/encoding.py +++ b/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, diff --git a/src/nwp500/models/status.py b/src/nwp500/models/status.py index 4315c2b8..d436d608 100644 --- a/src/nwp500/models/status.py +++ b/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: diff --git a/src/nwp500/mqtt/command_queue.py b/src/nwp500/mqtt/command_queue.py index dabfa2af..cf88eca5 100644 --- a/src/nwp500/mqtt/command_queue.py +++ b/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 diff --git a/src/nwp500/mqtt/control.py b/src/nwp500/mqtt/control.py index bab96c35..1ebc7415 100644 --- a/src/nwp500/mqtt/control.py +++ b/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") diff --git a/src/nwp500/mqtt/state_tracker.py b/src/nwp500/mqtt/state_tracker.py index ba91e4a7..ea7e8a40 100644 --- a/src/nwp500/mqtt/state_tracker.py +++ b/src/nwp500/mqtt/state_tracker.py @@ -135,8 +135,10 @@ async def process(self, device_mac: str, status: DeviceStatus) -> None: ) _logger.debug("Heating stopped") - # Error detection / clearance - if status.error_code and not prev.error_code: + # Error detection / clearance. Also emit when the error code + # CHANGES between two non-zero codes (e.g. E799 -> E407), so + # 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", ErrorDetectedEvent( diff --git a/src/nwp500/mqtt/utils.py b/src/nwp500/mqtt/utils.py index 6f6f76f6..12f75698 100644 --- a/src/nwp500/mqtt/utils.py +++ b/src/nwp500/mqtt/utils.py @@ -215,6 +215,10 @@ class MqttConnectionConfig: enable_command_queue: Enable command queueing when disconnected max_queued_commands: Maximum number of queued commands + max_queued_command_age: Maximum age in seconds for a queued + command to still be sent when the connection is restored; + older commands are discarded instead of being replayed to + the appliance. ``None`` disables expiry. """ endpoint: str = AWS_IOT_ENDPOINT @@ -239,6 +243,7 @@ class MqttConnectionConfig: # Command queue settings enable_command_queue: bool = True max_queued_commands: int = 100 + max_queued_command_age: float | None = 300.0 # seconds def __post_init__(self) -> None: """Generate client ID if not provided and validate settings.""" @@ -248,6 +253,26 @@ def __post_init__(self) -> None: ) if self.deep_reconnect_threshold < 1: object.__setattr__(self, "deep_reconnect_threshold", 1) + # Validate queue settings up front: a non-positive queue size + # would make enqueue() fail at runtime in hard-to-debug ways, + # and a negative max age would silently expire every command. + if self.max_queued_commands < 1: + raise ValueError( + "max_queued_commands must be >= 1, got " + f"{self.max_queued_commands}" + ) + if ( + self.max_queued_command_age is not None + and self.max_queued_command_age < 0 + ): + raise ValueError( + "max_queued_command_age must be >= 0 or None, got " + f"{self.max_queued_command_age}" + ) + if self.operation_timeout <= 0: + raise ValueError( + f"operation_timeout must be > 0, got {self.operation_timeout}" + ) @dataclass diff --git a/src/nwp500/temperature.py b/src/nwp500/temperature.py index 78ce8220..09dd729d 100644 --- a/src/nwp500/temperature.py +++ b/src/nwp500/temperature.py @@ -146,7 +146,7 @@ def from_fahrenheit(cls, fahrenheit: float) -> HalfCelsius: Example: >>> temp = HalfCelsius.from_fahrenheit(140.0) >>> temp.raw_value - 120 + 120.0 """ celsius = (fahrenheit - 32) * 5 / 9 raw_value = round(celsius * 2) @@ -165,7 +165,7 @@ def from_celsius(cls, celsius: float) -> HalfCelsius: Example: >>> temp = HalfCelsius.from_celsius(60.0) >>> temp.raw_value - 120 + 120.0 """ raw_value = round(celsius * 2) return cls(raw_value) @@ -215,7 +215,7 @@ def from_fahrenheit(cls, fahrenheit: float) -> DeciCelsius: Example: >>> temp = DeciCelsius.from_fahrenheit(140.0) >>> temp.raw_value - 600 + 600.0 """ celsius = (fahrenheit - 32) * 5 / 9 raw_value = round(celsius * 10) @@ -234,7 +234,7 @@ def from_celsius(cls, celsius: float) -> DeciCelsius: Example: >>> temp = DeciCelsius.from_celsius(60.0) >>> temp.raw_value - 600 + 600.0 """ raw_value = round(celsius * 10) return cls(raw_value) @@ -271,10 +271,11 @@ def to_fahrenheit(self) -> float: """Convert to Fahrenheit using standard rounding. Returns: - Temperature in Fahrenheit. + Temperature in Fahrenheit (rounded to a whole degree, but + returned as float for consistency with the base class API). """ celsius = self.to_celsius() - return round((celsius * 9 / 5) + 32) + return float(round((celsius * 9 / 5) + 32)) def to_fahrenheit_with_formula( self, formula_type: TempFormulaType @@ -292,8 +293,14 @@ def to_fahrenheit_with_formula( match formula_type: case TempFormulaType.ASYMMETRIC: - # Asymmetric Rounding: check remainder of raw value - remainder = int(self.raw_value) % 10 + # Asymmetric Rounding: check remainder of raw value. + # Use the truncated remainder (like the firmware/app's + # Java % operator), NOT Python's floored modulo: for + # negative raw values Python's % is always non-negative + # (-11 % 10 == 9), which would apply floor where the + # firmware applies ceil, giving off-by-one Fahrenheit + # values for sub-zero temperatures. + remainder = int(math.fmod(int(self.raw_value), 10)) match remainder: case 9: return float(math.floor(fahrenheit_value)) @@ -316,7 +323,7 @@ def from_fahrenheit(cls, fahrenheit: float) -> RawCelsius: Example: >>> temp = RawCelsius.from_fahrenheit(140.0) >>> temp.raw_value - 120 + 120.0 """ celsius = (fahrenheit - 32) * 5 / 9 raw_value = round(celsius * 2) @@ -335,7 +342,7 @@ def from_celsius(cls, celsius: float) -> RawCelsius: Example: >>> temp = RawCelsius.from_celsius(60.0) >>> temp.raw_value - 120 + 120.0 """ raw_value = round(celsius * 2) return cls(raw_value) @@ -389,7 +396,7 @@ class DeciCelsiusDelta(Temperature): >>> temp.to_celsius() 0.5 >>> temp.to_fahrenheit() - 0.9 # 0.5°C * 9/5 = 0.9°F, no +32 offset + 0.9 """ def to_celsius(self) -> float: @@ -422,7 +429,7 @@ def from_fahrenheit(cls, fahrenheit: float) -> DeciCelsiusDelta: Example: >>> temp = DeciCelsiusDelta.from_fahrenheit(0.9) >>> temp.raw_value - 5 + 5.0 """ celsius = fahrenheit * 5 / 9 raw_value = round(celsius * 10) @@ -441,7 +448,7 @@ def from_celsius(cls, celsius: float) -> DeciCelsiusDelta: Example: >>> temp = DeciCelsiusDelta.from_celsius(0.5) >>> temp.raw_value - 5 + 5.0 """ raw_value = round(celsius * 10) return cls(raw_value) diff --git a/tests/test_protocol_correctness.py b/tests/test_protocol_correctness.py new file mode 100644 index 00000000..3c8c67b3 --- /dev/null +++ b/tests/test_protocol_correctness.py @@ -0,0 +1,457 @@ +"""Regression tests for protocol correctness fixes. + +Covers: +- Weekly reservation / recirculation schedule payload shape (no double + nesting, no computed display fields leaked to the device) +- Command queue ordering on failed flush and stale-command expiry +- Negative-temperature ASYMMETRIC Fahrenheit conversion +- Freeze protection default raw values +- error_detected emitted when the error code changes between two errors +- TOU price encoding (half-up rounding, bool rejection) +""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from unittest.mock import AsyncMock, MagicMock + +import pytest +from awscrt import mqtt + +from nwp500.encoding import build_tou_period, encode_price +from nwp500.events import EventEmitter +from nwp500.models.schedule import ( + RecirculationSchedule, + RecirculationScheduleEntry, + WeeklyReservationEntry, + WeeklyReservationSchedule, +) +from nwp500.models.status import DeviceStatus +from nwp500.mqtt.command_queue import MqttCommandQueue +from nwp500.mqtt.control import MqttDeviceController +from nwp500.mqtt.state_tracker import DeviceStateTracker +from nwp500.mqtt.utils import MqttConnectionConfig +from nwp500.temperature import RawCelsius, TempFormulaType +from nwp500.unit_system import reset_unit_system + + +@pytest.fixture(autouse=True) +def _reset_units(): + yield + reset_unit_system() + + +def _status(device_status_dict: dict, **overrides) -> DeviceStatus: + data = {**device_status_dict, **overrides} + return DeviceStatus.model_validate(data) + + +@pytest.fixture +def mock_device(): + device = MagicMock() + device.device_info.mac_address = "aa:bb:cc:dd:ee:ff" + device.device_info.device_type = 52 + device.device_info.additional_value = "additional" + return device + + +def _make_controller(): + publish = AsyncMock(return_value=1) + controller = MqttDeviceController( + client_id="test-client", + session_id="test-session", + publish_func=publish, + ) + # Bypass capability lookup (would require a live device) + controller._get_device_features = AsyncMock(return_value=MagicMock()) + return controller, publish + + +class TestSchedulePayloads: + """Schedule commands must send flat, raw protocol payloads.""" + + @pytest.mark.asyncio + async def test_weekly_reservation_payload_shape(self, mock_device): + """Regression: the schedule model was dumped as-is, double-nesting + the payload (request.reservation.reservation) and leaking computed + display fields — including a unit-converted temperature alongside + the raw half-Celsius param — to the device.""" + controller, publish = _make_controller() + + schedule = WeeklyReservationSchedule( + reservationUse=2, + reservation=[ + WeeklyReservationEntry( + enable=2, week=84, hour=6, min=30, mode=3, param=120 + ) + ], + ) + + await controller.update_weekly_reservation(mock_device, schedule) + + publish.assert_awaited_once() + _topic, command = publish.await_args.args + + # Flat shape matching update_reservations() + request = command["request"] + assert request["reservationUse"] == 2 + entries = request["reservation"] + assert isinstance(entries, list) + assert entries[0] == { + "enable": 2, + "week": 84, + "hour": 6, + "min": 30, + "mode": 3, + "param": 120, + } + + @pytest.mark.asyncio + async def test_recirculation_schedule_payload_shape(self, mock_device): + """Regression: payload was nested as schedule.schedule with + computed fields (start_time, days, mode_name...) included.""" + controller, publish = _make_controller() + + schedule = RecirculationSchedule( + schedule=[ + RecirculationScheduleEntry( + enable=2, + week=84, + startHour=6, + startMin=0, + endHour=8, + endMin=30, + mode=2, + ) + ] + ) + + await controller.configure_recirculation_schedule(mock_device, schedule) + + publish.assert_awaited_once() + _topic, command = publish.await_args.args + + entries = command["request"]["schedule"] + assert isinstance(entries, list) + assert entries[0] == { + "enable": 2, + "week": 84, + "startHour": 6, + "startMin": 0, + "endHour": 8, + "endMin": 30, + "mode": 2, + } + + def test_to_protocol_dict_excludes_computed_fields(self): + entry = WeeklyReservationEntry( + enable=2, week=84, hour=6, min=30, mode=3, param=120 + ) + protocol = entry.to_protocol_dict() + assert set(protocol) == { + "enable", + "week", + "hour", + "min", + "mode", + "param", + } + # The display dump still has the computed fields + display = entry.model_dump() + assert "temperature" in display + assert "days" in display + + +def _queue(max_age: float | None = 300.0) -> MqttCommandQueue: + config = MqttConnectionConfig( + client_id="test-client", max_queued_command_age=max_age + ) + return MqttCommandQueue(config) + + +class TestCommandQueueOrdering: + """A failed flush must preserve command order.""" + + @pytest.mark.asyncio + async def test_failed_command_requeued_at_front(self): + """Regression: a failed command was re-queued at the TAIL behind + newer commands, so [power_on, set_temp] became [set_temp, + power_on] on the next flush.""" + queue = _queue() + queue.enqueue( + "topic/power_on", {"cmd": "power"}, mqtt.QoS.AT_LEAST_ONCE + ) + queue.enqueue("topic/set_temp", {"cmd": "temp"}, mqtt.QoS.AT_LEAST_ONCE) + + publish = AsyncMock(side_effect=RuntimeError("still down")) + + sent, failed = await queue.send_all(publish, lambda: True) + + assert (sent, failed) == (0, 1) + # Order preserved for the next flush + assert [c.topic for c in queue._queue] == [ + "topic/power_on", + "topic/set_temp", + ] + + publish_ok = AsyncMock(return_value=1) + sent, failed = await queue.send_all(publish_ok, lambda: True) + assert (sent, failed) == (2, 0) + sent_topics = [ + call.kwargs["topic"] for call in publish_ok.await_args_list + ] + assert sent_topics == ["topic/power_on", "topic/set_temp"] + + +class TestCommandQueueExpiry: + """Stale commands must not be replayed to the appliance.""" + + @pytest.mark.asyncio + async def test_expired_commands_discarded_on_flush(self): + queue = _queue(max_age=300.0) + queue.enqueue("topic/old", {"cmd": "old"}, mqtt.QoS.AT_LEAST_ONCE) + queue.enqueue("topic/new", {"cmd": "new"}, mqtt.QoS.AT_LEAST_ONCE) + # Age the first command past the limit + queue._queue[0].timestamp = datetime.now(UTC) - timedelta(hours=2) + + publish = AsyncMock(return_value=1) + sent, failed = await queue.send_all(publish, lambda: True) + + assert (sent, failed) == (1, 0) + sent_topics = [call.kwargs["topic"] for call in publish.await_args_list] + assert sent_topics == ["topic/new"] + + @pytest.mark.asyncio + async def test_expiry_disabled_with_none(self): + queue = _queue(max_age=None) + queue.enqueue("topic/old", {"cmd": "old"}, mqtt.QoS.AT_LEAST_ONCE) + queue._queue[0].timestamp = datetime.now(UTC) - timedelta(days=1) + + publish = AsyncMock(return_value=1) + sent, failed = await queue.send_all(publish, lambda: True) + + assert (sent, failed) == (1, 0) + + def test_overflow_drops_oldest(self): + config = MqttConnectionConfig( + client_id="test-client", max_queued_commands=2 + ) + queue = MqttCommandQueue(config) + for name in ("a", "b", "c"): + queue.enqueue(f"topic/{name}", {}, mqtt.QoS.AT_LEAST_ONCE) + + assert queue.count == 2 + assert [c.topic for c in queue._queue] == ["topic/b", "topic/c"] + + +class TestAsymmetricNegativeTemperatures: + """Firmware uses truncated remainder; Python's % is floored.""" + + def test_negative_raw_matches_firmware_rounding(self): + """Regression: raw -11 (-5.5degC) gave -11 % 10 == 9 -> floor -> + 22degF, while the firmware/app's truncated remainder (-1) applies + ceil -> 23degF.""" + temp = RawCelsius(-11) + assert ( + temp.to_fahrenheit_with_formula(TempFormulaType.ASYMMETRIC) == 23.0 + ) + + def test_positive_raw_behavior_unchanged(self): + # raw 119 -> 59.5degC -> 139.1degF; remainder 9 -> floor -> 139 + assert ( + RawCelsius(119).to_fahrenheit_with_formula( + TempFormulaType.ASYMMETRIC + ) + == 139.0 + ) + # raw 120 -> 60.0degC -> 140.0degF; remainder 0 -> ceil -> 140 + assert ( + RawCelsius(120).to_fahrenheit_with_formula( + TempFormulaType.ASYMMETRIC + ) + == 140.0 + ) + + def test_negative_raw_with_remainder_nine_equivalent(self): + # raw -19 -> -9.5degC -> 14.9degF; truncated remainder -9 -> ceil -> 15 + assert ( + RawCelsius(-19).to_fahrenheit_with_formula( + TempFormulaType.ASYMMETRIC + ) + == 15.0 + ) + + +class TestFreezeProtectionDefaults: + """Defaults must be raw half-Celsius values (43degF / 50degF).""" + + def test_defaults_decode_to_documented_range(self, device_status_dict): + """Regression: defaults 43/65 were Fahrenheit display values + pasted into raw half-Celsius fields, decoding to 70.7degF/90.5degF + instead of the documented fixed 43degF/50degF limits.""" + from nwp500.unit_system import set_unit_system + + set_unit_system("us_customary") + data = dict(device_status_dict) + data.pop("freezeProtectionTempMin", None) + data.pop("freezeProtectionTempMax", None) + status = DeviceStatus.model_validate(data) + + assert status.freeze_protection_temp_min_raw == 12 + assert status.freeze_protection_temp_max_raw == 20 + assert status.freeze_protection_temp_min == pytest.approx(43, abs=1) + assert status.freeze_protection_temp_max == pytest.approx(50, abs=1) + + +class TestErrorCodeChangeEvents: + """error_detected must fire when the error code changes.""" + + @pytest.mark.asyncio + async def test_error_code_change_emits_error_detected( + self, device_status_dict + ): + """Regression: only 0 -> non-zero emitted error_detected; a + transition E799 -> E407 emitted nothing, so consumers kept + displaying the stale error.""" + emitter = EventEmitter() + tracker = DeviceStateTracker(emitter) + + events = [] + emitter.on("error_detected", lambda e: events.append(int(e.error_code))) + + # Baseline snapshot (no events emitted for the first status) + await tracker.process("mac", _status(device_status_dict, errorCode=0)) + await tracker.process("mac", _status(device_status_dict, errorCode=799)) + await tracker.process("mac", _status(device_status_dict, errorCode=407)) + + assert events == [799, 407] + + @pytest.mark.asyncio + async def test_unchanged_error_code_not_re_emitted( + self, device_status_dict + ): + emitter = EventEmitter() + tracker = DeviceStateTracker(emitter) + + events = [] + emitter.on("error_detected", lambda e: events.append(int(e.error_code))) + + await tracker.process("mac", _status(device_status_dict, errorCode=0)) + await tracker.process("mac", _status(device_status_dict, errorCode=799)) + await tracker.process("mac", _status(device_status_dict, errorCode=799)) + + assert events == [799] + + @pytest.mark.asyncio + async def test_error_cleared_still_emitted(self, device_status_dict): + emitter = EventEmitter() + tracker = DeviceStateTracker(emitter) + + cleared = [] + emitter.on("error_cleared", lambda e: cleared.append(int(e.error_code))) + + await tracker.process("mac", _status(device_status_dict, errorCode=0)) + await tracker.process("mac", _status(device_status_dict, errorCode=799)) + await tracker.process("mac", _status(device_status_dict, errorCode=0)) + + assert cleared == [799] + + +class TestTouPriceEncoding: + """Price encoding must round half-up and reject bool as int.""" + + def test_encode_price_rounds_half_up(self): + """Regression: round() applies banker's rounding, under-encoding + exact halves at even boundaries.""" + assert encode_price(0.125, 2) == 13 + assert encode_price(0.135, 2) == 14 + assert encode_price(12.34, 2) == 1234 + + def test_build_tou_period_bool_price_not_treated_as_encoded(self): + """Regression: bool is an int subclass, so True passed the + 'already encoded' isinstance check and was sent as price 1.""" + period = build_tou_period( + week_days=["Monday"], + season_months=[1], + start_hour=0, + start_minute=0, + end_hour=6, + end_minute=0, + price_min=True, # nonsense input, but must be encoded not passed + price_max=2.5, + decimal_point=2, + ) + assert period["priceMin"] == 100 # encode_price(1.0, 2) + assert period["priceMax"] == 250 + + def test_build_tou_period_int_passthrough(self): + period = build_tou_period( + week_days=["Monday"], + season_months=[1], + start_hour=0, + start_minute=0, + end_hour=6, + end_minute=0, + price_min=500, # pre-encoded + price_max=0.075, + decimal_point=3, + ) + assert period["priceMin"] == 500 + assert period["priceMax"] == 75 + + +class TestConfigValidation: + """MqttConnectionConfig must reject unusable queue settings.""" + + def test_zero_max_queued_commands_rejected(self): + with pytest.raises(ValueError, match="max_queued_commands"): + MqttConnectionConfig(client_id="t", max_queued_commands=0) + + def test_negative_max_queued_commands_rejected(self): + with pytest.raises(ValueError, match="max_queued_commands"): + MqttConnectionConfig(client_id="t", max_queued_commands=-5) + + def test_negative_max_age_rejected(self): + with pytest.raises(ValueError, match="max_queued_command_age"): + MqttConnectionConfig(client_id="t", max_queued_command_age=-1.0) + + def test_none_max_age_allowed(self): + config = MqttConnectionConfig( + client_id="t", max_queued_command_age=None + ) + assert config.max_queued_command_age is None + + def test_zero_max_age_allowed(self): + config = MqttConnectionConfig(client_id="t", max_queued_command_age=0.0) + assert config.max_queued_command_age == 0.0 + + def test_nonpositive_operation_timeout_rejected(self): + with pytest.raises(ValueError, match="operation_timeout"): + MqttConnectionConfig(client_id="t", operation_timeout=0) + + +class TestEncodePriceDecimalPrecision: + """encode_price must not lose precision by round-tripping via float.""" + + def test_decimal_input_used_directly(self): + from decimal import Decimal + + # A value that is exact as Decimal but not as binary float + assert encode_price(Decimal("0.145"), 2) == 15 # HALF_UP on exact half + + def test_high_precision_decimal(self): + from decimal import Decimal + + assert encode_price(Decimal("0.1234567895"), 10) == 1234567895 + + def test_int_input(self): + assert encode_price(100, 0) == 100 + + def test_float_input_unchanged_behavior(self): + assert encode_price(12.34, 2) == 1234 + assert encode_price(0.125, 2) == 13 + + def test_rawcelsius_to_fahrenheit_returns_float(self): + result = RawCelsius(120).to_fahrenheit() + assert result == 140.0 + assert isinstance(result, float)