review: address Opus code-review findings on this branch

- config_flow: the #192 port-rescue made the "every port refused" fast-fail
  permanently unreachable (PREFERRED_PROBE_PORTS is always non-empty and
  always rescued), so removed the dead branch instead of leaving it as
  misleading dead code. A dead host now fails via the handshake loop's own
  error, which carries the real per-port reason.
- oven._has_option: added the is_stub_rep carve-out cooktop.py's identical
  per-token exists_fn already has on the same kind of href, so a not-yet
  sub-polled /mode/vs/0 doesn't permanently exclude energy_saving/
  cooktop_on_alert before their first real fetch lands.
- airconditioner.ENERGY_METER_LEGACY: build via dataclasses.replace() like
  its ENERGY_METER_GENERIC sibling, instead of hand-copying href/poll_tier
  (which would silently drift if common.ENERGY_METER ever gains a field).
- Fixed two stale comments: is_legacy_board()'s docstring still listed
  Volume among tokens needing legacy-only gating, though 'beep' now applies
  unconditionally across board generations; and common.py's UNIVERSAL
  invariant comment didn't mention that airconditioner also now excludes
  ENERGY_METER from the wholesale bundle.
- #191 (CAC token): added the fixture/golden/capability-test coverage the
  AVT token in the same branch got, including an honestly-documented list
  of the ten hrefs this board generation doesn't cover yet.
- fridge.cooler_temperature_setpoint: dropped the hardcoded "N °C" state
  labels -- the resource's own unit field isn't necessarily Celsius on a
  different model reporting the same href, and the options themselves are
  already read live via options_field, so a static per-value label risked
  asserting the wrong unit for a future device sharing this capability.
- airconditioner._legacy_cumulative_power_kwh: parse with float (matching
  common.wh_to_kwh's own numeric parsing) instead of this module's
  integer-only _int, so a decimal-formatted reading doesn't raise.
- test_config_flow: the port-rescue test bound the real 49154 directly,
  which could collide with an actually-running service on some machine;
  now monkeypatches PREFERRED_PROBE_PORTS to an OS-assigned port instead.
- oven.py: consolidated six byte-for-byte-identical single-token options
  write_fns (lamp/sound/fast_preheat/natural_steam/energy_saving/
  cooktop_on_alert) into one _option_switch_write(prefix) factory.

845 tests passing (up from 841 -- 4 new CAC coverage tests).
This commit is contained in:
Marc Billow committed 2026-07-29 18:04:23 +00:00
1 parent a003650d31
commit bd3ca80d3c
11 files changed
+1129 -117

No files matched your search

+7 -8
View File
@@ -248,14 +248,13 @@ def _probe_and_validate(host: str, ca_cert_pem: str, ca_key_pem: str) -> dict:
candidates = _find_live_ports(
host, PROBE_PORT_RANGE, LIVENESS_PROBE_TIMEOUT_S
)
if not candidates:
# Every port in the range actively refused: there is no DTLS/CoAP
# listener on this host, so a handshake can't succeed. Fail now with a
# clear message instead of retrying doomed ports.
raise CannotConnect(
f"no live DTLS port found on {host} "
f"in {PROBE_PORT_RANGE[0]}-{PROBE_PORT_RANGE[-1]}"
)
# No early "every port refused" fast-fail here: _find_live_ports always
# rescues PREFERRED_PROBE_PORTS (issue #192), so candidates is never
# empty as long as that table is non-empty and within PROBE_PORT_RANGE --
# both true today, which made this branch permanently unreachable. A
# genuinely dead host now fails via the handshake loop's own error below,
# which carries the actual per-port timeout/refusal reason instead of a
# generic "no live port found" message.
_LOGGER.debug("Live DTLS port candidates on %s: %s", host, candidates)
last_exc = None
@@ -288,13 +288,17 @@ def _option_token(rep, prefix):
def is_legacy_board(resources):
"""True for the board generation whose airflow lives in /airflow/vs/0.
Newer families carry several of the same option tokens (Volume, Sleep,
OutdoorTemp, Autoclean) *alongside* dedicated resources for those settings,
so an ungated token entity would either duplicate an existing one or apply a
scale calibrated elsewhere. Every AC dump on record has one shape or the
other: /airflow/vs/0 with no /wind/* at all, or /wind/strength/vs/0 with no
/airflow/vs/0. Same test as climate.py's _legacy_airflow(), so the entities
below and the climate entity can never disagree about the generation.
Newer families carry several of the same option tokens (Sleep, OutdoorTemp,
Autoclean) *alongside* dedicated resources for those settings, so an
ungated token entity would either duplicate an existing one or apply a
scale calibrated elsewhere. (Volume used to be in this list too, back when
it had its own gated buzzer_volume Number for this board generation --
issue #136 replaced that with the unified 'beep' switch, which applies
across every generation and isn't gated here at all.) Every AC dump on
record has one shape or the other: /airflow/vs/0 with no /wind/* at all,
or /wind/strength/vs/0 with no /airflow/vs/0. Same test as climate.py's
_legacy_airflow(), so the entities below and the climate entity can never
disagree about the generation.
"""
return HREF_AIRFLOW in resources and HREF_WIND_STRENGTH not in resources
@@ -310,13 +314,18 @@ def is_legacy_board(resources):
# 'energy_kwh' needs a replacement value_fn here; the rest pass through
# unchanged in case a future legacy dump ever reports them.
def _legacy_cumulative_power_kwh(v):
n = _int(v)
return round(n / 100000.0, 2) if n is not None else None
# float, not _int -- matches common.wh_to_kwh's own numeric parsing
# (float via _num) rather than this module's integer-only _int, so a
# decimal-formatted reading doesn't raise and silently go 'unknown'.
try:
n = float(v)
except (TypeError, ValueError):
return None
return round(n / 100000.0, 2)
ENERGY_METER_LEGACY = Capability(
href=common.ENERGY_METER.href,
poll_tier=common.ENERGY_METER.poll_tier,
ENERGY_METER_LEGACY = replace(
common.ENERGY_METER,
match_fn=lambda rep, resources: is_legacy_board(resources),
entities=tuple(
replace(e, value_fn=_legacy_cumulative_power_kwh)
@@ -611,15 +611,22 @@ SELF_CHECK = Capability(
# level: 2 of 6 families confirmed, blanket-added everywhere else).
#
# POWER is kept separate -- airconditioner is the one family that opts out
# of it. Canonical reason (see by_type/airconditioner.py and its test for
# pointers back here, not restatements): AC's climate entity already owns
# /power/0 and /power/vs/0 via bare, no-entity Capability objects
# (airconditioner.COVERAGE), and a second, real POWER_GENERIC/
# of it entirely. Canonical reason (see by_type/airconditioner.py and its
# test for pointers back here, not restatements): AC's climate entity
# already owns /power/0 and /power/vs/0 via bare, no-entity Capability
# objects (airconditioner.COVERAGE), and a second, real POWER_GENERIC/
# POWER_VS_FALLBACK cap on the same href would make _build() raise (a href
# with >1 cap must have every cap discriminated by rt_filter/match_fn, and
# the bare COVERAGE cap has neither). Kids-lock/remote-control don't have
# this conflict -- no AC dump has ever reported those hrefs -- so they stay
# in UNIVERSAL.
#
# Airconditioner also partially opts out of UNIVERSAL itself, not just
# POWER: issue #193 needs ENERGY_METER's cumulativePower scale to differ by
# board generation, so by_type/airconditioner.py excludes just that one
# member (`*[c for c in common.UNIVERSAL if c is not common.ENERGY_METER]`)
# and substitutes airconditioner.ENERGY_METER_GENERIC/ENERGY_METER_LEGACY in
# its place -- every other registry still unpacks UNIVERSAL wholesale.
# ---------------------------------------------------------------------------
UNIVERSAL = (
@@ -24,6 +24,7 @@ but is effectively read-only in practice.
"""
from datetime import datetime, timezone, timedelta
from ..batch import is_stub_rep
from ..capability import Capability
from ..entities import (
BinarySensorDesc, NumberDesc, SelectDesc, SensorDesc, SwitchDesc,
@@ -140,8 +141,15 @@ def _has_option(prefix):
options[] at all, so both switches were phantom controls: always read as
off, and toggling them wrote a token the firmware never recognized in
the first place, hence "does not appear to do anything."
`is_stub_rep(rep) or` keeps the same stub carve-out as cooktop.py's
identical per-token exists_fn on its own options[]-array href: a stub
/device/0 seed rep (not yet sub-polled) has no options[] at all, and
without this an entity whose token is genuinely present would never get
a first chance to bind, since exists_fn runs before that first real
fetch lands.
"""
return lambda rep, resources: _option_value(
return lambda rep, resources: is_stub_rep(rep) or _option_value(
rep.get('x.com.samsung.da.options'), prefix) is not None
@@ -214,64 +222,19 @@ def _oven_mode_write(p, rep, href=None):
return ['mode', 'vs', '0'], {'x.com.samsung.da.modes': [p]}
def _lamp_write(p, rep, href=None):
if p not in ('On', 'Off'):
return None
if not rep.get('x.com.samsung.da.options'):
return None
return ['mode', 'vs', '0'], {
'x.com.samsung.da.options': _option_write('UpperLamp', p),
}
def _sound_write(p, rep, href=None):
if p not in ('On', 'Off'):
return None
if not rep.get('x.com.samsung.da.options'):
return None
return ['mode', 'vs', '0'], {
'x.com.samsung.da.options': _option_write('Sound', p),
}
def _fastpreheat_write(p, rep, href=None):
if p not in ('On', 'Off'):
return None
if not rep.get('x.com.samsung.da.options'):
return None
return ['mode', 'vs', '0'], {
'x.com.samsung.da.options': _option_write('fastpreheat', p),
}
def _naturalsteam_write(p, rep, href=None):
if p not in ('On', 'Off'):
return None
if not rep.get('x.com.samsung.da.options'):
return None
return ['mode', 'vs', '0'], {
'x.com.samsung.da.options': _option_write('NaturalSteam', p),
}
def _energysaving_write(p, rep, href=None):
if p not in ('On', 'Off'):
return None
if not rep.get('x.com.samsung.da.options'):
return None
return ['mode', 'vs', '0'], {
'x.com.samsung.da.options': _option_write('EnergySaving', p),
}
def _burneronalert_write(p, rep, href=None):
if p not in ('On', 'Off'):
return None
if not rep.get('x.com.samsung.da.options'):
return None
return ['mode', 'vs', '0'], {
'x.com.samsung.da.options': _option_write('BurnerOnAlert', p),
}
def _option_switch_write(prefix):
"""Factory for a single-token on/off options-array write -- lamp, sound,
fast_preheat, natural_steam, energy_saving, and cooktop_on_alert were all
a byte-for-byte copy of this same shape, one per prefix."""
def write(p, rep, href=None):
if p not in ('On', 'Off'):
return None
if not rep.get('x.com.samsung.da.options'):
return None
return ['mode', 'vs', '0'], {
'x.com.samsung.da.options': _option_write(prefix, p),
}
return write
# ---------------------------------------------------------------------------
@@ -412,22 +375,22 @@ OVEN_MODE = Capability(
SwitchDesc(key='lamp', field='x.com.samsung.da.options',
icon='mdi:track-light',
value_fn=lambda opts: _option_value(opts, 'UpperLamp') == 'On',
write_fn=_lamp_write),
write_fn=_option_switch_write('UpperLamp')),
SwitchDesc(key='sound', field='x.com.samsung.da.options',
icon='mdi:volume-high',
entity_category='config',
value_fn=lambda opts: _option_value(opts, 'Sound') == 'On',
write_fn=_sound_write),
write_fn=_option_switch_write('Sound')),
SwitchDesc(key='fast_preheat', field='x.com.samsung.da.options',
icon='mdi:fire',
exists_fn=_has_option('fastpreheat'),
value_fn=lambda opts: _option_value(opts, 'fastpreheat') == 'On',
write_fn=_fastpreheat_write),
write_fn=_option_switch_write('fastpreheat')),
SwitchDesc(key='natural_steam', field='x.com.samsung.da.options',
icon='mdi:kettle-steam',
exists_fn=_has_option('NaturalSteam'),
value_fn=lambda opts: _option_value(opts, 'NaturalSteam') == 'On',
write_fn=_naturalsteam_write),
write_fn=_option_switch_write('NaturalSteam')),
# 120-hour energy-saving standby (issue #183): confirmed present in
# this unit's options[] (EnergySaving_On) and directly requested --
# unlike fast_preheat/natural_steam above, this token is real on this
@@ -436,7 +399,7 @@ OVEN_MODE = Capability(
icon='mdi:leaf', entity_category='config',
exists_fn=_has_option('EnergySaving'),
value_fn=lambda opts: _option_value(opts, 'EnergySaving') == 'On',
write_fn=_energysaving_write),
write_fn=_option_switch_write('EnergySaving')),
# Cooktop-on alert (issue #183): also confirmed present
# (BurnerOnAlert_Off) though the reporter noted it mainly matters for
# the SmartThings app's own alerting, not local automation.
@@ -444,6 +407,6 @@ OVEN_MODE = Capability(
icon='mdi:alert-circle-outline', entity_category='config',
exists_fn=_has_option('BurnerOnAlert'),
value_fn=lambda opts: _option_value(opts, 'BurnerOnAlert') == 'On',
write_fn=_burneronalert_write),
write_fn=_option_switch_write('BurnerOnAlert')),
),
)
@@ -212,14 +212,7 @@
}
},
"cooler_temperature_setpoint": {
"name": "Cooler temperature",
"state": {
"1": "1 °C",
"2": "2 °C",
"3": "3 °C",
"4": "4 °C",
"7": "7 °C"
}
"name": "Cooler temperature"
},
"day_brightness": {
"name": "Cabinet brightness",
@@ -212,14 +212,7 @@
}
},
"cooler_temperature_setpoint": {
"name": "Koeler temperatuur",
"state": {
"1": "1 °C",
"2": "2 °C",
"3": "3 °C",
"4": "4 °C",
"7": "7 °C"
}
"name": "Koeler temperatuur"
},
"day_brightness": {
"name": "Helderheid kast",