From 8ca2c3cb160d66480d68fd2ed6019df5a408915d Mon Sep 17 00:00:00 2001 From: Marc Billow Date: Thu, 23 Jul 2026 02:23:01 +0000 Subject: [PATCH] Address opus review: Fahrenheit setpoint bounds, OVEN_SPEC coverage, docstring - NumberDesc gains native_min_fn/native_max_fn/step_fn hooks (mirroring the existing unit_fn pattern) so an entity's slider bounds can track the live rep instead of staying pinned to whatever unit the descriptor was written against. Oven setpoint was hardcoded to Celsius bounds (30-270), which silently capped issue #44's Fahrenheit range at 270F -- below a normal 350F bake temp. Verified Fahrenheit bounds (175-550, step 5) come from that dump's /mode/vs/0 Bake modeSpec. - Wire oven.OVEN_SPEC into the oven registry, not just range -- it was only reachable from range before, so a standalone oven reporting /oven/spec/vs/0 would have false-tripped the coverage-gap repair. - Fix a docstring in test_golden_regression.py left over from the cooktop.py -> range.py rename. --- custom_components/localthings/number.py | 15 +++++++ .../localthings/registry/by_type/oven.py | 1 + .../localthings/registry/capabilities/oven.py | 33 ++++++++++++--- .../localthings/registry/entities.py | 7 ++++ tests/test_golden_regression.py | 2 +- tests/test_oven_capabilities.py | 40 +++++++++++++++++++ 6 files changed, 92 insertions(+), 6 deletions(-) diff --git a/custom_components/localthings/number.py b/custom_components/localthings/number.py index 2d8358b..ab04589 100644 --- a/custom_components/localthings/number.py +++ b/custom_components/localthings/number.py @@ -58,6 +58,9 @@ class LocalThingsNumber(LocalThingsEntity, NumberEntity): @property def native_min_value(self) -> float: + desc: NumberDesc = self._bound.desc + if desc.native_min_fn is not None: + return desc.native_min_fn(self.coordinator.resource(self._bound.href)) r = self._range_from_resource() if r is not None: return float(r[0]) @@ -67,6 +70,9 @@ class LocalThingsNumber(LocalThingsEntity, NumberEntity): @property def native_max_value(self) -> float: + desc: NumberDesc = self._bound.desc + if desc.native_max_fn is not None: + return desc.native_max_fn(self.coordinator.resource(self._bound.href)) r = self._range_from_resource() if r is not None: return float(r[1]) @@ -74,6 +80,15 @@ class LocalThingsNumber(LocalThingsEntity, NumberEntity): return self._attr_native_max_value return super().native_max_value + @property + def native_step(self) -> float: + desc: NumberDesc = self._bound.desc + if desc.step_fn is not None: + return desc.step_fn(self.coordinator.resource(self._bound.href)) + if hasattr(self, '_attr_native_step'): + return self._attr_native_step + return super().native_step + @property def native_value(self): return (self.coordinator.data or {}).get(self._state_key) diff --git a/custom_components/localthings/registry/by_type/oven.py b/custom_components/localthings/registry/by_type/oven.py index 2ae3341..c4028ce 100644 --- a/custom_components/localthings/registry/by_type/oven.py +++ b/custom_components/localthings/registry/by_type/oven.py @@ -14,5 +14,6 @@ REGISTRY = DeviceRegistry( oven.OVEN_OPERATIONAL_STATE, oven.OVEN_DOOR, oven.OVEN_CONNECTED, + oven.OVEN_SPEC, ]), ) diff --git a/custom_components/localthings/registry/capabilities/oven.py b/custom_components/localthings/registry/capabilities/oven.py index 7d66b8b..2eadfc4 100644 --- a/custom_components/localthings/registry/capabilities/oven.py +++ b/custom_components/localthings/registry/capabilities/oven.py @@ -39,6 +39,15 @@ SETPOINT_MIN_C = 30 SETPOINT_MAX_C = 270 SETPOINT_STEP_C = 5 +# Verified against issue #44's range dump (NSI6DG9100SRAA, unit reported as +# "Fahrenheit" on /temperatures/vs/0): Bake mode's modeSpec on /mode/vs/0 +# reports tempMinF/tempMaxF/tempIntervalF = 175/550/5. Kept as a separate +# constant set rather than converted from the Celsius bounds above, which +# are themselves unverified (no live dump; see module docstring). +SETPOINT_MIN_F = 175 +SETPOINT_MAX_F = 550 +SETPOINT_STEP_F = 5 + # Mode options seen on NV7000BS-class. No dump exists so this list is inferred # from Samsung documentation and firmware observations. The firmware will # reject unknown modes; missing entries here are a coverage gap, not a bug. @@ -129,8 +138,9 @@ def _oven_setpoint_write(p, rep, href=None): temp = float(p) except (TypeError, ValueError): return None - temp_i = int(round(temp / SETPOINT_STEP_C) * SETPOINT_STEP_C) - if not (SETPOINT_MIN_C <= temp_i <= SETPOINT_MAX_C): + min_v, max_v, step_v = _setpoint_bounds(rep) + temp_i = int(round(temp / step_v) * step_v) + if not (min_v <= temp_i <= max_v): return None items = rep.get('x.com.samsung.da.items') if not items: @@ -263,14 +273,24 @@ def _oven_temp_unit(rep): same aggregate `/temperatures/vs/0` items[] resource type, which on fridge hardware carries a per-item `x.com.samsung.da.unit` field ('Celsius'/'Fahrenheit') that was previously hardcoded away (issue #7). - No live oven dump has surfaced a non-Celsius reading yet, so this keeps - the verified '°C' default when the field is absent, but reads it live - if a device ever reports otherwise.""" + Keeps the verified '°C' default when the field is absent (the original + NV7000BS-class dump this module was written against), but reads it live + -- issue #44's range dump is the first to report 'Fahrenheit' here.""" items = rep.get('x.com.samsung.da.items') or [] unit = items[0].get('x.com.samsung.da.unit') if items else None return normalize_temp_unit(unit, default='°C') +def _setpoint_bounds(rep): + """(min, max, step) for the live unit -- see the SETPOINT_*_C/_F + constants above for provenance. Bounds must track the unit shown by + unit_fn (both read the same live rep), or the HA slider's range would + silently mismatch its own displayed unit.""" + if _oven_temp_unit(rep) == '°F': + return SETPOINT_MIN_F, SETPOINT_MAX_F, SETPOINT_STEP_F + return SETPOINT_MIN_C, SETPOINT_MAX_C, SETPOINT_STEP_C + + OVEN_SETPOINT = Capability( href='/temperatures/vs/0', poll_tier='hot', @@ -280,6 +300,9 @@ OVEN_SETPOINT = Capability( name='Setpoint', device_class='temperature', unit_fn=_oven_temp_unit, native_min=float(SETPOINT_MIN_C), native_max=float(SETPOINT_MAX_C), step=float(SETPOINT_STEP_C), icon='mdi:thermometer-chevron-up', + native_min_fn=lambda rep: float(_setpoint_bounds(rep)[0]), + native_max_fn=lambda rep: float(_setpoint_bounds(rep)[1]), + step_fn=lambda rep: float(_setpoint_bounds(rep)[2]), value_fn=lambda items: _int( (items[0].get('x.com.samsung.da.desired') if items else None)), write_fn=_oven_setpoint_write), diff --git a/custom_components/localthings/registry/entities.py b/custom_components/localthings/registry/entities.py index e1b98fc..322f83d 100644 --- a/custom_components/localthings/registry/entities.py +++ b/custom_components/localthings/registry/entities.py @@ -85,6 +85,13 @@ class NumberDesc(SamsungEntityDescription): native_min: Optional[float] = None native_max: Optional[float] = None step: Optional[float] = None + # Override native_min/native_max/step from the live rep, when set -- + # same "static default, live override" shape as unit_fn, for resources + # whose sane bounds depend on a per-device value (e.g. a temperature + # setpoint reported in Celsius on one device, Fahrenheit on another). + native_min_fn: Optional[Callable[[dict], float]] = None + native_max_fn: Optional[Callable[[dict], float]] = None + step_fn: Optional[Callable[[dict], float]] = None range_field: Optional[str] = None # resource field containing [min, max] list write_fn: WriteFn = None diff --git a/tests/test_golden_regression.py b/tests/test_golden_regression.py index a351bf8..2edbe74 100644 --- a/tests/test_golden_regression.py +++ b/tests/test_golden_regression.py @@ -203,7 +203,7 @@ def test_registry_reproduces_golden_state_keys_for_range(): """Range/cooktop-oven combo (model TP1X_DA-KS-RANGE-0102X, issue #44) -- reports no oneUiVersion; resolved via the '-RANGE-' modelNum token fallback in for_device_by_model. Reuses the oven family's cavity/ - setpoint/mode/operational-state capabilities and adds cooktop.py's + setpoint/mode/operational-state capabilities and adds range.py's per-burner capabilities for the 4 burners this dump reports.""" from tests.conftest import _load_device resources = _load_device('range') diff --git a/tests/test_oven_capabilities.py b/tests/test_oven_capabilities.py index 8799fa0..aafad2b 100644 --- a/tests/test_oven_capabilities.py +++ b/tests/test_oven_capabilities.py @@ -53,6 +53,46 @@ def test_oven_setpoint_rejects_missing_items(): assert desc.write_fn(200, {}) is None +# --------------------------------------------------------------------------- +# OVEN_SETPOINT — Fahrenheit bounds (issue #44 range dump reports +# unit="Fahrenheit"; bounds/step must track the live unit, not stay pinned +# to the Celsius defaults) +# --------------------------------------------------------------------------- + +def _fahrenheit_rep(desired='0'): + return {'x.com.samsung.da.items': [{ + 'x.com.samsung.da.desired': desired, + 'x.com.samsung.da.unit': 'Fahrenheit', + }]} + + +def test_oven_setpoint_write_uses_fahrenheit_bounds(): + desc = oven.OVEN_SETPOINT.entities[0] + rep = _fahrenheit_rep() + # 350 is within F bounds (175-550) but above the C max (270) -- + # confirms the write path isn't silently still clamping to Celsius. + _, body = desc.write_fn(350, rep) + assert body['x.com.samsung.da.items'][0]['x.com.samsung.da.desired'] == '350' + + +def test_oven_setpoint_rejects_out_of_range_fahrenheit(): + desc = oven.OVEN_SETPOINT.entities[0] + rep = _fahrenheit_rep() + assert desc.write_fn(100, rep) is None # below F min (175) + assert desc.write_fn(600, rep) is None # above F max (550) + + +def test_oven_setpoint_native_bounds_track_live_unit(): + desc = oven.OVEN_SETPOINT.entities[0] + celsius_rep = {'x.com.samsung.da.items': [{'x.com.samsung.da.unit': 'Celsius'}]} + assert desc.native_min_fn(celsius_rep) == 30.0 + assert desc.native_max_fn(celsius_rep) == 270.0 + fahrenheit_rep = _fahrenheit_rep() + assert desc.native_min_fn(fahrenheit_rep) == 175.0 + assert desc.native_max_fn(fahrenheit_rep) == 550.0 + assert desc.step_fn(fahrenheit_rep) == 5.0 + + # --------------------------------------------------------------------------- # OVEN_MODE — SelectDesc with non-empty options # ---------------------------------------------------------------------------