From 248e473abe5ddf46dbcbf37ff4abbe8bc946e303 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Sun, 9 Aug 2026 16:00:14 +0000 Subject: [PATCH 1/4] Fix AC temperature step quantization (PR #276, code review) Samsung local AC temperature writes always rounded to the nearest whole degree, dropping half-degree setpoints on boards that advertise a 0.5 step (CAC and TP1X FAC). Squashed from moridew's PR #276 with the review fixes applied: - The /temperatures/vs/0 fallback never matched: its increment lives inside the resource's items[] array, not at the top level (same shape _temps_vs_item() already unwraps for current/unit). The original fix only ever worked through /temperature/control/vs/0. - With no increment advertised anywhere (e.g. ARTIK051), writes went out unrounded instead of falling back to whole degrees the way climate.py's target_temperature_step already does. - A non-numeric payload now rejects the write (returns None) instead of posting {"temperature": null} -- coordinator.py's async_send_command already drops a write_fn result of None. - int/float normalization now happens once, in _quantize_temperature, instead of being duplicated (and skipped) per branch; a round(..., 2) guards against float division noise (e.g. 21.7 / 0.1). Tests rebuilt against real fixture resources (airconditioner_cac, airconditioner_artik051_krac_18k) instead of a fabricated flat resource shape no device produces. --- .../registry/capabilities/airconditioner.py | 66 ++++++++++++++++++- tests/test_airconditioner_capabilities.py | 53 +++++++++++++++ 2 files changed, 116 insertions(+), 3 deletions(-) diff --git a/custom_components/localthings/registry/capabilities/airconditioner.py b/custom_components/localthings/registry/capabilities/airconditioner.py index 4eac2be..9688776 100644 --- a/custom_components/localthings/registry/capabilities/airconditioner.py +++ b/custom_components/localthings/registry/capabilities/airconditioner.py @@ -491,7 +491,61 @@ def _humidity(rep): return None -def _climate_write(payload, rep, href=None): +def _quantize_temperature(value, step): + """Quantize a temperature to the device's advertised step size. + + Mirrors climate.py's `target_temperature_step`, which defaults to `1.0` + when no increment is advertised anywhere (`step=None` or `<=0` here) -- + so a board with no increment still gets whole-degree writes instead of + the raw value passed through unrounded. Returns `None` for a payload + that isn't numeric, so the caller can reject the write instead of + building a body around a null (coordinator.py's async_send_command + already logs and drops a write_fn result of None). + + Normalizes an integral result to `int` here, once, so both write + branches serialize `24` rather than `24.0` -- `round(..., 2)` guards + against float noise in the division (e.g. `21.7 / 0.1`). + """ + try: + numeric = float(value) + except (TypeError, ValueError): + return None + step_value = _num(step) or 1.0 + if step_value <= 0: + step_value = 1.0 + quantized = round(round(numeric / step_value) * step_value, 2) + return int(quantized) if quantized.is_integer() else quantized + + +def _temperature_step(resources): + """Device-reported temperature increment, read the same way and in the + same order as climate.py's `target_temperature_step`: OCF + `/temperature/control/vs/0`'s `increment` (or its vendor-prefixed + twin), falling back to vendor `/temperatures/vs/0`. The increment on + that second resource lives inside its `items[]` array like every other + per-item field, not at the resource's top level -- unwrapped via + `_temps_vs_item()`, the same helper `_temps_vs_current`/`_temps_vs_unit` + use above, rather than read off `resource` directly. + + `rep` (the bound entity's own resource, `/mode/vs/0` for ClimateDesc -- + see rep_fn=_first_mode below) never carries an increment, so it isn't a + candidate here; only the coordinator's `resources` snapshot is.""" + if not isinstance(resources, dict): + return None + control = resources.get(HREF_TEMP_CONTROL) + if isinstance(control, dict): + step = _num(control.get("increment")) or _num(control.get("x.com.samsung.da.increment")) + if step is not None: + return step + temps_vs = resources.get(HREF_TEMPS_VS) + if isinstance(temps_vs, dict): + step = _num(_temps_vs_item(temps_vs).get("x.com.samsung.da.increment")) + if step is not None: + return step + return None + + +def _climate_write(payload, rep, href=None, resources=None): """Maps a (kind, value) command from the climate platform to the (path_segs, body) for that one sub-write; `value` is already the raw device code. Power always goes to vendor `/power/vs/0` (OCF `/power/0` @@ -505,16 +559,22 @@ def _climate_write(payload, rep, href=None): if kind == "mode": return (["mode", "vs", "0"], {"x.com.samsung.da.modes": [value]}) if kind == "temperature_ocf": - return (["temperature", "desired", "0"], {"temperature": round(float(value))}) + quantized = _quantize_temperature(value, _temperature_step(resources)) + if quantized is None: + return None + return (["temperature", "desired", "0"], {"temperature": quantized}) if kind == "temperature": # 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"], { "x.com.samsung.da.items": [ { "x.com.samsung.da.id": "0", - "x.com.samsung.da.desired": str(round(float(value))), + "x.com.samsung.da.desired": str(quantized), } ] }, diff --git a/tests/test_airconditioner_capabilities.py b/tests/test_airconditioner_capabilities.py index fd9b8c3..92a0ca1 100644 --- a/tests/test_airconditioner_capabilities.py +++ b/tests/test_airconditioner_capabilities.py @@ -152,6 +152,59 @@ def test_climate_write_targets(): assert write(("bogus", 1), {}) is None +def test_climate_write_preserves_half_degree_temperature_steps(): + """CAC/TP1X FAC boards advertise `0.5` on both temperature resources. + The increment on /temperatures/vs/0 lives inside its items[] entry (the + same shape every fixture in the corpus uses), not at the resource's top + level -- a fabricated flat `{"/temperatures/vs/0": {"increment": ...}}` + resource would pass a step-reading bug like that silently.""" + climate_desc = next(e for e in airconditioner.CLIMATE.entities if isinstance(e, ClimateDesc)) + write = climate_desc.write_fn + resources = _load_device("airconditioner_cac") + assert write(("temperature_ocf", 24.5), {}, None, resources) == ( + ["temperature", "desired", "0"], + {"temperature": 24.5}, + ) + assert 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 + (climate.py) defaults to 1.0 there, so the write path must match rather + than pass the raw value through unrounded.""" + climate_desc = next(e for e in airconditioner.CLIMATE.entities if isinstance(e, ClimateDesc)) + write = climate_desc.write_fn + resources = _load_device("airconditioner_artik051_krac_18k") + assert airconditioner._temperature_step(resources) is None + assert write(("temperature", 23.6), {}, None, resources) == ( + ["temperatures", "vs", "0"], + { + "x.com.samsung.da.items": [ + {"x.com.samsung.da.id": "0", "x.com.samsung.da.desired": "24"} + ] + }, + ) + + +def test_climate_write_rejects_non_numeric_temperature(): + """A non-numeric payload must reject the write (return None) rather than + build a body with `{"temperature": None}` -- coordinator.py's + async_send_command logs and drops a write_fn result of None instead of + POSTing it.""" + climate_desc = next(e for e in airconditioner.CLIMATE.entities if isinstance(e, ClimateDesc)) + write = climate_desc.write_fn + assert write(("temperature_ocf", "not-a-number"), {}) is None + assert write(("temperature", None), {}) is None + + def test_climate_consumed_hrefs_declared_as_coverage(): """The climate-consumed and ambiguous hrefs are declared in the AC registry (as no-entity coverage caps) so they don't leak as gaps -- but produce no From 895fd87d2c2a75de2711bd688a9c4b507cb7a02a Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Sun, 9 Aug 2026 16:00:27 +0000 Subject: [PATCH 2/4] washer: fix swapped Towels/Bedding, add missing Table_02 course names Issue #343: DA_WM_TP1_21_COMMON's washer_cycle_table_02 had course codes 24 and 33 transposed -- selecting "Towels" in HA ran the washer's Bedding cycle and vice versa (confirmed against the reporter's diagnostics dump: Course_24 selected, courseTable Table_02). Swapped both codes' labels back in line with the 69/6A- 79/88 family's own Bedding/Towels pair (6f/70), across every locale catalog. Issue #342: added the four course codes the reporter's editCourseList carried with no catalog entry -- 06 (XXL Laundry), 08 (Rinse+Spin), and a0 (15' Quick Wash) were missing outright; 74 (Drum Clean) turned out to already be translated by the time this landed. The download-course request in the same issue (selecting which program a "Download" cycle fetches) is left for a follow-up -- still waiting on a confirmed local write path before building anything on top of the OneTimeCloudCourse/CloudCourse fields. --- .../localthings/translations/cs.json | 7 ++++-- .../localthings/translations/de.json | 7 ++++-- .../localthings/translations/en.json | 9 +++++--- .../localthings/translations/es.json | 7 ++++-- .../localthings/translations/it.json | 9 +++++--- .../localthings/translations/ko.json | 7 ++++-- .../localthings/translations/nl.json | 7 ++++-- tests/test_translations.py | 22 +++++++++++++++++++ 8 files changed, 59 insertions(+), 16 deletions(-) diff --git a/custom_components/localthings/translations/cs.json b/custom_components/localthings/translations/cs.json index 977b8db..95c2e5f 100644 --- a/custom_components/localthings/translations/cs.json +++ b/custom_components/localthings/translations/cs.json @@ -609,6 +609,8 @@ "state": { "01": "Normální", "04": "Rychlé praní", + "06": "XXL prádlo", + "08": "Máchání+odstřeďování", "17": "Stažený program", "1b": "Bavlna", "1c": "Eco 40-60", @@ -619,7 +621,7 @@ "21": "Barevné prádlo", "22": "Vlna", "23": "Outdoor", - "24": "Ručníky", + "24": "Ložní prádlo", "25": "Syntetika", "26": "Jemné prádlo", "27": "Máchání+odstřeďování", @@ -632,7 +634,7 @@ "2f": "Sportovní oblečení", "30": "Zataženo", "32": "Košile", - "33": "Ložní prádlo", + "33": "Ručníky", "34": "Smíšené", "36": "Praní+sušení", "37": "Sušení vzduchem", @@ -674,6 +676,7 @@ "88": "Péče o domácí mazlíčky", "8f": "Intenzivní studená", "96": "Méně mikrovláken", + "a0": "15min rychlé praní", "35": "Eko bavlna" } }, diff --git a/custom_components/localthings/translations/de.json b/custom_components/localthings/translations/de.json index cf31314..254f331 100644 --- a/custom_components/localthings/translations/de.json +++ b/custom_components/localthings/translations/de.json @@ -572,6 +572,8 @@ "state": { "01": "Normal", "04": "Schnellwäsche", + "06": "XXL-Wäsche", + "08": "Spülen + Schleudern", "1b": "Baumwolle", "1c": "Eco 40-60", "1d": "Super Speed", @@ -581,7 +583,7 @@ "21": "Buntwäsche", "22": "Wolle", "23": "Outdoor", - "24": "Handtücher", + "24": "Bettwäsche", "25": "Pflegeleicht", "26": "Feinwäsche", "27": "Spülen + Schleudern", @@ -594,7 +596,7 @@ "2f": "Sportkleidung", "30": "Bewölkter Tag", "32": "Hemden", - "33": "XXL-Wäsche", + "33": "Handtücher", "34": "Mischwäsche", "36": "Waschen + Trocknen", "37": "Air Wash", @@ -618,6 +620,7 @@ "87": "Download", "8f": "Kaltwäsche Intensiv", "96": "Weniger Mikrofasern", + "a0": "Schnelle Wäsche 15'", "17": "Heruntergeladen", "69": "KI-Wäsche", "6a": "Wolle", diff --git a/custom_components/localthings/translations/en.json b/custom_components/localthings/translations/en.json index 5b684be..43ce1f4 100644 --- a/custom_components/localthings/translations/en.json +++ b/custom_components/localthings/translations/en.json @@ -609,6 +609,8 @@ "state": { "01": "Normal", "04": "Quick Wash", + "06": "XXL Laundry", + "08": "Rinse+Spin", "17": "Downloaded", "1b": "Cotton", "1c": "Eco 40-60", @@ -619,7 +621,7 @@ "21": "Colors", "22": "Wool", "23": "Outdoor", - "24": "Towels", + "24": "Bedding", "25": "Synthetics", "26": "Delicates", "27": "Rinse+Spin", @@ -632,7 +634,7 @@ "2f": "Activewear", "30": "Cloudy Day", "32": "Shirts", - "33": "Bedding", + "33": "Towels", "34": "Mixed", "35": "E Cotton", "36": "Wash+Dry", @@ -674,7 +676,8 @@ "87": "Download", "88": "Pet Care", "8f": "Intense Cold", - "96": "Less Microfiber" + "96": "Less Microfiber", + "a0": "15' Quick Wash" } }, "washer_dry_level": { diff --git a/custom_components/localthings/translations/es.json b/custom_components/localthings/translations/es.json index f4b7d2a..6d2576f 100644 --- a/custom_components/localthings/translations/es.json +++ b/custom_components/localthings/translations/es.json @@ -758,6 +758,8 @@ "state": { "01": "Normal", "04": "Lavado rápido", + "06": "Colada XXL", + "08": "Aclarar + Centrifugar", "17": "Descargado", "1b": "Algodón", "1c": "Eco 40-60", @@ -768,7 +770,7 @@ "21": "Color", "22": "Lana", "23": "Exterior", - "24": "Toallas", + "24": "Ropa de cama", "25": "Sintéticos", "26": "Delicados", "27": "Aclarar + Centrifugar", @@ -781,7 +783,7 @@ "2f": "Ropa deportiva", "30": "Día nublado", "32": "Camisas", - "33": "Ropa de cama", + "33": "Toallas", "34": "Mezcla", "36": "Lavado + Secado", "37": "Lavado con aire", @@ -805,6 +807,7 @@ "87": "Descarga de Programas", "8f": "Lavado en frío", "96": "Menos microfibras", + "a0": "Lavado rápido 15'", "69": "Lavado IA", "6a": "Lana", "6b": "Denim", diff --git a/custom_components/localthings/translations/it.json b/custom_components/localthings/translations/it.json index f13c398..b9528b1 100644 --- a/custom_components/localthings/translations/it.json +++ b/custom_components/localthings/translations/it.json @@ -609,6 +609,8 @@ "state": { "01": "Normale", "04": "Lavaggio rapido", + "06": "Bucato XXL", + "08": "Risciacquo+Centrifuga", "17": "Scaricato", "1b": "Cotone", "1c": "Eco 40-60", @@ -619,7 +621,7 @@ "21": "Colorati", "22": "Lana", "23": "Capi outdoor", - "24": "Asciugamani", + "24": "Biancheria da letto", "25": "Sintetici", "26": "Delicati", "27": "Risciacquo+Centrifuga", @@ -632,7 +634,7 @@ "2f": "Abbigliamento sportivo", "30": "Giornata nuvolosa", "32": "Camicie", - "33": "Biancheria da letto", + "33": "Asciugamani", "34": "Misti", "36": "Lavaggio+Asciugatura", "37": "Lavaggio ad aria", @@ -674,7 +676,8 @@ "78": "Risciacquo+Centrifuga", "79": "Solo centrifuga", "88": "Cura animali", - "35": "Cotone E" + "35": "Cotone E", + "a0": "Rapido 15'" } }, "washer_dry_level": { diff --git a/custom_components/localthings/translations/ko.json b/custom_components/localthings/translations/ko.json index a27a9ef..043b1d3 100644 --- a/custom_components/localthings/translations/ko.json +++ b/custom_components/localthings/translations/ko.json @@ -609,6 +609,8 @@ "state": { "01": "표준세탁", "04": "쾌속세탁", + "06": "XXL 세탁", + "08": "헹굼+탈수", "17": "다운로드 코스", "1b": "면", "1c": "에코 40-60", @@ -619,7 +621,7 @@ "21": "컬러 의류", "22": "울", "23": "아웃도어", - "24": "타월", + "24": "이불", "25": "합성섬유", "26": "섬세의류", "27": "헹굼+탈수", @@ -632,7 +634,7 @@ "2f": "피트니스", "30": "흐린 날", "32": "셔츠", - "33": "이불", + "33": "타월", "34": "혼합", "36": "세탁+건조", "37": "에어워시", @@ -656,6 +658,7 @@ "87": "다운로드", "8f": "강력 냉수 세탁", "96": "미세플라스틱저감", + "a0": "15분 쾌속세탁", "69": "AI 맞춤세탁", "6a": "울", "6b": "데님", diff --git a/custom_components/localthings/translations/nl.json b/custom_components/localthings/translations/nl.json index 839a6a6..542826f 100644 --- a/custom_components/localthings/translations/nl.json +++ b/custom_components/localthings/translations/nl.json @@ -609,6 +609,8 @@ "state": { "01": "Normaal", "04": "Snelle was", + "06": "XXL was", + "08": "Spoelen+centrifugeren", "17": "Gedownload", "1b": "Katoen", "1c": "Eco 40-60", @@ -619,7 +621,7 @@ "21": "Bonte was", "22": "Wol", "23": "Outdoor", - "24": "Handdoeken", + "24": "Beddengoed", "25": "Synthetisch", "26": "Fijne was", "27": "Spoelen+centrifugeren", @@ -632,7 +634,7 @@ "2f": "Sportkleding", "30": "Bewolkte dag", "32": "Overhemden", - "33": "Beddengoed", + "33": "Handdoeken", "34": "Gemengd", "36": "Wassen+drogen", "37": "Air Wash", @@ -674,6 +676,7 @@ "88": "Huisdierverzorging", "8f": "Intensief koud", "96": "Minder microvezels", + "a0": "15' Snelle was", "35": "Eco katoen" } }, diff --git a/tests/test_translations.py b/tests/test_translations.py index a456e51..1fc1b60 100644 --- a/tests/test_translations.py +++ b/tests/test_translations.py @@ -160,6 +160,28 @@ def test_confirmed_korean_table_02_washer_course_names(): } +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" + + +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.""" + states = _load("en")["entity"]["select"]["washer_cycle_table_02"]["state"] + assert {code: states[code] for code in ("06", "08", "74", "a0")} == { + "06": "XXL Laundry", + "08": "Rinse+Spin", + "74": "Drum Clean", + "a0": "15' Quick Wash", + } + + def test_confirmed_dishwasher_course_names(): states = _load("en")["entity"]["select"]["dishwasher_cycle"]["state"] assert { From 846aefbcbac436e4f2afda6dca41264a39b2c91b Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Sun, 9 Aug 2026 16:06:22 +0000 Subject: [PATCH 3/4] Fix ty failures in new AC temperature-step tests (CI) ClimateDesc.write_fn is typed as WriteFn (Callable[[Any, dict], ...]), which only covers the (payload, rep) shape every other capability's write_fn honors -- calling it through that alias with the climate-only href/resources args, without first narrowing away the | None, failed ty two ways: the missing "is not None" check and the extra positional args past WriteFn's declared arity. Call _climate_write directly instead, same as test_coordinator_send_command.py and test_airconditioner_artik051_krac.py already do. --- tests/test_airconditioner_capabilities.py | 26 ++++++++++++----------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/tests/test_airconditioner_capabilities.py b/tests/test_airconditioner_capabilities.py index 92a0ca1..f9f1d3b 100644 --- a/tests/test_airconditioner_capabilities.py +++ b/tests/test_airconditioner_capabilities.py @@ -157,15 +157,21 @@ def test_climate_write_preserves_half_degree_temperature_steps(): The increment on /temperatures/vs/0 lives inside its items[] entry (the same shape every fixture in the corpus uses), not at the resource's top level -- a fabricated flat `{"/temperatures/vs/0": {"increment": ...}}` - resource would pass a step-reading bug like that silently.""" - climate_desc = next(e for e in airconditioner.CLIMATE.entities if isinstance(e, ClimateDesc)) - write = climate_desc.write_fn + resource would pass a step-reading bug like that silently. + + Calls _climate_write directly rather than through ClimateDesc.write_fn + (as test_climate_write_targets above does): WriteFn only declares the + (payload, rep) shape every other write_fn honors, so the type checker + rejects a call carrying the climate-only href/resources params through + that narrower alias -- same reason test_coordinator_send_command.py and + test_airconditioner_artik051_krac.py import the function directly too. + """ resources = _load_device("airconditioner_cac") - assert write(("temperature_ocf", 24.5), {}, None, resources) == ( + assert airconditioner._climate_write(("temperature_ocf", 24.5), {}, None, resources) == ( ["temperature", "desired", "0"], {"temperature": 24.5}, ) - assert write(("temperature", 24.5), {}, None, resources) == ( + assert airconditioner._climate_write(("temperature", 24.5), {}, None, resources) == ( ["temperatures", "vs", "0"], { "x.com.samsung.da.items": [ @@ -180,11 +186,9 @@ def test_climate_write_rounds_to_whole_degree_with_no_advertised_increment(): increment field on /temperatures/vs/0's item -- target_temperature_step (climate.py) defaults to 1.0 there, so the write path must match rather than pass the raw value through unrounded.""" - climate_desc = next(e for e in airconditioner.CLIMATE.entities if isinstance(e, ClimateDesc)) - write = climate_desc.write_fn resources = _load_device("airconditioner_artik051_krac_18k") assert airconditioner._temperature_step(resources) is None - assert write(("temperature", 23.6), {}, None, resources) == ( + assert airconditioner._climate_write(("temperature", 23.6), {}, None, resources) == ( ["temperatures", "vs", "0"], { "x.com.samsung.da.items": [ @@ -199,10 +203,8 @@ def test_climate_write_rejects_non_numeric_temperature(): build a body with `{"temperature": None}` -- coordinator.py's async_send_command logs and drops a write_fn result of None instead of POSTing it.""" - climate_desc = next(e for e in airconditioner.CLIMATE.entities if isinstance(e, ClimateDesc)) - write = climate_desc.write_fn - assert write(("temperature_ocf", "not-a-number"), {}) is None - assert write(("temperature", None), {}) is None + assert airconditioner._climate_write(("temperature_ocf", "not-a-number"), {}) is None + assert airconditioner._climate_write(("temperature", None), {}) is None def test_climate_consumed_hrefs_declared_as_coverage(): From 676074b2f8924250fe4e965c5b73797019faaaf7 Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Sun, 9 Aug 2026 16:25:12 +0000 Subject: [PATCH 4/4] AC: quantize subdevice temperature writes against their own step (Opus review) async_send_command handed write_fn/validate_fn the raw cache snapshot (real, on-the-wire hrefs), not a subdevice-scoped view. Every other consumer of a full resources dict (exists_fn, rep_fn, is_legacy_board, ...) reads through coordinator.canonical_resources() specifically to avoid this; write_fn/validate_fn didn't, so on a composite AC (issue #177) _temperature_step's resources.get(HREF_TEMP_CONTROL) saw the master's /temperature/control/vs/0 instead of the subdevice's own /temperature/control/vs/1, silently rounding a subdevice's 0.5-degree write to a whole degree. The remote-control gate stays on the raw snapshot -- /remotectrl/* is a shared, MAIN-only resource a subdevice's owned-hrefs-only canonical view would drop entirely. climate.py's target_temperature_step duplicated this same read-in-order logic; pointed it at airconditioner._temperature_step so the read and write paths can't drift again. Also, from the same review: - _quantize_temperature rejects non-finite floats (nan/inf survive float() but raise out of round()/division, escaping write_fn's documented None-on-bad-payload contract). - Deduplicated the quantize-and-check block shared by the temperature_ocf/temperature branches of _climate_write. - Added the /temperatures/vs/0 items[]-fallback test that was previously unreachable (every increment-carrying fixture also has /temperature/control/vs/0, which _temperature_step checks first). - Added a coordinator-level test seeding an indexed subdevice with its own step, distinct from the master's, covering the fix above. - The Towels/Bedding regression test now checks all 7 locale catalogs, not just English -- the bug is a code-mapping error, and test_every_language_mirrors_the_english_catalog only checks key topology, not values. --- custom_components/localthings/climate.py | 15 ++++------ custom_components/localthings/coordinator.py | 17 +++++++++-- .../registry/capabilities/airconditioner.py | 17 +++++++---- tests/test_airconditioner_capabilities.py | 20 +++++++++++++ tests/test_coordinator_send_command.py | 30 +++++++++++++++++++ tests/test_translations.py | 18 +++++++---- 6 files changed, 94 insertions(+), 23 deletions(-) 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",