bhetherman/localthings
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #344 from mbillow/claude/pr-276-squash-review-uplqdz

Browse Source 
Fix AC temperature step quantization + washer cycle translations (#342, #343)
This commit is contained in:
Marc Billow
2026-08-09 12:52:18 -04:00
committed by GitHub
parent 6828d0152b 676074b2f8
commit 16cb01ce5d
13 changed files with 262 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
custom_components/localthings/climate.py
+6 -9
View File
@@ -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 ----------------------------------------------------------
custom_components/localthings/coordinator.py
+14 -3
View File
@@ -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)
custom_components/localthings/registry/capabilities/airconditioner.py
+70 -5
View File
@@ -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
@@ -491,7 +492,68 @@ 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
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
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`
@@ -504,9 +566,12 @@ def _climate_write(payload, rep, href=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":
return (["temperature", "desired", "0"], {"temperature": round(float(value))})
if kind == "temperature":
if kind in ("temperature_ocf", "temperature"):
quantized = _quantize_temperature(value, _temperature_step(resources))
if quantized is None:
return None
if kind == "temperature_ocf":
return (["temperature", "desired", "0"], {"temperature": quantized})
# Vendor items[] array; only one item observed on every AC dump, id '0'.
return (
["temperatures", "vs", "0"],
@@ -514,7 +579,7 @@ def _climate_write(payload, rep, href=None):
"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),
}
]
},
custom_components/localthings/translations/cs.json
+5 -2
View File
@@ -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"
}
},
custom_components/localthings/translations/de.json
+5 -2
View File
@@ -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",
custom_components/localthings/translations/en.json
+6 -3
View File
@@ -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": {
custom_components/localthings/translations/es.json
+5 -2
View File
@@ -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",
custom_components/localthings/translations/it.json
+6 -3
View File
@@ -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": {
custom_components/localthings/translations/ko.json
+5 -2
View File
@@ -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": "데님",
custom_components/localthings/translations/nl.json
+5 -2
View File
@@ -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"
}
},
tests/test_airconditioner_capabilities.py
+75
View File
@@ -152,6 +152,81 @@ 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.
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 airconditioner._climate_write(("temperature_ocf", 24.5), {}, None, resources) == (
["temperature", "desired", "0"],
{"temperature": 24.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_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
(climate.py) defaults to 1.0 there, so the write path must match rather
than pass the raw value through unrounded."""
resources = _load_device("airconditioner_artik051_krac_18k")
assert airconditioner._temperature_step(resources) is None
assert airconditioner._climate_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."""
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():
"""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
tests/test_coordinator_send_command.py
+30
View File
@@ -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}
tests/test_translations.py
+30
View File
@@ -160,6 +160,36 @@ 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.
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; 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",
"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 {