diff --git a/custom_components/localthings/climate.py b/custom_components/localthings/climate.py index 6df7a55..d55626f 100644 --- a/custom_components/localthings/climate.py +++ b/custom_components/localthings/climate.py @@ -58,9 +58,6 @@ from .registry.capabilities.airconditioner import ( from .registry.capabilities.airconditioner import ( HREF_POWER_VS as POWER_VS_HREF, ) -from .registry.capabilities.airconditioner import ( - HREF_TEMP_CONTROL as TEMP_CONTROL_HREF, -) from .registry.capabilities.airconditioner import ( HREF_TEMP_CURRENT as TEMP_CURRENT_HREF, ) @@ -80,6 +77,7 @@ from .registry.capabilities.airconditioner import ( HREF_WIND_STRENGTH as WIND_STRENGTH_HREF, ) from .registry.capabilities.airconditioner import ( + _temperature_step, extend_option_code_bit, has_extend_option_code, has_option_code, @@ -558,12 +556,11 @@ class LocalThingsClimate(LocalThingsEntity, ClimateEntity): @property def target_temperature_step(self) -> float: - return ( - _num(self._rep(TEMP_CONTROL_HREF).get("increment")) - or _num(self._rep(TEMP_CONTROL_HREF).get("x.com.samsung.da.increment")) - or _num(self._temps_vs().get("x.com.samsung.da.increment")) - or 1.0 - ) + # Shared with the write path (airconditioner._climate_write) so a + # step read here always matches the step a write is quantized to -- + # self._resources is this entity's own subdevice's canonical view + # (issue #177), the same shape _temperature_step expects. + return _temperature_step(self._resources) or 1.0 # -- hvac mode ---------------------------------------------------------- diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index 67e37aa..0e8de00 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -1186,17 +1186,28 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): return href = bound_entity.href rep = self._cache.get(href or "") or {} - resources = self._cache.snapshot() + # The remote-control gate below keys off the raw on-the-wire href + # and a raw snapshot -- /remotectrl/* is a shared, MAIN-only + # resource that a subdevice's canonical_resources() view (owned + # hrefs only) would drop entirely. write_fn/validate_fn, by + # contrast, are written against canonical hrefs (same convention as + # exists_fn/rep_fn -- see entity.py's _resources), so they get this + # entity's own subdevice view instead of the raw snapshot: without + # it, a composite device's write_fn reading resources.get(some + # canonical href) (e.g. airconditioner._temperature_step) would + # silently see the master's resource instead of its own subdevice's. + raw_resources = self._cache.snapshot() bypass_remote_control = self._entry.options.get(CONF_BYPASS_REMOTE_CONTROL, False) if ( not bypass_remote_control - and remote_control_required_for_write(resources, href or "") - and not remote_control_enabled(resources) + and remote_control_required_for_write(raw_resources, href or "") + and not remote_control_enabled(raw_resources) ): raise ServiceValidationError( translation_domain=DOMAIN, translation_key="remote_control_disabled", ) + resources = self.canonical_resources(bound_entity.subdevice) validate_fn = getattr(desc, "validate_fn", None) if validate_fn is not None: error = validate_fn(payload, rep, resources) diff --git a/custom_components/localthings/registry/capabilities/airconditioner.py b/custom_components/localthings/registry/capabilities/airconditioner.py index 9688776..5c96473 100644 --- a/custom_components/localthings/registry/capabilities/airconditioner.py +++ b/custom_components/localthings/registry/capabilities/airconditioner.py @@ -9,6 +9,7 @@ here off the coordinator snapshot. These caps stay out of the global capabilities/__init__.py) -- AC-only, by_type registry only. """ +import math from dataclasses import replace from ..capability import Capability @@ -510,6 +511,13 @@ def _quantize_temperature(value, step): numeric = float(value) except (TypeError, ValueError): return None + if not math.isfinite(numeric): + # float('nan')/float('inf') pass the try/except above (they're + # valid floats) but round()/division on them raises ValueError/ + # OverflowError instead of returning -- reject here so a bad + # payload always comes back as a clean None, not an exception + # out of write_fn. + return None step_value = _num(step) or 1.0 if step_value <= 0: step_value = 1.0 @@ -558,16 +566,13 @@ def _climate_write(payload, rep, href=None, resources=None): return (["power", "vs", "0"], {"x.com.samsung.da.power": "On" if value else "Off"}) if kind == "mode": return (["mode", "vs", "0"], {"x.com.samsung.da.modes": [value]}) - if kind == "temperature_ocf": + if kind in ("temperature_ocf", "temperature"): quantized = _quantize_temperature(value, _temperature_step(resources)) if quantized is None: return None - return (["temperature", "desired", "0"], {"temperature": quantized}) - if kind == "temperature": + if kind == "temperature_ocf": + return (["temperature", "desired", "0"], {"temperature": quantized}) # Vendor items[] array; only one item observed on every AC dump, id '0'. - quantized = _quantize_temperature(value, _temperature_step(resources)) - if quantized is None: - return None return ( ["temperatures", "vs", "0"], { diff --git a/tests/test_airconditioner_capabilities.py b/tests/test_airconditioner_capabilities.py index f9f1d3b..023a24a 100644 --- a/tests/test_airconditioner_capabilities.py +++ b/tests/test_airconditioner_capabilities.py @@ -181,6 +181,26 @@ def test_climate_write_preserves_half_degree_temperature_steps(): ) +def test_temperature_step_falls_back_to_temps_vs_items_when_no_control_resource(): + """Isolates the /temperatures/vs/0 fallback: every fixture that carries + an increment there also carries /temperature/control/vs/0, which + _temperature_step checks first -- so without dropping that resource, + this fallback branch is never actually exercised, and reinstating the + original bug (reading the increment off /temperatures/vs/0's top level + instead of unwrapping its items[0]) would still pass every other test.""" + resources = dict(_load_device("airconditioner_cac")) + del resources[airconditioner.HREF_TEMP_CONTROL] + assert airconditioner._temperature_step(resources) == 0.5 + assert airconditioner._climate_write(("temperature", 24.5), {}, None, resources) == ( + ["temperatures", "vs", "0"], + { + "x.com.samsung.da.items": [ + {"x.com.samsung.da.id": "0", "x.com.samsung.da.desired": "24.5"} + ] + }, + ) + + def test_climate_write_rounds_to_whole_degree_with_no_advertised_increment(): """ARTIK051 boards have neither /temperature/control/vs/0 nor an increment field on /temperatures/vs/0's item -- target_temperature_step diff --git a/tests/test_coordinator_send_command.py b/tests/test_coordinator_send_command.py index 2b9b04a..bc04057 100644 --- a/tests/test_coordinator_send_command.py +++ b/tests/test_coordinator_send_command.py @@ -201,3 +201,33 @@ async def test_main_climate_write_unaffected_by_subdevice_translation(coordinato posted_path, _ = coordinator._session.post_calls[0] assert posted_path == ["power", "vs", "0"] + + +async def test_indexed_subdevice_climate_write_quantizes_with_its_own_temperature_step( + coordinator, +) -> None: + """A composite AC's write_fn (airconditioner._climate_write) reads the + temperature step off write_fn's own `resources` argument -- if + async_send_command handed it the raw cache snapshot (real, on-the-wire + hrefs) instead of this subdevice's canonical_resources() view, the + subdevice's own /temperature/control/vs/1 would be invisible under the + canonical HREF_TEMP_CONTROL key, and the write would quantize against + the master's step (or none at all) instead of its own.""" + sub1 = Subdevice(kind="indexed", key="1", seed_path=("device", "1")) + coordinator._observe.apply( + "/temperature/control/vs/0", + {"x.com.samsung.da.increment": "1"}, + source="poll", + ) + coordinator._observe.apply( + "/temperature/control/vs/1", + {"x.com.samsung.da.increment": "0.5"}, + source="poll", + ) + bound = _climate_bound("/mode/vs/1", sub1) + + await coordinator.async_send_command(bound, ("temperature_ocf", 24.5)) + + posted_path, posted_bytes = coordinator._session.post_calls[0] + assert posted_path == ["temperature", "desired", "1"] + assert cbor2.loads(posted_bytes) == {"temperature": 24.5} diff --git a/tests/test_translations.py b/tests/test_translations.py index 1fc1b60..9309a01 100644 --- a/tests/test_translations.py +++ b/tests/test_translations.py @@ -164,15 +164,23 @@ def test_confirmed_washer_table_02_towels_bedding_are_not_swapped(): """Issue #343: DA_WM_TP1_21_COMMON's Table_02 had 24/33 transposed -- selecting 'Towels' in HA ran the washer's Bedding cycle and vice versa. 24 must agree with the 69/6A-79/88 family's own Bedding/Towels pair - (6f/70), not with each other.""" - states = _load("en")["entity"]["select"]["washer_cycle_table_02"]["state"] - assert states["24"] == states["6f"] == "Bedding" - assert states["33"] == states["70"] == "Towels" + (6f/70), not with each other. + + Checked in every locale catalog, not just English: the underlying bug + is a device-code mapping error, not a wording issue, and + test_every_language_mirrors_the_english_catalog only checks key + topology -- a locale-specific 24/33 swap (the exact shape of #343) + would still pass that test.""" + for language in _languages(): + states = _load(language)["entity"]["select"]["washer_cycle_table_02"]["state"] + assert states["24"] == states["6f"], language + assert states["33"] == states["70"], language def test_confirmed_washer_table_02_missing_course_names(): """Issue #342: 06/08/a0 had no translation and fell back to the raw - device code in the UI.""" + device code in the UI; 74 was already translated by the time this + landed and is pinned here only as a "didn't regress" anchor.""" states = _load("en")["entity"]["select"]["washer_cycle_table_02"]["state"] assert {code: states[code] for code in ("06", "08", "74", "a0")} == { "06": "XXL Laundry",