diff --git a/.gitignore b/.gitignore index d82c869..c164dd6 100644 --- a/.gitignore +++ b/.gitignore @@ -49,6 +49,8 @@ build/* dist/* sdist/* docs/api/* +docs/reference/api/* +.obsidian/ docs/_rst/* docs/_build/* cover/* diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 770a152..2acc540 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,10 +5,15 @@ Changelog Unreleased ========== -**BREAKING CHANGES**: tank energy values were wrong in two independent -ways - a 2.5x unit-scale error and two actively misleading field names - -and both are corrected. Reported tank energy is now 2.5x smaller and two -public field names are removed. +**BREAKING CHANGES**: two independent corrections land together. + +Tank energy values were wrong in two ways - a 2.5x unit-scale error and +two actively misleading field names - and both are corrected. Reported +tank energy is now 2.5x smaller and two public field names are removed. + +Separately, eight status flags change type from ``bool`` to +``bool | None`` so the device's "unknown" state is no longer reported as +a definite OFF. Changed ------- @@ -56,6 +61,31 @@ Changed relabelled from "Total Capacity"/"Available Capacity" to "Full Recovery"/"Energy to Setpoint". +- **Status flags now preserve the device's unknown state.** The protocol + encodes these flags as ``0 = unknown, 1 = OFF, 2 = ON``, and the library + was collapsing 0 to ``False`` - inventing an OFF the device never claimed. + Confirmed against Navien's own NaviLink app (2.03.00, versionCode 141), + which decodes exactly this set of fields through an enum declared + ``UNKNOWN(0), OFF(1), ON(2)``; two sibling enums render their zero as + ``"-"`` and ``"Not Applied"`` rather than as an off state. + + Affected: ``operation_busy``, ``comp_use``, ``anti_legionella_use``, + ``anti_legionella_operation_busy``, ``heat_upper_use``, ``heat_lower_use``, + ``air_filter_alarm_use``, ``recirc_reservation_use``. + + ``None`` is falsy, so ``if status.comp_use:`` is unaffected. Code that + distinguishes ``is False`` from "not reported", or does arithmetic or + formatting on these fields, needs a ``None`` check. For Home Assistant + this is the wanted shape: ``None`` renders as "Unknown" instead of writing + a fabricated OFF into the recorder database. + +- ``OnOffFlag`` gains the vendor's ``UNKNOWN = 0`` member. It previously + started at ``OFF = 1``, leaving the device's reserved value unrepresented. + +- The CLI renders these flags as ``Unknown`` rather than ``No``. Both + affected rows are updated: "Busy" under OPERATION STATUS and + "Operation Busy" under ANTI-LEGIONELLA. + Added ----- - **``DeviceStatus.usable_energy``**: drawable energy in Watt-hours, @@ -69,6 +99,13 @@ Added standard deviation of 0.57 degF over 12275 samples. Rendered by the CLI as "Usable Energy". +- ``converters.device_tristate_to_python`` and + ``models.status.DeviceTriState`` for flags the device may decline to + report. ``converters.device_bool_to_python`` is unchanged and remains + correct for capability flags. +- New ``docs/explanation/unknown-values.rst`` recording which field families + use 0 as a sentinel and which do not, with the app evidence for each. + Removed ------- - **Misnamed energy fields**: removed ``DeviceStatus.total_energy_capacity`` @@ -119,6 +156,16 @@ Fixed including that Navien's own NaviLink app (2.03.00, versionCode 141) reads neither field, so no vendor-side corroboration exists. +- Documented that **temperature fields carry no sentinel at all**. The app + has no out-of-band constant (no ``0xFFFF``/``-999``/``-1``), no zero-guard + in any display path, and formats whatever arrives - so a temperature of + zero means zero. This closes a recurring source of bugs where zero-as-none + was applied to temperature converters and reported working sensors as + missing during cold-weather operation. +- Documented that capability flags are a distinct case: the app hides a + feature's entire UI when its DID ``Use`` flag reads 0, so 0 there means + "not fitted" and the existing ``bool`` mapping is correct. + Version 9.2.1 (2026-07-30) ========================== diff --git a/docs/explanation/index.rst b/docs/explanation/index.rst index a04ab75..56fd465 100644 --- a/docs/explanation/index.rst +++ b/docs/explanation/index.rst @@ -10,3 +10,4 @@ Understanding-oriented deep dives into the library's design and advanced feature advanced-features architecture tank-energy + unknown-values diff --git a/docs/explanation/unknown-values.rst b/docs/explanation/unknown-values.rst new file mode 100644 index 0000000..596426f --- /dev/null +++ b/docs/explanation/unknown-values.rst @@ -0,0 +1,259 @@ +============== +Unknown Values +============== + +The device sometimes declines to report a field. This page records what +the protocol actually does about that, which fields are affected, and - +just as importantly - which fields look affected but are not. + +The short answer: **there is no single rule about zero.** Zero is a +reserved sentinel in some field families, a real value in others, and +carries no special meaning at all for temperatures. Earlier attempts to +apply one rule library-wide produced bugs in both directions. + +.. contents:: + :local: + :depth: 2 + + +Where the evidence comes from +============================= + +Navien's own Android client, NaviLink, was decompiled - version 2.03.00, +versionCode 141, published March 2026. The app is a first-party decoder +for the same MQTT payloads this library parses, so its enum tables and +its null-handling are direct evidence of intent rather than inference +from observed traffic. + +Two cautions apply to everything below. The app is a *consumer* of the +protocol, not its specification, and it ignores a great deal of what the +device sends. Absence of handling in the app is therefore weak evidence +on its own; presence of an explicit sentinel is strong evidence. + + +Enum-coded flags: zero is a sentinel +==================================== + +The app decodes many status fields through enums in ``KDEnum.java``. The +generic on/off flag is declared: + +.. code:: java + + public enum MgppOnOFFFlag { + UNKNOWN(0, "Unknown"), + OFF(1, "OFF"), + ON(2, "ON"); + } + +Zero is reserved and real values start at 1. This is not an accident of +one enum - eight of the app's enums do it, and two render their zero as +something a user would read as "no data": + +.. list-table:: + :header-rows: 1 + :widths: 42 20 38 + + * - App enum + - Zero member + - Display text + * - ``MgppOnOFFFlag`` + - ``UNKNOWN`` + - "Unknown" + * - ``HPWHHeatSource`` + - ``UNKNOWN`` + - **"-"** + * - ``HPWHDREvent`` + - ``UNKNOWN`` + - **"Not Applied"** + * - ``MgppRecirculationOperationMode`` + - ``UNKNOWN`` + - "Unknown" + * - ``MgppDHWControlTypeFlag`` + - ``UNKNOWN`` + - "Unknown" + * - ``HydroElectricalEfficiencyMode`` + - ``UNKNOWN`` + - "Unknown" + * - ``MgppReservationMode`` + - ``NOT_RESERVATION`` + - "NOT RESERVATION" + * - ``firmwareType`` + - ``Unknown`` + - "Unknown" + + +But in five other enums zero is real +------------------------------------ + +The rule is not global, and this is where a blanket converter goes wrong: + +.. list-table:: + :header-rows: 1 + :widths: 42 58 + + * - App enum + - Zero means + * - ``MgppOperationMode`` + - ``STANDBY`` - a real, common state + * - ``OperationMode`` + - ``STANDBY`` + * - ``HydroOperationMode`` + - ``STOP`` + * - ``HydroFsmState`` + - ``INIT`` + * - ``FilterChange`` + - ``NORMAL`` - filter is fine + +A device sitting in standby reports 0 constantly. Treating that as +"unknown" would blank the operating mode most of the time. + + +What this library does +---------------------- + +:class:`~nwp500.enums.OnOffFlag` carries the vendor's ``UNKNOWN = 0`` +member. The app decodes nine status fields through ``MgppOnOFFFlag``; +eight of them exist here as flags and are typed +:data:`~nwp500.models.status.DeviceTriState`, which maps 0 to ``None``: + +- ``operation_busy`` +- ``comp_use`` +- ``anti_legionella_use`` +- ``anti_legionella_operation_busy`` +- ``heat_upper_use`` +- ``heat_lower_use`` +- ``air_filter_alarm_use`` +- ``recirc_reservation_use`` + +The ninth is ``drOverrideStatus``, which this library exposes as a raw +``int`` rather than a flag, so it is left alone. + +Every other flag keeps :data:`~nwp500.models.status.DeviceBool`. + + +Capability flags: zero means "not fitted" +========================================= + +The DID/feature ``Use`` flags are a third case, and the app treats them +differently from status flags - it does not decode them through an enum +at all, and checks them directly: + +.. code:: java + + if (... feature.getRecirculationUse() == 0) { + this.viewDataBinding.layoutHotButton.setVisibility(8); + this.viewDataBinding.linearLayoutControlRecirculation.setVisibility(8); + return; + } + +Zero hides the entire recirculation UI. For a capability flag, zero means +"this device does not have the feature", which is a definite answer, not +an absent one. ``False`` is the faithful mapping and +:data:`~nwp500.models.feature.CapabilityFlag` is unchanged. + + +Temperatures carry no sentinel +============================== + +This is the important negative result, because it is the one that has +been guessed wrong before. + +- **No out-of-band values.** No ``0xFFFF``, ``-999`` or ``-1`` appears + anywhere in the app's status handling. The only ``65535`` constants in + the app are CRC16 masks and infrared remote-control codes. +- **No zero-guard in any display path.** The only ``== 0`` comparisons on + the status screen are ``errorCode``, ``minorCode``, ``waterSprayStatus`` + and ``airFilterAlarmPeriod``. Not one is a temperature. +- **The formatter is unconditional.** ``getTempText()`` converts and + prints whatever arrives, so a device reporting 0 would render as + 32 degF. The app has no notion of an unavailable temperature. + +So a temperature of zero means zero. The library does not map any +temperature field to ``None``. + +.. warning:: + Do not add zero-as-none to a temperature field on the strength of it + "always reading 0" in a capture. Ambient and tank temperatures can + legitimately reach 0 degC, and a heat pump in a cold garage will get + there. A previous attempt did exactly this and had to be reverted after + it reported working sensors as missing during cold-weather operation. + + +Absent and zero are indistinguishable +===================================== + +All 126 numeric fields in the app's status model are declared as +primitive ``int`` - there is not a single boxed ``Integer`` among them. A +field missing from the JSON therefore deserializes to ``0``, and the +vendor's own client cannot tell "not reported" from "reported as zero" +either. + +There is consequently no protocol-level concept of "unreported" to +recover. Where a field is genuinely absent from the payload, that is +visible to this library through Pydantic's own missing-field handling, +not through any sentinel. + + +Fields the app never displays +============================= + +Several fields that look like natural candidates for N/A handling are +simply not rendered by the app for this device type. They exist as +getters on the status model with no UI call site: + +``outsideTemperature``, ``mixingRate``, ``currentInletTemperature``, +``dhwTemperature2``, ``recircTemperature``, ``recircFaucetTemperature``, +``heLowerOnTempSetting``. + +Observing that one of these "is always 0" in a capture is not evidence +that 0 is a sentinel. It is equally consistent with the sensor being +absent, the feature being unfitted, or the field being unused on this +model. The app makes no determination, so neither does this library. + +The one place the app does treat a numeric zero as "not configured" is +``airFilterAlarmPeriod``, and it handles it by substituting a different +*message* ("filter setup needed") rather than blanking a value - a +per-field UI decision, not a converter-level rule. + + +Migration +========= + +Eight fields change type from ``bool`` to ``bool | None``. ``None`` is +falsy, so truthiness checks are unaffected: + +.. code:: python + + if status.comp_use: # unchanged + ... + +The risk is in negative and identity checks, which no longer mean the +same thing: + +.. code:: python + + # Before: True for both OFF and unknown + # After: True for both OFF and unknown - but now you can tell them apart + if not status.comp_use: + ... + + # Distinguish explicitly + if status.comp_use is None: + ... # device is not reporting + elif status.comp_use: + ... # compressor running + +Anything doing arithmetic or formatting on these fields needs a ``None`` +check. For Home Assistant this is the desired shape: ``None`` renders as +"Unknown" rather than recording a fabricated OFF into the recorder +database and skewing history. + +The CLI renders these as ``Unknown`` rather than ``No``. + + +See also +======== + +- :doc:`tank-energy` - the other case where the protocol's own names + mislead, worked out from the same teardown +- :doc:`../reference/protocol/data_conversions` - protocol field conversions diff --git a/docs/how-to/manage-units.rst b/docs/how-to/manage-units.rst index 6dd038f..c145891 100644 --- a/docs/how-to/manage-units.rst +++ b/docs/how-to/manage-units.rst @@ -239,7 +239,7 @@ Temperature Conversions **Celsius to Fahrenheit** -.. code-block:: python +.. code-block:: text fahrenheit = (celsius * 9/5) + 32 @@ -248,7 +248,7 @@ Temperature Conversions **Fahrenheit to Celsius** -.. code-block:: python +.. code-block:: text celsius = (fahrenheit - 32) * 5/9 diff --git a/docs/project/authors.rst b/docs/project/authors.rst index cd8e091..24b9776 100644 --- a/docs/project/authors.rst +++ b/docs/project/authors.rst @@ -1,2 +1,2 @@ .. _authors: -.. include:: ../AUTHORS.rst +.. include:: ../../AUTHORS.rst diff --git a/docs/project/changelog.rst b/docs/project/changelog.rst index 871950d..9d6b6b3 100644 --- a/docs/project/changelog.rst +++ b/docs/project/changelog.rst @@ -1,2 +1,2 @@ .. _changes: -.. include:: ../CHANGELOG.rst +.. include:: ../../CHANGELOG.rst diff --git a/docs/project/license.rst b/docs/project/license.rst index 3989c51..3325f3f 100644 --- a/docs/project/license.rst +++ b/docs/project/license.rst @@ -4,4 +4,5 @@ License ======= -.. include:: ../LICENSE.txt +.. include:: ../../LICENSE.txt + :literal: diff --git a/docs/reference/index.rst b/docs/reference/index.rst index e4cb0b1..e6b3834 100644 --- a/docs/reference/index.rst +++ b/docs/reference/index.rst @@ -25,6 +25,7 @@ Protocol Reference .. toctree:: :maxdepth: 1 + protocol/quick_reference protocol/rest_api protocol/mqtt_protocol protocol/device_status diff --git a/docs/reference/installation.rst b/docs/reference/installation.rst index a0ba2f0..5bc2d6f 100644 --- a/docs/reference/installation.rst +++ b/docs/reference/installation.rst @@ -152,6 +152,6 @@ To upgrade to a specific version: Next Steps ========== -* :doc:`quickstart` - Get started with your first script +* :doc:`../tutorials/getting-started` - Get started with your first script * :doc:`configuration` - Configure credentials and options * :doc:`python_api/auth_client` - Learn about authentication diff --git a/examples/advanced/combined_callbacks.py b/examples/advanced/combined_callbacks.py index d0593ca..08cda0a 100755 --- a/examples/advanced/combined_callbacks.py +++ b/examples/advanced/combined_callbacks.py @@ -92,7 +92,13 @@ def on_status(status: DeviceStatus): print(f" Mode: {status.operation_mode.name}") print(f" DHW Temp: {status.dhw_temperature:.1f}{unit}") print(f" DHW Charge: {status.dhw_charge_per:.1f}%") - print(f" Compressor: {'On' if status.comp_use else 'Off'}") + # comp_use is tri-state: None means the device is not + # reporting, which must not be shown as "Off". + if status.comp_use is None: + comp = "Unknown" + else: + comp = "On" if status.comp_use else "Off" + print(f" Compressor: {comp}") # Callback for feature/capability info def on_feature(feature: DeviceFeature): diff --git a/src/nwp500/cli/presentation.py b/src/nwp500/cli/presentation.py index 27cbdf0..4aa424a 100644 --- a/src/nwp500/cli/presentation.py +++ b/src/nwp500/cli/presentation.py @@ -29,6 +29,17 @@ def _format_number(value: Any) -> str: return str(value) +def _yes_no_unknown(value: bool | None) -> str: + """Render a tri-state device flag. + + ``None`` means the device reported its reserved 0 and is not claiming a + state, so it must not collapse to "No". + """ + if value is None: + return "Unknown" + return "Yes" if value else "No" + + def _get_unit_suffix( field_name: str, model_class: Any = DeviceStatus, @@ -119,7 +130,7 @@ def build_device_status_rows(device_status: Any) -> list[StatusRow]: ( "OPERATION STATUS", "Busy", - "Yes" if device_status.operation_busy else "No", + _yes_no_unknown(device_status.operation_busy), ) ) if hasattr(device_status, "current_statenum"): @@ -576,7 +587,7 @@ def build_device_status_rows(device_status: Any) -> list[StatusRow]: ( "ANTI-LEGIONELLA", "Operation Busy", - "Yes" if device_status.anti_legionella_operation_busy else "No", + _yes_no_unknown(device_status.anti_legionella_operation_busy), ) ) diff --git a/src/nwp500/converters.py b/src/nwp500/converters.py index da6d3b5..dab4885 100644 --- a/src/nwp500/converters.py +++ b/src/nwp500/converters.py @@ -13,6 +13,7 @@ __all__ = [ "device_bool_to_python", "device_bool_from_python", + "device_tristate_to_python", "tou_override_to_python", "div_10", "energy_count_to_wh", @@ -54,28 +55,83 @@ def device_bool_to_python(value: Any) -> bool: """Convert device boolean representation to Python bool. - Device protocol uses: 1 = OFF/False, 2 = ON/True + Device protocol uses: 1 = OFF/False, 2 = ON/True, 0 = unknown/absent. - This design (using 1 and 2 instead of 0 and 1) is likely due to: - - 0 being reserved for null/uninitialized state - - 1 representing "off" in legacy firmware - - 2 representing "on" state + The 1/2 encoding is not arbitrary. The vendor's own client decodes these + fields through an enum declared ``UNKNOWN(0), OFF(1), ON(2)``, so 0 is a + reserved sentinel rather than a value. + + This converter collapses 0 to ``False``, which is correct for **capability + flags** (:data:`~nwp500.models.feature.CapabilityFlag`): the NaviLink app + hides a feature's entire UI when its ``Use`` flag reads 0, so 0 there means + "this device does not have the feature" and ``False`` is faithful. + + For **status flags**, where 0 means "the device is not reporting this right + now", collapsing to ``False`` invents an OFF the device never claimed. Use + :func:`device_tristate_to_python` for those. Args: value: Device value (typically 1 or 2). Returns: - Python boolean (1→False, 2→True). + Python boolean (1→False, 2→True, 0→False). Example: >>> device_bool_to_python(2) True >>> device_bool_to_python(1) False + >>> device_bool_to_python(0) + False """ return bool(value == 2) +def device_tristate_to_python(value: Any) -> bool | None: + """Convert a device on/off flag, preserving the device's unknown state. + + Identical to :func:`device_bool_to_python` except that the protocol's + reserved 0 maps to ``None`` instead of being flattened into ``False``. + + The device distinguishes three states and the library should not throw one + away. The NaviLink app decodes the affected status fields through + ``KDEnum.MgppOnOFFFlag``, which is declared ``UNKNOWN(0), OFF(1), ON(2)``; + two of the app's sibling enums render their zero as ``"-"`` and + ``"Not Applied"`` rather than as an off state. + + ``None`` is the right shape for the downstream consumer too: Home Assistant + renders it as "Unknown" instead of recording a fabricated OFF into the + history database. + + Note this applies only to flags the vendor itself decodes as an on/off + enum. It is **not** a general rule about zero - see + :func:`device_bool_to_python` for capability flags, and note that + temperature fields carry no sentinel at all. + + Args: + value: Device value (0, 1 or 2). + + Returns: + ``None`` when the device reports 0, otherwise 1→False, 2→True. + + Example: + >>> device_tristate_to_python(2) + True + >>> device_tristate_to_python(1) + False + >>> device_tristate_to_python(0) is None + True + """ + if value is None: + return None + try: + if int(value) == 0: + return None + except TypeError, ValueError: + return bool(value == 2) + return bool(int(value) == 2) + + def device_bool_from_python(value: bool) -> int: """Convert Python bool to device boolean representation. diff --git a/src/nwp500/enums.py b/src/nwp500/enums.py index ae72667..4a50688 100644 --- a/src/nwp500/enums.py +++ b/src/nwp500/enums.py @@ -19,8 +19,14 @@ class OnOffFlag(IntEnum): Used for: Power status, TOU status, recirculation status, vacation mode, anti-legionella, and many other boolean device settings. + + ``UNKNOWN`` is the device's own sentinel, not a library invention. The + NaviLink app decodes these fields through an enum declared + ``UNKNOWN(0), OFF(1), ON(2)``, reserving 0 and starting real values at 1. + See :doc:`/explanation/unknown-values`. """ + UNKNOWN = 0 OFF = 1 ON = 2 diff --git a/src/nwp500/models/status.py b/src/nwp500/models/status.py index e34f86b..2cf0f9a 100644 --- a/src/nwp500/models/status.py +++ b/src/nwp500/models/status.py @@ -5,6 +5,7 @@ from .._base import NavienBaseModel from ..converters import ( device_bool_to_python, + device_tristate_to_python, div_10, energy_count_to_wh, tou_override_to_python, @@ -29,6 +30,14 @@ from ..unit_system import get_unit_system DeviceBool = Annotated[bool, BeforeValidator(device_bool_to_python)] +#: An on/off flag the device may decline to report. 0 becomes ``None`` +#: rather than ``False``, because the device reserves 0 to mean "unknown" +#: and a fabricated OFF is worse than an absent value. Applied only to the +#: fields the vendor's own client decodes through its on/off enum - see +#: ``docs/explanation/unknown-values.rst``. +DeviceTriState = Annotated[ + bool | None, BeforeValidator(device_tristate_to_python) +] Div10 = Annotated[float, BeforeValidator(div_10)] EnergyCountToWh = Annotated[float, BeforeValidator(energy_count_to_wh)] TouStatus = Annotated[bool, BeforeValidator(bool)] @@ -268,7 +277,7 @@ class DeviceStatus(NavienBaseModel): did_reload: DeviceBool = Field( description="Indicates if the device has recently reloaded or restarted" ) - operation_busy: DeviceBool = Field( + operation_busy: DeviceTriState = Field( description=( "Indicates if the device is currently performing heating operations" ) @@ -307,7 +316,7 @@ class DeviceStatus(NavienBaseModel): "Whether ECO (Energy Cut Off) high-temp safety limit is triggered" ) ) - comp_use: DeviceBool = Field( + comp_use: DeviceTriState = Field( description=( "Compressor usage status (True=On, False=Off). " "The compressor is the main component of the heat pump" @@ -340,13 +349,13 @@ class DeviceStatus(NavienBaseModel): "Triggers error E799 if leak detected" ) ) - anti_legionella_use: DeviceBool = Field( + anti_legionella_use: DeviceTriState = Field( description=( "Whether anti-legionella function is enabled. " "Device periodically heats tank to prevent Legionella bacteria" ) ) - anti_legionella_operation_busy: DeviceBool = Field( + anti_legionella_operation_busy: DeviceTriState = Field( description=( "Whether the anti-legionella disinfection cycle " "is currently running" @@ -362,13 +371,13 @@ class DeviceStatus(NavienBaseModel): "1=Heat Pump, 2=Electric Element, 3=Both simultaneously" ) ) - heat_upper_use: DeviceBool = Field( + heat_upper_use: DeviceTriState = Field( description=( "Upper electric heating element usage status. " "Power: 3,755W @ 208V or 5,000W @ 240V" ) ) - heat_lower_use: DeviceBool = Field( + heat_lower_use: DeviceTriState = Field( description=( "Lower electric heating element usage status. " "Power: 3,755W @ 208V or 5,000W @ 240V" @@ -380,7 +389,7 @@ class DeviceStatus(NavienBaseModel): "Warning when water reaches potentially hazardous levels" ) ) - air_filter_alarm_use: DeviceBool = Field( + air_filter_alarm_use: DeviceTriState = Field( description=( "Air filter maintenance reminder enabled flag. " "Triggers alerts based on operating hours. Default: On" @@ -389,7 +398,7 @@ class DeviceStatus(NavienBaseModel): recirc_operation_busy: DeviceBool = Field( description="Recirculation operation busy status" ) - recirc_reservation_use: DeviceBool = Field( + recirc_reservation_use: DeviceTriState = Field( description="Recirculation reservation usage status" ) diff --git a/src/nwp500/mqtt_events.py b/src/nwp500/mqtt_events.py index 3ab6ac0..09986d9 100644 --- a/src/nwp500/mqtt_events.py +++ b/src/nwp500/mqtt_events.py @@ -224,7 +224,7 @@ class MqttClientEvents: print(f"Available events: {events}") See Also: - :doc:`../guides/event_system` - Comprehensive event handling guide + :doc:`/reference/python_api/events` - Comprehensive event handling guide """ # Connection lifecycle events diff --git a/tests/test_cli_tristate_presentation.py b/tests/test_cli_tristate_presentation.py new file mode 100644 index 0000000..01c0835 --- /dev/null +++ b/tests/test_cli_tristate_presentation.py @@ -0,0 +1,126 @@ +"""CLI rendering of tri-state status flags. + +The device encodes these flags as 0 = unknown, 1 = OFF, 2 = ON. A field +reading 0 must never be presented as "No" - that reports a definite OFF +the device never claimed. + +This module guards the presentation layer specifically, because the model +change is easy to make without updating every render site: a missed one +is silent, since ``None`` is falsy and a two-way ternary happily prints +"No". +""" + +import pytest + +# The CLI package pulls in the optional `cli` extra (click/rich). Skip +# rather than hard-fail when running against a bare install. +pytest.importorskip("rich", reason="requires the 'cli' extra") +pytest.importorskip("click", reason="requires the 'cli' extra") + +from nwp500.cli.presentation import ( + _yes_no_unknown, + build_device_status_rows, +) +from nwp500.models import DeviceStatus + +# Status flags typed DeviceTriState, with the row label the CLI gives each. +# Only those actually rendered by build_device_status_rows appear here. +RENDERED_TRISTATE_ROWS = { + "operation_busy": ("OPERATION STATUS", "Busy"), + "anti_legionella_operation_busy": ("ANTI-LEGIONELLA", "Operation Busy"), +} + +TRISTATE_ALIASES = { + "operation_busy": "operationBusy", + "anti_legionella_operation_busy": "antiLegionellaOperationBusy", +} + + +class TestYesNoUnknown: + """The formatter itself.""" + + def test_none_is_unknown(self): + assert _yes_no_unknown(None) == "Unknown" + + def test_true_is_yes(self): + assert _yes_no_unknown(True) == "Yes" + + def test_false_is_no(self): + assert _yes_no_unknown(False) == "No" + + def test_all_three_are_distinct(self): + rendered = [_yes_no_unknown(v) for v in (None, False, True)] + assert len(set(rendered)) == 3 + + +class TestTriStateRendering: + """End-to-end: a 0 on the wire must reach the CLI as "Unknown".""" + + def _rows(self, status_data, alias, raw): + """Build the CLI rows, keyed by (section, label).""" + data = dict(status_data) + data[alias] = raw + return { + (section, label): value + for section, label, value in build_device_status_rows( + DeviceStatus(**data) + ) + } + + @pytest.mark.parametrize( + ("field", "location"), list(RENDERED_TRISTATE_ROWS.items()) + ) + def test_zero_renders_unknown(self, device_status_dict, field, location): + """0 must not be presented as "No".""" + rows = self._rows(device_status_dict, TRISTATE_ALIASES[field], 0) + assert rows[location] == "Unknown" + + @pytest.mark.parametrize( + ("field", "location"), list(RENDERED_TRISTATE_ROWS.items()) + ) + def test_one_renders_no(self, device_status_dict, field, location): + """1 is a real OFF and still reads "No".""" + rows = self._rows(device_status_dict, TRISTATE_ALIASES[field], 1) + assert rows[location] == "No" + + @pytest.mark.parametrize( + ("field", "location"), list(RENDERED_TRISTATE_ROWS.items()) + ) + def test_two_renders_yes(self, device_status_dict, field, location): + """2 is a real ON and still reads "Yes".""" + rows = self._rows(device_status_dict, TRISTATE_ALIASES[field], 2) + assert rows[location] == "Yes" + + +def test_no_tristate_field_uses_a_two_way_ternary(): + """Structural guard against the miss this test module exists for. + + A two-way ``"Yes" if else "No"`` compiles and passes any + test that only exercises 1 and 2, then silently mislabels unknown as + "No". Catch it in the source instead. + """ + from pathlib import Path + + import nwp500.cli.presentation as presentation + + source = Path(presentation.__file__).read_text() + tristate_fields = ( + "operation_busy", + "comp_use", + "anti_legionella_use", + "anti_legionella_operation_busy", + "heat_upper_use", + "heat_lower_use", + "air_filter_alarm_use", + "recirc_reservation_use", + ) + offenders = [ + line.strip() + for line in source.splitlines() + if '"Yes" if' in line + and any(f".{field}" in line for field in tristate_fields) + ] + assert not offenders, ( + "tri-state fields rendered with a two-way ternary " + f"(use _yes_no_unknown): {offenders}" + ) diff --git a/tests/test_model_converters.py b/tests/test_model_converters.py index 1660e09..ac7a903 100644 --- a/tests/test_model_converters.py +++ b/tests/test_model_converters.py @@ -14,6 +14,7 @@ from nwp500.converters import ( WH_PER_ENERGY_COUNT, device_bool_to_python, + device_tristate_to_python, div_10, energy_count_to_wh, enum_validator, @@ -61,6 +62,16 @@ def test_float_off(self): """Float 1.0 is not equal to 2.""" assert device_bool_to_python(1.0) is False + def test_zero_collapses_to_false_for_capability_flags(self): + """0 stays False here; that is correct for capability flags. + + The NaviLink app hides a feature's entire UI when its DID ``Use`` + flag reads 0, so for capabilities 0 means "device does not have + this" and False is faithful. Status flags use + device_tristate_to_python instead. + """ + assert device_bool_to_python(0) is False + def test_float_on(self): """Float 2.0 equals int 2 in Python.""" assert device_bool_to_python(2.0) is True @@ -92,6 +103,55 @@ def test_off_value_variations(self, off_value): assert device_bool_to_python(off_value) is False +class TestDeviceTriStateConverter: + """Test device_tristate_to_python converter. + + The device encodes these flags as 0 = UNKNOWN, 1 = OFF, 2 = ON. The + vendor's own client decodes them through an enum declared + ``UNKNOWN(0), OFF(1), ON(2)``, so 0 is a reserved sentinel and must not + become False. + """ + + def test_unknown_value(self): + """Device value 0 converts to None, not False.""" + assert device_tristate_to_python(0) is None + + def test_off_value(self): + """Device value 1 converts to False.""" + assert device_tristate_to_python(1) is False + + def test_on_value(self): + """Device value 2 converts to True.""" + assert device_tristate_to_python(2) is True + + def test_none_passes_through(self): + """A missing value stays None rather than becoming False.""" + assert device_tristate_to_python(None) is None + + @pytest.mark.parametrize( + ("raw", "expected"), + [("0", None), ("1", False), ("2", True), (0.0, None), (2.0, True)], + ) + def test_coerces_numeric_strings_and_floats(self, raw, expected): + """Unlike device_bool_to_python, this coerces before comparing. + + device_bool_to_python compares with ``== 2`` and so reports False + for the string "2". That quirk is pinned by its own tests; this + converter does not reproduce it, because mapping a real ON to + False is exactly the failure mode it exists to prevent. + """ + assert device_tristate_to_python(raw) is expected + + def test_unparseable_value_does_not_raise(self): + """A non-numeric value falls back rather than blowing up.""" + assert device_tristate_to_python("garbage") is False + + def test_three_states_are_distinguishable(self): + """The whole point: 0, 1 and 2 map to three distinct results.""" + results = [device_tristate_to_python(v) for v in (0, 1, 2)] + assert results == [None, False, True] + + class TestBuiltinBoolForTouStatus: """Test built-in bool() for TOU status field (0/1 encoding). diff --git a/tests/test_models.py b/tests/test_models.py index 1cf17c3..3d8be68 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -1,3 +1,5 @@ +from typing import ClassVar + import pytest from nwp500.models import ( @@ -275,6 +277,78 @@ def test_skips_empty_entries(self): assert schedule.reservation[0].week == 62 +class TestTriStateFlags: + """Status flags the device may decline to report. + + The device encodes these as 0 = UNKNOWN, 1 = OFF, 2 = ON. The vendor's + own client decodes exactly this set through ``KDEnum.MgppOnOFFFlag``, + declared ``UNKNOWN(0), OFF(1), ON(2)``, so 0 must not become False. + """ + + #: Fields the NaviLink app decodes through its on/off enum. Kept here so + #: the evidenced set is asserted rather than assumed. + TRISTATE_FIELDS = ( + "operation_busy", + "comp_use", + "anti_legionella_use", + "anti_legionella_operation_busy", + "heat_upper_use", + "heat_lower_use", + "air_filter_alarm_use", + "recirc_reservation_use", + ) + + ALIASES: ClassVar[dict[str, str]] = { + "operation_busy": "operationBusy", + "comp_use": "compUse", + "anti_legionella_use": "antiLegionellaUse", + "anti_legionella_operation_busy": "antiLegionellaOperationBusy", + "heat_upper_use": "heatUpperUse", + "heat_lower_use": "heatLowerUse", + "air_filter_alarm_use": "airFilterAlarmUse", + "recirc_reservation_use": "recircReservationUse", + } + + @pytest.mark.parametrize("field", TRISTATE_FIELDS) + def test_zero_becomes_none(self, default_status_data, field): + """0 surfaces as None rather than a fabricated OFF.""" + data = dict(default_status_data) + data[self.ALIASES[field]] = 0 + assert getattr(DeviceStatus(**data), field) is None + + @pytest.mark.parametrize("field", TRISTATE_FIELDS) + def test_one_is_off(self, default_status_data, field): + """1 is a real OFF and stays False.""" + data = dict(default_status_data) + data[self.ALIASES[field]] = 1 + assert getattr(DeviceStatus(**data), field) is False + + @pytest.mark.parametrize("field", TRISTATE_FIELDS) + def test_two_is_on(self, default_status_data, field): + """2 is a real ON and stays True.""" + data = dict(default_status_data) + data[self.ALIASES[field]] = 2 + assert getattr(DeviceStatus(**data), field) is True + + def test_unknown_is_falsy_but_distinguishable(self, default_status_data): + """None is falsy, so naive `if flag:` keeps its old behaviour. + + The migration risk is the opposite check: `if not flag` and + `flag is False` no longer mean the same thing. + """ + data = dict(default_status_data) + data["compUse"] = 0 + status = DeviceStatus(**data) + assert not status.comp_use + assert status.comp_use is not False + + def test_capability_flags_are_unaffected(self, default_status_data): + """Plain DeviceBool status fields still collapse 0 to False.""" + data = dict(default_status_data) + data["dhwUse"] = 0 + assert DeviceStatus(**data).dhw_use is False + + class TestEnergyFields: """The energy capacity fields were misnamed and mis-scaled.