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

fix: address code review findings from post-onboarding audit

Browse Source 
- Add button platform (button.py + PLATFORMS) so oven/washer Start/Pause/Stop
  controls actually appear in HA
- Load DTLS cert chain from memory via _load_pem_chain(); eliminate all temp
  file writes for key material in config_flow and coordinator
- Fix PEM boundary: ensure leaf and CA blocks are separated by a newline in
  the stored fullchain regardless of whether the user pasted a trailing newline
- Dryer mode select: add static options tuple and value_fn to decode Course_XX
  hex strings back to human names; was permanently empty before
- Remove callable branch from LocalThingsSelect (options_field covers dynamic
  case; callable contract was broken — called with no args vs Callable[[dict]])
- Guard native_min/max_value overrides with hasattr + super() fallback to
  prevent AttributeError when native_min=None and range_field returns nothing
- Add async_close() public method to coordinator; __init__ now calls it instead
  of reaching in to _close_session directly
- Update tests: rename CONF_CERT_PEM/KEY_PEM → CA/LEAF variants, add
  leaf_cert_pem/leaf_key_pem to mock_probe return, fix mock PEM constants
  (no trailing newlines, matching what .strip() stores)
This commit is contained in:
Marc Billow committed 2026-06-30 16:34:39 -05:00
1 parent f25e616950
commit 57cf1cdd11
11 files changed
+176 -120
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

custom_components/localthings/__init__.py
+1 -1
View File
@@ -29,5 +29,5 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
unloaded = await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
if unloaded:
coordinator: LocalThingsCoordinator = hass.data[DOMAIN].pop(entry.entry_id)
await hass.async_add_executor_job(coordinator._close_session)
await coordinator.async_close()
return unloaded
custom_components/localthings/button.py
+36
View File
@@ -0,0 +1,36 @@
"""Button platform for Local Things."""
from __future__ import annotations
from homeassistant.components.button import ButtonEntity
from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant
from homeassistant.helpers.entity_platform import AddEntitiesCallback
from .ocf.registry.entities import ButtonDesc
from .const import DOMAIN
from .coordinator import LocalThingsCoordinator
from .entity import LocalThingsEntity
async def async_setup_entry(
hass: HomeAssistant,
entry: ConfigEntry,
async_add_entities: AddEntitiesCallback,
) -> None:
coordinator: LocalThingsCoordinator = hass.data[DOMAIN][entry.entry_id]
async_add_entities(
LocalThingsButton(coordinator, b)
for b in coordinator.bound
if isinstance(b.desc, ButtonDesc)
)
class LocalThingsButton(LocalThingsEntity, ButtonEntity):
def __init__(self, coordinator: LocalThingsCoordinator, bound) -> None:
super().__init__(coordinator, bound)
self._payload = bound.desc.payload
async def async_press(self) -> None:
await self.coordinator.async_send_command(self._bound, self._payload)
custom_components/localthings/config_flow.py
+48 -58
View File
@@ -3,11 +3,9 @@ from __future__ import annotations
import datetime
import logging
import os
import re
import socket
import ssl
import tempfile
from typing import Any
import voluptuous as vol
@@ -114,7 +112,11 @@ def _mint_leaf_cert(ca_cert_pem: str, ca_key_pem: str, uuid: str) -> tuple[str,
serialization.NoEncryption(),
).decode()
fullchain_pem = leaf_cert_pem + ca_cert_pem
# Ensure a newline separates the leaf and CA blocks regardless of
# whether the user's pasted CA PEM had a trailing newline.
fullchain_pem = leaf_cert_pem.rstrip('\n') + '\n' + ca_cert_pem
if not fullchain_pem.endswith('\n'):
fullchain_pem += '\n'
return fullchain_pem, leaf_key_pem
@@ -143,61 +145,49 @@ def _probe_and_validate(host: str, ca_cert_pem: str, ca_key_pem: str) -> dict:
raise CannotConnect(f"Failed to mint leaf cert: {exc}") from exc
_LOGGER.debug("Leaf cert minted successfully")
cert_file = tempfile.NamedTemporaryFile(mode='w', suffix='.pem', delete=False)
key_file = tempfile.NamedTemporaryFile(mode='w', suffix='.pem', delete=False)
try:
cert_file.write(fullchain_pem); cert_file.flush(); cert_file.close()
key_file.write(leaf_key_pem); key_file.flush(); key_file.close()
last_exc = None
for port in PROBE_PORTS:
sess = None
try:
sess = DtlsCoapSession(
host, port,
cert_path=cert_file.name,
key_path=key_file.name,
)
sess.connect()
sess.start_reader()
code, payload = sess.get(['device', '0'], timeout=15.0)
if code != 0x45 or not payload:
raise CannotConnect(f"port {port}: unexpected code {code:#04x}")
body = cbor2.loads(payload)
resources = (
parse_device0_batch(body) if isinstance(body, list) else {}
)
serial = (
resources
.get('/information/vs/0', {})
.get('x.com.samsung.da.serialNum', '')
)
if not serial:
serial = f"{host}:{port}"
return {
"port": port,
"serial": serial,
"leaf_cert_pem": fullchain_pem,
"leaf_key_pem": leaf_key_pem,
}
except CannotConnect:
raise
except Exception as exc:
last_exc = exc
_LOGGER.debug("port %d failed: %s", port, exc)
finally:
if sess is not None:
try:
sess.close()
except Exception:
pass
raise CannotConnect(f"no port responded on {host}: {last_exc}")
finally:
for path in (cert_file.name, key_file.name):
try:
os.unlink(path)
except OSError:
pass
last_exc = None
for port in PROBE_PORTS:
sess = None
try:
sess = DtlsCoapSession(
host, port,
cert_pem=fullchain_pem,
key_pem=leaf_key_pem,
)
sess.connect()
sess.start_reader()
code, payload = sess.get(['device', '0'], timeout=15.0)
if code != 0x45 or not payload:
raise CannotConnect(f"port {port}: unexpected code {code:#04x}")
body = cbor2.loads(payload)
resources = (
parse_device0_batch(body) if isinstance(body, list) else {}
)
serial = (
resources
.get('/information/vs/0', {})
.get('x.com.samsung.da.serialNum', '')
)
if not serial:
serial = f"{host}:{port}"
return {
"port": port,
"serial": serial,
"leaf_cert_pem": fullchain_pem,
"leaf_key_pem": leaf_key_pem,
}
except CannotConnect:
raise
except Exception as exc:
last_exc = exc
_LOGGER.debug("port %d failed: %s", port, exc)
finally:
if sess is not None:
try:
sess.close()
except Exception:
pass
raise CannotConnect(f"no port responded on {host}: {last_exc}")
class LocalThingsConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
custom_components/localthings/const.py
+1 -1
View File
@@ -1,6 +1,6 @@
DOMAIN = "localthings"
PLATFORMS = ["sensor", "binary_sensor", "switch", "number", "select"]
PLATFORMS = ["sensor", "binary_sensor", "switch", "number", "select", "button"]
CONF_HOST = "host"
CONF_PORT = "port"
custom_components/localthings/coordinator.py
+16 -28
View File
@@ -2,8 +2,6 @@
from __future__ import annotations
import logging
import tempfile
import os
from datetime import timedelta
from typing import Any
@@ -68,34 +66,21 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
return self._last_resources
def _connect_session(self) -> None:
host = self._entry.data[CONF_HOST]
port = self._entry.data[CONF_PORT]
cert_pem: str = self._entry.data[CONF_LEAF_CERT_PEM]
key_pem: str = self._entry.data[CONF_LEAF_KEY_PEM]
host = self._entry.data[CONF_HOST]
port = self._entry.data[CONF_PORT]
cert_pem = self._entry.data[CONF_LEAF_CERT_PEM]
key_pem = self._entry.data[CONF_LEAF_KEY_PEM]
cert_f = tempfile.NamedTemporaryFile(suffix='.pem', delete=False, mode='w')
key_f = tempfile.NamedTemporaryFile(suffix='.pem', delete=False, mode='w')
sess = DtlsCoapSession(host, port, cert_pem=cert_pem, key_pem=key_pem)
sess.connect()
sess.start_reader()
self._session = sess
_LOGGER.debug("DTLS connected to %s:%d", host, port)
try:
cert_f.write(cert_pem); cert_f.flush(); cert_f.close()
key_f.write(key_pem); key_f.flush(); key_f.close()
sess = DtlsCoapSession(host, port,
cert_path=cert_f.name,
key_path=key_f.name)
sess.connect()
sess.start_reader()
self._session = sess
_LOGGER.debug("DTLS connected to %s:%d", host, port)
try:
self._identity = read_identity(sess, None)
except Exception as e:
_LOGGER.debug("read_identity failed: %s", e)
self._identity = None
finally:
for f in (cert_f.name, key_f.name):
try:
os.unlink(f)
except OSError:
pass
self._identity = read_identity(sess, None)
except Exception as e:
_LOGGER.debug("read_identity failed: %s", e)
self._identity = None
def _close_session(self) -> None:
sess = self._session
@@ -106,6 +91,9 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
except Exception:
pass
async def async_close(self) -> None:
await self.hass.async_add_executor_job(self._close_session)
def _poll_once(self) -> dict[str, dict]:
"""GET /device/0, return parsed resources. Blocking."""
if self._session is None:
custom_components/localthings/number.py
+6 -2
View File
@@ -55,14 +55,18 @@ class LocalThingsNumber(LocalThingsEntity, NumberEntity):
r = self._range_from_resource()
if r is not None:
return float(r[0])
return self._attr_native_min_value
if hasattr(self, '_attr_native_min_value'):
return self._attr_native_min_value
return super().native_min_value
@property
def native_max_value(self) -> float:
r = self._range_from_resource()
if r is not None:
return float(r[1])
return self._attr_native_max_value
if hasattr(self, '_attr_native_max_value'):
return self._attr_native_max_value
return super().native_max_value
@property
def native_value(self):
custom_components/localthings/ocf/coap_dtls.py
+31 -6
View File
@@ -31,6 +31,33 @@ _OCF_ROOT_CA = str(Path(__file__).parent / 'ocf_root_ca.pem')
import logging
logger = logging.getLogger(__name__)
import re as _re
_PEM_CERT_RE = _re.compile(
rb'-----BEGIN CERTIFICATE-----.*?-----END CERTIFICATE-----',
_re.DOTALL,
)
def _load_pem_chain(ctx: SSL.Context, cert_pem: str, key_pem: str) -> None:
"""Load a PEM cert chain and private key into an SSL context in memory.
Parses all certificate blocks from cert_pem: the first is the leaf
(use_certificate), the rest are intermediates (add_extra_chain_cert).
No temp files are written.
"""
from OpenSSL import crypto
certs = _PEM_CERT_RE.findall(cert_pem.encode())
if not certs:
raise ValueError("No certificates found in cert_pem")
ctx.use_certificate(crypto.load_certificate(crypto.FILETYPE_PEM, certs[0]))
for extra in certs[1:]:
ctx.add_extra_chain_cert(
crypto.load_certificate(crypto.FILETYPE_PEM, extra)
)
ctx.use_privatekey(crypto.load_privatekey(crypto.FILETYPE_PEM, key_pem.encode()))
ctx.check_privatekey()
# Diagnostic logging — when DEBUG_BRIDGE=1 in env, the bridge dumps
# every received CoAP frame, every /operational/state/vs/0 + /oven/vs/0
# + /power/vs/0 + /mode/vs/0-options rep change, the full link tree at
@@ -188,12 +215,12 @@ class DtlsCoapSession:
READER_RECV_TIMEOUT_S = 1.0 # short so stop_event propagates quickly
MAX_BLOCKS = 32 # safety bound for Block2 fetches
def __init__(self, host, port, cert_path, key_path,
def __init__(self, host, port, cert_pem, key_pem,
on_notification=None, mtu=1200):
self.host = host
self.port = port
self.cert_path = str(cert_path)
self.key_path = str(key_path)
self.cert_pem = cert_pem
self.key_pem = key_pem
self.on_notification = on_notification # fn(href, payload_bytes)
self.mtu = mtu
@@ -235,9 +262,7 @@ class DtlsCoapSession:
# the OpenSSL instance cryptography bundles — ctypes and cffi bindings
# do not expose SSL_CTX_set_security_level on this build.
ctx.set_cipher_list(b'ECDHE-ECDSA-AES128-GCM-SHA256:@SECLEVEL=0')
ctx.use_certificate_chain_file(self.cert_path)
ctx.use_privatekey_file(self.key_path)
ctx.check_privatekey()
_load_pem_chain(ctx, self.cert_pem, self.key_pem)
conn = SSL.Connection(ctx, None)
conn.set_connect_state()
custom_components/localthings/ocf/registry/capabilities/dryer.py
+5 -1
View File
@@ -77,7 +77,11 @@ DRYER_COURSE = Capability(
entities=(
SelectDesc(key='dryer_mode', field='x.com.samsung.da.st.dryerMode',
name='Dryer mode', icon='mdi:tumble-dryer',
options=(), write_fn=_course_write),
options=tuple(_COURSE_CODE_BY_NAME),
value_fn=lambda v: _COURSE_NAMES.get(
int(v.split('_')[1], 16) if isinstance(v, str) and '_' in v else -1
),
write_fn=_course_write),
),
)
custom_components/localthings/select.py
+1 -2
View File
@@ -43,8 +43,7 @@ class LocalThingsSelect(LocalThingsEntity, SelectEntity):
super().__init__(coordinator, bound)
desc: SelectDesc = bound.desc
if not desc.options_field:
opts = desc.options
self._attr_options = list(opts() if callable(opts) else opts)
self._attr_options = list(desc.options)
@property
def options(self) -> list[str]:
tests/localthings/conftest.py
+18 -8
View File
@@ -32,7 +32,8 @@ def auto_enable_custom_integrations(enable_custom_integrations):
from custom_components.localthings.const import (
CONF_CERT_PEM, CONF_HOST, CONF_KEY_PEM, CONF_PORT, DOMAIN,
CONF_CA_CERT_PEM, CONF_CA_KEY_PEM,
CONF_HOST, CONF_LEAF_CERT_PEM, CONF_LEAF_KEY_PEM, CONF_PORT, DOMAIN,
)
FIXTURES = Path(__file__).resolve().parent.parent / 'fixtures'
@@ -40,14 +41,18 @@ FIXTURES = Path(__file__).resolve().parent.parent / 'fixtures'
MOCK_HOST = '10.0.0.254'
MOCK_PORT = 49154
MOCK_SERIAL = 'TEST-SERIAL-001'
MOCK_CERT_PEM = '-----BEGIN CERTIFICATE-----\nTEST\n-----END CERTIFICATE-----'
MOCK_KEY_PEM = '-----BEGIN PRIVATE KEY-----\nTEST\n-----END PRIVATE KEY-----'
MOCK_CA_CERT_PEM = '-----BEGIN CERTIFICATE-----\nTEST-CA\n-----END CERTIFICATE-----'
MOCK_CA_KEY_PEM = '-----BEGIN PRIVATE KEY-----\nTEST-CA-KEY\n-----END PRIVATE KEY-----'
MOCK_LEAF_CERT_PEM = '-----BEGIN CERTIFICATE-----\nTEST-LEAF\n-----END CERTIFICATE-----'
MOCK_LEAF_KEY_PEM = '-----BEGIN PRIVATE KEY-----\nTEST-LEAF-KEY\n-----END PRIVATE KEY-----'
ENTRY_DATA = {
CONF_HOST: MOCK_HOST,
CONF_PORT: MOCK_PORT,
CONF_CERT_PEM: MOCK_CERT_PEM,
CONF_KEY_PEM: MOCK_KEY_PEM,
CONF_HOST: MOCK_HOST,
CONF_PORT: MOCK_PORT,
CONF_CA_CERT_PEM: MOCK_CA_CERT_PEM,
CONF_CA_KEY_PEM: MOCK_CA_KEY_PEM,
CONF_LEAF_CERT_PEM: MOCK_LEAF_CERT_PEM,
CONF_LEAF_KEY_PEM: MOCK_LEAF_KEY_PEM,
}
@@ -67,7 +72,12 @@ def mock_probe():
"""Patch _probe_and_validate to succeed without a real DTLS connection."""
with patch(
'custom_components.localthings.config_flow._probe_and_validate',
return_value={'port': MOCK_PORT, 'serial': MOCK_SERIAL},
return_value={
'port': MOCK_PORT,
'serial': MOCK_SERIAL,
'leaf_cert_pem': MOCK_LEAF_CERT_PEM,
'leaf_key_pem': MOCK_LEAF_KEY_PEM,
},
) as m:
yield m
tests/localthings/test_config_flow.py
+13 -13
View File
@@ -9,27 +9,27 @@ from homeassistant.data_entry_flow import FlowResultType
from pytest_homeassistant_custom_component.common import MockConfigEntry
from custom_components.localthings.const import (
CONF_CERT_PEM, CONF_HOST, CONF_KEY_PEM, CONF_PORT, DOMAIN,
CONF_CA_CERT_PEM, CONF_CA_KEY_PEM, CONF_HOST, CONF_PORT, DOMAIN,
)
from .conftest import (
ENTRY_DATA, MOCK_CERT_PEM, MOCK_HOST, MOCK_KEY_PEM, MOCK_PORT, MOCK_SERIAL,
ENTRY_DATA, MOCK_CA_CERT_PEM, MOCK_CA_KEY_PEM, MOCK_HOST, MOCK_PORT, MOCK_SERIAL,
)
async def test_form_first_device(hass: HomeAssistant) -> None:
"""First device: form asks for host, cert, and key."""
"""First device: form asks for host, CA cert, and CA key."""
result = await hass.config_entries.flow.async_init(
DOMAIN, context={'source': 'user'}
)
assert result['type'] == FlowResultType.FORM
assert result['step_id'] == 'user'
assert CONF_CERT_PEM in result['data_schema'].schema
assert CONF_KEY_PEM in result['data_schema'].schema
assert CONF_CA_CERT_PEM in result['data_schema'].schema
assert CONF_CA_KEY_PEM in result['data_schema'].schema
async def test_form_second_device_reuses_creds(hass: HomeAssistant) -> None:
"""Second device: form only asks for host; cert/key schema fields absent."""
"""Second device: form only asks for host; CA cert/key schema fields absent."""
existing = MockConfigEntry(domain=DOMAIN, data=ENTRY_DATA)
existing.add_to_hass(hass)
@@ -37,8 +37,8 @@ async def test_form_second_device_reuses_creds(hass: HomeAssistant) -> None:
DOMAIN, context={'source': 'user'}
)
assert result['type'] == FlowResultType.FORM
assert CONF_CERT_PEM not in result['data_schema'].schema
assert CONF_KEY_PEM not in result['data_schema'].schema
assert CONF_CA_CERT_PEM not in result['data_schema'].schema
assert CONF_CA_KEY_PEM not in result['data_schema'].schema
async def test_successful_setup(hass: HomeAssistant, mock_probe) -> None:
@@ -48,12 +48,12 @@ async def test_successful_setup(hass: HomeAssistant, mock_probe) -> None:
)
result = await hass.config_entries.flow.async_configure(
result['flow_id'],
{CONF_HOST: MOCK_HOST, CONF_CERT_PEM: MOCK_CERT_PEM, CONF_KEY_PEM: MOCK_KEY_PEM},
{CONF_HOST: MOCK_HOST, CONF_CA_CERT_PEM: MOCK_CA_CERT_PEM, CONF_CA_KEY_PEM: MOCK_CA_KEY_PEM},
)
assert result['type'] == FlowResultType.CREATE_ENTRY
assert result['data'][CONF_HOST] == MOCK_HOST
assert result['data'][CONF_PORT] == MOCK_PORT
assert result['data'][CONF_CERT_PEM] == MOCK_CERT_PEM
assert result['data'][CONF_CA_CERT_PEM] == MOCK_CA_CERT_PEM
async def test_cannot_connect(hass: HomeAssistant) -> None:
@@ -69,7 +69,7 @@ async def test_cannot_connect(hass: HomeAssistant) -> None:
)
result = await hass.config_entries.flow.async_configure(
result['flow_id'],
{CONF_HOST: MOCK_HOST, CONF_CERT_PEM: MOCK_CERT_PEM, CONF_KEY_PEM: MOCK_KEY_PEM},
{CONF_HOST: MOCK_HOST, CONF_CA_CERT_PEM: MOCK_CA_CERT_PEM, CONF_CA_KEY_PEM: MOCK_CA_KEY_PEM},
)
assert result['type'] == FlowResultType.FORM
assert result['errors']['base'] == 'cannot_connect'
@@ -78,7 +78,7 @@ async def test_cannot_connect(hass: HomeAssistant) -> None:
async def test_duplicate_device_aborted(hass: HomeAssistant, mock_probe) -> None:
"""Second add of same serial: flow aborts.
When a device already exists the form only asks for host (cert/key are
When a device already exists the form only asks for host (CA creds are
reused), so we only submit CONF_HOST in the second configure call.
"""
existing = MockConfigEntry(
@@ -91,7 +91,7 @@ async def test_duplicate_device_aborted(hass: HomeAssistant, mock_probe) -> None
result = await hass.config_entries.flow.async_init(
DOMAIN, context={'source': 'user'}
)
# Second-device form only has CONF_HOST; cert/key are reused from existing.
# Second-device form only has CONF_HOST; CA creds are reused from existing.
result = await hass.config_entries.flow.async_configure(
result['flow_id'],
{CONF_HOST: MOCK_HOST},