diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 93b0ae01..511ce857 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -21,6 +21,30 @@ Fixed result/exception is always retrieved and logged at debug level instead of leaking as an unhandled asyncio warning. +Documentation +------------- +- **Unit-system preference is a deliberate process-wide global** (`#103 + `_): clarified and + locked in that the preference in ``nwp500.unit_system`` is an intentional + process-wide module-level global rather than a + ``contextvars.ContextVar``. A context-local value silently reverted to + auto-detect for real-time data because MQTT message handling runs in tasks + scheduled from AWS CRT callback threads whose context never inherits from + the application task. Added prominent docstring notes to the affected + models (``DeviceStatus``, ``DeviceFeature``, ``ReservationEntry``, + ``WeeklyReservationEntry``, and ``TOUPeriod``) explaining that unit-aware + computed fields read the preference at access time and share it across all + async tasks and threads. This is a non-breaking documentation and test + change; no API changes. + +Added +----- +- **Tests for process-wide unit-system semantics** (`#103 + `_): new + ``tests/test_unit_system_process_wide.py`` verifies that setting the global + preference affects already-constructed model instances at access time and + that the preference is visible across async tasks and threads. + Version 9.0.0 (2026-07-05) ========================== diff --git a/src/nwp500/models/feature.py b/src/nwp500/models/feature.py index 36bd7d7a..c9f8f64d 100644 --- a/src/nwp500/models/feature.py +++ b/src/nwp500/models/feature.py @@ -22,7 +22,19 @@ class DeviceFeature(NavienBaseModel): - """Device capabilities, configuration, and firmware info.""" + """Device capabilities, configuration, and firmware info. + + Unit-aware note: + Temperature computed fields (e.g. ``dhw_temperature_min``, + ``dhw_temperature_max``, ``recirc_temperature_min``) read the + *process-wide* unit-system preference via + :func:`nwp500.unit_system.get_unit_system` at access time, not at + construction time. Changing the preference with + :func:`nwp500.unit_system.set_unit_system` therefore affects values + read from already-constructed instances, and the preference is shared + across every async task and thread rather than being context-local + (see issue #103). + """ # IMPORTANT: temperature_type must remain the first field so computed # temperature properties can fall back to the device's native unit setting. @@ -308,7 +320,13 @@ class DeviceFeature(NavienBaseModel): ) def _is_celsius(self) -> bool: - """Return True if metric/Celsius units should be used.""" + """Return True if metric/Celsius units should be used. + + Reads the process-wide unit-system preference at call time and falls + back to the device's native ``temperature_type`` when no preference is + set. Every unit-aware computed property routes through this helper, so + they all reflect the current global preference. + """ unit_system = get_unit_system() if unit_system is not None: return unit_system == "metric" diff --git a/src/nwp500/models/schedule.py b/src/nwp500/models/schedule.py index 6483dcb7..91a1a528 100644 --- a/src/nwp500/models/schedule.py +++ b/src/nwp500/models/schedule.py @@ -25,6 +25,16 @@ class ReservationEntry(NavienBaseModel): - min: 0-59 - mode: DHW operation mode ID (1-6) - param: temperature in half-degrees Celsius + + Unit-aware note: + The ``temperature`` and ``unit`` computed fields read the + *process-wide* unit-system preference via + :func:`nwp500.unit_system.get_unit_system` at access time, not at + construction time. Changing the preference with + :func:`nwp500.unit_system.set_unit_system` therefore affects values + read from already-constructed instances, and the preference is shared + across every async task and thread rather than being context-local + (see issue #103). """ enable: int = 2 @@ -137,6 +147,16 @@ class WeeklyReservationEntry(NavienBaseModel): - min: 0-59 - mode: DHW operation mode ID (1-6) - param: temperature in half-degrees Celsius + + Unit-aware note: + The ``temperature`` and ``unit`` computed fields read the + *process-wide* unit-system preference via + :func:`nwp500.unit_system.get_unit_system` at access time, not at + construction time. Changing the preference with + :func:`nwp500.unit_system.set_unit_system` therefore affects values + read from already-constructed instances, and the preference is shared + across every async task and thread rather than being context-local + (see issue #103). """ enable: int = 2 diff --git a/src/nwp500/models/status.py b/src/nwp500/models/status.py index 7865693f..1c231502 100644 --- a/src/nwp500/models/status.py +++ b/src/nwp500/models/status.py @@ -36,7 +36,19 @@ class DeviceStatus(NavienBaseModel): - """Represents the status of the Navien water heater device.""" + """Represents the status of the Navien water heater device. + + Unit-aware note: + Temperature computed fields (e.g. ``dhw_temperature``, + ``outside_temperature``, ``dhw_temperature_setting``) read the + *process-wide* unit-system preference via + :func:`nwp500.unit_system.get_unit_system` at access time, not at + construction time. Changing the preference with + :func:`nwp500.unit_system.set_unit_system` therefore affects values + read from already-constructed instances, and the preference is shared + across every async task and thread rather than being context-local + (see issue #103). + """ # CRITICAL: temperature_type must remain the first field so computed # temperature properties can fall back to the device's native unit setting. @@ -586,7 +598,13 @@ class DeviceStatus(NavienBaseModel): ) def _is_celsius(self) -> bool: - """Return True if metric/Celsius units should be used.""" + """Return True if metric/Celsius units should be used. + + Reads the process-wide unit-system preference at call time and falls + back to the device's native ``temperature_type`` when no preference is + set. Every unit-aware computed property routes through this helper, so + they all reflect the current global preference. + """ unit_system = get_unit_system() if unit_system is not None: return unit_system == "metric" diff --git a/src/nwp500/models/tou.py b/src/nwp500/models/tou.py index 65009e66..3efe7249 100644 --- a/src/nwp500/models/tou.py +++ b/src/nwp500/models/tou.py @@ -68,6 +68,14 @@ class TOUPeriod(NavienBaseModel): - priceMin / priceMax: encoded integer prices (divide by 10^decimalPoint) - decimalPoint: number of decimal places for price values + + Unit-aware note: + Unlike temperature-bearing models such as + :class:`~nwp500.models.status.DeviceStatus`, the computed price and + time fields here are independent of the process-wide unit-system + preference (see :func:`nwp500.unit_system.get_unit_system` and issue + #103); pricing is always decoded the same way regardless of the + configured unit system. """ season: int = 0 diff --git a/tests/test_unit_system_process_wide.py b/tests/test_unit_system_process_wide.py new file mode 100644 index 00000000..288754f5 --- /dev/null +++ b/tests/test_unit_system_process_wide.py @@ -0,0 +1,133 @@ +"""Tests locking in the process-wide unit-system semantics (issue #103). + +The unit-system preference in :mod:`nwp500.unit_system` is a deliberate +process-wide module-level global rather than a :class:`contextvars.ContextVar`. +These tests document and lock in that behaviour: + +* Setting the global preference affects unit-aware computed fields on + already-constructed model instances at access time. +* The preference is visible across async tasks and threads, i.e. a value set + in one task/thread is observed by a model read in another — demonstrating it + is process-wide, not context-local. +""" + +import asyncio +import threading +from typing import Any + +import pytest + +from nwp500.models import ReservationEntry +from nwp500.unit_system import ( + get_unit_system, + reset_unit_system, + set_unit_system, +) + + +@pytest.fixture(autouse=True) +def _reset_unit_system() -> Any: + """Ensure global unit-system state does not leak between tests.""" + reset_unit_system() + try: + yield + finally: + reset_unit_system() + + +def test_set_unit_system_affects_existing_instance_at_access_time( + device_status_dict: dict[str, Any], +): + """Changing the global preference affects a pre-built instance's fields.""" + from nwp500.models import DeviceStatus + + data = device_status_dict.copy() + data["dhwTemperature"] = 120 # 60.0°C -> 140.0°F + + status = DeviceStatus.model_validate(data) + + set_unit_system("metric") + assert status.dhw_temperature == 60.0 + + set_unit_system("us_customary") + assert status.dhw_temperature == 140.0 + + reset_unit_system() + # Falls back to the device's native temperature_type (Fahrenheit here). + assert status.dhw_temperature == 140.0 + + +def test_reservation_entry_unit_follows_global_preference(): + """ReservationEntry temperature/unit reflect the global preference.""" + entry = ReservationEntry(param=120) # 60.0°C + + set_unit_system("metric") + assert entry.temperature == 60.0 + assert entry.unit == "°C" + + set_unit_system("us_customary") + assert entry.temperature == 140.0 + assert entry.unit == "°F" + + +def test_preference_set_in_one_task_seen_in_another_task(): + """A preference set in one asyncio task is visible in another task. + + A ``contextvars.ContextVar`` would isolate the value per task; the + process-wide global is shared, so the reader task observes the writer's + setting. + """ + + async def scenario() -> tuple[float, str]: + entry = ReservationEntry(param=120) # 60.0°C + writer_done = asyncio.Event() + + async def writer() -> None: + set_unit_system("metric") + writer_done.set() + + async def reader() -> tuple[float, str]: + await writer_done.wait() + return entry.temperature, entry.unit + + writer_task = asyncio.create_task(writer()) + reader_task = asyncio.create_task(reader()) + await writer_task + return await reader_task + + temperature, unit = asyncio.run(scenario()) + assert temperature == 60.0 + assert unit == "°C" + + +def test_preference_set_in_one_thread_seen_in_another_thread(): + """A preference set in one thread is visible from another thread. + + Model validation triggered by MQTT callbacks runs in AWS CRT callback + threads. This confirms the preference is shared across threads rather than + being isolated per-thread/context. + """ + entry = ReservationEntry(param=120) # 60.0°C + set_ready = threading.Event() + results: dict[str, Any] = {} + + def writer() -> None: + set_unit_system("metric") + set_ready.set() + + def reader() -> None: + set_ready.wait(timeout=5) + results["seen_preference"] = get_unit_system() + results["temperature"] = entry.temperature + results["unit"] = entry.unit + + writer_thread = threading.Thread(target=writer) + reader_thread = threading.Thread(target=reader) + reader_thread.start() + writer_thread.start() + writer_thread.join() + reader_thread.join() + + assert results["seen_preference"] == "metric" + assert results["temperature"] == 60.0 + assert results["unit"] == "°C"