From 5547c401ed498466d1859336fe0930f582744e5b Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Mon, 27 Jul 2026 05:11:58 +0000 Subject: [PATCH] Address independent review findings on the RAC finish-up branch - Pin the FAN_ONLY reverse-write fallback to 'Wind' (the original single spelling) instead of letting it silently flip to 'Fan' just because 'Fan' was added second to the dict -- _device_code_for_hvac() resolves the code from a unit's own supportedModes first, so this dict is only a fallback for a unit reporting none at all, and that fallback shouldn't change behavior as an unintended side effect of insertion order. - Add direct tests for _preset_to_ha() (pure function, previously untested) and the FAN_ONLY fallback pin. - Cross-reference the AC family's inverted Light_On/Light_Off polarity against air_purifier.py's plain-polarity use of the same token name on the same resource name, so a future refactor doesn't assume they're the same thing. - Fix a one-column continuation-line misalignment and a stale PR-number reference in a comment. --- custom_components/localthings/climate.py | 8 +++-- .../localthings/registry/by_type/__init__.py | 2 +- .../registry/capabilities/air_purifier.py | 7 +++++ tests/test_climate_ac_modes.py | 30 +++++++++++++++++++ tests/test_climate_windfree.py | 11 +++---- 5 files changed, 50 insertions(+), 8 deletions(-) diff --git a/custom_components/localthings/climate.py b/custom_components/localthings/climate.py index f78e9bb..063b811 100644 --- a/custom_components/localthings/climate.py +++ b/custom_components/localthings/climate.py @@ -71,9 +71,13 @@ _DEVICE_TO_HVAC: dict[str, HVACMode] = { # 'Fan' on others (e.g. TP1X_DA-AC-RAC-01011); both map to FAN_ONLY. The # reverse write can't rely on this map alone (two codes, one HA value) -- # _device_code_for_hvac() resolves the code from the unit's own - # supportedModes. - 'Wind': HVACMode.FAN_ONLY, + # supportedModes, so this is only a fallback for a unit reporting no + # supportedModes at all. 'Fan' is listed first so the {v: k} reverse + # comprehension below has 'Wind' win that fallback (last-key-wins), + # preserving the original single-spelling behavior rather than silently + # flipping it when 'Fan' was added. 'Fan': HVACMode.FAN_ONLY, + 'Wind': HVACMode.FAN_ONLY, # The device's 'Auto' is a single-setpoint "device decides" mode -> HA # HVACMode.AUTO (renders "Auto"). Not HEAT_COOL: that renders "Heat/cool" # and implies a two-setpoint heat+cool range these single-setpoint units diff --git a/custom_components/localthings/registry/by_type/__init__.py b/custom_components/localthings/registry/by_type/__init__.py index 4be6f42..712dace 100644 --- a/custom_components/localthings/registry/by_type/__init__.py +++ b/custom_components/localthings/registry/by_type/__init__.py @@ -171,7 +171,7 @@ def for_device_by_model(model_num: str, description: str) -> Optional[DeviceRegi # /otninformation/vs/0 ships no swVersionInfo block at all, so # oneUiVersion is empty. if key is None and ('_RAC_' in (model_num or '') - or '-RAC-' in (model_num or '').upper()): + or '-RAC-' in (model_num or '').upper()): key = 'airconditioner' # System air conditioners (multi-indoor-unit commercial installs, e.g. # A-CAWW-TP2-20-COMMON, issue #52) report no oneUiVersion either and diff --git a/custom_components/localthings/registry/capabilities/air_purifier.py b/custom_components/localthings/registry/capabilities/air_purifier.py index d953705..136d524 100644 --- a/custom_components/localthings/registry/capabilities/air_purifier.py +++ b/custom_components/localthings/registry/capabilities/air_purifier.py @@ -15,6 +15,13 @@ issue #56's follow-up (five diagnostics dumps captured with the physical unit set to Auto/Sleep/Low/Medium/High): Light_On / Light_Off -- a plain on/off flag; MODE below models it as a real switch, RMW-replacing just that one entry. + NOT the same polarity as the AC family's own + Light_On/Light_Off token on its own /mode/vs/0 + (airconditioner._display_light_on) -- that one is + confirmed inverted (Light_Off means the panel is + lit) on live hardware. Same token name, same + resource name, different device type and + opposite meaning -- don't unify them. Comode_Off -- read 'Off' on *every* one of the five dumps, including High/Low/Medium/Auto -- confirms this is NOT the fan-speed selector (ruling out the diff --git a/tests/test_climate_ac_modes.py b/tests/test_climate_ac_modes.py index 75b34af..e9905b9 100644 --- a/tests/test_climate_ac_modes.py +++ b/tests/test_climate_ac_modes.py @@ -8,6 +8,7 @@ from homeassistant.components.climate import HVACMode from custom_components.localthings.climate import ( _AI_COMFORT_MODE, _DEVICE_TO_HVAC, _HVAC_TO_DEVICE, PRESET_AI_COMFORT, + _preset_to_ha, ) @@ -42,5 +43,34 @@ def test_fan_only_still_reachable_via_wind(): assert _DEVICE_TO_HVAC['Wind'] == HVACMode.FAN_ONLY +def test_fan_only_still_reachable_via_fan(): + """'Fan' (e.g. TP1X_DA-AC-RAC-01011) is a second FAN_ONLY spelling + alongside 'Wind' -- issue #91.""" + assert _DEVICE_TO_HVAC['Fan'] == HVACMode.FAN_ONLY + + +def test_fan_only_reverse_fallback_prefers_wind(): + """_device_code_for_hvac() resolves FAN_ONLY from a unit's own + supportedModes first, so _HVAC_TO_DEVICE is only a fallback for a unit + reporting no supportedModes at all. That fallback must stay 'Wind' (the + original single spelling, predating 'Fan') rather than silently + flipping to whichever of the two duplicate-value entries happens to + come last in _DEVICE_TO_HVAC.""" + assert _HVAC_TO_DEVICE[HVACMode.FAN_ONLY] == 'Wind' + + def test_preset_ai_comfort_constant(): assert PRESET_AI_COMFORT == 'ai_comfort' + + +def test_preset_to_ha_off_maps_to_preset_none(): + from homeassistant.components.climate import PRESET_NONE + assert _preset_to_ha('Off') == PRESET_NONE + + +def test_preset_to_ha_lowercases_other_codes(): + """Every other device code is exposed as its lowercased self -- resolved + dynamically, not via a per-model table (issue #91).""" + assert _preset_to_ha('Sleep') == 'sleep' + assert _preset_to_ha('NanoSleep') == 'nanosleep' + assert _preset_to_ha('MotionIndirect') == 'motionindirect' diff --git a/tests/test_climate_windfree.py b/tests/test_climate_windfree.py index d9e74f0..f5d065b 100644 --- a/tests/test_climate_windfree.py +++ b/tests/test_climate_windfree.py @@ -3,11 +3,12 @@ test_climate_temperature_fallback.py). The preset side of issue #75 (WindFree/motion convenient modes not -surfacing) is intentionally not addressed here: PR #91 replaces -climate.py's static _DEVICE_TO_PRESET table with a generic resolver that -reads any device preset code straight off the unit's own supportedModes, -which covers WindFree/motion generically instead of a per-model dict -- -duplicating that here would just conflict with it. +surfacing) is intentionally not addressed here: climate.py's +_preset_to_ha() (issue #91) reads any device preset code straight off the +unit's own supportedModes, replacing the old static _DEVICE_TO_PRESET +table with a generic resolver that covers WindFree/motion generically +instead of a per-model dict -- duplicating that here would just conflict +with it. """ from custom_components.localthings.climate import _DEVICE_TO_SWING, _SWING_TO_DEVICE