From 16a9f89a60f3a61ab1700c1c72ecd5abe4e73bfd Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Thu, 2 Jul 2026 15:20:50 -0700 Subject: [PATCH 1/9] fix: detect proto3 sub-message absence with HasField in model parsing Truthiness and getattr-with-default absence checks never fire on real protobuf messages (unset sub-messages return truthy default instances), so sparse stream diffs zeroed room state, IDU controls, sensor readings, and controller temperatures on merge. Parse absence via HasField-based helpers, and preserve identity/relationship fields, controller temperatures, ODU performance data, and QSM led color in the SystemSnapshot.apply_* merges. Add real-proto round-trip merge tests that reproduce the notifier stream's serialize/parse path. --- src/quilt_hp/models/_helpers.py | 40 +++ src/quilt_hp/models/comfort.py | 12 +- src/quilt_hp/models/controller.py | 88 ++++--- src/quilt_hp/models/indoor_unit.py | 136 +++++++--- src/quilt_hp/models/outdoor_unit.py | 49 ++-- src/quilt_hp/models/qsm.py | 38 ++- src/quilt_hp/models/sensor.py | 61 +++-- src/quilt_hp/models/space.py | 97 +++++-- src/quilt_hp/models/system.py | 128 ++++++++-- tests/test_models_from_proto.py | 20 +- tests/test_models_real_proto_merge.py | 352 ++++++++++++++++++++++++++ 11 files changed, 823 insertions(+), 198 deletions(-) create mode 100644 tests/test_models_real_proto_merge.py diff --git a/src/quilt_hp/models/_helpers.py b/src/quilt_hp/models/_helpers.py index f9d754d..02ba2b3 100644 --- a/src/quilt_hp/models/_helpers.py +++ b/src/quilt_hp/models/_helpers.py @@ -1,5 +1,45 @@ from __future__ import annotations +from datetime import UTC, datetime + +from quilt_hp.const import PROTO_TIMESTAMP_UNSET_SECONDS + + +def proto_has_field(proto: object, name: str) -> bool: + """Return True when a message-typed field is present on the wire. + + Works with generated protobuf messages (via ``HasField``) and with + lightweight test stubs (attribute exists and is not ``None``). proto3 + scalar fields without presence semantics (``HasField`` raises + ``ValueError``) fall back to the attribute check. + + This is the only reliable proto3 absence test: truthiness does not work + because an unset sub-message returns a truthy default instance, and + ``getattr(msg, field, None)`` never returns ``None`` for real protos. + """ + has_field = getattr(proto, "HasField", None) + if callable(has_field): + try: + return bool(has_field(name)) + except ValueError: + pass # field without explicit presence — fall back to attribute check + return getattr(proto, name, None) is not None + + +def present_submsg(proto: object, name: str) -> object | None: + """Return the sub-message when present on the wire, else ``None``.""" + return getattr(proto, name) if proto_has_field(proto, name) else None + + +def timestamp_or_none(ts: object) -> datetime | None: + """Convert a proto Timestamp to a datetime, or None when unset/absent.""" + if ts is None: + return None + seconds = getattr(ts, "seconds", PROTO_TIMESTAMP_UNSET_SECONDS) + if seconds == PROTO_TIMESTAMP_UNSET_SECONDS: + return None + return datetime.fromtimestamp(seconds, tz=UTC) + def _id_variant_keys(raw: str) -> tuple[str, ...]: """Return ID variant keys in deterministic priority order (exact → tail → casefold).""" diff --git a/src/quilt_hp/models/comfort.py b/src/quilt_hp/models/comfort.py index d1f5c96..129d6a3 100644 --- a/src/quilt_hp/models/comfort.py +++ b/src/quilt_hp/models/comfort.py @@ -33,7 +33,15 @@ class ComfortSetting: cooling_setpoint_c: float fan_speed: FanSpeed louver_mode: LouverMode = LouverMode.UNSPECIFIED - louver_fixed_position: float = 0.0 # degrees, used when louver_mode=FIXED + # Position fraction 0.20–1.00 (see LouverAngle.to_wire), used when + # louver_mode=FIXED. 0.0 is the "not applicable" placeholder. + louver_fixed_position: float = 0.0 + # Raw wire FAN_SPEED_MODE / FAN_SPEED_PERCENT values. Needed because + # FanSpeed.from_wire(0, 0.0) (absent) and from_wire(1, 0.0) (AUTO) both + # decode to FanSpeed.AUTO; echoing the raw values back on update avoids + # converting "absent" into an explicit AUTO write. + fan_speed_mode_raw: int = 0 + fan_speed_percent_raw: float = 0.0 @property def has_standby_sentinel_setpoints(self) -> bool: @@ -79,6 +87,8 @@ def from_proto(cls, proto: object) -> ComfortSetting: heating_setpoint_c=a.heating_temperature_setpoint_c, cooling_setpoint_c=a.cooling_temperature_setpoint_c, fan_speed=FanSpeed.from_wire(a.fan_speed_mode, a.fan_speed_percent), + fan_speed_mode_raw=a.fan_speed_mode, + fan_speed_percent_raw=a.fan_speed_percent, louver_mode=LouverMode(a.louver_mode) if a.louver_mode else LouverMode.UNSPECIFIED, louver_fixed_position=a.louver_fixed_position, ) diff --git a/src/quilt_hp/models/controller.py b/src/quilt_hp/models/controller.py index 31445d4..6210cd1 100644 --- a/src/quilt_hp/models/controller.py +++ b/src/quilt_hp/models/controller.py @@ -6,8 +6,12 @@ from datetime import UTC, datetime from typing import Any, cast -from quilt_hp.const import PROTO_TIMESTAMP_UNSET_SECONDS -from quilt_hp.models._helpers import lookup_hardware, parse_wifi_state +from quilt_hp.models._helpers import ( + lookup_hardware, + parse_wifi_state, + present_submsg, + timestamp_or_none, +) from quilt_hp.models.enums import LocalCommsHealthStatus, RemoteSensorControlMode from quilt_hp.models.qsm import WifiInfo @@ -22,10 +26,12 @@ class Controller: system_id: str space_id: str name: str - raw_thermistor_c: float # ambient_temperature_c from raw Dial thermistor - pcb_temperature_a_c: float # temperature_f3 — PCB temp A (~30–50°C) - pcb_temperature_b_c: float # temperature_f4 — PCB temp B (hotter component, ~45–52°C) - calibrated_ambient_c: float # temperature_f5 — calibrated ext ambient sent to IDU + # Temperatures are None when the ``state`` sub-message was absent from a + # sparse stream diff; SystemSnapshot.apply_controller preserves them. + raw_thermistor_c: float | None # ambient_temperature_c from raw Dial thermistor + pcb_temperature_a_c: float | None # temperature_f3 — PCB temp A (~30–50°C) + pcb_temperature_b_c: float | None # temperature_f4 — PCB temp B (hotter component, ~45–52°C) + calibrated_ambient_c: float | None # temperature_f5 — calibrated ext ambient sent to IDU wifi_ssid: str | None wifi_ip: str | None wifi_signal_dbm: int | None @@ -51,11 +57,12 @@ class Controller: """ @property - def ambient_temperature_c(self) -> float: + def ambient_temperature_c(self) -> float | None: """Calibrated ambient temperature used for system control. Use this for display and logic. See also ``raw_thermistor_c`` for the - uncorrected on-chip reading (biased high by self-heating). + uncorrected on-chip reading (biased high by self-heating). ``None`` + when no state reading is available (e.g. unmerged stream diff). """ return self.calibrated_ambient_c @@ -92,27 +99,36 @@ def from_proto(cls, proto: object, hw_map: dict[str, object] | None = None) -> C load time. Stream diffs won't have it; fields default to None. """ p = cast("Any", proto) - w = p.hosted_wifi_state - ts = p.state.updated_ts - updated_at: datetime | None = None - if ts.seconds != PROTO_TIMESTAMP_UNSET_SECONDS: - updated_at = datetime.fromtimestamp(ts.seconds, tz=UTC) + st = cast("Any", present_submsg(proto, "state")) + updated_at = timestamp_or_none(getattr(st, "updated_ts", None)) if st is not None else None - wifi_last_seen: datetime | None = None - if w.updated_ts.seconds != PROTO_TIMESTAMP_UNSET_SECONDS: - wifi_last_seen = datetime.fromtimestamp(w.updated_ts.seconds, tz=UTC) + w = cast("Any", present_submsg(proto, "hosted_wifi_state")) + wifi_last_seen = ( + timestamp_or_none(getattr(w, "updated_ts", None)) if w is not None else None + ) - def _wifi(wstate: object) -> WifiInfo | None: + def _wifi(wstate: object | None) -> WifiInfo | None: + if wstate is None: + return None info = WifiInfo.from_proto(wstate) return info if info.connected else None - wifi_ssid, wifi_ip, wifi_signal_dbm = parse_wifi_state(w) + if w is not None: + wifi_ssid, wifi_ip, wifi_signal_dbm = parse_wifi_state(w) + wifi_freq_mhz = w.frequency_mhz or None + else: + wifi_ssid, wifi_ip, wifi_signal_dbm = None, None, None + wifi_freq_mhz = None + + rel = cast("Any", present_submsg(proto, "relationships")) + controls = cast("Any", present_submsg(proto, "controls")) + settings = cast("Any", present_submsg(proto, "settings")) serial: str | None = None model_sku: str | None = None fw_ver: str | None = None - if hw_map: - hw = lookup_hardware(hw_map, p.relationships.hardware_id) + if hw_map and rel is not None: + hw = lookup_hardware(hw_map, rel.hardware_id) if hw is not None: a = cast("Any", hw).attributes serial = a.serial_number or None @@ -122,22 +138,30 @@ def _wifi(wstate: object) -> WifiInfo | None: return cls( id=p.header.object_id, system_id=p.header.system_id, - space_id=p.relationships.space_id, - name=p.settings.name, - raw_thermistor_c=p.state.ambient_temperature_c, - pcb_temperature_a_c=p.state.temperature_f3, - pcb_temperature_b_c=p.state.temperature_f4, - calibrated_ambient_c=p.state.temperature_f5, + space_id=rel.space_id if rel is not None else "", + name=settings.name if settings is not None else "", + raw_thermistor_c=st.ambient_temperature_c if st is not None else None, + pcb_temperature_a_c=st.temperature_f3 if st is not None else None, + pcb_temperature_b_c=st.temperature_f4 if st is not None else None, + calibrated_ambient_c=st.temperature_f5 if st is not None else None, wifi_ssid=wifi_ssid, wifi_ip=wifi_ip, wifi_signal_dbm=wifi_signal_dbm, - wifi_freq_mhz=w.frequency_mhz or None, + wifi_freq_mhz=wifi_freq_mhz, wifi_last_seen=wifi_last_seen, - ap_wifi=_wifi(p.ap_wifi_state), - p2p_wifi=_wifi(p.p2p_wifi_state), - remote_sensor_mode=RemoteSensorControlMode(p.controls.remote_sensor_control_mode), - software_update_info_id=p.relationships.software_update_info_id or None, - firmware_update_info_id=p.relationships.firmware_update_info_id or None, + ap_wifi=_wifi(present_submsg(proto, "ap_wifi_state")), + p2p_wifi=_wifi(present_submsg(proto, "p2p_wifi_state")), + remote_sensor_mode=( + RemoteSensorControlMode(controls.remote_sensor_control_mode) + if controls is not None + else RemoteSensorControlMode.UNSPECIFIED + ), + software_update_info_id=( + (rel.software_update_info_id or None) if rel is not None else None + ), + firmware_update_info_id=( + (rel.firmware_update_info_id or None) if rel is not None else None + ), serial_number=serial, model_sku=model_sku, firmware_version=fw_ver, diff --git a/src/quilt_hp/models/indoor_unit.py b/src/quilt_hp/models/indoor_unit.py index a885f75..918d30c 100644 --- a/src/quilt_hp/models/indoor_unit.py +++ b/src/quilt_hp/models/indoor_unit.py @@ -4,12 +4,13 @@ from dataclasses import dataclass from datetime import UTC, datetime +from typing import Any, cast from quilt_hp.const import ( ABSENT_FAN_SPEED_MODE_SENTINEL, LOUVER_FIXED_POSITION_SENTINEL, - PROTO_TIMESTAMP_UNSET_SECONDS, ) +from quilt_hp.models._helpers import present_submsg, timestamp_or_none from quilt_hp.models.enums import ( FallbackControlCommand, FanSpeed, @@ -279,17 +280,20 @@ def effective_occupancy_state(self) -> int | None: def _idu_from_proto(proto: object) -> IndoorUnit: - """Internal: convert a proto IndoorUnit to our model.""" + """Internal: convert a proto IndoorUnit to our model. + + Sub-messages absent from a sparse stream diff parse to ``None`` (for + optional model fields) or sentinel defaults (for ``controls``/``state``/ + ``settings``) that ``SystemSnapshot.apply_indoor_unit`` uses to preserve + existing snapshot data. Presence is detected with ``HasField`` — proto3 + truthiness cannot detect absent sub-messages. + """ from quilt_hp.models.enums import HVACMode, HVACState - c = proto.controls # type: ignore[attr-defined] - s = proto.state # type: ignore[attr-defined] - st = proto.settings # type: ignore[attr-defined] - pd = proto.performance_data # type: ignore[attr-defined] - pm = proto.performance_metrics # type: ignore[attr-defined] - + pd = present_submsg(proto, "performance_data") perf_data = None - if pd.updated_ts: + if pd is not None: + pd = cast("Any", pd) perf_data = IndoorUnitPerformanceData( measurement_interval_s=pd.measurement_interval_s, energy_measurement_j=pd.energy_measurement_j, @@ -304,8 +308,10 @@ def _idu_from_proto(proto: object) -> IndoorUnit: liquid_pipe_temperature_c=pd.liquid_pipe_temperature_c, ) + pm = present_submsg(proto, "performance_metrics") perf_metrics = None - if pm.updated_ts: + if pm is not None: + pm = cast("Any", pm) perf_metrics = IndoorUnitPerformanceMetrics( capacity_w=pm.capacity_w, coefficient_of_performance=pm.coefficient_of_performance, @@ -320,8 +326,9 @@ def _idu_from_proto(proto: object) -> IndoorUnit: ) hvac_inputs = None - hi = proto.hvac_inputs # type: ignore[attr-defined] - if hi.updated_ts: + hi = present_submsg(proto, "hvac_inputs") + if hi is not None: + hi = cast("Any", hi) hvac_inputs = IndoorUnitHvacInputs( external_ambient_temperature_c=hi.external_ambient_temperature_c, ambient_temperature_source=hi.ambient_temperature_source, @@ -332,15 +339,17 @@ def _idu_from_proto(proto: object) -> IndoorUnit: ) commands = None - cmd = proto.commands # type: ignore[attr-defined] - if cmd.updated_ts: + cmd = present_submsg(proto, "commands") + if cmd is not None: + cmd = cast("Any", cmd) commands = IndoorUnitCommands( fallback_control_command=FallbackControlCommand(cmd.fallback_control_command), ) conditions = None - co = proto.conditions # type: ignore[attr-defined] - if co.updated_ts: + co = present_submsg(proto, "conditions") + if co is not None: + co = cast("Any", co) conditions = IndoorUnitConditions( mode_conflict=co.mode_conflict, anti_cold_wind=co.anti_cold_wind, @@ -356,26 +365,26 @@ def _idu_from_proto(proto: object) -> IndoorUnit: ) presence_state = None - if hasattr(proto, "presence") and proto.presence.updated_ts: + pres = present_submsg(proto, "presence") + if pres is not None: + pres = cast("Any", pres) presence_state = IndoorUnitPresence( - sensor0_presence=Presence(proto.presence.sensor0_presence), - sensor1_presence=Presence(proto.presence.sensor1_presence), + sensor0_presence=Presence(pres.sensor0_presence), + sensor1_presence=Presence(pres.sensor1_presence), ) occupancy_state = None - if hasattr(proto, "occupancy") and proto.occupancy.updated_ts: + occ = present_submsg(proto, "occupancy") + if occ is not None: + occ = cast("Any", occ) occupancy_state = IndoorUnitOccupancy( - occupancy_state=proto.occupancy.occupancy_state, + occupancy_state=occ.occupancy_state, ) - return IndoorUnit( - id=proto.header.object_id, # type: ignore[attr-defined] - system_id=proto.header.system_id, # type: ignore[attr-defined] - space_id=proto.relationships.space_id, # type: ignore[attr-defined] - outdoor_unit_id=proto.relationships.outdoor_unit_id or None, # type: ignore[attr-defined] - hardware_id=proto.relationships.hardware_id, # type: ignore[attr-defined] - qsm_id=proto.relationships.quilt_smart_module_id or None, # type: ignore[attr-defined] - settings=IndoorUnitSettings( + st = present_submsg(proto, "settings") + if st is not None: + st = cast("Any", st) + settings = IndoorUnitSettings( name=st.name, description=st.description, light_brightness_default_percent=st.light_brightness_default_percent, @@ -383,8 +392,22 @@ def _idu_from_proto(proto: object) -> IndoorUnit: presence_fence_right_m=st.presence_fence_right_m, presence_fence_forward_m=st.presence_fence_forward_m, radar_sensor_distance_from_floor_m=st.radar_sensor_distance_from_floor_m, - ), - controls=IndoorUnitControls( + ) + else: + settings = IndoorUnitSettings( + name="", + description="", + light_brightness_default_percent=0.0, + presence_fence_left_m=0.0, + presence_fence_right_m=0.0, + presence_fence_forward_m=0.0, + radar_sensor_distance_from_floor_m=0.0, + ) + + c = present_submsg(proto, "controls") + if c is not None: + c = cast("Any", c) + controls = IndoorUnitControls( fan_speed=FanSpeed.from_wire(c.fan_speed_mode, c.fan_speed_percent), fan_speed_mode_raw=c.fan_speed_mode, fan_speed_percent_raw=c.fan_speed_percent, @@ -396,8 +419,22 @@ def _idu_from_proto(proto: object) -> IndoorUnit: led_brightness=c.led_color_brightness_percent, led_animation=LedAnimation(c.led_animation), led_state=LightState(c.led_state), - ), - state=IndoorUnitState( + ) + else: + # All-sentinel controls: apply_indoor_unit detects and preserves. + controls = IndoorUnitControls( + fan_speed=FanSpeed.from_wire(0, 0.0), + louver_mode=LouverMode.UNSPECIFIED, + louver_fixed_position=LOUVER_FIXED_POSITION_SENTINEL, + led_color_code=0, + led_brightness=0.0, + led_animation=LedAnimation.UNSPECIFIED, + ) + + s = present_submsg(proto, "state") + if s is not None: + s = cast("Any", s) + state = IndoorUnitState( hvac_mode=HVACMode(s.hvac_mode), hvac_state=HVACState(s.hvac_state), ambient_temperature_c=s.ambient_temperature_c, @@ -411,12 +448,31 @@ def _idu_from_proto(proto: object) -> IndoorUnit: outlet_temperature_c=s.outlet_temperature_c, calculated_ambient_temperature_c=s.calculated_ambient_temperature_c, louver_angle_up_down_degrees=s.louver_angle_up_down_degrees, - updated_at=( - datetime.fromtimestamp(s.updated_ts.seconds, tz=UTC) - if s.updated_ts and s.updated_ts.seconds != PROTO_TIMESTAMP_UNSET_SECONDS - else None - ), - ), + updated_at=timestamp_or_none(getattr(s, "updated_ts", None)), + ) + else: + state = IndoorUnitState( + hvac_mode=HVACMode.UNSPECIFIED, + hvac_state=HVACState.UNSPECIFIED, + ambient_temperature_c=0.0, + ambient_humidity_percent=0.0, + fan_speed_rpm=0.0, + fan_speed_setpoint_rpm=0.0, + presence_detection_level=0.0, + ) + + rel = cast("Any", present_submsg(proto, "relationships")) + p = cast("Any", proto) + return IndoorUnit( + id=p.header.object_id, + system_id=p.header.system_id, + space_id=rel.space_id if rel is not None else "", + outdoor_unit_id=(rel.outdoor_unit_id or None) if rel is not None else None, + hardware_id=rel.hardware_id if rel is not None else "", + qsm_id=(rel.quilt_smart_module_id or None) if rel is not None else None, + settings=settings, + controls=controls, + state=state, hvac_inputs=hvac_inputs, conditions=conditions, performance_data=perf_data, @@ -424,7 +480,7 @@ def _idu_from_proto(proto: object) -> IndoorUnit: presence=presence_state, occupancy=occupancy_state, firmware_update_info_id=( - proto.relationships.firmware_update_info_id or None # type: ignore[attr-defined] + (rel.firmware_update_info_id or None) if rel is not None else None ), commands=commands, ) diff --git a/src/quilt_hp/models/outdoor_unit.py b/src/quilt_hp/models/outdoor_unit.py index 0a8b392..018b6f7 100644 --- a/src/quilt_hp/models/outdoor_unit.py +++ b/src/quilt_hp/models/outdoor_unit.py @@ -3,20 +3,10 @@ from __future__ import annotations from dataclasses import dataclass +from typing import Any, cast -from quilt_hp.models._helpers import lookup_hardware - - -def _has_performance_data(proto: object) -> bool: - if not hasattr(proto, "performance_data"): - return False - has_field = getattr(proto, "HasField", None) - if callable(has_field): - try: - return bool(has_field("performance_data")) - except ValueError: - pass - return True +from quilt_hp.models._helpers import lookup_hardware, present_submsg +from quilt_hp.models.enums import HVACState @dataclass(slots=True) @@ -40,7 +30,7 @@ class OutdoorUnit: id: str system_id: str space_id: str - hvac_state: int + hvac_state: HVACState model_sku: str | None serial_number: str | None firmware_version: str | None @@ -50,12 +40,13 @@ class OutdoorUnit: @classmethod def from_proto(cls, proto: object, hw_map: dict[str, object] | None = None) -> OutdoorUnit: """Construct from a protobuf OutdoorUnit message.""" - hw_id = proto.relationships.hardware_id # type: ignore[attr-defined] - hw = lookup_hardware(hw_map, hw_id) if hw_map else None + rel = cast("Any", present_submsg(proto, "relationships")) + hw_id = rel.hardware_id if rel is not None else None + hw = lookup_hardware(hw_map, hw_id) if hw_map and hw_id else None pd = None - if _has_performance_data(proto): - p = proto.performance_data # type: ignore[attr-defined] + p = cast("Any", present_submsg(proto, "performance_data")) + if p is not None: pd = OutdoorUnitPerformanceData( measurement_interval_s=p.measurement_interval_s, energy_measurement_j=p.energy_measurement_j, @@ -67,16 +58,22 @@ def from_proto(cls, proto: object, hw_map: dict[str, object] | None = None) -> O low_pressure_kpa=p.low_pressure_kpa, ) + st = cast("Any", present_submsg(proto, "state")) + try: + hvac_state = HVACState(st.hvac_state) if st is not None else HVACState.UNSPECIFIED + except ValueError: + hvac_state = HVACState.UNSPECIFIED + return cls( - id=proto.header.object_id, # type: ignore[attr-defined] - system_id=proto.header.system_id, # type: ignore[attr-defined] - space_id=proto.relationships.space_id, # type: ignore[attr-defined] - hvac_state=proto.state.hvac_state, # type: ignore[attr-defined] - model_sku=(hw.attributes.model_sku or None) if hw else None, # type: ignore[attr-defined] - serial_number=(hw.attributes.serial_number or None) if hw else None, # type: ignore[attr-defined] - firmware_version=(hw.attributes.firmware_version or None) if hw else None, # type: ignore[attr-defined] + id=cast("Any", proto).header.object_id, + system_id=cast("Any", proto).header.system_id, + space_id=rel.space_id if rel is not None else "", + hvac_state=hvac_state, + model_sku=(cast("Any", hw).attributes.model_sku or None) if hw else None, + serial_number=(cast("Any", hw).attributes.serial_number or None) if hw else None, + firmware_version=(cast("Any", hw).attributes.firmware_version or None) if hw else None, firmware_update_info_id=( - proto.relationships.firmware_update_info_id or None # type: ignore[attr-defined] + (rel.firmware_update_info_id or None) if rel is not None else None ), performance_data=pd, ) diff --git a/src/quilt_hp/models/qsm.py b/src/quilt_hp/models/qsm.py index e0db7ed..97829de 100644 --- a/src/quilt_hp/models/qsm.py +++ b/src/quilt_hp/models/qsm.py @@ -10,8 +10,9 @@ from __future__ import annotations from dataclasses import dataclass, field +from typing import Any, cast -from quilt_hp.models._helpers import parse_wifi_state +from quilt_hp.models._helpers import parse_wifi_state, present_submsg from quilt_hp.models.enums import LocalCommsHealthStatus @@ -75,12 +76,18 @@ class QuiltSmartModule: @classmethod def from_proto(cls, proto: object) -> QuiltSmartModule: - """Construct from a protobuf QuiltSmartModule message.""" - c = proto.controls # type: ignore[attr-defined] - s = proto.state # type: ignore[attr-defined] + """Construct from a protobuf QuiltSmartModule message. + + Sub-messages absent from a sparse stream diff parse to ``None`` / + sentinel defaults; ``SystemSnapshot.apply_qsm`` preserves existing + snapshot data for them. + """ + p = cast("Any", proto) + c = cast("Any", present_submsg(proto, "controls")) + s = cast("Any", present_submsg(proto, "state")) sensors: QsmSensors | None = None - if s.updated_ts: + if s is not None: sensors = QsmSensors( phase_detected_raw=s.phase_detected_raw, target_detected_raw=s.target_detected_raw, @@ -92,23 +99,26 @@ def from_proto(cls, proto: object) -> QuiltSmartModule: accel_z_raw=s.accel_z_raw, ) - def _wifi(w: object) -> WifiInfo | None: + def _wifi(w: object | None) -> WifiInfo | None: + if w is None: + return None info = WifiInfo.from_proto(w) return info if info.connected else None + rel = cast("Any", present_submsg(proto, "relationships")) return cls( - id=proto.header.object_id, # type: ignore[attr-defined] - system_id=proto.header.system_id, # type: ignore[attr-defined] - led_color_code=c.led_color_code, + id=p.header.object_id, + system_id=p.header.system_id, + led_color_code=c.led_color_code if c is not None else 0, sensors=sensors, - hosted_wifi=_wifi(proto.hosted_wifi_state), # type: ignore[attr-defined] - ap_wifi=_wifi(proto.ap_wifi_state), # type: ignore[attr-defined] - p2p_wifi=_wifi(proto.p2p_wifi_state), # type: ignore[attr-defined] + hosted_wifi=_wifi(present_submsg(proto, "hosted_wifi_state")), + ap_wifi=_wifi(present_submsg(proto, "ap_wifi_state")), + p2p_wifi=_wifi(present_submsg(proto, "p2p_wifi_state")), software_update_info_id=( - proto.relationships.software_update_info_id or None # type: ignore[attr-defined] + (rel.software_update_info_id or None) if rel is not None else None ), firmware_update_info_id=( - proto.relationships.firmware_update_info_id or None # type: ignore[attr-defined] + (rel.firmware_update_info_id or None) if rel is not None else None ), local_comms_health=LocalCommsHealthStatus( getattr(getattr(proto, "local_comms_status", None), "health", 0) diff --git a/src/quilt_hp/models/sensor.py b/src/quilt_hp/models/sensor.py index 466cbb0..46cd22c 100644 --- a/src/quilt_hp/models/sensor.py +++ b/src/quilt_hp/models/sensor.py @@ -11,26 +11,45 @@ from __future__ import annotations from dataclasses import dataclass +from typing import Any, cast +from quilt_hp.models._helpers import present_submsg from quilt_hp.models.enums import RemoteSensorControlMode def _parse_state( - s: object, + s: object | None, ) -> tuple[float | None, float | None, float | None, int | None]: - """Return ambient temp, humidity, battery, and signal from proto state.""" - ambient_temperature_c = getattr(s, "ambient_temperature_c", None) - humidity_percent = getattr(s, "humidity_percent", None) - battery_level_percent = getattr(s, "battery_level_percent", None) - signal_level_dbm = getattr(s, "signal_level_dbm", None) + """Return ambient temp, humidity, battery, and signal from proto state. + + ``s`` is ``None`` when the ``state`` sub-message was absent from the wire + (sparse stream diff) — all readings are then ``None`` so that + ``SystemSnapshot.apply_*`` preserves existing values. + """ + if s is None: + return (None, None, None, None) return ( - ambient_temperature_c if ambient_temperature_c is not None else None, - humidity_percent if humidity_percent is not None else None, - battery_level_percent if battery_level_percent is not None else None, - signal_level_dbm if signal_level_dbm is not None else None, + getattr(s, "ambient_temperature_c", None), + getattr(s, "humidity_percent", None), + getattr(s, "battery_level_percent", None), + getattr(s, "signal_level_dbm", None), ) +def _parse_control_mode(proto: object) -> RemoteSensorControlMode: + controls = cast("Any", present_submsg(proto, "controls")) + if controls is None: + return RemoteSensorControlMode.UNSPECIFIED + return RemoteSensorControlMode(controls.control_mode) + + +def _parse_mac(proto: object) -> str | None: + attributes = cast("Any", present_submsg(proto, "attributes")) + if attributes is None: + return None + return attributes.mac or None + + @dataclass(slots=True) class RemoteSensor: """A standalone BLE remote sensor linked to an IndoorUnit.""" @@ -47,16 +66,17 @@ class RemoteSensor: @classmethod def from_proto(cls, proto: object) -> RemoteSensor: """Construct from a protobuf RemoteSensor message.""" - at, hum, bat, sig = _parse_state(proto.state) # type: ignore[attr-defined] + at, hum, bat, sig = _parse_state(present_submsg(proto, "state")) + rel = cast("Any", present_submsg(proto, "relationships")) return cls( - id=proto.header.object_id, # type: ignore[attr-defined] - indoor_unit_id=proto.relationships.indoor_unit_id, # type: ignore[attr-defined] - mac=proto.attributes.mac or None, # type: ignore[attr-defined] + id=cast("Any", proto).header.object_id, + indoor_unit_id=rel.indoor_unit_id if rel is not None else "", + mac=_parse_mac(proto), ambient_temperature_c=at, humidity_percent=hum, battery_level_percent=bat, signal_level_dbm=sig, - control_mode=RemoteSensorControlMode(proto.controls.control_mode), # type: ignore[attr-defined] + control_mode=_parse_control_mode(proto), ) @@ -81,14 +101,15 @@ class ControllerRemoteSensor: @classmethod def from_proto(cls, proto: object) -> ControllerRemoteSensor: """Construct from a protobuf ControllerRemoteSensor message.""" - at, hum, bat, sig = _parse_state(proto.state) # type: ignore[attr-defined] + at, hum, bat, sig = _parse_state(present_submsg(proto, "state")) + rel = cast("Any", present_submsg(proto, "relationships")) return cls( - id=proto.header.object_id, # type: ignore[attr-defined] - controller_id=proto.relationships.controller_id, # type: ignore[attr-defined] - mac=proto.attributes.mac or None, # type: ignore[attr-defined] + id=cast("Any", proto).header.object_id, + controller_id=rel.controller_id if rel is not None else "", + mac=_parse_mac(proto), ambient_temperature_c=at, humidity_percent=hum, battery_level_percent=bat, signal_level_dbm=sig, - control_mode=RemoteSensorControlMode(proto.controls.control_mode), # type: ignore[attr-defined] + control_mode=_parse_control_mode(proto), ) diff --git a/src/quilt_hp/models/space.py b/src/quilt_hp/models/space.py index 042087b..c5a15a5 100644 --- a/src/quilt_hp/models/space.py +++ b/src/quilt_hp/models/space.py @@ -11,6 +11,7 @@ STANDBY_COOL_SENTINEL_C, STANDBY_HEAT_SENTINEL_C, ) +from quilt_hp.models._helpers import present_submsg from quilt_hp.models.enums import ( BoostMode, ComfortSettingOverride, @@ -77,7 +78,9 @@ def fmt(val_c: float) -> str: return f"{val_c:.1f}°C" mode = self.hvac_mode - if mode in (HVACMode.STANDBY, HVACMode.UNSPECIFIED, HVACMode.FAN): + if mode in (HVACMode.STANDBY, HVACMode.UNSPECIFIED, HVACMode.FAN, HVACMode.DRY): + # DRY has no user-configurable setpoint; temperature_setpoint_c may + # hold a stale mirror value in that mode. return "--" if mode == HVACMode.COOL: return fmt(self.cooling_setpoint_c) @@ -208,15 +211,18 @@ def from_proto(cls, proto: object) -> Space: def _space_from_proto(proto: object) -> Space: - """Internal: convert a proto Space to our model.""" + """Internal: convert a proto Space to our model. + + Sub-messages absent from a sparse stream diff parse to sentinel values + (``settings.name == ""``, ``controls.hvac_mode == UNSPECIFIED``, + ``state.ambient_temperature_c is None``) that + ``SystemSnapshot.apply_space`` uses to preserve existing snapshot data. + """ p = cast("Any", proto) - sg = p.settings - return Space( - id=p.header.object_id, - system_id=p.header.system_id, - name=sg.name, - parent_space_id=p.relationships.parent_space_id or None, - settings=SpaceSettings( + + sg = cast("Any", present_submsg(p, "settings")) + if sg is not None: + settings = SpaceSettings( name=sg.name, timezone=sg.timezone, occupancy_mode=OccupancyMode(sg.occupancy), @@ -224,20 +230,61 @@ def _space_from_proto(proto: object) -> Space: unoccupied_timeout_s=sg.unoccupied_timeout_s, safety_heating=SafetyHeatingMode(sg.safety_heating), hvac_controller_type=HvacControllerType(sg.hvac_controller_type), - ), - controls=SpaceControls( - hvac_mode=HVACMode(p.controls.hvac_mode), - temperature_setpoint_c=p.controls.temperature_setpoint_c, - cooling_setpoint_c=p.controls.cooling_temperature_setpoint_c, - heating_setpoint_c=p.controls.heating_temperature_setpoint_c, - comfort_setting_id=p.controls.comfort_setting_id_string, - comfort_setting_override=ComfortSettingOverride(p.controls.comfort_setting_override), - boost_mode=BoostMode(p.controls.boost_mode), - ), - state=SpaceState( - ambient_temperature_c=p.state.ambient_temperature_c if p.state.updated_ts else None, - hvac_state=HVACState(p.state.hvac_state), - setpoint_c=p.state.setpoint_temperature_c if p.state.updated_ts else None, - comfort_setting_id=p.state.comfort_setting_id, - ), + ) + else: + settings = SpaceSettings( + name="", + timezone="", + occupancy_mode=OccupancyMode.UNSPECIFIED, + occupied_timeout_s=0.0, + unoccupied_timeout_s=0.0, + safety_heating=SafetyHeatingMode.UNSPECIFIED, + ) + + c = cast("Any", present_submsg(p, "controls")) + if c is not None: + controls = SpaceControls( + hvac_mode=HVACMode(c.hvac_mode), + temperature_setpoint_c=c.temperature_setpoint_c, + cooling_setpoint_c=c.cooling_temperature_setpoint_c, + heating_setpoint_c=c.heating_temperature_setpoint_c, + comfort_setting_id=c.comfort_setting_id_string, + comfort_setting_override=ComfortSettingOverride(c.comfort_setting_override), + boost_mode=BoostMode(c.boost_mode), + ) + else: + controls = SpaceControls( + hvac_mode=HVACMode.UNSPECIFIED, + temperature_setpoint_c=0.0, + cooling_setpoint_c=0.0, + heating_setpoint_c=0.0, + comfort_setting_id="", + comfort_setting_override=ComfortSettingOverride.UNSPECIFIED, + ) + + st = cast("Any", present_submsg(p, "state")) + if st is not None: + state = SpaceState( + ambient_temperature_c=st.ambient_temperature_c, + hvac_state=HVACState(st.hvac_state), + setpoint_c=st.setpoint_temperature_c, + comfort_setting_id=st.comfort_setting_id, + ) + else: + state = SpaceState( + ambient_temperature_c=None, + hvac_state=HVACState.UNSPECIFIED, + setpoint_c=None, + comfort_setting_id="", + ) + + rel = cast("Any", present_submsg(p, "relationships")) + return Space( + id=p.header.object_id, + system_id=p.header.system_id, + name=settings.name, + parent_space_id=(rel.parent_space_id or None) if rel is not None else None, + settings=settings, + controls=controls, + state=state, ) diff --git a/src/quilt_hp/models/system.py b/src/quilt_hp/models/system.py index 5e7214e..48801a0 100644 --- a/src/quilt_hp/models/system.py +++ b/src/quilt_hp/models/system.py @@ -2,7 +2,7 @@ from __future__ import annotations -from dataclasses import dataclass +from dataclasses import dataclass, replace from typing import Any, cast from quilt_hp.models._helpers import _id_variants @@ -11,6 +11,7 @@ from quilt_hp.models.enums import ( ComfortSettingType, HVACMode, + HVACState, LightState, LocalCommsHealthStatus, LouverMode, @@ -166,11 +167,9 @@ def apply_space(self, space: Space) -> Space: self.enrich_space(space) for i, s in enumerate(self.spaces): if s.id == space.id: - from dataclasses import replace - updates: dict[str, Any] = {} # state absent: ambient_temperature_c is None - # (set only when updated_ts present) + # (set only when the state sub-message is present on the wire) if ( space.state.ambient_temperature_c is None and s.state.ambient_temperature_c is not None @@ -183,11 +182,20 @@ def apply_space(self, space: Space) -> Space: and s.controls.hvac_mode != HVACMode.UNSPECIFIED ): updates["controls"] = s.controls + # enrich_space resolved the diff's (empty) comfort setting + # id; restore the type matching the preserved controls. + updates["active_comfort_setting_type"] = s.active_comfort_setting_type # settings absent: name is empty (settings.name is always # non-empty in a real Space) if not space.settings.name and s.settings.name: updates["settings"] = s.settings updates["name"] = s.name + # relationships absent: parent_space_id is None — preserve so + # is_room stays valid and the room doesn't vanish from rooms. + if space.parent_space_id is None and s.parent_space_id is not None: + updates["parent_space_id"] = s.parent_space_id + if not space.system_id and s.system_id: + updates["system_id"] = s.system_id if updates: space = replace(space, **updates) self.spaces[i] = space @@ -210,31 +218,41 @@ def apply_indoor_unit(self, idu: IndoorUnit) -> IndoorUnit: """ for i, u in enumerate(self.indoor_units): if u.id == idu.id: - from dataclasses import replace - updates: dict[str, Any] = {} + # relationships absent: identity/link fields are empty/None + if not idu.space_id and u.space_id: + updates["space_id"] = u.space_id + if not idu.hardware_id and u.hardware_id: + updates["hardware_id"] = u.hardware_id + if not idu.system_id and u.system_id: + updates["system_id"] = u.system_id if idu.qsm_id is None and u.qsm_id: updates["qsm_id"] = u.qsm_id if idu.outdoor_unit_id is None and u.outdoor_unit_id: updates["outdoor_unit_id"] = u.outdoor_unit_id + if idu.firmware_update_info_id is None and u.firmware_update_info_id: + updates["firmware_update_info_id"] = u.firmware_update_info_id if idu.hvac_inputs is None and u.hvac_inputs is not None: updates["hvac_inputs"] = u.hvac_inputs if idu.conditions is None and u.conditions is not None: updates["conditions"] = u.conditions + if idu.commands is None and u.commands is not None: + updates["commands"] = u.commands + # settings absent from diff: name is empty + if not idu.settings.name and u.settings.name: + updates["settings"] = u.settings # state absent from diff: updated_at is None — preserve # existing so is_online stays valid if idu.state.updated_at is None and u.state.updated_at is not None: updates["state"] = u.state - # controls absent detection: state IS in diff - # (timestamped state-only update), and all control sentinel - # fields are at proto3 defaults. When controls are genuinely - # sent: led_color_code is non-zero, OR led_state is explicit - # ON/OFF, OR louver_mode is set. Brightness alone is not a - # safe sentinel because mobile_led_scheduling_enabled preserves + # controls absent detection: all control sentinel fields are + # at proto3 defaults. When controls are genuinely sent: + # led_color_code is non-zero, OR led_state is explicit ON/OFF, + # OR louver_mode is set. Brightness alone is not a safe + # sentinel because mobile_led_scheduling_enabled preserves # brightness when LED is OFF. if ( - idu.state.updated_at is not None - and idu.controls.fan_speed_mode_raw == 0 + idu.controls.fan_speed_mode_raw == 0 and idu.controls.louver_mode == LouverMode.UNSPECIFIED and idu.controls.led_color_code == 0 and idu.controls.led_state == LightState.UNSPECIFIED @@ -265,14 +283,22 @@ def apply_outdoor_unit(self, odu: OutdoorUnit) -> OutdoorUnit: lack hardware info (no hw_map available at parse time). Preserve any existing non-default values so partial updates don't erase them. """ - from dataclasses import replace - for i, u in enumerate(self.outdoor_units): if u.id == odu.id: updates: dict[str, Any] = {} # Preserve hvac_state when stream diff has a default-zero state - if not odu.hvac_state and u.hvac_state: + if odu.hvac_state == HVACState.UNSPECIFIED and u.hvac_state: updates["hvac_state"] = u.hvac_state + # Preserve identity/link fields absent from a sparse diff + if not odu.space_id and u.space_id: + updates["space_id"] = u.space_id + if not odu.system_id and u.system_id: + updates["system_id"] = u.system_id + if odu.firmware_update_info_id is None and u.firmware_update_info_id: + updates["firmware_update_info_id"] = u.firmware_update_info_id + # Preserve compressor telemetry when the diff omits it + if odu.performance_data is None and u.performance_data is not None: + updates["performance_data"] = u.performance_data # Preserve hardware info — stream diffs are parsed without hw_map if odu.model_sku is None and u.model_sku is not None: updates["model_sku"] = u.model_sku @@ -293,13 +319,32 @@ def apply_controller(self, ctrl: Controller) -> Controller: Hardware info (serial, model_sku, firmware_version) is only populated at initial snapshot load and is never in stream diffs; always preserve it. """ - from dataclasses import replace - for i, c in enumerate(self.controllers): if c.id == ctrl.id: updates: dict[str, Any] = {} if not ctrl.name and c.name: updates["name"] = c.name + if not ctrl.space_id and c.space_id: + updates["space_id"] = c.space_id + if not ctrl.system_id and c.system_id: + updates["system_id"] = c.system_id + # state absent from diff: temperatures parse to None — preserve + # the last real readings (calibrated_ambient_c is the primary + # display value). + if ctrl.calibrated_ambient_c is None and c.calibrated_ambient_c is not None: + updates["calibrated_ambient_c"] = c.calibrated_ambient_c + if ctrl.raw_thermistor_c is None and c.raw_thermistor_c is not None: + updates["raw_thermistor_c"] = c.raw_thermistor_c + if ctrl.pcb_temperature_a_c is None and c.pcb_temperature_a_c is not None: + updates["pcb_temperature_a_c"] = c.pcb_temperature_a_c + if ctrl.pcb_temperature_b_c is None and c.pcb_temperature_b_c is not None: + updates["pcb_temperature_b_c"] = c.pcb_temperature_b_c + if ctrl.state_updated_at is None and c.state_updated_at is not None: + updates["state_updated_at"] = c.state_updated_at + if ctrl.software_update_info_id is None and c.software_update_info_id: + updates["software_update_info_id"] = c.software_update_info_id + if ctrl.firmware_update_info_id is None and c.firmware_update_info_id: + updates["firmware_update_info_id"] = c.firmware_update_info_id if ctrl.wifi_ssid is None and c.wifi_ssid is not None: updates["wifi_ssid"] = c.wifi_ssid updates["wifi_ip"] = c.wifi_ip @@ -339,11 +384,18 @@ def apply_qsm(self, qsm: QuiltSmartModule) -> QuiltSmartModule: Stream diffs are sparse — a controls diff omits state (sensors) and wifi state sub-messages. Preserve existing non-None values. """ - from dataclasses import replace - for i, q in enumerate(self.quilt_smart_modules): if q.id == qsm.id: updates: dict[str, Any] = {} + if not qsm.system_id and q.system_id: + updates["system_id"] = q.system_id + # controls absent: led_color_code defaults to 0 + if qsm.led_color_code == 0 and q.led_color_code != 0: + updates["led_color_code"] = q.led_color_code + if qsm.software_update_info_id is None and q.software_update_info_id: + updates["software_update_info_id"] = q.software_update_info_id + if qsm.firmware_update_info_id is None and q.firmware_update_info_id: + updates["firmware_update_info_id"] = q.firmware_update_info_id if qsm.sensors is None and q.sensors is not None: updates["sensors"] = q.sensors if qsm.hosted_wifi is None and q.hosted_wifi is not None: @@ -372,11 +424,14 @@ def apply_remote_sensor(self, rs: RemoteSensor) -> RemoteSensor: omits state, zeroing all sensor readings. Preserve existing non-None values. """ - from dataclasses import replace - for i, r in enumerate(self.remote_sensors): if r.id == rs.id: updates: dict[str, Any] = {} + # relationships/attributes absent: link and mac are empty/None + if not rs.indoor_unit_id and r.indoor_unit_id: + updates["indoor_unit_id"] = r.indoor_unit_id + if rs.mac is None and r.mac is not None: + updates["mac"] = r.mac # controls absent: control_mode defaults to UNSPECIFIED. if ( rs.control_mode == RemoteSensorControlMode.UNSPECIFIED @@ -403,11 +458,13 @@ def apply_controller_remote_sensor( self, crs: ControllerRemoteSensor ) -> ControllerRemoteSensor: """Patch a stream-updated ControllerRemoteSensor into the snapshot.""" - from dataclasses import replace - for i, r in enumerate(self.controller_remote_sensors): if r.id == crs.id: updates: dict[str, Any] = {} + if not crs.controller_id and r.controller_id: + updates["controller_id"] = r.controller_id + if crs.mac is None and r.mac is not None: + updates["mac"] = r.mac if ( crs.control_mode == RemoteSensorControlMode.UNSPECIFIED and r.control_mode != RemoteSensorControlMode.UNSPECIFIED @@ -428,6 +485,20 @@ def apply_controller_remote_sensor( self.controller_remote_sensors.append(crs) return crs + def apply_software_update_info(self, sui: SoftwareUpdateInfo) -> SoftwareUpdateInfo: + """Patch a stream-updated SoftwareUpdateInfo into the snapshot. + + Update records are replaced wholesale: an all-empty/zero record is a + legitimate state ("no update pending"), so there is no absence + sentinel to preserve against. + """ + for i, existing in enumerate(self.software_update_infos): + if existing.id == sui.id: + self.software_update_infos[i] = sui + return sui + self.software_update_infos.append(sui) + return sui + def odu_for_idu(self, idu: IndoorUnit) -> OutdoorUnit | None: """Return the OutdoorUnit connected to the given IDU, or None.""" if not idu.outdoor_unit_id: @@ -448,7 +519,12 @@ def stream_topics(self) -> list[str]: """Return the NotifierService topic strings for this snapshot. Pass the result directly to ``client.stream(topics)``. Covers all - rooms, indoor units, outdoor units, and controllers. + rooms, indoor units, outdoor units, controllers, QSMs, remote + sensors, and software-update records. + + Note: comfort settings, schedules, and locations are not delivered + over the notifier stream — re-fetch a snapshot to pick up changes to + those (e.g. after editing presets in the Quilt app). """ topics: list[str] = [] for space in self.rooms: diff --git a/tests/test_models_from_proto.py b/tests/test_models_from_proto.py index b68b54f..99f2e95 100644 --- a/tests/test_models_from_proto.py +++ b/tests/test_models_from_proto.py @@ -191,9 +191,9 @@ def test_space_ambient_temperature_f() -> None: def test_space_ambient_none() -> None: - # When state.updated_ts is falsy (no server state data), temps are None. + # When the state sub-message is absent from the wire, temps are None. proto = _make_space_proto(ambient_c=22.0) - proto.state.updated_ts = None # simulate empty state sub-message + proto.state = None # simulate absent state sub-message space = Space.from_proto(proto) assert space.state.ambient_temperature_c is None assert space.ambient_temperature_f is None @@ -539,7 +539,9 @@ def test_idu_offline_when_state_timestamp_stale() -> None: def test_idu_no_perf_data() -> None: proto = _make_idu_proto() - # performance_data.updated_ts=None → no perf data regardless of field values + # performance sub-messages absent from the wire → no perf data + proto.performance_data = None + proto.performance_metrics = None idu = IndoorUnit.from_proto(proto) assert idu.performance_data is None assert idu.performance_metrics is None @@ -814,17 +816,7 @@ def test_qsm_from_proto_no_sensors() -> None: header=_make_header("qsm-2"), relationships=_ns(software_update_info_id="", firmware_update_info_id=""), controls=_ns(led_color_code=0, updated_ts=None), - state=_ns( - updated_ts=None, - phase_detected_raw=0.0, - target_detected_raw=0.0, - als_illuminance_raw=0, - als_ir_raw=0, - als_both_raw=0, - accel_x_raw=0, - accel_y_raw=0, - accel_z_raw=0, - ), + state=None, # absent state sub-message → no sensor data hosted_wifi_state=_ns(ssid="", ipv4_address="", signal_level_dbm=0), ap_wifi_state=_ns(ssid="", ipv4_address="", signal_level_dbm=0), p2p_wifi_state=_ns(ssid="", ipv4_address="", signal_level_dbm=0), diff --git a/tests/test_models_real_proto_merge.py b/tests/test_models_real_proto_merge.py new file mode 100644 index 0000000..1574923 --- /dev/null +++ b/tests/test_models_real_proto_merge.py @@ -0,0 +1,352 @@ +"""Sparse-diff merge tests using REAL generated protobuf messages. + +These tests reproduce exactly what ``NotifierStream._parse_event`` does: +build a real ``hds.*`` proto, ``SerializeToString`` → ``ParseFromString``, +convert with ``from_proto``, and merge with ``SystemSnapshot.apply_*``. + +They exist because proto3 absence cannot be detected with truthiness or +``getattr`` defaults — only ``HasField`` works — and ``SimpleNamespace`` +stubs cannot catch that class of bug. +""" + +from __future__ import annotations + +import pytest + +from quilt_hp._proto import quilt_hds_pb2 as hds +from quilt_hp.models.controller import Controller +from quilt_hp.models.enums import HVACMode, HVACState, LightState, LouverMode +from quilt_hp.models.indoor_unit import IndoorUnit +from quilt_hp.models.outdoor_unit import OutdoorUnit +from quilt_hp.models.qsm import QuiltSmartModule +from quilt_hp.models.sensor import RemoteSensor +from quilt_hp.models.space import Space +from quilt_hp.models.system import SystemSnapshot + + +def _roundtrip[T](msg: T) -> T: + """Serialize and re-parse a proto, as the notifier stream does.""" + fresh = type(msg)() + fresh.ParseFromString(msg.SerializeToString()) # type: ignore[attr-defined] + return fresh + + +def _empty_snapshot() -> SystemSnapshot: + return SystemSnapshot( + spaces=[], + indoor_units=[], + outdoor_units=[], + controllers=[], + quilt_smart_modules=[], + comfort_settings=[], + schedule_weeks=[], + schedule_days=[], + remote_sensors=[], + controller_remote_sensors=[], + software_update_infos=[], + locations=[], + timezone=None, + ) + + +def _full_space_proto() -> hds.Space: + return hds.Space( + header=hds.EntityMetadata(object_id="space-1", system_id="sys-1"), + relationships=hds.SpaceRelationships(parent_space_id="root-1"), + settings=hds.SpaceSettings(name="Living Room", timezone="UTC"), + controls=hds.SpaceControls( + hvac_mode=hds.HVAC_MODE_HEAT, + temperature_setpoint_c=21.0, + heating_temperature_setpoint_c=21.0, + cooling_temperature_setpoint_c=26.0, + comfort_setting_id_string="cs-1", + ), + state=hds.SpaceState( + ambient_temperature_c=21.7, + hvac_state=hds.HVAC_STATE_HEAT, + setpoint_temperature_c=21.0, + ), + ) + + +def test_real_proto_unset_submessages_parse_as_absent() -> None: + """A diff with only controls set must not fabricate state/settings.""" + diff = _roundtrip( + hds.Space( + header=hds.EntityMetadata(object_id="space-1", system_id="sys-1"), + controls=hds.SpaceControls(hvac_mode=hds.HVAC_MODE_COOL), + ) + ) + space = Space.from_proto(diff) + assert space.state.ambient_temperature_c is None + assert space.state.setpoint_c is None + assert space.settings.name == "" + assert space.controls.hvac_mode == HVACMode.COOL + + +def test_apply_space_controls_only_diff_preserves_state_and_identity() -> None: + snapshot = _empty_snapshot() + full = Space.from_proto(_roundtrip(_full_space_proto())) + snapshot.spaces.append(full) + assert snapshot.rooms # sanity: identity fields intact + + # Controls-only diff (user changed a setpoint) + diff = _roundtrip( + hds.Space( + header=hds.EntityMetadata(object_id="space-1", system_id="sys-1"), + controls=hds.SpaceControls( + hvac_mode=hds.HVAC_MODE_HEAT, + temperature_setpoint_c=22.0, + heating_temperature_setpoint_c=22.0, + cooling_temperature_setpoint_c=26.0, + comfort_setting_id_string="cs-1", + ), + ) + ) + merged = snapshot.apply_space(Space.from_proto(diff)) + + assert merged.controls.heating_setpoint_c == 22.0 # diff applied + assert merged.state.ambient_temperature_c == pytest.approx(21.7) # state preserved + assert merged.state.hvac_state == HVACState.HEAT + assert merged.name == "Living Room" # settings preserved + assert merged.parent_space_id == "root-1" # relationships preserved + assert merged.is_room is True + assert snapshot.rooms # room did not vanish + + +def test_apply_space_state_only_diff_preserves_controls() -> None: + snapshot = _empty_snapshot() + snapshot.spaces.append(Space.from_proto(_roundtrip(_full_space_proto()))) + + diff = _roundtrip( + hds.Space( + header=hds.EntityMetadata(object_id="space-1", system_id="sys-1"), + state=hds.SpaceState( + ambient_temperature_c=23.4, + hvac_state=hds.HVAC_STATE_STANDBY, + ), + ) + ) + merged = snapshot.apply_space(Space.from_proto(diff)) + + assert merged.state.ambient_temperature_c == pytest.approx(23.4) # diff applied + assert merged.controls.hvac_mode == HVACMode.HEAT # controls preserved + assert merged.controls.heating_setpoint_c == 21.0 + assert merged.name == "Living Room" + + +def _full_idu_proto() -> hds.IndoorUnit: + import time + + from google.protobuf.timestamp_pb2 import Timestamp + + ts = Timestamp() + ts.FromSeconds(int(time.time())) + return hds.IndoorUnit( + header=hds.EntityMetadata(object_id="idu-1", system_id="sys-1"), + relationships=hds.IndoorUnitRelationships( + space_id="space-1", + hardware_id="hw-1", + quilt_smart_module_id="qsm-1", + outdoor_unit_id="odu-1", + ), + settings=hds.IndoorUnitSettings(name="Living Room IDU"), + controls=hds.IndoorUnitControls( + fan_speed_mode=2, + fan_speed_percent=0.6, + louver_mode=hds.LOUVER_MODE_SWEEP, + led_color_code=255, + led_color_brightness_percent=0.8, + led_state=hds.LIGHT_STATE_ON, + ), + state=hds.IndoorUnitState( + updated_ts=ts, + hvac_mode=hds.HVAC_MODE_HEAT, + hvac_state=hds.HVAC_STATE_HEAT, + ambient_temperature_c=21.5, + fan_speed_rpm=800.0, + ), + hvac_inputs=hds.IndoorUnitHvacInputs( + external_ambient_temperature_c=20.0, + temperature_setpoint_c=21.0, + ), + ) + + +def test_apply_idu_presence_only_diff_preserves_everything() -> None: + """A presence-only diff must not clobber controls/state/links (C1+H3+H5).""" + snapshot = _empty_snapshot() + snapshot.indoor_units.append(IndoorUnit.from_proto(_roundtrip(_full_idu_proto()))) + + diff = _roundtrip( + hds.IndoorUnit( + header=hds.EntityMetadata(object_id="idu-1", system_id="sys-1"), + presence=hds.IndoorUnitPresenceState(sensor0_presence=1, sensor1_presence=2), + ) + ) + merged = snapshot.apply_indoor_unit(IndoorUnit.from_proto(diff)) + + assert merged.presence is not None # diff applied + # Controls preserved (no state present in the diff either — the old + # merge logic required state to be present and failed here) + assert merged.controls.led_color_code == 255 + assert merged.controls.led_state == LightState.ON + assert merged.controls.louver_mode == LouverMode.SWEEP + assert merged.controls.fan_speed_percent_raw == pytest.approx(0.6) + # State, links, settings preserved + assert merged.state.ambient_temperature_c == 21.5 + assert merged.state.updated_at is not None + assert merged.space_id == "space-1" + assert merged.hardware_id == "hw-1" + assert merged.qsm_id == "qsm-1" + assert merged.outdoor_unit_id == "odu-1" + assert merged.settings.name == "Living Room IDU" + # hvac_inputs preserved (all-zero fabricated object would be a bug) + assert merged.hvac_inputs is not None + assert merged.hvac_inputs.external_ambient_temperature_c == 20.0 + + +def test_apply_controller_sparse_diff_preserves_temperatures() -> None: + full = _roundtrip( + hds.Controller( + header=hds.EntityMetadata(object_id="ctrl-1", system_id="sys-1"), + relationships=hds.ControllerRelationships(space_id="space-1"), + settings=hds.ControllerSettings(name="Dial"), + state=hds.ControllerState( + ambient_temperature_c=24.0, + temperature_f3=35.0, + temperature_f4=45.0, + temperature_f5=19.5, + ), + ) + ) + snapshot = _empty_snapshot() + snapshot.controllers.append(Controller.from_proto(full)) + + # Controls-only diff (remote sensor mode toggle) + diff = _roundtrip( + hds.Controller( + header=hds.EntityMetadata(object_id="ctrl-1", system_id="sys-1"), + controls=hds.ControllerControls(remote_sensor_control_mode=2), + ) + ) + merged = snapshot.apply_controller(Controller.from_proto(diff)) + + assert merged.calibrated_ambient_c == 19.5 # preserved, not zeroed + assert merged.ambient_temperature_c == 19.5 + assert merged.raw_thermistor_c == 24.0 + assert merged.name == "Dial" + assert merged.space_id == "space-1" + + +def test_apply_remote_sensor_controls_only_diff_preserves_readings() -> None: + full = _roundtrip( + hds.RemoteSensor( + header=hds.EntityMetadata(object_id="rs-1", system_id="sys-1"), + relationships=hds.RemoteSensorRelationships(indoor_unit_id="idu-1"), + attributes=hds.RemoteSensorAttributes(mac="AA:BB"), + state=hds.RemoteSensorState( + ambient_temperature_c=22.5, + humidity_percent=45.0, + battery_level_percent=80.0, + ), + ) + ) + snapshot = _empty_snapshot() + snapshot.remote_sensors.append(RemoteSensor.from_proto(full)) + + diff = _roundtrip( + hds.RemoteSensor( + header=hds.EntityMetadata(object_id="rs-1", system_id="sys-1"), + controls=hds.RemoteSensorControls(control_mode=2), + ) + ) + merged = snapshot.apply_remote_sensor(RemoteSensor.from_proto(diff)) + + assert merged.ambient_temperature_c == 22.5 # preserved, not zeroed + assert merged.humidity_percent == 45.0 + assert merged.battery_level_percent == 80.0 + assert merged.indoor_unit_id == "idu-1" + assert merged.mac == "AA:BB" + + +def test_apply_qsm_controls_only_diff_preserves_sensors() -> None: + full = _roundtrip( + hds.QuiltSmartModule( + header=hds.EntityMetadata(object_id="qsm-1", system_id="sys-1"), + controls=hds.QuiltSmartModuleControls(led_color_code=255), + state=hds.QuiltSmartModuleState( + phase_detected_raw=1.5, + als_illuminance_raw=320, + ), + ) + ) + snapshot = _empty_snapshot() + snapshot.quilt_smart_modules.append(QuiltSmartModule.from_proto(full)) + + # State-only diff + diff = _roundtrip( + hds.QuiltSmartModule( + header=hds.EntityMetadata(object_id="qsm-1", system_id="sys-1"), + state=hds.QuiltSmartModuleState(phase_detected_raw=2.0), + ) + ) + merged = snapshot.apply_qsm(QuiltSmartModule.from_proto(diff)) + assert merged.sensors is not None + assert merged.sensors.phase_detected_raw == 2.0 # diff applied + assert merged.led_color_code == 255 # controls preserved + + # Controls-only diff must preserve sensors + diff2 = _roundtrip( + hds.QuiltSmartModule( + header=hds.EntityMetadata(object_id="qsm-1", system_id="sys-1"), + controls=hds.QuiltSmartModuleControls(led_color_code=128), + ) + ) + merged2 = snapshot.apply_qsm(QuiltSmartModule.from_proto(diff2)) + assert merged2.led_color_code == 128 + assert merged2.sensors is not None + assert merged2.sensors.phase_detected_raw == 2.0 + + +def test_apply_odu_state_only_diff_preserves_performance_data() -> None: + full = _roundtrip( + hds.OutdoorUnit( + header=hds.EntityMetadata(object_id="odu-1", system_id="sys-1"), + relationships=hds.OutdoorUnitRelationships(space_id="root-1"), + state=hds.OutdoorUnitState(hvac_state=hds.HVAC_STATE_HEAT), + performance_data=hds.OutdoorUnitPerformanceData( + compressor_frequency_hz=42.0, + ambient_temperature_c=5.0, + ), + ) + ) + snapshot = _empty_snapshot() + snapshot.outdoor_units.append(OutdoorUnit.from_proto(full)) + + diff = _roundtrip( + hds.OutdoorUnit( + header=hds.EntityMetadata(object_id="odu-1", system_id="sys-1"), + state=hds.OutdoorUnitState(hvac_state=hds.HVAC_STATE_STANDBY), + ) + ) + merged = snapshot.apply_outdoor_unit(OutdoorUnit.from_proto(diff)) + + assert merged.hvac_state == HVACState.STANDBY # diff applied + assert merged.performance_data is not None # telemetry preserved + assert merged.performance_data.compressor_frequency_hz == 42.0 + assert merged.space_id == "root-1" + + +def test_full_snapshot_parse_from_real_protos_unaffected() -> None: + """Snapshot-path values still parse correctly with presence gating.""" + space = Space.from_proto(_roundtrip(_full_space_proto())) + assert space.state.ambient_temperature_c == pytest.approx(21.7) + assert space.controls.hvac_mode == HVACMode.HEAT + assert space.settings.timezone == "UTC" + assert space.is_room is True + + idu = IndoorUnit.from_proto(_roundtrip(_full_idu_proto())) + assert idu.state.ambient_temperature_c == 21.5 + assert idu.controls.led_color_code == 255 + assert idu.is_online is True From 16c50879e7a446195b7c81859d2ab5aaceb66ec6 Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Thu, 2 Jul 2026 15:26:02 -0700 Subject: [PATCH 2/9] fix: make UNAUTHENTICATED retry functional and unify gRPC error translation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In grpc.aio, awaiting the interceptor continuation only returns the call object — AioRpcError surfaces when the call itself is awaited, so the token-refresh retry was dead code and clients broke permanently on token expiry. Await the call inside the interceptor and retry once on UNAUTHENTICATED. Also: never duplicate authorization metadata already present on a call, pass CancelledError/KeyboardInterrupt through grpc_call untranslated, map NOT_FOUND to QuiltNotFoundError, route all hds/user RPCs through grpc_call so UNAVAILABLE surfaces as QuiltConnectionError everywhere, echo raw comfort-setting fan wire values (absent vs AUTO), and fetch-and-echo the current phone number when update_current_user is called without one. --- src/quilt_hp/services/__init__.py | 9 ++- src/quilt_hp/services/hds.py | 80 +++++++++-------------- src/quilt_hp/services/user.py | 34 +++++----- src/quilt_hp/transport.py | 61 ++++++++++++----- tests/test_client_service_error_paths.py | 2 +- tests/test_hds_service_branches.py | 2 + tests/test_transport_interceptor_extra.py | 63 +++++++++++++++--- 7 files changed, 157 insertions(+), 94 deletions(-) diff --git a/src/quilt_hp/services/__init__.py b/src/quilt_hp/services/__init__.py index d28958c..d486b75 100644 --- a/src/quilt_hp/services/__init__.py +++ b/src/quilt_hp/services/__init__.py @@ -10,7 +10,7 @@ import grpc import grpc.aio -from quilt_hp.exceptions import QuiltConnectionError, QuiltError +from quilt_hp.exceptions import QuiltConnectionError, QuiltError, QuiltNotFoundError logger = logging.getLogger(__name__) @@ -41,6 +41,11 @@ async def __aexit__(self, exc_type: object, exc: BaseException | None, tb: objec del exc_type, tb if exc is None: return False + # Never translate CancelledError / KeyboardInterrupt / SystemExit — + # swallowing cancellation breaks asyncio.timeout() and task + # cancellation semantics for the caller. + if not isinstance(exc, Exception): + return False translated = self._translate_exception(exc) if translated is exc: raise exc @@ -83,6 +88,8 @@ def _translate_exception(self, exc: BaseException) -> QuiltError: if isinstance(exc, grpc.aio.AioRpcError): if exc.code() in _TRANSIENT_GRPC_CODES: return QuiltConnectionError(f"{self._operation} failed: {exc.details()}") + if exc.code() == grpc.StatusCode.NOT_FOUND: + return QuiltNotFoundError(f"{self._operation} failed: {exc.details()}") return QuiltError(f"{self._operation} failed: {exc.details()}") logger.debug("Unexpected error in %s: %s", self._operation, exc) return QuiltError(f"{self._operation} failed: {exc}") diff --git a/src/quilt_hp/services/hds.py b/src/quilt_hp/services/hds.py index 8c67237..631f9e9 100644 --- a/src/quilt_hp/services/hds.py +++ b/src/quilt_hp/services/hds.py @@ -8,20 +8,23 @@ import logging import time from collections.abc import Callable, Sequence -from typing import Protocol, cast +from typing import TYPE_CHECKING, Protocol, cast -import grpc.aio from google.protobuf.timestamp_pb2 import Timestamp +if TYPE_CHECKING: + import grpc.aio + from quilt_hp._proto import quilt_hds_pb2 as hds from quilt_hp._proto import quilt_hds_pb2_grpc as hds_grpc -from quilt_hp.exceptions import QuiltError, QuiltNotFoundError +from quilt_hp.exceptions import QuiltNotFoundError from quilt_hp.models.comfort import ComfortSetting from quilt_hp.models.enums import FanSpeed, HVACMode, LouverMode from quilt_hp.models.indoor_unit import IndoorUnit from quilt_hp.models.schedule import ScheduleDay, ScheduleEvent, ScheduleWeek, ScheduleWeekDay from quilt_hp.models.space import Space from quilt_hp.models.system import SystemSnapshot +from quilt_hp.services import grpc_call logger = logging.getLogger(__name__) @@ -86,13 +89,12 @@ async def get_system(self, system_id: str) -> SystemSnapshot: """Fetch a full system snapshot.""" logger.debug("RPC GetHomeDatastoreSystem system_id=%s", system_id) try: - snap = await self._stub.GetHomeDatastoreSystem( - hds.GetHomeDatastoreSystemRequest(system_id=system_id) - ) - except grpc.aio.AioRpcError as exc: - if exc.code() == grpc.StatusCode.NOT_FOUND: - raise QuiltNotFoundError(f"System {system_id} not found") from exc - raise QuiltError(f"GetHomeDatastoreSystem failed: {exc.details()}") from exc + async with grpc_call("GetHomeDatastoreSystem"): + snap = await self._stub.GetHomeDatastoreSystem( + hds.GetHomeDatastoreSystemRequest(system_id=system_id) + ) + except QuiltNotFoundError as exc: + raise QuiltNotFoundError(f"System {system_id} not found") from exc return SystemSnapshot.from_proto(snap) async def update_space( @@ -160,10 +162,8 @@ async def update_space( logger.debug( "RPC UpdateSpace space_id=%s system_id=%s", snapshot_space.id, snapshot_space.system_id ) - try: + async with grpc_call("UpdateSpace"): result = await self._stub.UpdateSpace(hds.UpdateSpaceRequest(diff=diff)) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"UpdateSpace failed: {exc.details()}") from exc return Space.from_proto(result) async def update_space_settings( @@ -206,10 +206,8 @@ async def update_space_settings( snapshot_space.id, snapshot_space.system_id, ) - try: + async with grpc_call("UpdateSpace settings"): result = await self._stub.UpdateSpace(hds.UpdateSpaceRequest(diff=diff)) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"UpdateSpace settings failed: {exc.details()}") from exc return Space.from_proto(result) async def update_indoor_unit( @@ -255,10 +253,8 @@ async def update_indoor_unit( ), ) logger.debug("RPC UpdateIndoorUnit indoor_unit_id=%s system_id=%s", idu.id, idu.system_id) - try: + async with grpc_call("UpdateIndoorUnit"): result = await self._stub.UpdateIndoorUnit(hds.UpdateIndoorUnitRequest(diff=diff)) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"UpdateIndoorUnit failed: {exc.details()}") from exc return IndoorUnit.from_proto(result) async def update_indoor_unit_settings( @@ -309,10 +305,8 @@ async def update_indoor_unit_settings( logger.debug( "RPC UpdateIndoorUnit settings indoor_unit_id=%s system_id=%s", idu.id, idu.system_id ) - try: + async with grpc_call("UpdateIndoorUnit settings"): result = await self._stub.UpdateIndoorUnit(hds.UpdateIndoorUnitRequest(diff=diff)) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"UpdateIndoorUnit settings failed: {exc.details()}") from exc return IndoorUnit.from_proto(result) async def update_comfort_setting( @@ -326,9 +320,13 @@ async def update_comfort_setting( fan_speed: FanSpeed | None = None, ) -> ComfortSetting: """Update a comfort setting preset.""" - fan_mode_val, fan_pct = ( - fan_speed.to_wire() if fan_speed is not None else setting.fan_speed.to_wire() - ) + if fan_speed is not None: + fan_mode_val, fan_pct = fan_speed.to_wire() + else: + # Echo the raw wire values back: from_wire cannot distinguish + # "absent" (mode 0) from an explicit AUTO (mode 1), so re-encoding + # via the FanSpeed enum would convert absent into an AUTO write. + fan_mode_val, fan_pct = setting.fan_speed_mode_raw, setting.fan_speed_percent_raw diff = hds.ComfortSetting( header=hds.EntityMetadata( object_id=setting.id, @@ -357,12 +355,10 @@ async def update_comfort_setting( setting.id, setting.system_id, ) - try: + async with grpc_call("UpdateComfortSetting"): result = await self._stub.UpdateComfortSetting( hds.UpdateComfortSettingRequest(comfort_setting=diff) ) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"UpdateComfortSetting failed: {exc.details()}") from exc return ComfortSetting.from_proto(result) async def create_schedule_day( @@ -381,12 +377,10 @@ async def create_schedule_day( events=wire_events, ) logger.debug("RPC CreateScheduleDay system_id=%s space_id=%s", system_id, space_id) - try: + async with grpc_call("CreateScheduleDay"): result = await self._stub.CreateScheduleDay( hds.CreateScheduleDayRequest(schedule_day=diff) ) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"CreateScheduleDay failed: {exc.details()}") from exc return ScheduleDay.from_proto(result) async def create_schedule_week( @@ -403,12 +397,10 @@ async def create_schedule_week( days=wire_days, ) logger.debug("RPC CreateScheduleWeek system_id=%s space_id=%s", system_id, space_id) - try: + async with grpc_call("CreateScheduleWeek"): result = await self._stub.CreateScheduleWeek( hds.CreateScheduleWeekRequest(schedule_week=diff) ) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"CreateScheduleWeek failed: {exc.details()}") from exc return ScheduleWeek.from_proto(result) async def update_schedule_week( @@ -434,23 +426,19 @@ async def update_schedule_week( system_id, space_id, ) - try: + async with grpc_call("UpdateScheduleWeek"): result = await self._stub.UpdateScheduleWeek( hds.UpdateScheduleWeekRequest(schedule_week=diff) ) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"UpdateScheduleWeek failed: {exc.details()}") from exc return ScheduleWeek.from_proto(result) async def delete_schedule_day(self, schedule_day_id: str) -> None: """Delete a schedule day program.""" logger.debug("RPC DeleteScheduleDay schedule_day_id=%s", schedule_day_id) - try: + async with grpc_call("DeleteScheduleDay"): await self._stub.DeleteScheduleDay( hds.DeleteScheduleDayRequest(schedule_day_id=schedule_day_id) ) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"DeleteScheduleDay failed: {exc.details()}") from exc async def update_schedule_day( self, @@ -478,23 +466,19 @@ async def update_schedule_day( system_id, space_id, ) - try: + async with grpc_call("UpdateScheduleDay"): result = await self._stub.UpdateScheduleDay( hds.UpdateScheduleDayRequest(schedule_day=diff) ) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"UpdateScheduleDay failed: {exc.details()}") from exc return ScheduleDay.from_proto(result) async def delete_schedule_week(self, schedule_week_id: str) -> None: """Delete a schedule week.""" logger.debug("RPC DeleteScheduleWeek schedule_week_id=%s", schedule_week_id) - try: + async with grpc_call("DeleteScheduleWeek"): await self._stub.DeleteScheduleWeek( hds.DeleteScheduleWeekRequest(schedule_week_id=schedule_week_id) ) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"DeleteScheduleWeek failed: {exc.details()}") from exc async def update_location_schedule_execution( self, @@ -516,7 +500,5 @@ async def update_location_schedule_execution( controls=hds.LocationControls(schedule_execution=execution), ) logger.debug("RPC UpdateLocation location_id=%s system_id=%s", location_id, system_id) - try: + async with grpc_call("UpdateLocation"): await self._stub.UpdateLocation(hds.UpdateLocationRequest(location=diff)) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"UpdateLocation failed: {exc.details()}") from exc diff --git a/src/quilt_hp/services/user.py b/src/quilt_hp/services/user.py index d95c25d..6d705d3 100644 --- a/src/quilt_hp/services/user.py +++ b/src/quilt_hp/services/user.py @@ -6,13 +6,14 @@ from collections.abc import Callable from dataclasses import dataclass from enum import IntEnum -from typing import Any, Protocol, cast +from typing import TYPE_CHECKING, Any, Protocol, cast -import grpc.aio +if TYPE_CHECKING: + import grpc.aio from quilt_hp._proto import quilt_services_pb2 as svc from quilt_hp._proto import quilt_services_pb2_grpc as svc_grpc -from quilt_hp.exceptions import QuiltError +from quilt_hp.services import grpc_call logger = logging.getLogger(__name__) @@ -79,13 +80,11 @@ def __init__(self, channel: grpc.aio.Channel) -> None: async def get_current_user(self) -> User: """Get the currently authenticated user.""" logger.debug("Getting current user") - try: + async with grpc_call("GetLoggedInUser"): response = cast( "Any", await self._stub.GetLoggedInUser(svc.GetLoggedInUserRequest()), ) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"GetLoggedInUser failed: {exc.details()}") from exc return _to_user(response) async def update_current_user( @@ -95,33 +94,36 @@ async def update_current_user( last_name: str, phone_number: str | None = None, ) -> User: - """Update first/last name and optional phone number for current user.""" + """Update first/last name and optional phone number for current user. + + ``phone_number=None`` means "leave unchanged": the current value is + fetched and echoed back, because the wire field has no presence and + an empty string would clear the stored number server-side. + """ logger.debug("Updating current user") - try: + if phone_number is None: + phone_number = (await self.get_current_user()).phone_number + async with grpc_call("UpdateLoggedInUser"): response = cast( "Any", await self._stub.UpdateLoggedInUser( svc.UpdateLoggedInUserRequest( first_name=first_name, last_name=last_name, - phone_number=phone_number or "", + phone_number=phone_number, ) ), ) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"UpdateLoggedInUser failed: {exc.details()}") from exc return _to_user(response) async def get_user_attributes(self) -> UserAttributes: """Get current user's additional attributes.""" logger.debug("Getting user attributes") - try: + async with grpc_call("GetUserAttributes"): response = cast( "Any", await self._stub.GetUserAttributes(svc.GetUserAttributesRequest()), ) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"GetUserAttributes failed: {exc.details()}") from exc return _to_user_attributes(response) async def patch_user_attributes( @@ -131,7 +133,7 @@ async def patch_user_attributes( ) -> UserAttributes: """Patch user attributes for the current user.""" logger.debug("Patching user attributes") - try: + async with grpc_call("PatchUserAttributes"): response = cast( "Any", await self._stub.PatchUserAttributes( @@ -145,6 +147,4 @@ async def patch_user_attributes( ) ), ) - except grpc.aio.AioRpcError as exc: - raise QuiltError(f"PatchUserAttributes failed: {exc.details()}") from exc return _to_user_attributes(response) diff --git a/src/quilt_hp/transport.py b/src/quilt_hp/transport.py index f84655e..f2f1265 100644 --- a/src/quilt_hp/transport.py +++ b/src/quilt_hp/transport.py @@ -4,6 +4,7 @@ import logging from collections.abc import Awaitable, Callable +from typing import Any, cast import grpc import grpc.aio @@ -66,37 +67,49 @@ def _metadata(self) -> list[tuple[str, str]]: def _patch( self, client_call_details: grpc.aio.ClientCallDetails ) -> grpc.aio.ClientCallDetails: + existing = list(client_call_details.metadata or []) + # Never duplicate keys: explicit per-call metadata (e.g. the notifier + # stream's metadata_provider) wins over interceptor-supplied values. + # Some proxies reject repeated authorization headers. + existing_keys = {key.lower() for key, _ in existing} + patched = existing + [ + (key, value) for key, value in self._metadata() if key.lower() not in existing_keys + ] return grpc.aio.ClientCallDetails( method=client_call_details.method, timeout=client_call_details.timeout, - metadata=list(client_call_details.metadata or []) + self._metadata(), + metadata=patched, credentials=client_call_details.credentials, wait_for_ready=client_call_details.wait_for_ready, ) - async def _refresh_and_retry( + async def _refresh(self) -> None: + if self._refresh_callback is not None: + await invoke_refresh_callback( + self._refresh_callback, + TokenRefreshContext( + reason=TokenRefreshReason.TRANSPORT_UNAUTHENTICATED, + source="transport", + ), + ) + + async def _refresh_and_retry_unary( self, continuation: Callable[..., Awaitable[object]], client_call_details: grpc.aio.ClientCallDetails, - *args: object, + request: object, ) -> object: - """Refresh the token and retry the call once. + """Refresh the token and retry a unary RPC once. Raises: QuiltAuthError: If the retry still receives UNAUTHENTICATED after the token refresh, indicating the refresh token is also expired or the credentials are otherwise invalid. """ - if self._refresh_callback is not None: - await invoke_refresh_callback( - self._refresh_callback, - TokenRefreshContext( - reason=TokenRefreshReason.TRANSPORT_UNAUTHENTICATED, - source="transport", - ), - ) + await self._refresh() + call = await continuation(self._patch(client_call_details), request) try: - return await continuation(self._patch(client_call_details), *args) + return await cast("Awaitable[object]", call) except grpc.aio.AioRpcError as exc: if exc.code() == grpc.StatusCode.UNAUTHENTICATED: raise QuiltAuthError( @@ -113,12 +126,19 @@ async def intercept_unary_unary( client_call_details: grpc.aio.ClientCallDetails, request: object, ) -> object: + # NOTE: awaiting the continuation only returns the *call* object; the + # RPC result (and any AioRpcError) surfaces when the call itself is + # awaited. The call must be awaited here for UNAUTHENTICATED retry + # to work — returning the un-awaited call would make this dead code. + call = await continuation(self._patch(client_call_details), request) try: - return await continuation(self._patch(client_call_details), request) + return await cast("Awaitable[object]", call) except grpc.aio.AioRpcError as exc: if exc.code() == grpc.StatusCode.UNAUTHENTICATED and self._refresh_callback: logger.warning("Retrying unary RPC after UNAUTHENTICATED response") - return await self._refresh_and_retry(continuation, client_call_details, request) + return await self._refresh_and_retry_unary( + continuation, client_call_details, request + ) raise async def intercept_unary_stream( @@ -130,13 +150,16 @@ async def intercept_unary_stream( client_call_details: grpc.aio.ClientCallDetails, request: object, ) -> object: + call = await continuation(self._patch(client_call_details), request) try: - return await continuation(self._patch(client_call_details), request) + await cast("Any", call).wait_for_connection() except grpc.aio.AioRpcError as exc: if exc.code() == grpc.StatusCode.UNAUTHENTICATED and self._refresh_callback: logger.warning("Retrying streaming RPC setup after UNAUTHENTICATED response") - return await self._refresh_and_retry(continuation, client_call_details, request) + await self._refresh() + return await continuation(self._patch(client_call_details), request) raise + return call async def intercept_stream_unary( self, @@ -192,7 +215,9 @@ def create_channel( def auth_metadata(token_provider: TokenProviderLike) -> list[tuple[str, str]]: """Build gRPC metadata with auth headers. - Useful for stream-stream RPCs where the channel interceptor may not fire. + Used by the notifier stream to capture fresh credentials per (re)connect. + The channel interceptor skips keys already present in per-call metadata, + so headers built here are never duplicated on the wire. """ resolved_provider = _resolve_token_provider(token_provider) logger.debug("Building auth metadata") diff --git a/tests/test_client_service_error_paths.py b/tests/test_client_service_error_paths.py index 81398fb..9d300e5 100644 --- a/tests/test_client_service_error_paths.py +++ b/tests/test_client_service_error_paths.py @@ -326,7 +326,7 @@ async def test_user_service_success_and_error_paths(monkeypatch: pytest.MonkeyPa with pytest.raises(QuiltError, match="GetLoggedInUser failed"): await svc_err.get_current_user() with pytest.raises(QuiltError, match="UpdateLoggedInUser failed"): - await svc_err.update_current_user(first_name="A", last_name="B") + await svc_err.update_current_user(first_name="A", last_name="B", phone_number="+1555") with pytest.raises(QuiltError, match="GetUserAttributes failed"): await svc_err.get_user_attributes() with pytest.raises(QuiltError, match="PatchUserAttributes failed"): diff --git a/tests/test_hds_service_branches.py b/tests/test_hds_service_branches.py index 2f9ded3..013425f 100644 --- a/tests/test_hds_service_branches.py +++ b/tests/test_hds_service_branches.py @@ -102,6 +102,8 @@ async def test_hds_success_paths(monkeypatch: pytest.MonkeyPatch) -> None: heating_setpoint_c=20.0, cooling_setpoint_c=24.0, fan_speed=SimpleNamespace(to_wire=lambda: (1, 25)), + fan_speed_mode_raw=1, + fan_speed_percent_raw=25.0, type=SimpleNamespace(value=1), ) assert await svc.update_comfort_setting(comfort, name="Sleep") == "comfort" diff --git a/tests/test_transport_interceptor_extra.py b/tests/test_transport_interceptor_extra.py index 06eb950..d78c25c 100644 --- a/tests/test_transport_interceptor_extra.py +++ b/tests/test_transport_interceptor_extra.py @@ -76,6 +76,30 @@ async def _with_context(context: tokens.TokenRefreshContext) -> None: assert inspect_signature.call_count == 1 +class _FakeCall: + """Models a grpc.aio call object. + + Errors surface when the call is awaited, not when the continuation is + awaited (which only constructs the call). + """ + + def __init__(self, result: object = None, error: Exception | None = None) -> None: + self._result = result + self._error = error + + def __await__(self) -> object: + async def _run() -> object: + if self._error is not None: + raise self._error + return self._result + + return _run().__await__() + + async def wait_for_connection(self) -> None: + if self._error is not None: + raise self._error + + @pytest.mark.asyncio async def test_auth_interceptor_retry_paths() -> None: refreshed: list[str] = [] @@ -99,14 +123,16 @@ async def _continuation(call_details: grpc.aio.ClientCallDetails, request: objec calls += 1 assert ("authorization", "Bearer abc") in list(call_details.metadata or []) if calls == 1: - raise _FakeRpcError(grpc.StatusCode.UNAUTHENTICATED, "expired") - return request + return _FakeCall(error=_FakeRpcError(grpc.StatusCode.UNAUTHENTICATED, "expired")) + return _FakeCall(result=request) assert await interceptor.intercept_unary_unary(_continuation, details, "req") == "req" - assert ( - await interceptor.intercept_unary_stream(_continuation, details, "stream-req") - == "stream-req" - ) + assert refreshed == ["yes"] + + calls = 0 + refreshed.clear() + stream_call = await interceptor.intercept_unary_stream(_continuation, details, "stream-req") + assert await stream_call == "stream-req" # type: ignore[misc] assert refreshed == ["yes"] @@ -138,7 +164,7 @@ async def _stream_continuation( ) async def _failing(_call_details: grpc.aio.ClientCallDetails, _request: object) -> object: - raise _FakeRpcError(grpc.StatusCode.INTERNAL, "boom") + return _FakeCall(error=_FakeRpcError(grpc.StatusCode.INTERNAL, "boom")) with pytest.raises(_FakeRpcError): await interceptor.intercept_unary_unary(_failing, details, "req") @@ -164,7 +190,7 @@ async def _refresh(_context: transport.TokenRefreshContext) -> None: async def _always_unauthenticated( _call_details: grpc.aio.ClientCallDetails, _request: object ) -> object: - raise _FakeRpcError(grpc.StatusCode.UNAUTHENTICATED, "Jwt is expired") + return _FakeCall(error=_FakeRpcError(grpc.StatusCode.UNAUTHENTICATED, "Jwt is expired")) with pytest.raises(QuiltAuthError, match="re-login required"): await interceptor.intercept_unary_unary(_always_unauthenticated, details, "req") @@ -172,6 +198,27 @@ async def _always_unauthenticated( assert refreshed == ["yes"], "refresh callback should have been called exactly once" +@pytest.mark.asyncio +async def test_auth_interceptor_does_not_duplicate_authorization_metadata() -> None: + interceptor = transport._AuthInterceptor(lambda: "fresh-value") + details = grpc.aio.ClientCallDetails( + method="/svc/method", + timeout=1, + metadata=[("authorization", "explicit-value"), ("x-test", "1")], + credentials=None, + wait_for_ready=False, + ) + + async def _continuation(call_details: grpc.aio.ClientCallDetails, request: object) -> object: + metadata = list(call_details.metadata or []) + auth_values = [v for k, v in metadata if k.lower() == "authorization"] + assert auth_values == ["explicit-value"], "explicit metadata must win, never duplicate" + assert ("x-quilt-app-version", transport.APP_VERSION) in metadata + return _FakeCall(result=request) + + assert await interceptor.intercept_unary_unary(_continuation, details, "req") == "req" + + def test_create_channel_and_provider_resolution(monkeypatch: pytest.MonkeyPatch) -> None: class _Provider: def get_current_token(self) -> str: From 53369e604e2fe6f9337998fb08709e74ae1f49d5 Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Thu, 2 Jul 2026 15:38:31 -0700 Subject: [PATCH 3/9] fix: harden notifier stream reconnect and error handling - Reset reconnect budget and back-off after a healthy connection, so routine server-side stream recycling no longer escalates every reconnect to 60s or permanently kills the stream after N lifetime disconnects - Catch non-gRPC failures (parse crashes, metadata provider errors, channel usage errors) in the reconnect loop: reconnect while budget remains, then surface via on_error instead of silently dying - Skip malformed events instead of letting one bad payload kill the stream - Reconnect immediately after a successful token refresh - Add jitter to back-off to avoid fleet-wide reconnect stampedes - Add on_connected callback so consumers can re-snapshot after gaps - on_* registrations return unsubscribe callables - subscribe/unsubscribe before start no longer queue duplicate requests - stop() logs pre-existing task errors instead of masking the caller's exception; derive wire-scan field numbers from the proto descriptor --- src/quilt_hp/services/streaming.py | 215 ++++++++++++++---- tests/test_streaming.py | 14 +- ...test_streaming_reconnect_dispatch_extra.py | 157 ++++++++++++- 3 files changed, 339 insertions(+), 47 deletions(-) diff --git a/src/quilt_hp/services/streaming.py b/src/quilt_hp/services/streaming.py index 984f97e..8565fff 100644 --- a/src/quilt_hp/services/streaming.py +++ b/src/quilt_hp/services/streaming.py @@ -10,6 +10,7 @@ import asyncio import contextlib import logging +import random import time from collections.abc import AsyncIterator, Awaitable, Callable, Sequence from dataclasses import dataclass, field @@ -33,6 +34,11 @@ logger = logging.getLogger(__name__) +# A connection that stays up at least this long is considered healthy: the +# reconnect budget and exponential back-off reset when it ends, so routine +# server-side stream recycling never escalates reconnect latency permanently. +_HEALTHY_CONNECTION_S = 30.0 + # Callbacks may be sync or async. SpaceCallback = Callable[[Space], Awaitable[None] | None] IndoorUnitCallback = Callable[[IndoorUnit], Awaitable[None] | None] @@ -43,6 +49,10 @@ ControllerRemoteSensorCallback = Callable[[ControllerRemoteSensor], Awaitable[None] | None] SoftwareUpdateInfoCallback = Callable[[SoftwareUpdateInfo], Awaitable[None] | None] ErrorCallback = Callable[[Exception], Awaitable[None] | None] +ConnectedCallback = Callable[[], Awaitable[None] | None] + +# Returned by the on_* registration methods; call it to unregister. +Unsubscribe = Callable[[], None] class _NotifierServiceStub(Protocol): @@ -95,6 +105,21 @@ def _get_len_field(data: bytes, field_num: int) -> bytes | None: return None +# Entity field numbers inside HomeDatastoreObjectDiff, which mirrors the +# HomeDatastoreSystem field layout. Derived from the generated descriptor so +# a proto regeneration cannot silently desynchronize the hand-rolled wire +# scan in _parse_event. +_HDS_FIELDS = hds.HomeDatastoreSystem.DESCRIPTOR.fields_by_name +_SPACE_FIELD: int = _HDS_FIELDS["spaces"].number +_ODU_FIELD: int = _HDS_FIELDS["outdoor_units"].number +_QSM_FIELD: int = _HDS_FIELDS["quilt_smart_modules"].number +_IDU_FIELD: int = _HDS_FIELDS["indoor_units"].number +_CTRL_FIELD: int = _HDS_FIELDS["controllers"].number +_RS_FIELD: int = _HDS_FIELDS["remote_sensors"].number +_CRS_FIELD: int = _HDS_FIELDS["controller_remote_sensors"].number +_SUI_FIELD: int = _HDS_FIELDS["software_update_infos"].number + + def _make_subscribe_request(topics: list[str]) -> notifier.SubscribeRequest: """Build a SubscribeRequest for the given topic list.""" return notifier.SubscribeRequest( @@ -158,8 +183,10 @@ class NotifierStream: authenticate: Optional async callable (no args) that refreshes the auth token. When provided and the stream gets ``UNAUTHENTICATED``, the callable is awaited before reconnecting. - max_reconnects: Maximum reconnect attempts per disconnect event. - ``-1`` means unlimited (default). + max_reconnects: Maximum consecutive reconnect attempts. The counter + resets after a connection stays healthy for a while, so this + bounds retries per disconnect event rather than per stream + lifetime. ``-1`` means unlimited (default). reconnect_delay_s: Initial back-off delay in seconds before the first reconnect. Doubles on each subsequent attempt, capped at 60 s. Default: ``1.0``. @@ -185,6 +212,7 @@ class NotifierStream: _crs_callbacks: list[ControllerRemoteSensorCallback] = field(default_factory=list, init=False) _sui_callbacks: list[SoftwareUpdateInfoCallback] = field(default_factory=list, init=False) _error_callbacks: list[ErrorCallback] = field(default_factory=list, init=False) + _connected_callbacks: list[ConnectedCallback] = field(default_factory=list, init=False) _request_queue: asyncio.Queue[notifier.SubscribeRequest] = field(init=False) _subscription_lock: asyncio.Lock = field(init=False) _lifecycle_lock: asyncio.Lock = field(init=False) @@ -238,42 +266,69 @@ def create( ) # --- Callback registration --- + # + # Each on_* method returns an unsubscribe callable so long-lived + # consumers (e.g. HA config entries) can detach callbacks without + # tearing down the stream. + # + # Callbacks run on the event loop: keep them non-blocking. A slow or + # blocking callback delays delivery of subsequent stream events. - def on_space_update(self, callback: SpaceCallback) -> None: + @staticmethod + def _register[T](callbacks: list[T], callback: T) -> Unsubscribe: + callbacks.append(callback) + + def _unsubscribe() -> None: + with contextlib.suppress(ValueError): + callbacks.remove(callback) + + return _unsubscribe + + def on_space_update(self, callback: SpaceCallback) -> Unsubscribe: """Register a callback for space change events (sync or async).""" - self._space_callbacks.append(callback) + return self._register(self._space_callbacks, callback) - def on_indoor_unit_update(self, callback: IndoorUnitCallback) -> None: + def on_indoor_unit_update(self, callback: IndoorUnitCallback) -> Unsubscribe: """Register a callback for indoor unit change events (sync or async).""" - self._idu_callbacks.append(callback) + return self._register(self._idu_callbacks, callback) - def on_outdoor_unit_update(self, callback: OutdoorUnitCallback) -> None: + def on_outdoor_unit_update(self, callback: OutdoorUnitCallback) -> Unsubscribe: """Register callback for outdoor unit change events.""" - self._odu_callbacks.append(callback) + return self._register(self._odu_callbacks, callback) - def on_controller_update(self, callback: ControllerCallback) -> None: + def on_controller_update(self, callback: ControllerCallback) -> Unsubscribe: """Register callback for controller (Dial) change events.""" - self._ctrl_callbacks.append(callback) + return self._register(self._ctrl_callbacks, callback) - def on_qsm_update(self, callback: QsmCallback) -> None: + def on_qsm_update(self, callback: QsmCallback) -> Unsubscribe: """Register callback for QuiltSmartModule change events.""" - self._qsm_callbacks.append(callback) + return self._register(self._qsm_callbacks, callback) - def on_remote_sensor_update(self, callback: RemoteSensorCallback) -> None: + def on_remote_sensor_update(self, callback: RemoteSensorCallback) -> Unsubscribe: """Register callback for RemoteSensor change events.""" - self._rs_callbacks.append(callback) + return self._register(self._rs_callbacks, callback) - def on_controller_remote_sensor_update(self, callback: ControllerRemoteSensorCallback) -> None: + def on_controller_remote_sensor_update( + self, callback: ControllerRemoteSensorCallback + ) -> Unsubscribe: """Register callback for ControllerRemoteSensor change events.""" - self._crs_callbacks.append(callback) + return self._register(self._crs_callbacks, callback) - def on_software_update_info(self, callback: SoftwareUpdateInfoCallback) -> None: + def on_software_update_info(self, callback: SoftwareUpdateInfoCallback) -> Unsubscribe: """Register callback for SoftwareUpdateInfo change events.""" - self._sui_callbacks.append(callback) + return self._register(self._sui_callbacks, callback) - def on_error(self, callback: ErrorCallback) -> None: + def on_error(self, callback: ErrorCallback) -> Unsubscribe: """Register a callback invoked when the stream encounters a fatal error.""" - self._error_callbacks.append(callback) + return self._register(self._error_callbacks, callback) + + def on_connected(self, callback: ConnectedCallback) -> Unsubscribe: + """Register a callback invoked on every successful (re)connect. + + Events published while the stream was disconnected are lost; use this + to re-fetch a snapshot after a reconnect and close the gap. + """ + return self._register(self._connected_callbacks, callback) @property def error(self) -> Exception | None: @@ -298,10 +353,14 @@ def stream_state(self) -> str: # --- Subscription management --- async def subscribe(self, topics: list[str]) -> None: - """Add more topics to the subscription (after stream is started).""" + """Add more topics to the subscription (before or after start).""" async with self._subscription_lock: self._topics.extend(topics) - await self._request_queue.put(_make_subscribe_request(topics)) + # Before start, _topics alone is the source of truth: the initial + # request snapshots it. Queueing here as well would send the + # topics twice on the first stream. + if self._running: + await self._request_queue.put(_make_subscribe_request(topics)) async def unsubscribe(self, topics: list[str]) -> None: """Remove topics from the subscription.""" @@ -314,7 +373,8 @@ async def unsubscribe(self, topics: list[str]) -> None: for t in topics: if t in self._topics: self._topics.remove(t) - await self._request_queue.put(req) + if self._running: + await self._request_queue.put(req) # --- Internal stream machinery --- @@ -363,49 +423,49 @@ def _parse_event(self, evt: object) -> StreamEvent | None: obj_diff = _get_len_field(inner_notif, 2) if obj_diff: - space_bytes = _get_len_field(obj_diff, 3) + space_bytes = _get_len_field(obj_diff, _SPACE_FIELD) if space_bytes: updated = hds.Space() updated.ParseFromString(space_bytes) event.space = Space.from_proto(updated) - idu_bytes = _get_len_field(obj_diff, 9) + idu_bytes = _get_len_field(obj_diff, _IDU_FIELD) if idu_bytes: updated_idu = hds.IndoorUnit() updated_idu.ParseFromString(idu_bytes) event.indoor_unit = IndoorUnit.from_proto(updated_idu) - odu_bytes = _get_len_field(obj_diff, 6) + odu_bytes = _get_len_field(obj_diff, _ODU_FIELD) if odu_bytes: updated_odu = hds.OutdoorUnit() updated_odu.ParseFromString(odu_bytes) event.outdoor_unit = OutdoorUnit.from_proto(updated_odu) - ctrl_bytes = _get_len_field(obj_diff, 11) + ctrl_bytes = _get_len_field(obj_diff, _CTRL_FIELD) if ctrl_bytes: updated_ctrl = hds.Controller() updated_ctrl.ParseFromString(ctrl_bytes) event.controller = Controller.from_proto(updated_ctrl) - qsm_bytes = _get_len_field(obj_diff, 7) + qsm_bytes = _get_len_field(obj_diff, _QSM_FIELD) if qsm_bytes: updated_qsm = hds.QuiltSmartModule() updated_qsm.ParseFromString(qsm_bytes) event.qsm = QuiltSmartModule.from_proto(updated_qsm) - rs_bytes = _get_len_field(obj_diff, 12) + rs_bytes = _get_len_field(obj_diff, _RS_FIELD) if rs_bytes: updated_rs = hds.RemoteSensor() updated_rs.ParseFromString(rs_bytes) event.remote_sensor = RemoteSensor.from_proto(updated_rs) - crs_bytes = _get_len_field(obj_diff, 16) + crs_bytes = _get_len_field(obj_diff, _CRS_FIELD) if crs_bytes: updated_crs = hds.ControllerRemoteSensor() updated_crs.ParseFromString(crs_bytes) event.controller_remote_sensor = ControllerRemoteSensor.from_proto(updated_crs) - sui_bytes = _get_len_field(obj_diff, 18) + sui_bytes = _get_len_field(obj_diff, _SUI_FIELD) if sui_bytes: updated_sui = hds.SoftwareUpdateInfo() updated_sui.ParseFromString(sui_bytes) @@ -557,16 +617,29 @@ async def _run_one_stream(self) -> None: ) self._active_call = call self._stream_state = "connected" + for connected_cb in self._connected_callbacks: + try: + result = connected_cb() + if asyncio.iscoroutine(result): + await result + except Exception: + logger.exception("Error in connected callback") try: async for response in call: saw_event = False for ctrl in response.control_events: saw_event = True - event_name = notifier.ControlEventType.Name(ctrl.type) - logger.debug("Control event: %s topics=%s", event_name, list(ctrl.topics)) + if logger.isEnabledFor(logging.DEBUG): + event_name = notifier.ControlEventType.Name(ctrl.type) + logger.debug("Control event: %s topics=%s", event_name, list(ctrl.topics)) for evt in response.notifier_events: - parsed = self._parse_event(evt) + try: + parsed = self._parse_event(evt) + except Exception: + # One malformed event must not kill the stream. + logger.exception("Failed to parse stream event; skipping") + continue if parsed is None: continue saw_event = True @@ -577,12 +650,30 @@ async def _run_one_stream(self) -> None: if self._active_call is call: self._active_call = None + async def _prepare_reconnect(self, delay: float) -> bool: + """Back off, then reset the request queue. Returns True when stopped.""" + # Jitter avoids a synchronized reconnect stampede across clients + # after a server-side restart. + if await self._wait_for_stop(delay * random.uniform(0.5, 1.5)): + return True + async with self._subscription_lock: + logger.info( + "Resetting subscription queue before reconnect; " + "tracked topics will be re-subscribed on the next stream" + ) + # _topics is the source of truth. The next request iterator + # snapshots the current topics and sends them as its first + # request, so discarding any stale queued requests is safe. + self._request_queue = asyncio.Queue() + return False + async def _run_stream_with_reconnect(self) -> None: """Run the stream with automatic reconnect and exponential back-off.""" attempt = 0 delay = self._reconnect_delay_s while self._running: + connected_at = time.monotonic() try: self._error = None await self._run_one_stream() @@ -591,8 +682,16 @@ async def _run_stream_with_reconnect(self) -> None: except grpc.aio.AioRpcError as exc: if not self._running: break + # A connection that stayed healthy for a while resets the + # reconnect budget and back-off: max_reconnects bounds + # *consecutive* failures, and routine server-side stream + # recycling must not permanently escalate the delay. + if time.monotonic() - connected_at >= _HEALTHY_CONNECTION_S: + attempt = 0 + delay = self._reconnect_delay_s is_unauth = exc.code() == grpc.StatusCode.UNAUTHENTICATED can_retry = self._max_reconnects < 0 or attempt < self._max_reconnects + wait_s = delay if is_unauth and self._authenticate is not None and can_retry: self._stream_state = "reconnecting" @@ -614,6 +713,9 @@ async def _run_stream_with_reconnect(self) -> None: self._error = exc self._stream_state = "error" break + # Refresh succeeded — reconnect promptly instead of + # serving the escalated back-off for routine token expiry. + wait_s = 0.0 elif can_retry: self._stream_state = "reconnecting" details = exc.details() or "" @@ -648,19 +750,35 @@ async def _run_stream_with_reconnect(self) -> None: self._stream_state = "error" break - if await self._wait_for_stop(delay): + if await self._prepare_reconnect(wait_s): + break + delay = min(delay * 2, 60.0) + attempt += 1 + except Exception as exc: + # Non-gRPC failures (metadata provider errors, channel usage + # errors, unexpected parse crashes) must not silently kill the + # stream: reconnect if budget remains, otherwise surface via + # on_error like any other fatal condition. + if not self._running: + break + if time.monotonic() - connected_at >= _HEALTHY_CONNECTION_S: + attempt = 0 + delay = self._reconnect_delay_s + if self._max_reconnects >= 0 and attempt >= self._max_reconnects: + logger.exception("Unexpected stream error; max reconnects reached") + self._error = QuiltStreamError(f"Stream error: {exc}") + self._stream_state = "error" + break + self._stream_state = "reconnecting" + logger.exception( + "Unexpected stream error; reconnecting in %.1fs (attempt %d)", + delay, + attempt + 1, + ) + if await self._prepare_reconnect(delay): break delay = min(delay * 2, 60.0) attempt += 1 - async with self._subscription_lock: - logger.info( - "Resetting subscription queue before reconnect; " - "tracked topics will be re-subscribed on the next stream" - ) - # _topics is the source of truth. The next request iterator - # snapshots the current topics and sends them as its first - # request, so discarding any stale queued requests is safe. - self._request_queue = asyncio.Queue() if self._error is None and self._stream_state != "stopped": self._stream_state = "stopped" @@ -749,8 +867,15 @@ async def stop(self) -> None: if task is not None: task.cancel() - with contextlib.suppress(asyncio.CancelledError, QuiltStreamError): + try: await task + except asyncio.CancelledError, QuiltStreamError: + pass + except Exception: + # The task may have already died with an unrelated error; + # log it rather than masking the caller's own exception + # (stop() often runs from __aexit__). + logger.exception("NotifierStream task raised during stop") await self._cancel_pending_dispatches() diff --git a/tests/test_streaming.py b/tests/test_streaming.py index 90071ab..bcc5f72 100644 --- a/tests/test_streaming.py +++ b/tests/test_streaming.py @@ -162,11 +162,11 @@ async def test_unsubscribe_removes_topics() -> None: @pytest.mark.asyncio async def test_subscribe_after_queue_reset_resubscribes_from_topics() -> None: stream = _make_stream(["topic-a"]) + stream._running = True # simulate a live stream mid-reconnect stream._request_queue = asyncio.Queue() await stream.subscribe(["topic-b"]) - stream._running = True request_iterator = stream._request_iterator(list(stream._topics), stream._request_queue) initial = await anext(request_iterator) queued = await anext(request_iterator) @@ -176,6 +176,18 @@ async def test_subscribe_after_queue_reset_resubscribes_from_topics() -> None: assert [sub.topic for sub in queued.append.subscriptions] == ["topic-b"] +@pytest.mark.asyncio +async def test_subscribe_before_start_does_not_queue_duplicate_request() -> None: + stream = _make_stream(["topic-a"]) + + await stream.subscribe(["topic-b"]) + + # Before start, only _topics is extended; the initial request snapshots + # it, so queueing as well would send topic-b twice on the first stream. + assert stream._topics == ["topic-a", "topic-b"] + assert stream._request_queue.empty() + + # ─── lifecycle ─────────────────────────────────────────────────────────────── diff --git a/tests/test_streaming_reconnect_dispatch_extra.py b/tests/test_streaming_reconnect_dispatch_extra.py index c5a50dc..8b10d5d 100644 --- a/tests/test_streaming_reconnect_dispatch_extra.py +++ b/tests/test_streaming_reconnect_dispatch_extra.py @@ -125,7 +125,9 @@ async def _fake_sleep(delay: float) -> None: await stream._run_stream_with_reconnect() assert calls == 2 - assert sleep_calls == [1.0] + # Back-off is 1.0s with ±50% jitter applied at sleep time. + assert len(sleep_calls) == 1 + assert 0.5 <= sleep_calls[0] <= 1.5 assert stream._request_queue is not old_queue assert "Resetting subscription queue before reconnect" in caplog.text @@ -173,3 +175,156 @@ async def test_on_task_done_logs_only_for_exceptions() -> None: error_task = MagicMock(cancelled=lambda: False, exception=lambda: RuntimeError("boom")) stream._on_task_done(error_task) + + +@pytest.mark.asyncio +async def test_non_grpc_error_reconnects_then_surfaces_via_on_error( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Unexpected (non-AioRpcError) failures reconnect and, once the budget + is exhausted, surface via on_error instead of silently killing the task.""" + stream = _make_stream() + stream._running = True + stream._max_reconnects = 1 + + calls = 0 + + async def _broken() -> None: + nonlocal calls + calls += 1 + raise RuntimeError("metadata provider exploded") + + stream._run_one_stream = _broken # type: ignore[method-assign] + monkeypatch.setattr("quilt_hp.services.streaming.asyncio.sleep", AsyncMock()) + + errors: list[Exception] = [] + stream.on_error(errors.append) + + await stream._run_stream_with_reconnect() + + assert calls == 2 # initial + one reconnect + assert stream.stream_state == "error" + assert len(errors) == 1 + assert isinstance(errors[0], QuiltStreamError) + + +@pytest.mark.asyncio +async def test_backoff_and_budget_reset_after_healthy_connection( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A connection that stays up past the health threshold resets attempt + and delay, so routine stream recycling never escalates to 60s waits.""" + stream = _make_stream() + stream._running = True + stream._max_reconnects = 1 # one consecutive retry allowed + + clock = {"now": 0.0} + monkeypatch.setattr("quilt_hp.services.streaming.time.monotonic", lambda: clock["now"]) + + sleeps: list[float] = [] + + async def _fake_sleep(delay: float) -> None: + sleeps.append(delay) + + monkeypatch.setattr("quilt_hp.services.streaming.asyncio.sleep", _fake_sleep) + + calls = 0 + + async def _recycled() -> None: + nonlocal calls + calls += 1 + # Each connection lives 120s (> healthy threshold) before the server + # recycles it — with per-lifetime counting the old code died here. + clock["now"] += 120.0 + if calls >= 4: + stream._running = False + return + raise _FakeRpcError(grpc.StatusCode.CANCELLED, "recycled") + + stream._run_one_stream = _recycled # type: ignore[method-assign] + + await stream._run_stream_with_reconnect() + + assert calls == 4 # three recycles + final clean run — budget never exhausted + assert stream.error is None + # Delay never escalated: every wait is the initial 1.0s (±50% jitter). + assert all(0.5 <= s <= 1.5 for s in sleeps) + + +@pytest.mark.asyncio +async def test_token_refresh_success_reconnects_without_backoff( + monkeypatch: pytest.MonkeyPatch, +) -> None: + stream = _make_stream() + stream._running = True + refreshed: list[str] = [] + + async def _refresh() -> None: + refreshed.append("yes") + + stream._authenticate = _refresh + + calls = 0 + + async def _unauth_once() -> None: + nonlocal calls + calls += 1 + if calls == 1: + raise _FakeRpcError(grpc.StatusCode.UNAUTHENTICATED, "expired") + stream._running = False + + stream._run_one_stream = _unauth_once # type: ignore[method-assign] + + sleeps: list[float] = [] + + async def _fake_sleep(delay: float) -> None: + sleeps.append(delay) + + monkeypatch.setattr("quilt_hp.services.streaming.asyncio.sleep", _fake_sleep) + + await stream._run_stream_with_reconnect() + + assert refreshed == ["yes"] + assert calls == 2 + assert sleeps == [0.0] # prompt reconnect after successful refresh + + +@pytest.mark.asyncio +async def test_on_connected_fires_per_connect_and_unsubscribe_detaches() -> None: + stream = _make_stream() + connected: list[str] = [] + unsubscribe = stream.on_connected(lambda: connected.append("up")) + + async def _empty_iter() -> asyncio.AsyncIterator[object]: + return + yield + + stream._stub = MagicMock(Subscribe=lambda *_a, **_k: _empty_iter()) + await stream._run_one_stream() + assert connected == ["up"] + + unsubscribe() + await stream._run_one_stream() + assert connected == ["up"] # detached — no second invocation + + +@pytest.mark.asyncio +async def test_malformed_event_is_skipped_not_fatal() -> None: + stream = _make_stream() + seen: list[object] = [] + stream.on_space_update(seen.append) + + good_event = StreamEvent(topic="t", space=object()) + parse = MagicMock(side_effect=[IndexError("truncated varint"), good_event]) + stream._parse_event = parse # type: ignore[method-assign] + + response = SimpleNamespace(control_events=[], notifier_events=[object(), object()]) + + async def _iter() -> asyncio.AsyncIterator[object]: + yield response + + stream._stub = MagicMock(Subscribe=lambda *_a, **_k: _iter()) + await stream._run_one_stream() # must not raise + + assert parse.call_count == 2 + assert len(seen) == 1 # the good event still dispatched From 5b6c3126a6f0d078ba61d21eef1293ca41bc9583 Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Thu, 2 Jul 2026 15:41:34 -0700 Subject: [PATCH 4/9] fix: force refresh on server-rejected tokens and add single-flight guards - authenticate() no longer returns a locally-unexpired token that the server just rejected: transport/stream UNAUTHENTICATED contexts force a Cognito refresh - Persist rotated refresh tokens instead of silently discarding them - Measure token expiry from receipt time, not from before the OTP prompt - QuiltClient.refresh_token() is single-flight: concurrent 401s trigger one refresh instead of a Cognito stampede with racing store writes - get_snapshot() cold-cache fetch is single-flight - get_system_id(home=...) with a non-default home no longer poisons the cached default system (and matches the configured home case-insensitively) - close() stops tracked NotifierStreams before closing the channel - _resolve_snapshot_item and home resolution raise QuiltNotFoundError - tokens.py: parenthesize except (works on 3.13 parsers/tools too) and fix the refresh-callback signature cache to key bound methods by __func__ so it actually hits --- src/quilt_hp/auth.py | 25 ++++++-- src/quilt_hp/client.py | 125 +++++++++++++++++++++++++------------ src/quilt_hp/tokens.py | 13 ++-- tests/test_auth.py | 58 +++++++++++++++++ tests/test_client_cache.py | 88 ++++++++++++++++++++++++++ 5 files changed, 261 insertions(+), 48 deletions(-) diff --git a/src/quilt_hp/auth.py b/src/quilt_hp/auth.py index 684f1eb..802fbdb 100644 --- a/src/quilt_hp/auth.py +++ b/src/quilt_hp/auth.py @@ -207,12 +207,20 @@ async def authenticate( Token persistence is delegated to *token_store*. Pass ``None`` for purely in-memory/stateless operation (caller handles caching). + + When *refresh_context* indicates the server rejected the current token + (transport/stream ``UNAUTHENTICATED``), the locally cached IdToken is + not trusted even if unexpired — a refresh is forced. Otherwise a + revoked-but-unexpired token would be returned right back to the caller. """ - now = time.time() cached = await _load_tokens(token_store, email) if token_store else None + server_rejected_token = refresh_context is not None and refresh_context.reason in ( + TokenRefreshReason.TRANSPORT_UNAUTHENTICATED, + TokenRefreshReason.STREAM_UNAUTHENTICATED, + ) # 1. Valid cached IdToken - if cached is not None and not cached.is_expired: + if cached is not None and not cached.is_expired and not server_rejected_token: logger.debug("Using cached token") return cached.id_token @@ -239,10 +247,15 @@ async def authenticate( raise logger.warning("Refresh failed; falling back to OTP") else: + # Cognito user pools may rotate the refresh token; persist the + # new one or the stored token dies after first use. + rotated = result.get("RefreshToken") tokens = CachedTokens( id_token=_require_str(result, "IdToken"), - refresh_token=cached.refresh_token, - expires_at=now + _expires_in_s(result), + refresh_token=( + rotated if isinstance(rotated, str) and rotated else cached.refresh_token + ), + expires_at=time.time() + _expires_in_s(result), ) if token_store: await _save_tokens(token_store, email, tokens) @@ -260,10 +273,12 @@ async def authenticate( ) result = await _do_otp_login(email, otp_callback) + # Expiry is measured from token receipt — the user may take minutes to + # enter the OTP code. tokens = CachedTokens( id_token=_require_str(result, "IdToken"), refresh_token=_require_str(result, "RefreshToken") if "RefreshToken" in result else "", - expires_at=now + _expires_in_s(result), + expires_at=time.time() + _expires_in_s(result), ) if token_store: await _save_tokens(token_store, email, tokens) diff --git a/src/quilt_hp/client.py b/src/quilt_hp/client.py index 824925b..452ce07 100644 --- a/src/quilt_hp/client.py +++ b/src/quilt_hp/client.py @@ -13,6 +13,8 @@ from __future__ import annotations +import asyncio +import contextlib import logging import time from collections.abc import Callable @@ -20,7 +22,7 @@ from quilt_hp.auth import OtpCallback, authenticate from quilt_hp.const import Environment -from quilt_hp.exceptions import QuiltAuthError, QuiltError +from quilt_hp.exceptions import QuiltAuthError, QuiltError, QuiltNotFoundError from quilt_hp.services.hds import HomeDatastoreService from quilt_hp.services.streaming import NotifierStream from quilt_hp.services.system import SystemInformationService @@ -104,6 +106,13 @@ def __init__( # Snapshot cache self._snapshot_cache: SystemSnapshot | None = None self._snapshot_cached_at: float = 0.0 + # Single-flight guards: N concurrent 401s must trigger one Cognito + # refresh, and N concurrent cold-cache reads one snapshot RPC. + self._auth_lock = asyncio.Lock() + self._snapshot_lock = asyncio.Lock() + # Streams created via stream(); stopped in close() so a live stream + # never outlives its channel. + self._streams: list[NotifierStream] = [] def get_current_token(self) -> str: """Token provider callable for the transport interceptor.""" @@ -161,7 +170,7 @@ async def _resolve_snapshot_item( for candidate in items(snapshot): if candidate.id == item: return candidate - raise QuiltError(f"{kind} {item!r} not found") + raise QuiltNotFoundError(f"{kind} {item!r} not found") # --- Auth --- @@ -191,18 +200,28 @@ async def login(self, otp_callback: OtpCallback | None = None) -> None: logger.info("Login succeeded") async def refresh_token(self, context: TokenRefreshContext | None = None) -> None: - """Refresh the auth token without OTP when refresh token is valid.""" + """Refresh the auth token without OTP when refresh token is valid. + + Single-flight: when several concurrent RPCs hit ``UNAUTHENTICATED`` + at once, only the first waiter performs the Cognito refresh; the + rest observe the already-updated token and return. + """ resolved_context = context or TokenRefreshContext( reason=TokenRefreshReason.EXPIRED_CACHED_TOKEN, source="client", ) - self._token = await authenticate( - self._email, - token_store=self._token_store, - refresh_context=resolved_context, - refresh_hooks=self._token_refresh_hooks, - refresh_policy=self._token_refresh_policy, - ) + token_before = self._token + async with self._auth_lock: + if self._token is not None and self._token != token_before: + logger.debug("Token already refreshed by a concurrent caller") + return + self._token = await authenticate( + self._email, + token_store=self._token_store, + refresh_context=resolved_context, + refresh_hooks=self._token_refresh_hooks, + refresh_policy=self._token_refresh_policy, + ) # --- System discovery --- @@ -216,14 +235,20 @@ async def list_systems(self) -> list[SystemInfo]: return await self._require_sysinfo().list_systems() async def get_system_id(self, home: str | None = None) -> str: - """Get primary system ID, cached after first call unless home changes.""" + """Get primary system ID, cached after first call. + + Passing an explicit ``home`` different from the client's configured + default resolves that system *without* touching the cached default — + subsequent no-argument calls keep operating on the configured home. + """ target_home = home or self._home + is_default_request = home is None or ( + self._home is not None and home.lower() == self._home.lower() + ) logger.debug("Resolving system for home filter %r", target_home) - if self._system_id is not None: - # Bypass the cache only when a different home is requested. - if not home or home == self._home: - logger.debug("Using cached system id %s", self._system_id) - return self._system_id + if self._system_id is not None and is_default_request: + logger.debug("Using cached system id %s", self._system_id) + return self._system_id systems = await self.list_systems() if not systems: @@ -233,16 +258,18 @@ async def get_system_id(self, home: str | None = None) -> str: matches = [s for s in systems if target_home.lower() in s.name.lower()] if not matches: names = [s.name for s in systems] - raise QuiltError(f"No home matching {target_home!r}. Available: {names}") - self._system_id = matches[0].id - self._system_name = matches[0].name + raise QuiltNotFoundError(f"No home matching {target_home!r}. Available: {names}") + resolved = matches[0] else: # No home filter — use the first system (primary home) - self._system_id = systems[0].id - self._system_name = systems[0].name + resolved = systems[0] + + if is_default_request: + self._system_id = resolved.id + self._system_name = resolved.name - logger.info("Selected system %s (%s)", self._system_name, self._system_id) - return self._system_id + logger.info("Selected system %s (%s)", resolved.name, resolved.id) + return resolved.id async def get_snapshot(self, system_id: str | None = None) -> SystemSnapshot: """Fetch a full system snapshot. @@ -261,15 +288,20 @@ async def get_snapshot(self, system_id: str | None = None) -> SystemSnapshot: if self._snapshot_cache is not None and age < self._snapshot_ttl_s: logger.debug("Snapshot cache hit for system %s", sid) return self._snapshot_cache - logger.debug("Snapshot cache miss for system %s", sid) - - snapshot = await hds.get_system(sid) - - if system_id is None and self._snapshot_ttl_s > 0: - self._snapshot_cache = snapshot - self._snapshot_cached_at = time.monotonic() - - return snapshot + # Single-flight: concurrent cold-cache callers (e.g. HA startup) + # must not each issue a full-system RPC. + async with self._snapshot_lock: + age = time.monotonic() - self._snapshot_cached_at + if self._snapshot_cache is not None and age < self._snapshot_ttl_s: + logger.debug("Snapshot cache hit for system %s (filled while waiting)", sid) + return self._snapshot_cache + logger.debug("Snapshot cache miss for system %s", sid) + snapshot = await hds.get_system(sid) + self._snapshot_cache = snapshot + self._snapshot_cached_at = time.monotonic() + return snapshot + + return await hds.get_system(sid) def invalidate_snapshot(self) -> None: """Discard the cached snapshot so the next call fetches fresh data.""" @@ -573,23 +605,30 @@ def stream( entity before dispatching the latest event. ``0.0`` disables debouncing. - Returns a ``NotifierStream`` that can be used as: + Returns a ``NotifierStream``. Stream events are **sparse diffs** — + always merge them into a snapshot via ``snapshot.apply_*`` before + use; a raw stream entity has empty names/controls for any field the + diff didn't carry. - **Background task** (for integrations):: - async with client.stream(topics) as stream: - stream.on_space_update(my_callback) + snapshot = await client.get_snapshot() + async with client.stream(snapshot.stream_topics()) as stream: + stream.on_space_update( + lambda space: on_change(snapshot.apply_space(space)) + ) # stream runs in background, do other work here await asyncio.sleep(3600) - **Blocking** (for CLI / scripts):: - s = client.stream(topics) - s.on_space_update(my_callback) + snapshot = await client.get_snapshot() + s = client.stream(snapshot.stream_topics()) + s.on_space_update(lambda space: snapshot.apply_space(space)) await s.run_forever() """ channel = self._require_channel() - return NotifierStream.create( + stream = NotifierStream.create( channel, topics, metadata_provider=lambda: auth_metadata(self), @@ -598,6 +637,10 @@ def stream( reconnect_delay_s=reconnect_delay_s, debounce_s=debounce_s, ) + # Track for close(); drop references to streams that already stopped. + self._streams = [s for s in self._streams if s.stream_state not in ("stopped", "error")] + self._streams.append(stream) + return stream # --- User --- @@ -636,7 +679,11 @@ async def patch_user_attributes( # --- Lifecycle --- async def close(self) -> None: - """Close the gRPC channel.""" + """Stop any live streams and close the gRPC channel.""" + streams, self._streams = self._streams, [] + for stream in streams: + with contextlib.suppress(Exception): + await stream.stop() if self._channel is not None: await self._channel.close() self._channel = None diff --git a/src/quilt_hp/tokens.py b/src/quilt_hp/tokens.py index b9fbd2a..b5c4df7 100644 --- a/src/quilt_hp/tokens.py +++ b/src/quilt_hp/tokens.py @@ -137,11 +137,16 @@ async def invoke_refresh_callback( """Invoke a refresh callback, passing context only if it accepts a parameter. Whether each callback accepts a ``TokenRefreshContext`` argument is cached - per-callable in a WeakKeyDictionary so that ``inspect.signature`` is only - called once per unique callback object. + per underlying function in a WeakKeyDictionary so that + ``inspect.signature`` is only called once per callback. Bound methods are + cached by their ``__func__`` — each attribute access creates a new bound + method object, which would never hit an identity-keyed cache; the + parameter count (computed from the bound signature, which excludes + ``self``) is identical for all instances of the same class. """ + cache_key = getattr(refresh_callback, "__func__", refresh_callback) try: - has_params = _REFRESH_CALLBACK_HAS_PARAMS.get(refresh_callback) + has_params = _REFRESH_CALLBACK_HAS_PARAMS.get(cache_key) except TypeError: has_params = None # non-weakrefable callable — skip cache if has_params is None: @@ -150,7 +155,7 @@ async def invoke_refresh_callback( except TypeError, ValueError: has_params = False try: - _REFRESH_CALLBACK_HAS_PARAMS[refresh_callback] = has_params + _REFRESH_CALLBACK_HAS_PARAMS[cache_key] = has_params except TypeError: pass # non-weakrefable callable — skip caching if has_params: diff --git a/tests/test_auth.py b/tests/test_auth.py index 7902b46..9bb809f 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -186,3 +186,61 @@ async def _fake_refresh(_refresh: str) -> dict[str, str | int]: assert len(hooks.starts) == 1 assert hooks.successes == [] assert len(hooks.failures) == 1 + + +@pytest.mark.asyncio +async def test_authenticate_forces_refresh_when_server_rejected_token( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A locally-unexpired token the server just rejected must not be + returned as-is — the UNAUTHENTICATED context forces a refresh.""" + import time + + store = _AsyncStore( + loaded=CachedTokens( + id_token="rejected-but-unexpired", + refresh_token="refresh-1", + expires_at=time.time() + 3600, + ) + ) + + async def _fake_refresh(_refresh: str) -> dict[str, str | int]: + return {"IdToken": "fresh-token", "ExpiresIn": 3600} + + monkeypatch.setattr(auth, "_do_refresh", _fake_refresh) + + token = await auth.authenticate( + "user@example.com", + token_store=store, + refresh_context=TokenRefreshContext( + reason=TokenRefreshReason.STREAM_UNAUTHENTICATED, + source="streaming", + ), + ) + assert token == "fresh-token" + + +@pytest.mark.asyncio +async def test_authenticate_persists_rotated_refresh_token( + monkeypatch: pytest.MonkeyPatch, +) -> None: + import time + + store = _AsyncStore( + loaded=CachedTokens( + id_token="expired", + refresh_token="old-refresh", + expires_at=time.time() - 10, + ) + ) + + async def _fake_refresh(_refresh: str) -> dict[str, str | int]: + return {"IdToken": "fresh", "RefreshToken": "rotated-refresh", "ExpiresIn": 3600} + + monkeypatch.setattr(auth, "_do_refresh", _fake_refresh) + + await auth.authenticate("user@example.com", token_store=store) + + assert store.saved, "rotated tokens must be persisted" + _, saved_tokens = store.saved[-1] + assert saved_tokens.refresh_token == "rotated-refresh" diff --git a/tests/test_client_cache.py b/tests/test_client_cache.py index b41bff5..91050e6 100644 --- a/tests/test_client_cache.py +++ b/tests/test_client_cache.py @@ -433,3 +433,91 @@ async def test_get_system_id_home_filter() -> None: sid = await client.get_system_id() assert sid == "sys-2" + + +@pytest.mark.asyncio +async def test_get_system_id_explicit_home_does_not_poison_default_cache() -> None: + from quilt_hp.models.system import SystemInfo + + client, _ = _make_client() + client._system_id = None + client._home = "Beach" + + mock_sysinfo = MagicMock() + mock_sysinfo.list_systems = AsyncMock( + return_value=[ + SystemInfo(id="sys-beach", name="Beach House", timezone="UTC"), + SystemInfo(id="sys-lake", name="Lake House", timezone="UTC"), + ] + ) + client._sysinfo = mock_sysinfo + + assert await client.get_system_id() == "sys-beach" + # Explicit different home resolves without overwriting the default… + assert await client.get_system_id("Lake") == "sys-lake" + # …so a subsequent default call still targets the configured home. + assert await client.get_system_id() == "sys-beach" + assert client.system_name == "Beach House" + # Case-insensitive match against the configured home hits the cache. + calls_before = mock_sysinfo.list_systems.call_count + assert await client.get_system_id("beach") == "sys-beach" + assert mock_sysinfo.list_systems.call_count == calls_before + + +@pytest.mark.asyncio +async def test_refresh_token_is_single_flight(monkeypatch: pytest.MonkeyPatch) -> None: + import asyncio + + client, _ = _make_client() + client._token = "old-token" + calls = 0 + + async def _fake_authenticate(*_args: object, **_kwargs: object) -> str: + nonlocal calls + calls += 1 + await asyncio.sleep(0) # let concurrent waiters pile up on the lock + return "new-token" + + monkeypatch.setattr("quilt_hp.client.authenticate", _fake_authenticate) + + await asyncio.gather(*(client.refresh_token() for _ in range(5))) + + assert client._token == "new-token" + assert calls == 1 # only the first waiter hit Cognito + + +@pytest.mark.asyncio +async def test_concurrent_cold_snapshot_fetch_is_single_flight() -> None: + import asyncio + + client, mock_hds = _make_client(ttl=60) + snapshot = _make_snapshot() + + async def _slow_get_system(_sid: str) -> SystemSnapshot: + await asyncio.sleep(0) + return snapshot + + mock_hds.get_system = AsyncMock(side_effect=_slow_get_system) + + results = await asyncio.gather(*(client.get_snapshot() for _ in range(5))) + + assert all(r is snapshot for r in results) + assert mock_hds.get_system.await_count == 1 + + +@pytest.mark.asyncio +async def test_close_stops_tracked_streams() -> None: + client, _ = _make_client() + fake_stream = MagicMock() + fake_stream.stop = AsyncMock() + fake_stream.stream_state = "connected" + client._streams.append(fake_stream) + + channel = client._channel + channel.close = AsyncMock() + + await client.close() + + fake_stream.stop.assert_awaited_once() + channel.close.assert_awaited_once() + assert client._streams == [] From 943a1b3f4cc0ea1ef35489a06ae1edff692b9014 Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Thu, 2 Jul 2026 15:47:13 -0700 Subject: [PATCH 5/9] fix: CLI/TUI resilience, input validation, and token store hardening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TUI: register stream on_error with visible disconnect indicator and automatic recovery (re-snapshot + restart with backoff); OTP modal so first-time users can log in; boot failures show an error screen instead of a stuck spinner; app-owned snapshot shared by all screens (stacked screens no longer render from a stale object after auto-refresh); dashboard cursor preserved across refresh and rows re-rendered on resume; app-level °C/°F preference; setpoint clamping with named bounds; LED toggle-on restores stored/default brightness; 0.0 readings render as values instead of '--'; dead code removed and duplicated schedule/ODU logic extracted. CLI: energy --period validates choices; --heat/--cool enforce range; 'quilt set' with no options errors instead of a no-op RPC; FALLBACK_* modes rejected; OTP prompt no longer blocks the event loop. Store: corrupt tokens.json is quarantined to a timestamped backup and recovered (was a permanent hard failure), atomic writes fsync before replace, advisory flock around read-modify-write, config dir 0700. --- src/quilt_hp/_paths.py | 8 +- src/quilt_hp/cli/constants.py | 25 + src/quilt_hp/cli/main.py | 87 +++- src/quilt_hp/cli/store.py | 156 +++--- src/quilt_hp/cli/tui.py | 658 +++++++++++++++++------- tests/test_auth_store_settings_edges.py | 51 +- tests/test_cli_surfaces_extra.py | 35 +- tests/test_tokens.py | 20 +- tests/test_tui_bindings.py | 52 ++ 9 files changed, 812 insertions(+), 280 deletions(-) create mode 100644 src/quilt_hp/cli/constants.py diff --git a/src/quilt_hp/_paths.py b/src/quilt_hp/_paths.py index 04fe69a..76ca13f 100644 --- a/src/quilt_hp/_paths.py +++ b/src/quilt_hp/_paths.py @@ -20,7 +20,11 @@ def app_config_dir() -> Path: - """Return the platform-appropriate config directory, creating if needed.""" + """Return the platform-appropriate config directory, creating if needed. + + The directory is created user-only (0o700) because it holds cached + authentication tokens. + """ d = Path(user_config_dir(_APP)) - d.mkdir(parents=True, exist_ok=True) + d.mkdir(parents=True, exist_ok=True, mode=0o700) return d diff --git a/src/quilt_hp/cli/constants.py b/src/quilt_hp/cli/constants.py new file mode 100644 index 0000000..0f19319 --- /dev/null +++ b/src/quilt_hp/cli/constants.py @@ -0,0 +1,25 @@ +"""Shared CLI/TUI constants. + +Setpoint bounds +--------------- +Quilt uses 8.0 °C as the STANDBY heat sentinel (``STANDBY_HEAT_SENTINEL_C`` +in ``quilt_hp.models.space``), which is the lowest temperature the system +will ever hold. 32 °C is a sensible upper bound for a user-facing comfort +setpoint (the 40 °C STANDBY cool sentinel is a placeholder, not a real +setpoint). User-supplied heating/cooling setpoints are validated/clamped to +this range. +""" + +from __future__ import annotations + +SETPOINT_MIN_C = 8.0 +SETPOINT_MAX_C = 32.0 + +# Fallbacks used when a space has no current setpoint to step from. +DEFAULT_HEAT_SETPOINT_C = 20.0 +DEFAULT_COOL_SETPOINT_C = 26.0 + + +def clamp_setpoint_c(value_c: float) -> float: + """Clamp a setpoint to the supported [SETPOINT_MIN_C, SETPOINT_MAX_C] range.""" + return max(SETPOINT_MIN_C, min(SETPOINT_MAX_C, value_c)) diff --git a/src/quilt_hp/cli/main.py b/src/quilt_hp/cli/main.py index 68b79a3..c4b30d2 100644 --- a/src/quilt_hp/cli/main.py +++ b/src/quilt_hp/cli/main.py @@ -19,6 +19,7 @@ sys.exit(1) from quilt_hp import __version__ +from quilt_hp.cli.constants import SETPOINT_MAX_C, SETPOINT_MIN_C from quilt_hp.cli.settings import SettingsStore from quilt_hp.cli.store import FileStore from quilt_hp.client import QuiltClient @@ -39,6 +40,25 @@ class OutputMode(StrEnum): JSON = "json" +class EnergyPeriod(StrEnum): + """Reporting period for the energy command.""" + + DAY = "day" + WEEK = "week" + MONTH = "month" + + +# User-settable HVAC modes (FALLBACK_* are device-side fallback states). +_SETTABLE_MODES = ( + HVACMode.COOL, + HVACMode.HEAT, + HVACMode.AUTO, + HVACMode.FAN, + HVACMode.DRY, + HVACMode.STANDBY, +) + + class _EntityWithId(Protocol): id: str @@ -415,7 +435,9 @@ async def _prompt_for_otp(challenge_email: str) -> str: console.print( f"[yellow]✉ OTP sent to {challenge_email} — check your email.[/yellow]" ) - return cast("str", typer.prompt("Enter OTP code")).strip() + # typer.prompt blocks on stdin — run it off the event loop. + code = await asyncio.to_thread(typer.prompt, "Enter OTP code") + return cast("str", code).strip() await client.login(otp_callback=_prompt_for_otp) console.print("[green]✓ Successfully logged in![/green]") @@ -647,8 +669,12 @@ async def _presets() -> None: console.print("\n[bold]═══ Comfort Settings ═══[/bold]") for cs in settings: mode = cs.hvac_mode.name - heat = f"{cs.heating_setpoint_c:.1f}°C" if cs.heating_setpoint_c else "--" - cool = f"{cs.cooling_setpoint_c:.1f}°C" if cs.cooling_setpoint_c else "--" + heat = ( + f"{cs.heating_setpoint_c:.1f}°C" if cs.heating_setpoint_c is not None else "--" + ) + cool = ( + f"{cs.cooling_setpoint_c:.1f}°C" if cs.cooling_setpoint_c is not None else "--" + ) fan = cs.fan_speed.name console.print(f"\n [cyan]{cs.name}[/cyan] ({cs.type.name})") console.print(f" Mode: {mode} Heat: {heat} Cool: {cool} Fan: {fan}") @@ -701,7 +727,10 @@ async def _schedules() -> None: def energy( email: str | None = typer.Option(None, envvar="QUILT_EMAIL", help="Quilt account email"), home: str | None = typer.Option(None, help="Specific home name to connect to"), - period: str = typer.Option("day", help="Time period: day, week, month"), + period: EnergyPeriod = typer.Option( # noqa: B008 + EnergyPeriod.DAY, + help="Time period: day, week, month", + ), ) -> None: """Show energy consumption metrics.""" email, home = _resolve(email, home) @@ -716,12 +745,12 @@ async def _energy() -> None: now = datetime.now(tz=zoneinfo.ZoneInfo(snapshot.timezone or "UTC")) start = now.replace(hour=0, minute=0, second=0, microsecond=0) - if period == "day": + if period == EnergyPeriod.DAY: end = start + timedelta(days=1) - timedelta(seconds=1) - elif period == "week": + elif period == EnergyPeriod.WEEK: start = start - timedelta(days=start.weekday()) end = start + timedelta(weeks=1) - timedelta(seconds=1) - else: # month + else: # EnergyPeriod.MONTH start = start.replace(day=1) if start.month == 12: end = start.replace(year=start.year + 1, month=1) - timedelta(seconds=1) @@ -733,7 +762,7 @@ async def _energy() -> None: console.print(f"\n [bold][{period.upper()}][/bold] {header}\n") for sm in metrics: name = name_by_id.get(sm.space_id, sm.space_id[:8]) - total = getattr(sm, "total_kwh", 0) + total = sm.total_kwh if total == 0: continue console.print(f" {name:<22} total={total:.3f} kWh") @@ -745,14 +774,42 @@ async def _energy() -> None: def set_space( space_name: str = typer.Argument(..., help="Exact name of the room to update"), mode: str | None = typer.Option(None, help="HVAC mode: COOL, HEAT, AUTO, FAN, DRY, STANDBY"), - heat: float | None = typer.Option(None, help="Heating setpoint in °C"), - cool: float | None = typer.Option(None, help="Cooling setpoint in °C"), + heat: float | None = typer.Option( + None, + min=SETPOINT_MIN_C, + max=SETPOINT_MAX_C, + help=f"Heating setpoint in °C ({SETPOINT_MIN_C:.0f}–{SETPOINT_MAX_C:.0f})", + ), + cool: float | None = typer.Option( + None, + min=SETPOINT_MIN_C, + max=SETPOINT_MAX_C, + help=f"Cooling setpoint in °C ({SETPOINT_MIN_C:.0f}–{SETPOINT_MAX_C:.0f})", + ), email: str | None = typer.Option(None, envvar="QUILT_EMAIL", help="Quilt account email"), home: str | None = typer.Option(None, help="Specific home name to connect to"), ) -> None: """Update HVAC mode and setpoints for a room.""" + if mode is None and heat is None and cool is None: + console.print( + "[red]Nothing to update:[/red] provide at least one of --mode, --heat, --cool." + ) + raise typer.Exit(1) + email, home = _resolve(email, home) + if mode: + try: + hvac_mode: HVACMode | None = HVACMode[mode.upper()] + if hvac_mode not in _SETTABLE_MODES: + raise KeyError(mode) + except KeyError: + valid = ", ".join(m.name.lower() for m in _SETTABLE_MODES) + console.print(f"[red]Invalid mode {mode!r}. Valid: {valid}[/red]") + raise typer.Exit(1) from None + else: + hvac_mode = None + async def _set() -> None: async with _client_snapshot(email, home) as (client, snap): space = next( @@ -763,16 +820,6 @@ async def _set() -> None: console.print(f"[red]Room {space_name!r} not found.[/red]") raise typer.Exit(1) - if mode: - try: - hvac_mode: HVACMode | None = HVACMode[mode.upper()] - except KeyError: - valid = ", ".join(m.name.lower() for m in HVACMode if m.value) - console.print(f"[red]Invalid mode {mode!r}. Valid: {valid}[/red]") - raise typer.Exit(1) from None - else: - hvac_mode = None - await client.set_space( space.id, mode=hvac_mode, diff --git a/src/quilt_hp/cli/store.py b/src/quilt_hp/cli/store.py index 622f564..51fd0de 100644 --- a/src/quilt_hp/cli/store.py +++ b/src/quilt_hp/cli/store.py @@ -8,17 +8,28 @@ from __future__ import annotations import asyncio +import contextlib import json import logging import os from dataclasses import asdict +from datetime import UTC, datetime from pathlib import Path +from typing import TYPE_CHECKING, Any from uuid import uuid4 from quilt_hp._paths import app_config_dir from quilt_hp.exceptions import QuiltAuthError from quilt_hp.tokens import CachedTokens +if TYPE_CHECKING: + from collections.abc import Iterator + +try: + import fcntl +except ImportError: # pragma: no cover - non-POSIX platforms + fcntl = None # type: ignore[assignment] + logger = logging.getLogger(__name__) @@ -35,6 +46,69 @@ class FileStore: def _token_path(self) -> Path: return app_config_dir() / "tokens.json" + @contextlib.contextmanager + def _file_lock(self) -> Iterator[None]: + """Advisory inter-process lock around read-modify-write cycles. + + Uses ``fcntl.flock`` on a sibling ``.lock`` file (Linux/macOS). On + platforms or filesystems without flock support this degrades to a + no-op — the atomic-replace write still prevents torn files. + """ + if fcntl is None: + yield + return + lock_path = self._token_path().with_name("tokens.json.lock") + try: + lock_path.parent.mkdir(parents=True, exist_ok=True) + fd = os.open(lock_path, os.O_CREAT | os.O_WRONLY, 0o600) + except OSError: + yield + return + try: + with contextlib.suppress(OSError): + fcntl.flock(fd, fcntl.LOCK_EX) + yield + finally: + with contextlib.suppress(OSError): + fcntl.flock(fd, fcntl.LOCK_UN) + os.close(fd) + + def _recover_corruption(self, reason: str) -> None: + """Move a corrupt token file aside so the store can start empty. + + Worst case is one re-login instead of a permanent QuiltAuthError. + """ + path = self._token_path() + timestamp = datetime.now(UTC).strftime("%Y%m%dT%H%M%SZ") + backup = path.with_name(f"{path.name}.corrupt-{timestamp}-{reason}") + logger.warning( + "Token store %s is corrupt (%s); moving it to %s and starting empty", + path, + reason, + backup, + ) + with contextlib.suppress(OSError): + path.replace(backup) + + def _read_all(self) -> dict[str, Any]: + """Read the full token file, recovering from corruption.""" + path = self._token_path() + logger.debug("Loading token file %s", path) + try: + data = json.loads(path.read_text()) + except FileNotFoundError: + return {} + except json.JSONDecodeError: + self._recover_corruption("invalid-json") + return {} + except OSError as exc: + _warn_if_permission_error("reading", path, exc) + raise QuiltAuthError("Failed to read token store.") from exc + if not isinstance(data, dict): + self._recover_corruption("invalid-shape") + return {} + return data + def _atomic_write(self, payload: dict[str, object]) -> None: path = self._token_path() path.parent.mkdir(parents=True, exist_ok=True) @@ -45,6 +119,8 @@ def _atomic_write(self, payload: dict[str, object]) -> None: fd = os.open(tmp, os.O_CREAT | os.O_WRONLY | os.O_TRUNC, 0o600) with os.fdopen(fd, "w") as f: f.write(json.dumps(payload, indent=2)) + f.flush() + os.fsync(f.fileno()) os.replace(tmp, path) os.chmod(path, 0o600) except OSError: @@ -59,18 +135,7 @@ async def load(self, email: str) -> CachedTokens | None: return await asyncio.to_thread(self._load_sync, email) def _load_sync(self, email: str) -> CachedTokens | None: - path = self._token_path() - logger.debug("Loading token file %s", path) - try: - data = json.loads(path.read_text()) - except FileNotFoundError: - return None - except json.JSONDecodeError as exc: - raise QuiltAuthError("Token store contains invalid JSON.") from exc - except OSError as exc: - _warn_if_permission_error("reading", path, exc) - raise QuiltAuthError("Failed to read token store.") from exc - + data = self._read_all() try: entry = data[email] return CachedTokens( @@ -89,56 +154,31 @@ async def save(self, email: str, tokens: CachedTokens) -> None: def _save_sync(self, email: str, tokens: CachedTokens) -> None: path = self._token_path() - logger.debug("Saving token file %s", path) - try: - data = json.loads(path.read_text()) - except FileNotFoundError: - data = {} - except json.JSONDecodeError as exc: - raise QuiltAuthError("Token store contains invalid JSON.") from exc - except OSError as exc: - _warn_if_permission_error("reading", path, exc) - raise QuiltAuthError("Failed to read token store.") from exc - data[email] = asdict(tokens) - try: - self._atomic_write(data) - except OSError as exc: - _warn_if_permission_error("writing", path, exc) - raise QuiltAuthError("Failed to persist token store.") from exc + with self._file_lock(): + data = self._read_all() + data[email] = asdict(tokens) + logger.debug("Saving token file %s", path) + try: + self._atomic_write(data) + except OSError as exc: + _warn_if_permission_error("writing", path, exc) + raise QuiltAuthError("Failed to persist token store.") from exc def clear_tokens(self, email: str) -> None: """Remove cached tokens for *email*.""" path = self._token_path() - logger.debug("Loading token file %s", path) - try: - data = json.loads(path.read_text()) - except FileNotFoundError: - return - except json.JSONDecodeError as exc: - raise QuiltAuthError("Token store contains invalid JSON.") from exc - except OSError as exc: - _warn_if_permission_error("reading", path, exc) - raise QuiltAuthError("Failed to read token store.") from exc - - data.pop(email, None) - logger.debug("Saving token file %s", path) - try: - self._atomic_write(data) - except OSError as exc: - _warn_if_permission_error("writing", path, exc) - raise QuiltAuthError("Failed to persist token store.") from exc + with self._file_lock(): + data = self._read_all() + if email not in data: + return + data.pop(email, None) + logger.debug("Saving token file %s", path) + try: + self._atomic_write(data) + except OSError as exc: + _warn_if_permission_error("writing", path, exc) + raise QuiltAuthError("Failed to persist token store.") from exc def list_emails(self) -> list[str]: """All email addresses that have cached tokens.""" - path = self._token_path() - logger.debug("Loading token file %s", path) - try: - data = json.loads(path.read_text()) - except FileNotFoundError: - return [] - except json.JSONDecodeError as exc: - raise QuiltAuthError("Token store contains invalid JSON.") from exc - except OSError as exc: - _warn_if_permission_error("reading", path, exc) - raise QuiltAuthError("Failed to read token store.") from exc - return [k for k in data if isinstance(k, str)] + return [k for k in self._read_all() if isinstance(k, str)] diff --git a/src/quilt_hp/cli/tui.py b/src/quilt_hp/cli/tui.py index 9139a4c..7492181 100644 --- a/src/quilt_hp/cli/tui.py +++ b/src/quilt_hp/cli/tui.py @@ -8,8 +8,11 @@ from __future__ import annotations +import asyncio import contextlib import datetime +import logging +from dataclasses import replace from typing import TYPE_CHECKING, ClassVar from rich.text import Text @@ -24,11 +27,12 @@ ) from textual.css.query import NoMatches from textual.reactive import reactive -from textual.screen import Screen +from textual.screen import ModalScreen, Screen from textual.widgets import ( DataTable, Footer, Header, + Input, Label, ListItem, ListView, @@ -39,9 +43,15 @@ TabPane, ) +from quilt_hp.cli.constants import ( + DEFAULT_COOL_SETPOINT_C, + DEFAULT_HEAT_SETPOINT_C, + clamp_setpoint_c, +) from quilt_hp.cli.settings import SettingsStore from quilt_hp.cli.store import FileStore from quilt_hp.client import QuiltClient +from quilt_hp.exceptions import QuiltAuthError from quilt_hp.models.controller import Controller from quilt_hp.models.enums import ( FanSpeed, @@ -62,6 +72,9 @@ if TYPE_CHECKING: from quilt_hp.models.space import Space from quilt_hp.models.system import SystemSnapshot + from quilt_hp.services.streaming import NotifierStream + +logger = logging.getLogger(__name__) # ────────────────────────────────────────────────────────────────── # Persistent settings (delegates to quilt_hp.cli.settings) @@ -156,7 +169,16 @@ LouverMode.FIXED, LouverMode.CLOSED, ] -_OCC_CYCLE = [OccupancyMode.DISABLED, OccupancyMode.ENABLED] + +_WEEKDAY_NAMES = [ + "Monday", + "Tuesday", + "Wednesday", + "Thursday", + "Friday", + "Saturday", + "Sunday", +] def _tc(val_c: float | None, use_f: bool) -> str: @@ -202,12 +224,6 @@ def _led_color_str(color_code: int) -> str: return f"#{r:02X}{g:02X}{b:02X}w{w:02X}" -def _bar(level: float, width: int = 10) -> str: - """Render a simple block-character progress bar.""" - filled = max(0, min(width, round(level * width))) - return "█" * filled + "░" * (width - filled) - - def _sku_or_none(model_sku: str | None) -> str | None: """Return a displayable SKU value or None for empty/placeholder values.""" if not model_sku: @@ -289,6 +305,38 @@ def _cycle_next(current: object, cycle: list) -> object: return cycle[0] +def _odu_for_space( + snapshot: SystemSnapshot, space_id: str, idu: IndoorUnit | None +) -> OutdoorUnit | None: + """Resolve a room's ODU from the IDU link first, then by room relationship.""" + if idu: + odu = snapshot.odu_for_idu(idu) + if odu is not None: + return odu + space_ids = _id_tokens(space_id) + return next( + (u for u in snapshot.outdoor_units if _id_tokens(u.space_id) & space_ids), + None, + ) + + +def _patch_schedule_paused(snapshot: SystemSnapshot, paused: bool) -> None: + """Patch the cached snapshot's primary location with a new paused state.""" + loc = snapshot.primary_location + if loc is None: + return + idx = snapshot.locations.index(loc) + snapshot.locations[idx] = replace(loc, schedule_paused=paused) + + +async def _set_schedule_paused( + client: QuiltClient, snapshot: SystemSnapshot, paused: bool +) -> None: + """Toggle schedule execution server-side and patch the local cache.""" + await client.set_schedule_execution(paused) + _patch_schedule_paused(snapshot, paused) + + # ────────────────────────────────────────────────────────────────── # CSS # ────────────────────────────────────────────────────────────────── @@ -309,6 +357,41 @@ def _cycle_next(current: object, cycle: list) -> object: color: $text-muted; } +/* Boot error */ +#boot-error-container { + align: center middle; + height: 100%; +} +#boot-error-title { + text-style: bold; + color: $error; + text-align: center; +} +#boot-error-message { + margin-top: 1; + text-align: center; +} +#boot-error-hint { + margin-top: 2; + text-align: center; + color: $text-muted; +} + +/* OTP modal */ +OtpScreen { + align: center middle; +} +#otp-dialog { + width: 60; + height: auto; + border: round $primary; + padding: 1 2; + background: $surface; +} +#otp-label { + margin-bottom: 1; +} + /* Dashboard */ #dashboard-list { height: 1fr; @@ -481,6 +564,46 @@ def set_status(self, msg: str) -> None: self.query_one("#loading-label", Label).update(msg) +class BootErrorScreen(Screen): + """Shown when startup fails — no perpetual spinner.""" + + def __init__(self, message: str, hint: str | None = None) -> None: + super().__init__() + self._message = message + self._hint = hint or "Check your connection and try again." + + def compose(self) -> ComposeResult: + with Container(id="boot-error-container"): + yield Label("✗ Failed to start", id="boot-error-title") + yield Label(self._message, id="boot-error-message") + yield Label(f"{self._hint}\nPress q to quit.", id="boot-error-hint") + + +class OtpScreen(ModalScreen[str]): + """Modal prompting for the one-time passcode emailed during login.""" + + def __init__(self, email: str) -> None: + super().__init__() + self._email = email + + def compose(self) -> ComposeResult: + with Vertical(id="otp-dialog"): + yield Label( + f"✉ A one-time code was sent to {self._email}.\nEnter it below:", + id="otp-label", + ) + yield Input(placeholder="123456", id="otp-input") + + def on_mount(self) -> None: + self.query_one("#otp-input", Input).focus() + + @on(Input.Submitted, "#otp-input") + def _submit(self, event: Input.Submitted) -> None: + code = event.value.strip() + if code: + self.dismiss(code) + + # ────────────────────────────────────────────────────────────────── # DashboardScreen # ────────────────────────────────────────────────────────────────── @@ -494,6 +617,7 @@ def __init__(self, space: Space, idu: IndoorUnit | None = None, use_f: bool = Fa self._space_id = space.id self._space_name = space.name self._idu = idu + self._use_f = use_f self.update_space(space, idu, use_f) @property @@ -535,6 +659,7 @@ def update_space( self, space: Space, idu: IndoorUnit | None = None, use_f: bool = False ) -> None: self._space = space + self._use_f = use_f if idu is not None: self._idu = idu with contextlib.suppress(NoMatches): @@ -542,7 +667,7 @@ def update_space( def compose(self) -> ComposeResult: yield Static( - self._build_row(self._space, self._idu, False), + self._build_row(self._space, self._idu, self._use_f), id=f"room-row-{self._space_id}", ) @@ -557,18 +682,34 @@ class DashboardScreen(Screen): Binding("enter", "select_room", "Room Detail"), ] - use_f: reactive[bool] = reactive(False) - def __init__( self, snapshot: SystemSnapshot, client: QuiltClient, ) -> None: super().__init__() - self._snapshot = snapshot + self._snapshot = snapshot # fallback when not attached to a QuiltApp self._client = client self._items: dict[str, RoomListItem] = {} # space_id → ListItem + @property + def snapshot(self) -> SystemSnapshot: + """The app-owned snapshot, falling back to the constructor value.""" + with contextlib.suppress(Exception): + app = self.app + if isinstance(app, QuiltApp) and app.snapshot is not None: + return app.snapshot + return self._snapshot + + @property + def use_f(self) -> bool: + """App-level °C/°F preference (falls back to °C when unmounted).""" + with contextlib.suppress(Exception): + app = self.app + if isinstance(app, QuiltApp): + return app.use_f + return False + def compose(self) -> ComposeResult: yield Header(show_clock=True) yield ListView(id="dashboard-list") @@ -577,51 +718,71 @@ def compose(self) -> ComposeResult: def _idu_for(self, space_id: str) -> IndoorUnit | None: return next( - (u for u in self._snapshot.indoor_units if u.space_id == space_id), + (u for u in self.snapshot.indoor_units if u.space_id == space_id), None, ) def _odu_for(self, space_id: str, idu: IndoorUnit | None) -> OutdoorUnit | None: """Resolve room ODU from IDU link first, then by room relationship.""" - if idu: - odu = self._snapshot.odu_for_idu(idu) - if odu is not None: - return odu - space_ids = _id_tokens(space_id) - return next( - (u for u in self._snapshot.outdoor_units if _id_tokens(u.space_id) & space_ids), - None, - ) + return _odu_for_space(self.snapshot, space_id, idu) def on_mount(self) -> None: lv = self.query_one(ListView) - for space in self._snapshot.rooms: + for space in self.snapshot.rooms: item = RoomListItem(space, self._idu_for(space.id), self.use_f) self._items[space.id] = item lv.append(item) self._refresh_statusbar() self.set_interval(60, self._auto_refresh) + def on_screen_resume(self) -> None: + """Re-render rows from the merged snapshot when returning to this screen.""" + self.refresh_units() + + def refresh_units(self) -> None: + """Re-render all room rows and the statusbar from the current snapshot.""" + snap = self.snapshot + use_f = self.use_f + for space_id, item in self._items.items(): + space = next((s for s in snap.rooms if s.id == space_id), None) + if space: + item.update_space(space, self._idu_for(space_id), use_f) + item.refresh() + self._refresh_statusbar() + async def _apply_snapshot(self, snap: SystemSnapshot) -> None: - """Replace the current snapshot and rebuild the room list in-place.""" + """Adopt a fresh snapshot and rebuild the room list in-place.""" self._snapshot = snap - # Keep app-level snapshot in sync for stream dispatcher comfort maps. - self.app._snapshot = snap # type: ignore[attr-defined] - self._items.clear() + app = self.app + if isinstance(app, QuiltApp): + app.update_snapshot(snap) lv = self.query_one(ListView) + # Preserve the highlighted room across the rebuild. + highlighted = lv.highlighted_child + selected_id = highlighted.space_id if isinstance(highlighted, RoomListItem) else None + old_index = lv.index + self._items.clear() await lv.clear() for space in snap.rooms: item = RoomListItem(space, self._idu_for(space.id), self.use_f) self._items[space.id] = item lv.append(item) + if self._items: + ids = list(self._items) + if selected_id in self._items: + lv.index = ids.index(selected_id) + elif old_index is not None: + lv.index = min(old_index, len(ids) - 1) self._refresh_statusbar() @work async def _auto_refresh(self) -> None: """Periodic silent re-sync with server state (called every 60 s).""" - with contextlib.suppress(Exception): + try: snap = await self._client.get_snapshot() await self._apply_snapshot(snap) + except Exception as exc: + logger.warning("Dashboard auto-refresh failed: %s", exc) @work async def action_refresh(self) -> None: @@ -633,7 +794,7 @@ async def action_refresh(self) -> None: self.notify(f"Refresh failed: {exc}", severity="error") def _refresh_statusbar(self) -> None: - snap = self._snapshot + snap = self.snapshot tz = snap.timezone or "?" loc = snap.primary_location sched = "⏸ PAUSED" if (loc and loc.schedule_paused) else "▶ RUNNING" @@ -654,23 +815,31 @@ def update_space(self, space: Space) -> None: item.update_space(space, idu, self.use_f) item.refresh() - def update_odu(self, odu: OutdoorUnit) -> None: - """Called when an ODU stream event arrives — refresh the statusbar.""" - self._refresh_statusbar() + def update_idu(self, idu: IndoorUnit) -> None: + """Called from stream callbacks — refresh the row for the IDU's room.""" + space = next((s for s in self.snapshot.rooms if s.id == idu.space_id), None) + if space is None: + return + item = self._items.get(space.id) + if item: + item.update_space(space, idu, self.use_f) + item.refresh() - def watch_use_f(self, use_f: bool) -> None: - for space_id, item in self._items.items(): - space = next((s for s in self._snapshot.spaces if s.id == space_id), None) - if space: - item.update_space(space, None, use_f) - item.refresh() + def update_odu(self, _odu: OutdoorUnit) -> None: + """Called when an ODU stream event arrives — refresh the statusbar. + + The updated ODU is already merged into the snapshot, which the + statusbar reads directly. + """ + self._refresh_statusbar() def action_toggle_units(self) -> None: - self.use_f = not self.use_f - self.app._persist() + app = self.app + if isinstance(app, QuiltApp): + app.use_f = not app.use_f def action_system(self) -> None: - self.app.push_screen(SystemScreen(self._snapshot, self._client, use_f=self.use_f)) + self.app.push_screen(SystemScreen(self.snapshot, self._client, use_f=self.use_f)) def action_select_room(self) -> None: lv = self.query_one(ListView) @@ -686,19 +855,20 @@ def on_room_selected(self, event: ListView.Selected) -> None: self._open_room(event.item.space_id) def _open_room(self, space_id: str) -> None: - space = next((s for s in self._snapshot.rooms if s.id == space_id), None) + snap = self.snapshot + space = next((s for s in snap.rooms if s.id == space_id), None) if space is None: return idu = next( - (u for u in self._snapshot.indoor_units if u.space_id == space_id), + (u for u in snap.indoor_units if u.space_id == space_id), None, ) ctrl = next( - (c for c in self._snapshot.controllers if c.space_id == space_id), + (c for c in snap.controllers if c.space_id == space_id), None, ) odu = self._odu_for(space_id, idu) - qsm = self._snapshot.qsm_for_idu(idu) if idu else None + qsm = snap.qsm_for_idu(idu) if idu else None self.app.push_screen( RoomScreen( space=space, @@ -706,7 +876,7 @@ def _open_room(self, space_id: str) -> None: controller=ctrl, odu=odu, qsm=qsm, - snapshot=self._snapshot, + snapshot=snap, client=self._client, use_f=self.use_f, ) @@ -744,7 +914,6 @@ class RoomScreen(Screen): Binding("f", "cycle_fan", "Fan"), Binding("l", "cycle_louver", "Louver"), Binding("L", "toggle_led", "LED"), - Binding("o", "cycle_occupancy", "Occ"), Binding("p", "toggle_schedule", "Pause Sched"), Binding("e", "refresh_energy", "Energy ↻"), Binding("[", "away_timeout_dec", "Away-5m", show=False), @@ -760,8 +929,6 @@ class RoomScreen(Screen): Binding("alt+t", "radar_height_dec", "Radar H-", show=False), ] - use_f: reactive[bool] = reactive(False) - def __init__( self, space: Space, @@ -779,12 +946,52 @@ def __init__( self._controller = controller self._odu = odu self._qsm = qsm - self._snapshot = snapshot + self._snapshot = snapshot # fallback when not attached to a QuiltApp self._client = client - self.use_f = use_f + self._use_f = use_f self.title = space.name self.sub_title = "Room" + @property + def snapshot(self) -> SystemSnapshot: + """The app-owned snapshot, falling back to the constructor value.""" + with contextlib.suppress(Exception): + app = self.app + if isinstance(app, QuiltApp) and app.snapshot is not None: + return app.snapshot + return self._snapshot + + @property + def use_f(self) -> bool: + """App-level °C/°F preference (falls back to the constructor value).""" + with contextlib.suppress(Exception): + app = self.app + if isinstance(app, QuiltApp): + return app.use_f + return self._use_f + + # Entity IDs for the app-level stream dispatchers. + + @property + def space_id(self) -> str: + return self._space.id + + @property + def idu_id(self) -> str | None: + return self._idu.id if self._idu else None + + @property + def odu_id(self) -> str | None: + return self._odu.id if self._odu else None + + @property + def controller_id(self) -> str | None: + return self._controller.id if self._controller else None + + @property + def qsm_id(self) -> str | None: + return self._qsm.id if self._qsm else None + # ── Layout ────────────────────────────────────────────────── def compose(self) -> ComposeResult: @@ -969,7 +1176,7 @@ def _compose_energy(self) -> ComposeResult: yield _KVStatic(id="e-7day") yield _KVStatic(id="e-30day") with Vertical(classes="energy-chart") as v: - v.border_title = "Last 24 Hours — Hourly (kWh)" + v.border_title = "Today — Hourly (kWh)" yield Static("", id="e-sparkline") yield DataTable(id="e-table") @@ -995,7 +1202,7 @@ def _populate_status(self) -> None: preset_name = "--" if c.comfort_setting_id: cs = next( - (x for x in self._snapshot.comfort_settings if x.id == c.comfort_setting_id), + (x for x in self.snapshot.comfort_settings if x.id == c.comfort_setting_id), None, ) if cs: @@ -1022,7 +1229,11 @@ def _populate_status(self) -> None: "Louver", idu.controls.louver_mode.name if idu else "--", ) - if idu and idu.controls.louver_mode.name == "FIXED" and idu.controls.louver_fixed_position: + if ( + idu + and idu.controls.louver_mode == LouverMode.FIXED + and idu.controls.louver_fixed_position + ): self._kv( "ctl-louver-pos", " Fixed Pos", @@ -1092,7 +1303,7 @@ def _populate_status(self) -> None: state_preset_name = "--" if s.comfort_setting_id: cs_state = next( - (x for x in self._snapshot.comfort_settings if x.id == s.comfort_setting_id), + (x for x in self.snapshot.comfort_settings if x.id == s.comfort_setting_id), None, ) if cs_state: @@ -1132,7 +1343,7 @@ def _populate_status(self) -> None: away_cs = next( ( cs - for cs in self._snapshot.comfort_settings + for cs in self.snapshot.comfort_settings if cs.space_id == space.id and cs.type.name == "AWAY" ), None, @@ -1165,7 +1376,7 @@ def _populate_status(self) -> None: _tc(s.ambient_temperature_c, use_f), "green", ) - if idu and idu.state.calculated_ambient_temperature_c: + if idu and idu.state.calculated_ambient_temperature_c is not None: self._kv( "sen-calc-ambient", "Ambient (calc)", @@ -1177,14 +1388,19 @@ def _populate_status(self) -> None: "sen-humidity", "Humidity", f"{idu.state.ambient_humidity_percent:.0f}%" - if idu and idu.state.ambient_humidity_percent + if idu and idu.state.ambient_humidity_percent is not None else "--", ) fan_rpm = idu.state.fan_speed_rpm if idu and idu.state else None + if fan_rpm is None: + fan_rpm_str = "--" + else: + # 0 RPM is a real reading — the fan is off. + fan_rpm_str = f"{fan_rpm:.0f} RPM" if fan_rpm else "Off" self._kv( "sen-fan-rpm", "Fan Speed (actual)", - f"{fan_rpm:.0f} RPM" if fan_rpm else "Off", + fan_rpm_str, ) fan_sp_rpm = idu.state.fan_speed_setpoint_rpm if idu and idu.state else None self._kv( @@ -1193,7 +1409,7 @@ def _populate_status(self) -> None: f"{fan_sp_rpm:.0f} RPM" if fan_sp_rpm else "--", ) self._kv("sen-setpoint", "Active Setpoint", _tc(s.setpoint_c, use_f)) - if idu and idu.state.inlet_temperature_c: + if idu and idu.state.inlet_temperature_c is not None: self._kv( "sen-inlet", "Inlet Temp", @@ -1201,7 +1417,7 @@ def _populate_status(self) -> None: ) else: self._kv("sen-inlet", "Inlet Temp", "--") - if idu and idu.state.outlet_temperature_c: + if idu and idu.state.outlet_temperature_c is not None: self._kv( "sen-outlet", "Outlet Temp", @@ -1209,7 +1425,7 @@ def _populate_status(self) -> None: ) else: self._kv("sen-outlet", "Outlet Temp", "--") - if idu and idu.state.louver_angle_up_down_degrees: + if idu and idu.state.louver_angle_up_down_degrees is not None: self._kv( "sen-louver-angle", "Louver Angle", @@ -1360,11 +1576,7 @@ def _wifi_str(w: object | None) -> str: self._kv("dial-remote-sensor", "Zone Sensor", rsm_str, rsm_style) # ControllerRemoteSensor — Dial acting as zone sensor crs = next( - ( - r - for r in self._snapshot.controller_remote_sensors - if r.controller_id == ctrl.id - ), + (r for r in self.snapshot.controller_remote_sensors if r.controller_id == ctrl.id), None, ) if crs: @@ -1377,12 +1589,14 @@ def _wifi_str(w: object | None) -> str: self._kv( "dial-crs-humidity", " Zone Humidity", - f"{crs.humidity_percent:.0f}%" if crs.humidity_percent else "--", + f"{crs.humidity_percent:.0f}%" if crs.humidity_percent is not None else "--", ) self._kv( "dial-crs-battery", " Battery", - f"{crs.battery_level_percent:.0f}%" if crs.battery_level_percent else "--", + f"{crs.battery_level_percent:.0f}%" + if crs.battery_level_percent is not None + else "--", ) self._kv( "dial-crs-signal", @@ -1757,22 +1971,12 @@ def _cs(val: int) -> tuple[str, str]: def _populate_schedule(self) -> None: space_id = self._space.id - snap = self._snapshot + snap = self.snapshot week = next((w for w in snap.schedule_weeks if w.space_id == space_id), None) self._sched_day_by_id = {d.id: d for d in snap.schedule_days} self._sched_cs_by_id = {cs.id: cs for cs in snap.comfort_settings} - _DAYS = [ - "Monday", - "Tuesday", - "Wednesday", - "Thursday", - "Friday", - "Saturday", - "Sunday", - ] - week_table: DataTable = self.query_one("#sched-week", DataTable) if not week_table.columns: week_table.add_columns("Day") @@ -1780,14 +1984,14 @@ def _populate_schedule(self) -> None: week_table.clear() # Each weekday can map to multiple day programs (one event each). # _sched_row_day_ids[i] is a list of day_ids for weekday i+1. - self._sched_row_day_ids: list[list[str]] = [[] for _ in _DAYS] + self._sched_row_day_ids: list[list[str]] = [[] for _ in _WEEKDAY_NAMES] if week: for wd in week.days: idx = wd.weekday - 1 # weekday 1=Mon … 7=Sun → 0-based if 0 <= idx < 7: self._sched_row_day_ids[idx].append(wd.day_id) - for day_name in _DAYS: + for day_name in _WEEKDAY_NAMES: week_table.add_row(day_name) # Show Monday's events by default self._populate_day_events( @@ -1800,7 +2004,7 @@ def _populate_schedule(self) -> None: label="Monday", ) else: - for day_name in _DAYS: + for day_name in _WEEKDAY_NAMES: week_table.add_row(day_name) self._populate_day_events([], {}, label="Monday") @@ -1809,22 +2013,13 @@ def _populate_schedule(self) -> None: @on(DataTable.RowHighlighted, "#sched-week") def _on_sched_week_row_highlighted(self, event: DataTable.RowHighlighted) -> None: - _DAYS = [ - "Monday", - "Tuesday", - "Wednesday", - "Thursday", - "Friday", - "Saturday", - "Sunday", - ] idx = event.cursor_row - row_ids = getattr(self, "_sched_row_day_ids", [[] for _ in _DAYS]) + row_ids = getattr(self, "_sched_row_day_ids", [[] for _ in _WEEKDAY_NAMES]) cs_by_id = getattr(self, "_sched_cs_by_id", {}) day_by_id = getattr(self, "_sched_day_by_id", {}) day_ids = row_ids[idx] if idx < len(row_ids) else [] days = [day_by_id[did] for did in day_ids if did in day_by_id] - self._populate_day_events(days, cs_by_id, label=_DAYS[idx] if idx < 7 else "") + self._populate_day_events(days, cs_by_id, label=_WEEKDAY_NAMES[idx] if idx < 7 else "") def _populate_day_events(self, days: list, cs_by_id: dict, label: str = "") -> None: from quilt_hp.models.enums import HVACMode as _HM @@ -1903,7 +2098,7 @@ async def _fetch_energy(self) -> None: try: self._set_energy_status("⟳ Loading energy data…") tz = datetime.UTC - snap_tz = self._snapshot.timezone + snap_tz = self.snapshot.timezone if snap_tz: try: import zoneinfo @@ -1928,7 +2123,7 @@ def _set_energy_status(self, msg: str) -> None: except NoMatches: pass - def _populate_energy(self, metrics: object | None, tz: datetime.timezone) -> None: + def _populate_energy(self, metrics: object | None, tz: datetime.tzinfo) -> None: from quilt_hp.models.energy import SpaceEnergyMetrics table: DataTable = self.query_one("#e-table", DataTable) @@ -2027,14 +2222,7 @@ def update_space(self, space: Space) -> None: def update_idu(self, idu: IndoorUnit) -> None: self._idu = idu - odu = self._snapshot.odu_for_idu(idu) - if odu is None: - space_ids = _id_tokens(self._space.id) - odu = next( - (u for u in self._snapshot.outdoor_units if _id_tokens(u.space_id) & space_ids), - None, - ) - self._odu = odu + self._odu = _odu_for_space(self.snapshot, self._space.id, idu) self._populate_status() self._populate_perf() @@ -2060,8 +2248,12 @@ def action_refresh_energy(self) -> None: self._fetch_energy() def action_toggle_units(self) -> None: - self.use_f = not self.use_f - self.app._persist() + app = self.app + if isinstance(app, QuiltApp): + app.use_f = not app.use_f + + def refresh_units(self) -> None: + """Re-render temperature-bearing panels after a °C/°F change.""" self._populate_status() self._populate_perf() @@ -2093,10 +2285,10 @@ def _delta_setpoint(self, which: str, delta: float) -> None: return c = self._space.controls if which == "heat": - val = (c.heating_setpoint_c or 20.0) + delta + val = clamp_setpoint_c((c.heating_setpoint_c or DEFAULT_HEAT_SETPOINT_C) + delta) self._mutate_space(heat_setpoint_c=val) else: - val = (c.cooling_setpoint_c or 26.0) + delta + val = clamp_setpoint_c((c.cooling_setpoint_c or DEFAULT_COOL_SETPOINT_C) + delta) self._mutate_space(cool_setpoint_c=val) def action_cycle_fan(self) -> None: @@ -2114,16 +2306,15 @@ def action_cycle_louver(self) -> None: def action_toggle_led(self) -> None: if not self._idu: return - new_brightness = 0.0 if self._idu.controls.light_on else 1.0 - self._mutate_idu(led_brightness=new_brightness) - - def action_cycle_occupancy(self) -> None: - if not self._space: + if self._idu.controls.light_on: + self._mutate_idu(led_brightness=0.0) return - nxt = _cycle_next(self._space.settings.occupancy_mode, _OCC_CYCLE) - # occupancy_mode is a settings field; mutate via a future API if added. - # For now notify the user it's read-only in this version. - self.notify(f"Occupancy mode would → {nxt.name} (not yet wired)", timeout=3) + # Restore the stored brightness (preserved server-side when off); + # fall back to the configured default, then full brightness. + restore = self._idu.controls.led_brightness + if restore <= 0.0: + restore = self._idu.settings.light_brightness_default_percent or 1.0 + self._mutate_idu(led_brightness=restore) _AWAY_TIMEOUT_STEP_S: float = 300.0 # 5 minutes _RETURN_TIMEOUT_STEP_S: float = 60.0 # 1 minute @@ -2158,7 +2349,7 @@ def action_return_timeout_inc(self) -> None: self._mutate_settings(occupied_timeout_s=cur + self._RETURN_TIMEOUT_STEP_S) def action_toggle_schedule(self) -> None: - loc = self._snapshot.primary_location + loc = self.snapshot.primary_location if loc is None: self.notify("No location found", severity="error") return @@ -2288,14 +2479,7 @@ async def _mutate_idu_settings( @work async def _do_toggle_schedule(self, paused: bool) -> None: try: - await self._client.set_schedule_execution(paused) - loc = self._snapshot.primary_location - if loc: - # patch local cache - from dataclasses import replace - - patched = replace(loc, schedule_paused=paused) - self._snapshot.locations[0] = patched + await _set_schedule_paused(self._client, self.snapshot, paused) self._update_schedule_status(paused) self.notify("Schedules " + ("paused" if paused else "resumed"), timeout=2) except Exception as exc: @@ -2316,8 +2500,6 @@ class SystemScreen(Screen): Binding("p", "toggle_schedule", "Pause Sched"), ] - use_f: reactive[bool] = reactive(False) - def __init__( self, snapshot: SystemSnapshot, @@ -2326,9 +2508,27 @@ def __init__( use_f: bool = False, ) -> None: super().__init__() - self._snapshot = snapshot + self._snapshot = snapshot # fallback when not attached to a QuiltApp self._client = client - self.use_f = use_f + self._use_f = use_f + + @property + def snapshot(self) -> SystemSnapshot: + """The app-owned snapshot, falling back to the constructor value.""" + with contextlib.suppress(Exception): + app = self.app + if isinstance(app, QuiltApp) and app.snapshot is not None: + return app.snapshot + return self._snapshot + + @property + def use_f(self) -> bool: + """App-level °C/°F preference (falls back to the constructor value).""" + with contextlib.suppress(Exception): + app = self.app + if isinstance(app, QuiltApp): + return app.use_f + return self._use_f def compose(self) -> ComposeResult: yield Header(show_clock=True) @@ -2340,12 +2540,12 @@ def compose(self) -> ComposeResult: # ODU row — one panel per outdoor unit with Horizontal(id="odu-row"): - if self._snapshot.outdoor_units: - for i in range(len(self._snapshot.outdoor_units)): + if self.snapshot.outdoor_units: + for i in range(len(self.snapshot.outdoor_units)): with Vertical(classes="odu-panel") as v: v.border_title = ( f"Outdoor Unit {i + 1}" - if len(self._snapshot.outdoor_units) > 1 + if len(self.snapshot.outdoor_units) > 1 else "Outdoor Unit" ) yield Static(id=f"sys-odu-{i}") @@ -2372,7 +2572,7 @@ def on_mount(self) -> None: self._populate() def _populate(self) -> None: - snap = self._snapshot + snap = self.snapshot use_f = self.use_f # Header @@ -2475,8 +2675,10 @@ def _populate(self) -> None: idu_to_room.get(rs.indoor_unit_id, rs.indoor_unit_id[:8]), Text(mode_str, style=mode_style), _tc(rs.ambient_temperature_c, use_f), - f"{rs.humidity_percent:.0f}%" if rs.humidity_percent else "--", - f"{rs.battery_level_percent:.0f}%" if rs.battery_level_percent else "--", + f"{rs.humidity_percent:.0f}%" if rs.humidity_percent is not None else "--", + f"{rs.battery_level_percent:.0f}%" + if rs.battery_level_percent is not None + else "--", f"{rs.signal_level_dbm} dBm" if rs.signal_level_dbm else "--", ) for crs in snap.controller_remote_sensors: @@ -2505,8 +2707,10 @@ def _populate(self) -> None: room_name, Text(mode_str, style=mode_style), _tc(crs.ambient_temperature_c, use_f), - f"{crs.humidity_percent:.0f}%" if crs.humidity_percent else "--", - f"{crs.battery_level_percent:.0f}%" if crs.battery_level_percent else "--", + f"{crs.humidity_percent:.0f}%" if crs.humidity_percent is not None else "--", + f"{crs.battery_level_percent:.0f}%" + if crs.battery_level_percent is not None + else "--", f"{crs.signal_level_dbm} dBm" if crs.signal_level_dbm else "--", ) @@ -2568,8 +2772,12 @@ def _fw_row(device_name: str, sw_id: str | None, fw_id: str | None) -> None: def action_back(self) -> None: self.app.pop_screen() - def update_odu(self, odu: OutdoorUnit) -> None: - """Called by QuiltApp stream dispatcher when an ODU update arrives.""" + def update_odu(self, _odu: OutdoorUnit) -> None: + """Called by QuiltApp stream dispatcher when an ODU update arrives. + + The updated ODU is already merged into the snapshot, which + ``_populate`` reads directly. + """ self._populate() def update_remote_sensor(self, rs: RemoteSensor) -> None: @@ -2577,12 +2785,16 @@ def update_remote_sensor(self, rs: RemoteSensor) -> None: self._populate() def action_toggle_units(self) -> None: - self.use_f = not self.use_f + app = self.app + if isinstance(app, QuiltApp): + app.use_f = not app.use_f + + def refresh_units(self) -> None: + """Re-render the system panels after a °C/°F change.""" self._populate() - self.app._persist() def action_toggle_schedule(self) -> None: - loc = self._snapshot.primary_location + loc = self.snapshot.primary_location if loc is None: self.notify("No location found", severity="error") return @@ -2591,13 +2803,7 @@ def action_toggle_schedule(self) -> None: @work async def _do_toggle_schedule(self, paused: bool) -> None: try: - await self._client.set_schedule_execution(paused) - loc = self._snapshot.primary_location - if loc: - from dataclasses import replace - - patched = replace(loc, schedule_paused=paused) - self._snapshot.locations[0] = patched + await _set_schedule_paused(self._client, self.snapshot, paused) self._populate() self.notify("Schedules " + ("paused" if paused else "resumed"), timeout=2) except Exception as exc: @@ -2619,27 +2825,50 @@ class QuiltApp(App[None]): Binding("d", "toggle_dark", "Dark/Light", priority=True), ] + _STREAM_RECOVERY_DELAYS_S: ClassVar = (5.0, 15.0, 30.0) + + use_f: reactive[bool] = reactive(False) + def __init__(self, email: str, home: str | None = None) -> None: super().__init__() self._email = email self._home = home self._client = QuiltClient(email, home=home, snapshot_ttl_s=30, token_store=_token_store) - self._stream = None - self._snapshot = None + self._stream: NotifierStream | None = None + self._snapshot: SystemSnapshot | None = None self._settings = _settings_store.load() - # Apply persisted dark/light before first render + # Apply persisted preferences before first render; set_reactive avoids + # triggering watch_use_f before the app is running. + self.set_reactive(QuiltApp.use_f, self._settings.use_fahrenheit) if self._settings.dark is not None: self.theme = "textual-dark" if self._settings.dark else "textual-light" + # ── Shared state (single source of truth for all screens) ──── + + @property + def snapshot(self) -> SystemSnapshot | None: + """The current system snapshot shared by all screens.""" + return self._snapshot + + def update_snapshot(self, snap: SystemSnapshot) -> None: + """Adopt a freshly fetched snapshot as the shared source of truth.""" + self._snapshot = snap + + def watch_use_f(self, use_f: bool) -> None: + """Persist the °C/°F preference and re-render every stacked screen.""" + self._persist() + for screen in self.screen_stack: + refresh = getattr(screen, "refresh_units", None) + if callable(refresh): + refresh() + @property def _is_dark(self) -> bool: return self.theme != "textual-light" def _persist(self) -> None: """Save current toggleable settings to disk.""" - screen = self.screen - use_f = getattr(screen, "use_f", self._settings.use_fahrenheit) - self._settings = _settings_store.update(use_fahrenheit=use_f, dark=self._is_dark) + self._settings = _settings_store.update(use_fahrenheit=self.use_f, dark=self._is_dark) def action_toggle_dark(self) -> None: self.theme = "textual-light" if self._is_dark else "textual-dark" @@ -2650,6 +2879,10 @@ def on_mount(self) -> None: self.push_screen(self._loading_screen) self._boot() + async def _prompt_otp(self, email: str) -> str: + """OTP callback for first-time logins — modal input in the TUI.""" + return await self.push_screen_wait(OtpScreen(email)) + @work async def _boot(self) -> None: """Log in, fetch snapshot, and replace LoadingScreen.""" @@ -2663,7 +2896,7 @@ def _set_status(msg: str) -> None: try: _set_status("Authenticating…") - await self._client.login() + await self._client.login(otp_callback=self._prompt_otp) _set_status("Loading system snapshot…") snap = await self._client.get_snapshot() self._snapshot = snap @@ -2679,21 +2912,30 @@ def _set_status(msg: str) -> None: dashboard = DashboardScreen(snap, self._client) await self.switch_screen(dashboard) - # Restore persisted use_fahrenheit - # (dark mode already applied in __init__) - if self._settings.use_fahrenheit: - dashboard.use_f = True - # Start the shared stream self._start_stream(snap) + except QuiltAuthError as exc: + self._show_boot_error( + str(exc), + "Authentication failed. Run `quilt login` in a terminal and retry.", + ) except Exception as exc: - self.notify(f"Boot failed: {exc}", severity="error") + logger.exception("TUI boot failed") + self._show_boot_error(str(exc)) - @work(exclusive=True) + def _show_boot_error(self, message: str, hint: str | None = None) -> None: + """Replace the loading spinner with an actionable error screen.""" + self.notify(f"Boot failed: {message}", severity="error") + self.switch_screen(BootErrorScreen(message, hint)) + + # ── Stream lifecycle ───────────────────────────────────────── + + @work(exclusive=True, group="stream") async def _start_stream(self, snap: SystemSnapshot) -> None: """Open shared NotifierStream, dispatch events to the active screen.""" stream = self._client.stream(snap.stream_topics()) + self._stream = stream # Stream callbacks are invoked from within async code on the same event # loop — call UI dispatch methods directly (no call_from_thread). @@ -2703,16 +2945,67 @@ async def _start_stream(self, snap: SystemSnapshot) -> None: stream.on_controller_update(self._dispatch_ctrl) stream.on_qsm_update(self._dispatch_qsm) stream.on_remote_sensor_update(self._dispatch_remote_sensor) + stream.on_error(self._on_stream_error) - with contextlib.suppress(Exception): + try: await stream.run_forever() + except asyncio.CancelledError: + raise + except Exception as exc: + # Fatal errors are normally reported via on_error; anything that + # still escapes run_forever is unexpected — surface it too. + logger.exception("NotifierStream terminated unexpectedly") + self._on_stream_error(exc) + + def _on_stream_error(self, exc: Exception) -> None: + """Fatal stream error — tell the user and try to recover.""" + logger.error("Notifier stream failed: %s", exc) + self._set_stream_disconnected(True) + self.notify( + f"Live updates disconnected: {exc}. Attempting to reconnect…", + severity="error", + timeout=8, + ) + self._recover_stream() + + def _set_stream_disconnected(self, disconnected: bool) -> None: + """Show/clear a visible 'stream disconnected' indicator.""" + self.sub_title = "⚠ live updates disconnected" if disconnected else "" + + @work(exclusive=True, group="stream-recovery") + async def _recover_stream(self) -> None: + """Re-fetch a snapshot and restart the stream, with backoff.""" + for delay in self._STREAM_RECOVERY_DELAYS_S: + await asyncio.sleep(delay) + try: + snap = await self._client.get_snapshot() + except Exception as exc: + logger.warning("Stream recovery snapshot fetch failed: %s", exc) + continue + self.update_snapshot(snap) + for screen in self.screen_stack: + refresh = getattr(screen, "refresh_units", None) + if callable(refresh): + refresh() + self._set_stream_disconnected(False) + self.notify("Live updates restored", timeout=3) + self._start_stream(snap) + return + self.notify( + "Could not restore live updates. Data may be stale — press r to refresh, " + "or restart the app.", + severity="error", + timeout=10, + ) + + # ── Stream event dispatchers ───────────────────────────────── def _dispatch_space(self, space: Space) -> None: if self._snapshot: space = self._snapshot.apply_space(space) screen = self.screen if isinstance(screen, DashboardScreen) or ( - isinstance(screen, RoomScreen) and screen._space.id == space.id + isinstance(screen, RoomScreen) and screen.space_id == space.id ): screen.update_space(space) @@ -2720,29 +3013,17 @@ def _dispatch_idu(self, idu: IndoorUnit) -> None: if self._snapshot: idu = self._snapshot.apply_indoor_unit(idu) screen = self.screen - if isinstance(screen, RoomScreen) and screen._idu and screen._idu.id == idu.id: + if (isinstance(screen, RoomScreen) and screen.idu_id == idu.id) or isinstance( + screen, DashboardScreen + ): screen.update_idu(idu) - elif isinstance(screen, DashboardScreen): - space = ( - next( - (s for s in self._snapshot.rooms if s.id == idu.space_id), - None, - ) - if self._snapshot - else None - ) - if space: - item = screen._items.get(space.id) - if item: - item.update_space(space, idu, screen.use_f) - item.refresh() def _dispatch_odu(self, odu: OutdoorUnit) -> None: if self._snapshot: odu = self._snapshot.apply_outdoor_unit(odu) screen = self.screen if isinstance(screen, (DashboardScreen, SystemScreen)) or ( - isinstance(screen, RoomScreen) and screen._odu and screen._odu.id == odu.id + isinstance(screen, RoomScreen) and screen.odu_id == odu.id ): screen.update_odu(odu) @@ -2750,18 +3031,14 @@ def _dispatch_ctrl(self, ctrl: Controller) -> None: if self._snapshot: ctrl = self._snapshot.apply_controller(ctrl) screen = self.screen - if ( - isinstance(screen, RoomScreen) - and screen._controller - and screen._controller.id == ctrl.id - ): + if isinstance(screen, RoomScreen) and screen.controller_id == ctrl.id: screen.update_ctrl(ctrl) def _dispatch_qsm(self, qsm: QuiltSmartModule) -> None: if self._snapshot: qsm = self._snapshot.apply_qsm(qsm) screen = self.screen - if isinstance(screen, RoomScreen) and screen._qsm and screen._qsm.id == qsm.id: + if isinstance(screen, RoomScreen) and screen.qsm_id == qsm.id: screen.update_qsm(qsm) def _dispatch_remote_sensor(self, rs: RemoteSensor) -> None: @@ -2772,4 +3049,9 @@ def _dispatch_remote_sensor(self, rs: RemoteSensor) -> None: screen.update_remote_sensor(rs) async def on_unmount(self) -> None: + stream = self._stream + self._stream = None + if stream is not None: + with contextlib.suppress(Exception): + await stream.stop() await self._client.close() diff --git a/tests/test_auth_store_settings_edges.py b/tests/test_auth_store_settings_edges.py index a4105ff..03b60b4 100644 --- a/tests/test_auth_store_settings_edges.py +++ b/tests/test_auth_store_settings_edges.py @@ -12,6 +12,11 @@ from quilt_hp.tokens import CachedTokens, RefreshFailureAction, TokenRefreshContext +def _corrupt_backups(directory: Path) -> list[Path]: + """Sync helper so async tests avoid direct Path.glob (ASYNC240).""" + return sorted(directory.glob("tokens.json.corrupt-*")) + + class _RaisePolicy: def on_refresh_failure( self, _context: TokenRefreshContext, _error: Exception @@ -80,14 +85,52 @@ async def test_filestore_malformed_entry_and_clear_json_errors(tmp_path: Path) - m.setattr(store, "_token_path", lambda: path) await store.load("user@example.com") + # Corrupt JSON no longer raises: the file is quarantined and the store + # starts empty, so the worst case is one re-login. path.write_text("{bad-json") - with ( - pytest.raises(QuiltAuthError, match="invalid JSON"), - pytest.MonkeyPatch.context() as m, - ): + with pytest.MonkeyPatch.context() as m: m.setattr(store, "_token_path", lambda: path) store.clear_tokens("user@example.com") + assert not path.exists() + backups = _corrupt_backups(tmp_path) + assert len(backups) == 1 + assert backups[0].read_text() == "{bad-json" + + +async def test_filestore_recovers_from_corrupt_file_on_load(tmp_path: Path) -> None: + store = FileStore() + path = tmp_path / "tokens.json" + path.write_text("{bad-json") + + with pytest.MonkeyPatch.context() as m: + m.setattr(store, "_token_path", lambda: path) + assert await store.load("user@example.com") is None + # File was quarantined; a subsequent save works and round-trips. + tokens = CachedTokens( + id_token="id", + refresh_token="ref", + expires_at=9999999999.0, + ) + await store.save("user@example.com", tokens) + loaded = await store.load("user@example.com") + + assert loaded is not None + assert loaded.refresh_token == "ref" + assert _corrupt_backups(tmp_path) + + +async def test_filestore_recovers_from_non_dict_payload(tmp_path: Path) -> None: + store = FileStore() + path = tmp_path / "tokens.json" + path.write_text(json.dumps(["not", "a", "dict"])) + + with pytest.MonkeyPatch.context() as m: + m.setattr(store, "_token_path", lambda: path) + assert await store.load("user@example.com") is None + + assert _corrupt_backups(tmp_path) + def test_settings_store_corruption_and_schema_edges(tmp_path: Path) -> None: path = tmp_path / "settings.json" diff --git a/tests/test_cli_surfaces_extra.py b/tests/test_cli_surfaces_extra.py index dcd020e..f4d6cc2 100644 --- a/tests/test_cli_surfaces_extra.py +++ b/tests/test_cli_surfaces_extra.py @@ -4,6 +4,7 @@ from typing import ClassVar from unittest.mock import patch +import pytest from typer.testing import CliRunner from quilt_hp.cli import main as cli_main @@ -125,12 +126,44 @@ async def get_snapshot(self) -> SimpleNamespace: patch.object(cli_main, "_resolve", return_value=("user@example.com", None)), patch.object(cli_main, "QuiltClient", _NoRoomClient), ): - result = runner.invoke(cli_main.app, ["set", "Missing Room"]) + result = runner.invoke(cli_main.app, ["set", "Missing Room", "--mode", "COOL"]) assert result.exit_code == 1 assert "not found" in result.stdout +def test_set_command_without_options_is_an_error() -> None: + result = runner.invoke(cli_main.app, ["set", "Living Room"]) + + assert result.exit_code == 1 + assert "Nothing to update" in result.stdout + + +def test_set_command_rejects_fallback_modes() -> None: + with ( + patch.object(cli_main, "_resolve", return_value=("user@example.com", None)), + patch.object(cli_main, "QuiltClient", _FakeClient), + ): + result = runner.invoke(cli_main.app, ["set", "Living Room", "--mode", "FALLBACK_AUTO"]) + + assert result.exit_code == 1 + assert "Invalid mode" in result.stdout + + +@pytest.mark.parametrize("option", ["--heat", "--cool"]) +@pytest.mark.parametrize("value", ["7.9", "32.1"]) +def test_set_command_rejects_out_of_range_setpoints(option: str, value: str) -> None: + result = runner.invoke(cli_main.app, ["set", "Living Room", option, value]) + + assert result.exit_code == 2 + + +def test_energy_command_rejects_unknown_period() -> None: + result = runner.invoke(cli_main.app, ["energy", "--period", "fortnight"]) + + assert result.exit_code == 2 + + def test_tui_command_handles_missing_optional_dependency(monkeypatch) -> None: # type: ignore[no-untyped-def] original_import = __import__ diff --git a/tests/test_tokens.py b/tests/test_tokens.py index 15e289e..9367b1b 100644 --- a/tests/test_tokens.py +++ b/tests/test_tokens.py @@ -9,7 +9,6 @@ import pytest from quilt_hp.cli.store import FileStore -from quilt_hp.exceptions import QuiltAuthError from quilt_hp.tokens import CachedTokens @@ -89,15 +88,22 @@ async def test_list_emails(tmp_path: Path) -> None: @pytest.mark.asyncio -async def test_load_invalid_json_raises(tmp_path: Path) -> None: +async def test_load_invalid_json_recovers_with_backup(tmp_path: Path) -> None: store = FileStore() path = tmp_path / "tokens.json" path.write_text("{not-json") - with ( - patch.object(store, "_token_path", return_value=path), - pytest.raises(QuiltAuthError, match="invalid JSON"), - ): - await store.load("user@test.com") + with patch.object(store, "_token_path", return_value=path): + # Corruption is quarantined (renamed to a .corrupt-* backup) and the + # store starts empty — worst case is one re-login, never a hard lock. + assert await store.load("user@test.com") is None + _assert_corrupt_backup(tmp_path, path) + + +def _assert_corrupt_backup(tmp_path: Path, path: Path) -> None: + assert not path.exists() + backups = list(tmp_path.glob("tokens.json.corrupt-*")) + assert len(backups) == 1 + assert backups[0].read_text() == "{not-json" def test_is_expired() -> None: diff --git a/tests/test_tui_bindings.py b/tests/test_tui_bindings.py index 8ce02d9..deb467b 100644 --- a/tests/test_tui_bindings.py +++ b/tests/test_tui_bindings.py @@ -1,5 +1,7 @@ from __future__ import annotations +from typing import ClassVar + import pytest pytest.importorskip("textual") @@ -68,3 +70,53 @@ def odu_for_idu(self, _idu: object) -> object | None: screen = DashboardScreen(snapshot=_Snap(), client=object()) odu = screen._odu_for("space/space-1", idu=object()) assert odu is not None + + +def test_room_screen_has_no_occupancy_cycle_binding() -> None: + keymap = _key_action_map() + assert "o" not in keymap + assert not hasattr(RoomScreen, "action_cycle_occupancy") + + +def test_setpoint_clamp_constants() -> None: + from quilt_hp.cli.constants import SETPOINT_MAX_C, SETPOINT_MIN_C, clamp_setpoint_c + + assert clamp_setpoint_c(SETPOINT_MIN_C - 5) == SETPOINT_MIN_C + assert clamp_setpoint_c(SETPOINT_MAX_C + 5) == SETPOINT_MAX_C + assert clamp_setpoint_c(21.5) == 21.5 + + +def test_patch_schedule_paused_replaces_location_in_place() -> None: + from dataclasses import dataclass + + from quilt_hp.cli.tui import _patch_schedule_paused + + @dataclass + class _Loc: + schedule_paused: bool + + class _Snap: + def __init__(self) -> None: + self.locations = [_Loc(schedule_paused=False)] + + @property + def primary_location(self) -> _Loc | None: + return self.locations[0] if self.locations else None + + snap = _Snap() + _patch_schedule_paused(snap, paused=True) # type: ignore[arg-type] + assert snap.locations[0].schedule_paused is True + + +def test_odu_for_space_prefers_idu_link() -> None: + from quilt_hp.cli.tui import _odu_for_space + + sentinel = object() + + class _Snap: + outdoor_units: ClassVar[list[object]] = [] + + def odu_for_idu(self, _idu: object) -> object: + return sentinel + + assert _odu_for_space(_Snap(), "space/space-1", idu=object()) is sentinel # type: ignore[arg-type] From 5bcc5bfb328192923b2a2832af24ad80cc153a60 Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Thu, 2 Jul 2026 15:47:13 -0700 Subject: [PATCH 6/9] docs: fix raw-stream example, stale references, and proto comments - README Quick Start now merges stream diffs via snapshot.apply_space instead of demonstrating the documented misuse; client.stream() docstring examples likewise - Re-export NotifierStream, StreamEvent, and token protocol types at the package top level so consumers don't import from quilt_hp.services - Update reference docs: real OutdoorUnit/Controller dataclasses (the documented OutdoorUnitState/ControllerState never existed), nullable controller temperatures, apply_software_update_info, on_connected and unsubscribe callbacks, reconnect back-off semantics, rotated refresh token persistence - Fix contradictory proto comments: led_animation/led_state field-number note and louver_fixed_position fraction-vs-degrees --- README.md | 14 ++++-- docs/explanation/streaming-protocol.md | 2 +- docs/reference/client.md | 4 +- docs/reference/hds-entities.md | 4 +- docs/reference/models.md | 69 ++++++++++++++++++-------- docs/reference/token-management.md | 5 ++ proto/cleaned/quilt_hds.proto | 4 +- src/quilt_hp/__init__.py | 24 ++++++++- src/quilt_hp/client.py | 6 ++- 9 files changed, 98 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index ccec9b0..16ec6bf 100644 --- a/README.md +++ b/README.md @@ -36,11 +36,15 @@ async def main(): spaces = await client.list_spaces() await client.set_space(spaces[0].id, mode=HVACMode.COOL, cool_setpoint_c=22.0) - # Stream real-time updates - spaces = await client.list_spaces() - topics = [f"hds/space/{s.id}" for s in spaces] - async with client.stream(topics) as stream: - stream.on_space_update(lambda s: print(f"{s.name}: {s.state.ambient_temperature_c}°C")) + # Stream real-time updates — stream events are sparse diffs, so + # always merge them into a snapshot before use + snapshot = await client.get_snapshot() + async with client.stream(snapshot.stream_topics()) as stream: + def on_space(space): + merged = snapshot.apply_space(space) + print(f"{merged.name}: {merged.state.ambient_temperature_c}°C") + + stream.on_space_update(on_space) await asyncio.sleep(60) asyncio.run(main()) diff --git a/docs/explanation/streaming-protocol.md b/docs/explanation/streaming-protocol.md index 9d3c8c4..be74e0b 100644 --- a/docs/explanation/streaming-protocol.md +++ b/docs/explanation/streaming-protocol.md @@ -84,7 +84,7 @@ stateDiagram-v2 Fatal --> [*] ``` -The back-off starts at `reconnect_delay_s` (default 1 second), doubles on each failed attempt, and caps at 60 seconds. The request queue is reset before each reconnect so the server receives a fresh subscription request with the full topic list. +The back-off starts at `reconnect_delay_s` (default 1 second), doubles on each consecutive failed attempt with ±50% jitter, and caps at 60 seconds. Both the back-off and the `max_reconnects` budget reset once a connection stays healthy for a while, so routine server-side stream recycling never escalates reconnect latency. After a successful token refresh the stream reconnects immediately. The request queue is reset before each reconnect so the server receives a fresh subscription request with the full topic list. `on_connected` callbacks fire on every successful (re)connect — use them to re-fetch a snapshot, since events published while disconnected are lost. The `UNAUTHENTICATED` path is special: the stream refreshes the token before waiting, because reconnecting immediately with a stale token would fail again. If the refresh itself fails, the stream gives up rather than entering an infinite retry loop with an invalid token. diff --git a/docs/reference/client.md b/docs/reference/client.md index e470391..6338e59 100644 --- a/docs/reference/client.md +++ b/docs/reference/client.md @@ -112,7 +112,7 @@ async def __aenter__(self) -> QuiltClient: ... async def __aexit__(self, *_: object) -> None: ... ``` -`__aexit__` calls `close()` to close the gRPC channel. Prefer the async context manager, or call `await close()` yourself when managing lifecycle manually. +`__aexit__` calls `close()`, which stops any live `NotifierStream`s created via `stream()` and closes the gRPC channel. Prefer the async context manager, or call `await close()` yourself when managing lifecycle manually. --- @@ -528,7 +528,7 @@ Creates a `NotifierStream`. It does not start the stream; call `start()`, `run_f **Parameters:** - `topics`: topic strings. Use `snapshot.stream_topics()` to get all topics for a system. -- `max_reconnects`: maximum reconnect attempts per disconnect. `-1` = unlimited (default). `0` = no retries. +- `max_reconnects`: maximum *consecutive* reconnect attempts — the counter resets after a connection stays healthy, so this bounds retries per disconnect event. `-1` = unlimited (default). `0` = no retries. - `reconnect_delay_s`: initial back-off in seconds. Doubles on each attempt, capped at 60 s. **Returns:** `NotifierStream` instance. diff --git a/docs/reference/hds-entities.md b/docs/reference/hds-entities.md index c86723a..969dee1 100644 --- a/docs/reference/hds-entities.md +++ b/docs/reference/hds-entities.md @@ -142,8 +142,8 @@ The Quilt Dial is a compact circular thermostat (58 mm diameter) that can be wal | `id` | `str` | Unique object ID | | `space_id` | `str` | Space this Dial controls | | `name` | `str` | Display name | -| `ambient_temperature_c` | `float` | Temperature measured by the Dial's built-in sensor | -| `raw_thermistor_c` | `float` | Raw uncalibrated thermistor reading | +| `ambient_temperature_c` | `float \| None` | Temperature measured by the Dial's built-in sensor; `None` when no state reading is available | +| `raw_thermistor_c` | `float \| None` | Raw uncalibrated thermistor reading; `None` when no state reading is available | | `remote_sensor_mode` | `HvacControllerType` | How the Dial's temperature reading influences the space setpoint | | `model_sku` | `str \| None` | Hardware model identifier | | `serial_number` | `str \| None` | Unit serial number | diff --git a/docs/reference/models.md b/docs/reference/models.md index e441774..d65785f 100644 --- a/docs/reference/models.md +++ b/docs/reference/models.md @@ -65,7 +65,7 @@ service = UserService(channel) ### `NotifierStream` ```python -from quilt_hp.services.streaming import NotifierStream +from quilt_hp import NotifierStream # also importable from quilt_hp.services.streaming # metadata_provider returns gRPC call metadata (e.g. auth headers). # Obtain a token from your QuiltClient or token store. @@ -84,10 +84,11 @@ stream = NotifierStream.create( See [Streaming protocol behavior](../explanation/streaming-protocol.md) for the full state machine, event types, and reconnect behavior. -Callback registration methods: +Callback registration methods (each returns an *unsubscribe* callable — +call it to detach the callback without stopping the stream): ```python -stream.on_space_update(callback) +unsub = stream.on_space_update(callback) stream.on_indoor_unit_update(callback) stream.on_outdoor_unit_update(callback) stream.on_controller_update(callback) @@ -95,9 +96,15 @@ stream.on_qsm_update(callback) stream.on_remote_sensor_update(callback) stream.on_controller_remote_sensor_update(callback) stream.on_software_update_info(callback) -stream.on_error(callback) +stream.on_error(callback) # fatal stream errors +stream.on_connected(callback) # fired on every successful (re)connect +unsub() # detach the space callback ``` +`on_connected` fires on every successful connect and reconnect. Events +published while disconnected are lost — use it to re-fetch a snapshot and +close the gap. + Lifecycle methods: ```python @@ -158,6 +165,7 @@ snapshot.apply_controller(controller) snapshot.apply_qsm(qsm) snapshot.apply_remote_sensor(sensor) snapshot.apply_controller_remote_sensor(sensor) +snapshot.apply_software_update_info(info) ``` --- @@ -291,19 +299,28 @@ class IndoorUnitState: class OutdoorUnit: id: str system_id: str + space_id: str + hvac_state: HVACState + model_sku: str | None serial_number: str | None - model_name: str | None - state: OutdoorUnitState + firmware_version: str | None + firmware_update_info_id: str | None + performance_data: OutdoorUnitPerformanceData | None ``` -#### `OutdoorUnitState` +#### `OutdoorUnitPerformanceData` ```python @dataclass -class OutdoorUnitState: - updated_at: datetime | None - outdoor_temp_c: float | None - is_online: bool +class OutdoorUnitPerformanceData: + measurement_interval_s: float + energy_measurement_j: float + compressor_frequency_hz: float + ambient_temperature_c: float + coil_temperature_c: float + exhaust_temperature_c: float + high_pressure_kpa: float + low_pressure_kpa: float ``` --- @@ -315,19 +332,31 @@ class OutdoorUnitState: class Controller: id: str system_id: str + space_id: str + name: str + raw_thermistor_c: float | None # None when no state reading available + pcb_temperature_a_c: float | None + pcb_temperature_b_c: float | None + calibrated_ambient_c: float | None # exposed as ambient_temperature_c + wifi_ssid: str | None + wifi_ip: str | None + wifi_signal_dbm: int | None + wifi_freq_mhz: int | None + wifi_last_seen: datetime | None + ap_wifi: WifiInfo | None + p2p_wifi: WifiInfo | None + remote_sensor_mode: RemoteSensorControlMode + software_update_info_id: str | None + firmware_update_info_id: str | None serial_number: str | None + model_sku: str | None firmware_version: str | None - state: ControllerState + state_updated_at: datetime | None + local_comms_health: LocalCommsHealthStatus ``` -#### `ControllerState` - -```python -@dataclass -class ControllerState: - updated_at: datetime | None - is_online: bool -``` +Useful properties: `ambient_temperature_c` (→ `calibrated_ambient_c`, `None` +when no state reading is available), `wifi_band`, `is_online`. --- diff --git a/docs/reference/token-management.md b/docs/reference/token-management.md index d446695..928c0ca 100644 --- a/docs/reference/token-management.md +++ b/docs/reference/token-management.md @@ -223,6 +223,11 @@ class TokenRefreshReason(StrEnum): - `TRANSPORT_UNAUTHENTICATED`: The gRPC transport interceptor received `UNAUTHENTICATED` on a unary RPC. - `STREAM_UNAUTHENTICATED`: The `NotifierStream` received `UNAUTHENTICATED` and is refreshing before reconnecting. +For the two `*_UNAUTHENTICATED` reasons, `authenticate()` does **not** trust a +locally-unexpired cached IdToken — the server just rejected it, so a Cognito +refresh is forced. If the Cognito response contains a rotated `RefreshToken`, +the new value is persisted to the token store. + --- ## `RefreshFailureAction` diff --git a/proto/cleaned/quilt_hds.proto b/proto/cleaned/quilt_hds.proto index 5713236..d4c5cb7 100644 --- a/proto/cleaned/quilt_hds.proto +++ b/proto/cleaned/quilt_hds.proto @@ -333,7 +333,7 @@ message IndoorUnitSettings { // f1,f2=absent; f3=ledColorCode(varint/uint32), f4=ledColorBrightnessPercent(float), // f5=fanSpeedMode(varint), f6=fanSpeedPercent(float), f7=updatedTs(Timestamp), // f8,f9=absent; f10=louverMode(varint), f11=louverFixedPosition(float), -// f12=lightState(varint), f13=lightAnimation(varint). +// f12=lightAnimation(varint), f13=lightState(varint). // NOTE: These are FLAT fields, NOT nested sub-messages. message IndoorUnitControls { // f1, f2 absent from wire captures @@ -344,7 +344,7 @@ message IndoorUnitControls { google.protobuf.Timestamp updated_ts = 7; // f8, f9: uuid-string field confirmed at f9 in captures; purpose TBD IndoorUnitLouverMode louver_mode = 10; - float louver_fixed_position = 11; // degrees, used when louver_mode=FIXED + float louver_fixed_position = 11; // position fraction 0.20–1.00 (see LouverAngle.to_wire), used when louver_mode=FIXED; 0.0 = not applicable LightAnimation led_animation = 12; // LED_ANIMATION_FIELD_NUMBER=12; wire-confirmed LightState led_state = 13; // wire-confirmed f13: UNSPECIFIED=0,ON=1,OFF=2 // sent when mobile_led_scheduling_enabled Statsig gate is on; diff --git a/src/quilt_hp/__init__.py b/src/quilt_hp/__init__.py index 0497c10..1f61593 100644 --- a/src/quilt_hp/__init__.py +++ b/src/quilt_hp/__init__.py @@ -1,5 +1,6 @@ """quilt_hp — Async Python client for Quilt mini-split HVAC systems.""" +from quilt_hp.auth import OtpCallback from quilt_hp.client import QuiltClient from quilt_hp.const import Environment from quilt_hp.exceptions import ( @@ -9,16 +10,37 @@ QuiltNotFoundError, QuiltStreamError, ) +from quilt_hp.services.streaming import NotifierStream, StreamEvent +from quilt_hp.tokens import ( + CachedTokens, + LegacyTokenStore, + RefreshFailureAction, + TokenRefreshContext, + TokenRefreshHooks, + TokenRefreshPolicy, + TokenRefreshReason, + TokenStore, +) __version__ = "0.5.3" __all__ = [ + "CachedTokens", "Environment", + "LegacyTokenStore", + "NotifierStream", + "OtpCallback", "QuiltAuthError", "QuiltClient", "QuiltConnectionError", "QuiltError", "QuiltNotFoundError", "QuiltStreamError", - "__version__", + "RefreshFailureAction", + "StreamEvent", + "TokenRefreshContext", + "TokenRefreshHooks", + "TokenRefreshPolicy", + "TokenRefreshReason", + "TokenStore", ] diff --git a/src/quilt_hp/client.py b/src/quilt_hp/client.py index 452ce07..1004d19 100644 --- a/src/quilt_hp/client.py +++ b/src/quilt_hp/client.py @@ -115,7 +115,11 @@ def __init__( self._streams: list[NotifierStream] = [] def get_current_token(self) -> str: - """Token provider callable for the transport interceptor.""" + """Token provider callable for the transport interceptor. + + Internal transport hook (implements ``CurrentTokenProvider``) — not + intended for application use. + """ if self._token is None: raise QuiltAuthError("Not authenticated. Call login() first.") return self._token From 12355b867a18f44dae20f48cedbb1e8fb8c8cad3 Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Thu, 2 Jul 2026 15:57:48 -0700 Subject: [PATCH 7/9] chore: regenerate protobuf stubs for proto comment fixes The .pyi stubs embed proto comments as docstrings, so the comment-only corrections in quilt_hds.proto require regenerated stubs to keep the proto-sync CI check green. --- src/quilt_hp/_proto/quilt_hds_pb2.pyi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/quilt_hp/_proto/quilt_hds_pb2.pyi b/src/quilt_hp/_proto/quilt_hds_pb2.pyi index 4107ccd..f840617 100644 --- a/src/quilt_hp/_proto/quilt_hds_pb2.pyi +++ b/src/quilt_hp/_proto/quilt_hds_pb2.pyi @@ -1039,7 +1039,7 @@ class IndoorUnitControls(_message.Message): f1,f2=absent; f3=ledColorCode(varint/uint32), f4=ledColorBrightnessPercent(float), f5=fanSpeedMode(varint), f6=fanSpeedPercent(float), f7=updatedTs(Timestamp), f8,f9=absent; f10=louverMode(varint), f11=louverFixedPosition(float), - f12=lightState(varint), f13=lightAnimation(varint). + f12=lightAnimation(varint), f13=lightState(varint). NOTE: These are FLAT fields, NOT nested sub-messages. """ @@ -1066,7 +1066,7 @@ class IndoorUnitControls(_message.Message): louver_mode: Global___IndoorUnitLouverMode.ValueType """f8, f9: uuid-string field confirmed at f9 in captures; purpose TBD""" louver_fixed_position: _builtins.float - """degrees, used when louver_mode=FIXED""" + """position fraction 0.20–1.00 (see LouverAngle.to_wire), used when louver_mode=FIXED; 0.0 = not applicable""" led_animation: Global___LightAnimation.ValueType """LED_ANIMATION_FIELD_NUMBER=12; wire-confirmed""" led_state: Global___LightState.ValueType From bc5d1cd86e2c8549e7cbd2fc39ec2c8702d765b3 Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Thu, 2 Jul 2026 16:08:38 -0700 Subject: [PATCH 8/9] fix: address PR review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - intercept_unary_stream: wait_for_connection on the retried call and raise QuiltAuthError when the retry is still UNAUTHENTICATED, matching the unary path instead of deferring the failure to first iteration - streaming: dispatch callbacks over a snapshot of the registration lists so a callback can unsubscribe itself (or others) mid-dispatch - store: add a uuid suffix to corruption backup names — the 1s timestamp resolution could collide and leave the corrupt file in place --- src/quilt_hp/cli/store.py | 5 +++- src/quilt_hp/services/streaming.py | 10 +++++--- src/quilt_hp/transport.py | 11 ++++++++- tests/test_transport_interceptor_extra.py | 29 +++++++++++++++++++++++ 4 files changed, 50 insertions(+), 5 deletions(-) diff --git a/src/quilt_hp/cli/store.py b/src/quilt_hp/cli/store.py index 51fd0de..246a02c 100644 --- a/src/quilt_hp/cli/store.py +++ b/src/quilt_hp/cli/store.py @@ -80,7 +80,10 @@ def _recover_corruption(self, reason: str) -> None: """ path = self._token_path() timestamp = datetime.now(UTC).strftime("%Y%m%dT%H%M%SZ") - backup = path.with_name(f"{path.name}.corrupt-{timestamp}-{reason}") + # uuid suffix: timestamps have 1s resolution, so concurrent or + # repeated recoveries must not collide on the backup name (a failed + # replace would leave the corrupt file in place forever). + backup = path.with_name(f"{path.name}.corrupt-{timestamp}-{uuid4().hex[:8]}-{reason}") logger.warning( "Token store %s is corrupt (%s); moving it to %s and starting empty", path, diff --git a/src/quilt_hp/services/streaming.py b/src/quilt_hp/services/streaming.py index 8565fff..6ac3267 100644 --- a/src/quilt_hp/services/streaming.py +++ b/src/quilt_hp/services/streaming.py @@ -491,7 +491,9 @@ async def _invoke_callbacks[T]( arg: T, error_message: str, ) -> None: - for callback in callbacks: + # Iterate a snapshot: a callback may unsubscribe itself (or others) + # while being dispatched. + for callback in list(callbacks): try: await _dispatch(callback, arg) except Exception: @@ -617,7 +619,9 @@ async def _run_one_stream(self) -> None: ) self._active_call = call self._stream_state = "connected" - for connected_cb in self._connected_callbacks: + # Iterate a snapshot: a callback may unsubscribe itself (or others) + # while being dispatched. + for connected_cb in list(self._connected_callbacks): try: result = connected_cb() if asyncio.iscoroutine(result): @@ -784,7 +788,7 @@ async def _run_stream_with_reconnect(self) -> None: self._stream_state = "stopped" if self._error is not None: - for cb in self._error_callbacks: + for cb in list(self._error_callbacks): try: await _dispatch(cb, self._error) except Exception: diff --git a/src/quilt_hp/transport.py b/src/quilt_hp/transport.py index f2f1265..2cff92a 100644 --- a/src/quilt_hp/transport.py +++ b/src/quilt_hp/transport.py @@ -157,7 +157,16 @@ async def intercept_unary_stream( if exc.code() == grpc.StatusCode.UNAUTHENTICATED and self._refresh_callback: logger.warning("Retrying streaming RPC setup after UNAUTHENTICATED response") await self._refresh() - return await continuation(self._patch(client_call_details), request) + retried = await continuation(self._patch(client_call_details), request) + try: + await cast("Any", retried).wait_for_connection() + except grpc.aio.AioRpcError as retry_exc: + if retry_exc.code() == grpc.StatusCode.UNAUTHENTICATED: + raise QuiltAuthError( + "Token refresh did not restore authentication; re-login required" + ) from retry_exc + raise + return retried raise return call diff --git a/tests/test_transport_interceptor_extra.py b/tests/test_transport_interceptor_extra.py index d78c25c..0bebe76 100644 --- a/tests/test_transport_interceptor_extra.py +++ b/tests/test_transport_interceptor_extra.py @@ -234,3 +234,32 @@ def get_current_token(self) -> str: channel = transport.create_channel(lambda: "Bearer x", environment=Environment.STAGING) assert channel == "channel" secure.assert_called_once() + + +@pytest.mark.asyncio +async def test_unary_stream_retry_still_unauthenticated_raises_auth_error() -> None: + """A retried streaming call that is still UNAUTHENTICATED must surface a + clear QuiltAuthError at interception time, matching the unary path.""" + refreshed: list[str] = [] + + async def _refresh(_context: transport.TokenRefreshContext) -> None: + refreshed.append("yes") + + interceptor = transport._AuthInterceptor(lambda: "stale", refresh_callback=_refresh) + details = grpc.aio.ClientCallDetails( + method="/svc/method", + timeout=1, + metadata=None, + credentials=None, + wait_for_ready=False, + ) + + async def _always_unauthenticated( + _call_details: grpc.aio.ClientCallDetails, _request: object + ) -> object: + return _FakeCall(error=_FakeRpcError(grpc.StatusCode.UNAUTHENTICATED, "expired")) + + with pytest.raises(QuiltAuthError, match="re-login required"): + await interceptor.intercept_unary_stream(_always_unauthenticated, details, "req") + + assert refreshed == ["yes"] From 541694a5b20fde1a651246d7f447717c5afb7eaf Mon Sep 17 00:00:00 2001 From: Emmanuel Levijarvi Date: Thu, 2 Jul 2026 16:17:15 -0700 Subject: [PATCH 9/9] docs: update CHANGELOG for evaluation fixes --- CHANGELOG.md | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c21bcc2..afd33ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,64 @@ ## [Unreleased] +Fixes all findings from a full architecture/code/performance/bug-hunt evaluation. + +### Critical + +- **Proto3 absence detection never fired on real protos.** Truthiness and + getattr-with-default checks always pass on generated messages (unset + sub-messages return truthy default instances), so sparse stream diffs zeroed + room state, IDU controls, sensor readings, and controller temperatures on + merge. All model parsing now uses `HasField`-based helpers, and + `SystemSnapshot.apply_*` preserves identity/relationship fields, controller + temperatures, ODU telemetry, and QSM LED state. New test suite + (`tests/test_models_real_proto_merge.py`) reproduces the notifier stream's + serialize/parse path with real generated protos. +- **Transport UNAUTHENTICATED retry was dead code.** In `grpc.aio`, awaiting the + interceptor continuation returns the call object; `AioRpcError` only surfaces + when the call is awaited — so token refresh/retry never executed and clients + broke permanently ~1h after token expiry. The interceptor now awaits the call + and retries once. Tests updated to model real `grpc.aio` semantics. + +### High + +- Stream reconnect backoff/budget reset after a healthy connection (routine LB + stream recycling no longer escalates to permanent 60s delays or kills the + stream after N lifetime disconnects); non-gRPC failures reconnect and surface + via `on_error` instead of dying silently; malformed events are skipped; + jitter added; prompt reconnect after successful token refresh. +- `authenticate()` no longer returns a locally-unexpired token the server just + rejected; rotated Cognito refresh tokens are persisted; expiry measured from + token receipt. +- `grpc_call` passes `CancelledError` through untranslated (asyncio.timeout/ + cancellation semantics restored). +- TUI: fatal stream death now shows a disconnect indicator and auto-recovers; + first-time users get an OTP modal; boot failures show an error screen + instead of a stuck spinner. + +### Medium / Low + +- Unified error translation: all hds/user RPCs route through `grpc_call`; + `UNAVAILABLE`/`DEADLINE_EXCEEDED` → `QuiltConnectionError` and `NOT_FOUND` → + `QuiltNotFoundError` everywhere. +- Single-flight guards on token refresh and cold snapshot fetch; + `get_system_id(home=...)` no longer poisons the default-system cache; + `close()` stops tracked streams; duplicate authorization metadata + eliminated. +- Stream API: `on_connected` callback, unsubscribe handles from all `on_*` + registrations, `apply_software_update_info`, descriptor-derived wire-scan + field numbers. +- CLI/TUI: app-owned snapshot (stacked screens no longer go stale), cursor + preservation, setpoint bounds, energy period validation, app-level °C/°F, + LED brightness restoration, 0.0 readings rendered correctly, token store + corruption recovery + fsync + flock, config dir 0700, dead code removal. +- Docs: README no longer demonstrates raw-stream usage; stream/token types + re-exported at package top level; stale OutdoorUnit/Controller reference + listings corrected; contradictory proto comments fixed. + +Intentionally unchanged: Python >= 3.14 floor and boto3 dependency (HA 2026.3+ +runs Python 3.14). + ## [0.5.3] - 2026-06-30 ## [0.5.2] - 2026-06-30