diff --git a/.github/mutations/plan.json b/.github/mutations/plan.json index d399a7a..b482420 100644 --- a/.github/mutations/plan.json +++ b/.github/mutations/plan.json @@ -44,8 +44,8 @@ { "name": "a factory reset with a lost reply keeps the stale cache", "path": "custom_components/neumann_kh/button.py", - "find": " await self.coordinator.async_invalidate_and_refresh()\n raise HomeAssistantError(", - "replace": " raise HomeAssistantError(", + "find": " on_unreachable=self.coordinator.async_invalidate_and_refresh,", + "replace": " on_unreachable=None,", "expect": "tests/test_factory_reset.py::test_a_reset_whose_reply_is_lost_still_drops_the_cache" }, { @@ -166,5 +166,75 @@ "find": " if \"osc\" in payload or json_tokens(payload) + _XID_TOKENS > _MAX_REQUEST_TOKENS:", "replace": " if \"osc\" in payload:", "expect": "tests/test_ssc_client.py::test_a_request_over_the_token_limit_goes_without_an_xid" + }, + { + "name": "a restore cut off mid-way does not re-read the value in flight", + "path": "custom_components/neumann_kh/export_actions.py", + "find": " coordinator.request_reread(path)\n raise _stop(\"restore_interrupted\", error=str(err)) from err", + "replace": " raise _stop(\"restore_interrupted\", error=str(err)) from err", + "expect": "tests/test_export_actions.py::test_a_restore_cut_off_mid_way_keeps_what_landed_and_rereads_the_rest" + }, + { + "name": "a value the device adjusted counts as written", + "path": "custom_components/neumann_kh/export_actions.py", + "find": " if confirmed != value:", + "replace": " if False:", + "expect": "tests/test_export_actions.py::test_a_value_the_device_adjusts_is_counted_as_adjusted" + }, + { + "name": "a clean end of stream before any answer is a silent None", + "path": "custom_components/neumann_kh/ssc_client.py", + "find": " else:\n raise SSCConnectionError(\n f\"Connection to {mask_host(self._host)} interrupted\"\n ) from err", + "replace": " else:\n break", + "expect": "tests/test_ssc_client.py::test_a_speaker_closing_without_an_answer_is_a_connection_error" + }, + { + "name": "a JSON line that is not an object reaches deep_merge", + "path": "custom_components/neumann_kh/ssc_client.py", + "find": " if not isinstance(parsed, dict) or nesting_exceeds(parsed, _MAX_NESTING):", + "replace": " if isinstance(parsed, dict) and nesting_exceeds(parsed, _MAX_NESTING):", + "expect": "tests/test_ssc_client.py::test_a_json_line_that_is_not_an_object_is_skipped" + }, + { + "name": "a setup whose platforms fail keeps its connection", + "path": "custom_components/neumann_kh/__init__.py", + "find": " # any more: it lives on the entry, and the next attempt overwrites it.\n await client.close()", + "replace": " # any more: it lives on the entry, and the next attempt overwrites it.\n pass", + "expect": "tests/test_setup_cleanup.py::test_a_setup_whose_platforms_fail_closes_the_connection" + }, + { + "name": "an error key in the code has no text", + "path": "custom_components/neumann_kh/export_actions.py", + "find": "translation_key=\"restore_no_backup\"", + "replace": "translation_key=\"restore_no_backups\"", + "expect": "tests/test_translations.py::test_every_error_key_in_the_code_has_a_text" + }, + { + "name": "a backup that ran out of time reports a generic failure", + "path": "custom_components/neumann_kh/export_actions.py", + "find": " except BackupTimeoutError as err:", + "replace": " except KeyError as err:", + "expect": "tests/test_export_actions.py::test_a_backup_that_fails_to_read_says_why" + }, + { + "name": "a discovery that ran out of time reports a generic failure", + "path": "custom_components/neumann_kh/export_actions.py", + "find": " except DiscoveryTimeoutError as err:", + "replace": " except KeyError as err:", + "expect": "tests/test_export_actions.py::test_a_discovery_that_fails_to_read_says_why" + }, + { + "name": "the name field forgets the name the speaker had before", + "path": "custom_components/neumann_kh/config_flow.py", + "find": " return self.add_suggested_values_to_schema(schema, {CONF_NAME: remembered_name})", + "replace": " return schema", + "expect": "tests/test_config_flow.py::test_the_name_field_offers_the_name_the_speaker_had_before" + }, + { + "name": "a refused write drops the device's reason from the message", + "path": "custom_components/neumann_kh/entity.py", + "find": " translation_key=rejected_key,\n translation_placeholders={\"error\": str(err)},", + "replace": " translation_key=rejected_key,", + "expect": "tests/test_entity_writes.py::test_a_write_error_tells_the_user_what_the_device_said" } ] diff --git a/README.de.md b/README.de.md index 4a5a6fd..8cd5586 100644 --- a/README.de.md +++ b/README.de.md @@ -17,9 +17,6 @@ Protokoll- und Gerätedetails (SSC, gemessenes Lautsprecher-Verhalten): siehe **Benötigt Home Assistant 2026.10 oder neuer.** -Eigenes Icon/Logo unter `custom_components/neumann_kh/brand/` (eigenständiges -Design, keine Kopie des offiziellen Neumann-Firmenlogos). - Die SSC-Adresspfade stammen ursprünglich aus dem [khtool-Projekt](https://github.com/schwinn/khtool); die von der Integration genutzten Pfade und ihr Verhalten sind inzwischen an KH 120 II und KH 750 DSP diff --git a/README.md b/README.md index fc8fe37..af3b1b6 100644 --- a/README.md +++ b/README.md @@ -17,9 +17,6 @@ Protocol and device details (SSC, measured speaker behaviour): see the **Requires Home Assistant 2026.10 or newer.** -Custom icon/logo in `custom_components/neumann_kh/brand/` (an independent -design, not a copy of the official Neumann company logo). - The SSC address paths were first taken from the [khtool project](https://github.com/schwinn/khtool); the paths the integration uses and how they behave have since been checked on KH 120 II and KH 750 DSP diff --git a/custom_components/neumann_kh/__init__.py b/custom_components/neumann_kh/__init__.py index 2ff71cc..79f68cf 100644 --- a/custom_components/neumann_kh/__init__.py +++ b/custom_components/neumann_kh/__init__.py @@ -101,7 +101,7 @@ async def _async_relocate(hass: HomeAssistant, entry: ConfigEntry) -> bool: # address but moved to another port is precisely the case this repair # exists for. current = (entry.data.get(CONF_HOST), entry.data.get(CONF_PORT, DEFAULT_PORT)) - candidates = [s for s in speakers if (s.host, s.port) != current] + candidates = [speaker for speaker in speakers if (speaker.host, speaker.port) != current] # Asked concurrently, a few at a time. One after another this cost the # connection timeout per silent candidate, and this runs on every failed @@ -118,9 +118,18 @@ async def _serial_of(speaker: DiscoveredSpeaker) -> Any: finally: await client.close() - found_serials = await asyncio.gather(*(_serial_of(s) for s in candidates)) + # The announcement names its serial: a candidate naming another speaker + # needs no connection, and one naming this speaker is taken as it is. + # Unlike async_step_zeroconf, which asks the device before repointing a + # working entry, this runs only after a failed setup, and the retry's + # serial check (_async_verify_identity) is that confirmation. + announced = [speaker for speaker in candidates if speaker.serial] + unannounced = [speaker for speaker in candidates if not speaker.serial] + found_serials = [speaker.serial for speaker in announced] + await asyncio.gather( + *(_serial_of(speaker) for speaker in unannounced) + ) - for speaker, found in zip(candidates, found_serials, strict=True): + for speaker, found in zip(announced + unannounced, found_serials, strict=True): if not serial_matches(serial, found): continue diff --git a/custom_components/neumann_kh/button.py b/custom_components/neumann_kh/button.py index e017c14..d709f01 100644 --- a/custom_components/neumann_kh/button.py +++ b/custom_components/neumann_kh/button.py @@ -31,18 +31,16 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.const import EntityCategory from homeassistant.core import HomeAssistant -from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.entity_platform import AddEntitiesCallback from ._util import localized from .const import ( CONF_MODEL, - DOMAIN, PATH_RESTORE, RESTORE_FACTORY_DEFAULTS_VALUE, ) from .coordinator import NeumannKHConfigEntry, NeumannKHCoordinator -from .entity import NeumannKHEntity +from .entity import NeumannKHEntity, device_errors from .eq import build_eq_reset_buttons from .export_actions import ( async_check_restorable, @@ -50,7 +48,6 @@ async_run_discovery, async_run_restore, ) -from .ssc_client import SSCConnectionError, SSCDeviceError, SSCLocalModeError, SSCTimeoutError # Upper bound of the confirmation window shared by factory reset and restore # backup. After it elapses, the button must be armed again. @@ -134,31 +131,18 @@ async def async_press(self) -> None: # Second press after the minimum gap, within the window -> reset. self._armed_at = None async_dismiss_notification(self.hass, self._notification_id) - try: - async with self.coordinator.claim_device(): - await self.coordinator.client.set(PATH_RESTORE, RESTORE_FACTORY_DEFAULTS_VALUE) - except SSCLocalModeError as err: - raise HomeAssistantError( - translation_domain=DOMAIN, translation_key="local_mode" - ) from err - except SSCDeviceError as err: - raise HomeAssistantError( - translation_domain=DOMAIN, - translation_key="factory_reset_rejected", - translation_placeholders={"error": str(err)}, - ) from err - except (SSCConnectionError, SSCTimeoutError) as err: - # The command may already have reached the speaker: a reset - # reboots it, so a missing reply is what success looks like - # from here too. Reading that as "nothing happened" kept every - # value from before a reset that did in fact run - forever, - # for values that are only ever written once. - await self.coordinator.async_invalidate_and_refresh() - raise HomeAssistantError( - translation_domain=DOMAIN, - translation_key="device_unreachable", - translation_placeholders={"error": str(err)}, - ) from err + # A lost reply drops the cache too: a reset reboots the speaker, + # so a missing reply is what success looks like from here as well. + # Reading that as "nothing happened" kept every value from before + # a reset that did in fact run - forever, for values written once. + async with ( + device_errors( + rejected_key="factory_reset_rejected", + on_unreachable=self.coordinator.async_invalidate_and_refresh, + ), + self.coordinator.claim_device(), + ): + await self.coordinator.client.set(PATH_RESTORE, RESTORE_FACTORY_DEFAULTS_VALUE) # A reset rewrites everything at once, so every cached value is # stale - including the slow-polled ones, which would otherwise # keep showing pre-reset settings for up to five minutes. diff --git a/custom_components/neumann_kh/config_flow.py b/custom_components/neumann_kh/config_flow.py index 1453b75..fbdf560 100644 --- a/custom_components/neumann_kh/config_flow.py +++ b/custom_components/neumann_kh/config_flow.py @@ -93,8 +93,8 @@ async def _async_get_interface_options(hass: HomeAssistant) -> list[selector.Sel name = adapter.get("name") if not name: continue - addresses = [ip["address"] for ip in adapter.get("ipv6", []) if ip.get("address")] - addresses += [ip["address"] for ip in adapter.get("ipv4", []) if ip.get("address")] + entries = [*adapter.get("ipv6", []), *adapter.get("ipv4", [])] + addresses = [entry["address"] for entry in entries if entry.get("address")] label = f"{name} ({', '.join(addresses)})" if addresses else name options.append(selector.SelectOptionDict(value=name, label=label)) @@ -595,38 +595,26 @@ async def async_step_zeroconf_confirm( self, user_input: dict[str, Any] | None = None ) -> ConfigFlowResult: """Name a speaker that mDNS turned up and that is not configured yet.""" - errors: dict[str, str] = {} entry_data = self._pending_entry identity = self._pending_identity if entry_data is None or identity is None: return self.async_abort(reason="discovery_expired") - if user_input is not None: - name = user_input.get(CONF_NAME, "").strip() - if not name: - errors["base"] = "name_required" - else: - entry_data = {**entry_data, CONF_NAME: name} - if identity.serial: - await storage.async_remember_name(self.hass, identity.serial, name) - return await self._async_finish(entry_data, identity) - - suggested = None - if identity.serial: - suggested = await storage.async_get_remembered_name(self.hass, identity.serial) - - schema = vol.Schema({vol.Required(CONF_NAME): str}) - if suggested: - schema = self.add_suggested_values_to_schema(schema, {CONF_NAME: suggested}) + name = (user_input or {}).get(CONF_NAME, "").strip() + if not name: + # First visit, or the name was left empty. + return self.async_show_form( + step_id="zeroconf_confirm", + data_schema=await self._name_schema(identity), + errors={"base": "name_required"} if user_input is not None else {}, + description_placeholders={ + "device": f"{identity.product or FALLBACK_MODEL} – {entry_data[CONF_HOST]}" + }, + ) - return self.async_show_form( - step_id="zeroconf_confirm", - data_schema=schema, - errors=errors, - description_placeholders={ - "device": f"{identity.product or FALLBACK_MODEL} – {entry_data[CONF_HOST]}" - }, - ) + if identity.serial: + await storage.async_remember_name(self.hass, identity.serial, name) + return await self._async_finish({**entry_data, CONF_NAME: name}, identity) # --- Path 2: Active mDNS scan, then naming ------------------------------ @@ -672,21 +660,23 @@ async def async_step_scan(self, user_input: dict[str, Any] | None = None) -> Con return self.async_show_form(step_id="scan", data_schema=self._build_scan_schema()) + async def _name_schema(self, identity: DeviceIdentity) -> vol.Schema: + """A name field, pre-filled if this speaker carried a name before.""" + schema = vol.Schema({vol.Required(CONF_NAME): str}) + if not identity.serial: + return schema + remembered_name = await storage.async_get_remembered_name(self.hass, identity.serial) + if not remembered_name: + return schema + return self.add_suggested_values_to_schema(schema, {CONF_NAME: remembered_name}) + async def _show_scan_confirm_form( self, identity: DeviceIdentity, candidate: DiscoveredSpeaker, error: str | None ) -> ConfigFlowResult: """Ask for a name, pre-filled if this speaker carried one before.""" - remembered_name = None - if identity.serial: - remembered_name = await storage.async_get_remembered_name(self.hass, identity.serial) - - schema = vol.Schema({vol.Required(CONF_NAME): str}) - if remembered_name: - schema = self.add_suggested_values_to_schema(schema, {CONF_NAME: remembered_name}) - return self.async_show_form( step_id="scan_confirm", - data_schema=schema, + data_schema=await self._name_schema(identity), errors={"base": error} if error else {}, description_placeholders={ "device": f"{identity.product or FALLBACK_MODEL} – {candidate.host}" diff --git a/custom_components/neumann_kh/coordinator.py b/custom_components/neumann_kh/coordinator.py index 364fcf4..7959e7b 100644 --- a/custom_components/neumann_kh/coordinator.py +++ b/custom_components/neumann_kh/coordinator.py @@ -42,7 +42,7 @@ SUBWOOFER_SLOW_POLL_PATHS, UPDATE_INTERVAL_SECONDS, ) -from .eq_containers import eq_containers_for_model +from .eq_containers import EQ_ENABLED, eq_containers_for_model from .ssc_client import ( ERROR_NOT_FOUND, SSCClient, @@ -147,7 +147,7 @@ def __init__( # Change only through user action (which applies the confirmed value # immediately) - therefore in the slow poll. for container in eq_containers_for_model(model): - self._slow_poll_paths.append(container.path + ("enabled",)) + self._slow_poll_paths.append(container.leaf(EQ_ENABLED)) # Counts the poll cycles to include the slow paths only every Nth round. # Starts at 0, so that the slow paths are queried right on the very first # cycle (values available immediately). @@ -274,10 +274,6 @@ async def _poll_all_paths(self, paths: list[tuple[str, ...]]) -> dict[str, Any]: answered = False try: for path in paths: - # Priority path: briefly yield to a waiting user action. - if self.client.priority_waiting.is_set(): - await asyncio.sleep(0.05) - try: value = await self.client.get(path) except SSCDeviceError: @@ -403,8 +399,8 @@ async def async_persist(self) -> None: return try: await self.client.set(PATH_SAVE_SETTINGS, True) - except SSCDeviceError as err: - if err.code == ERROR_NOT_FOUND: + except (SSCDeviceError, SSCConnectionError, SSCTimeoutError) as err: + if isinstance(err, SSCDeviceError) and err.code == ERROR_NOT_FOUND: # The KH 80 need is reported, not measured: a firmware without # the path has nothing to save, and must not fail every change. _LOGGER.debug("%s has no save command - nothing to save", self.name) @@ -414,12 +410,6 @@ async def async_persist(self) -> None: translation_key="settings_not_saved", translation_placeholders={"error": str(err)}, ) from err - except (SSCConnectionError, SSCTimeoutError) as err: - raise HomeAssistantError( - translation_domain=DOMAIN, - translation_key="settings_not_saved", - translation_placeholders={"error": str(err)}, - ) from err def value(self, path: tuple[str, ...]) -> Any: """Convenient access to a value from the last polled data.""" diff --git a/custom_components/neumann_kh/discovery.py b/custom_components/neumann_kh/discovery.py index 57a2763..d60ad81 100644 --- a/custom_components/neumann_kh/discovery.py +++ b/custom_components/neumann_kh/discovery.py @@ -21,6 +21,7 @@ from zeroconf.asyncio import AsyncServiceBrowser, AsyncServiceInfo from .const import SCAN_DURATION_SECONDS, SSC_ZEROCONF_SERVICE_TYPE +from .identity import as_identity_text _LOGGER = logging.getLogger(__name__) @@ -40,6 +41,7 @@ class DiscoveredSpeaker: mdns_name: str # full mDNS service name, e.g. "Right._ssc._tcp.local." host: str # IP address; for link-local incl. "%" port: int + serial: str | None = None # TXT "id" of the announcement, if it carries one def pick_host(addresses: list[str]) -> str | None: @@ -89,7 +91,7 @@ async def async_scan_for_speakers( failed retry - for minutes. What is dropped is logged rather than silently left out. """ - aiozc = await ha_zeroconf.async_get_async_instance(hass) + zeroconf_instance = await ha_zeroconf.async_get_async_instance(hass) found_names: set[str] = set() def _on_change( # skipcq: PYL-W0613 @@ -106,7 +108,9 @@ def _on_change( # skipcq: PYL-W0613 if state_change is ServiceStateChange.Added: found_names.add(name) - browser = AsyncServiceBrowser(aiozc.zeroconf, SSC_ZEROCONF_SERVICE_TYPE, handlers=[_on_change]) + browser = AsyncServiceBrowser( + zeroconf_instance.zeroconf, SSC_ZEROCONF_SERVICE_TYPE, handlers=[_on_change] + ) try: await asyncio.sleep(duration) finally: @@ -125,7 +129,7 @@ def _on_change( # skipcq: PYL-W0613 info = AsyncServiceInfo(SSC_ZEROCONF_SERVICE_TYPE, mdns_name) try: - resolved = await info.async_request(aiozc.zeroconf, _RESOLVE_TIMEOUT_MS) + resolved = await info.async_request(zeroconf_instance.zeroconf, _RESOLVE_TIMEOUT_MS) except Exception: _LOGGER.debug("Could not resolve announced SSC service %d", index, exc_info=True) continue @@ -138,6 +142,10 @@ def _on_change( # skipcq: PYL-W0613 _LOGGER.debug("Announced SSC service %d has no IPv6 address", index) continue - speakers.append(DiscoveredSpeaker(mdns_name=mdns_name, host=host, port=info.port)) + # Cleaned as the zeroconf step cleans it: a blank id announces nothing. + serial = as_identity_text(info.decoded_properties.get("id")) + speakers.append( + DiscoveredSpeaker(mdns_name=mdns_name, host=host, port=info.port, serial=serial) + ) return speakers diff --git a/custom_components/neumann_kh/entity.py b/custom_components/neumann_kh/entity.py index 343f057..2b55d1c 100644 --- a/custom_components/neumann_kh/entity.py +++ b/custom_components/neumann_kh/entity.py @@ -8,6 +8,8 @@ from __future__ import annotations +import contextlib +from collections.abc import AsyncIterator, Awaitable, Callable from typing import Any from homeassistant.config_entries import ConfigEntry @@ -29,6 +31,37 @@ from .ssc_client import SSCConnectionError, SSCDeviceError, SSCLocalModeError, SSCTimeoutError +@contextlib.asynccontextmanager +async def device_errors( + rejected_key: str, on_unreachable: Callable[[], Awaitable[None]] | None = None +) -> AsyncIterator[None]: + """Turn the SSC errors of a write into the messages the user sees. + + One owner for the order: SSCLocalModeError is an SSCDeviceError, so it has + to be caught first - every writer had to be fixed by hand when LOCAL came. + on_unreachable runs before the error is raised: the request may have + reached the speaker before the line dropped. + """ + try: + yield + except SSCLocalModeError as err: + raise HomeAssistantError(translation_domain=DOMAIN, translation_key="local_mode") from err + except SSCDeviceError as err: + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key=rejected_key, + translation_placeholders={"error": str(err)}, + ) from err + except (SSCConnectionError, SSCTimeoutError) as err: + if on_unreachable is not None: + await on_unreachable() + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="device_unreachable", + translation_placeholders={"error": str(err)}, + ) from err + + def unique_id_base(entry: ConfigEntry) -> str: """Prefix of every unique ID of this entry's entities.""" return str(entry.data.get(CONF_SERIAL) or entry.entry_id) @@ -72,25 +105,12 @@ async def _async_write( path is re-read instead: a missing echo or a line dropped after sending does not mean the device kept its old value. """ - try: - confirmed = await self.coordinator.client.set(path, value) - except SSCLocalModeError as err: - raise HomeAssistantError( - translation_domain=DOMAIN, translation_key="local_mode" - ) from err - except SSCDeviceError as err: - raise HomeAssistantError( - translation_domain=DOMAIN, - translation_key=rejected_key, - translation_placeholders={"error": str(err)}, - ) from err - except (SSCConnectionError, SSCTimeoutError) as err: + + async def _reread() -> None: self.coordinator.request_reread(path) - raise HomeAssistantError( - translation_domain=DOMAIN, - translation_key="device_unreachable", - translation_placeholders={"error": str(err)}, - ) from err + + async with device_errors(rejected_key=rejected_key, on_unreachable=_reread): + confirmed = await self.coordinator.client.set(path, value) if confirmed is None: self.coordinator.request_reread(path) else: diff --git a/custom_components/neumann_kh/eq.py b/custom_components/neumann_kh/eq.py index 8ab7eb3..dfb71c6 100644 --- a/custom_components/neumann_kh/eq.py +++ b/custom_components/neumann_kh/eq.py @@ -20,13 +20,10 @@ from homeassistant.components.switch import SwitchEntity, SwitchEntityDescription from homeassistant.config_entries import ConfigEntry from homeassistant.const import EntityCategory -from homeassistant.exceptions import HomeAssistantError -from .const import DOMAIN from .coordinator import NeumannKHCoordinator -from .entity import NeumannKHEntity -from .eq_containers import EQContainer, eq_containers_for_model -from .ssc_client import SSCConnectionError, SSCDeviceError, SSCLocalModeError, SSCTimeoutError +from .entity import NeumannKHEntity, device_errors +from .eq_containers import EQ_BOOST, EQ_ENABLED, EQ_GAIN, EQContainer, eq_containers_for_model # --- Container on/off switch ------------------------------------------------- @@ -37,7 +34,7 @@ def _reported_band_count(coordinator: NeumannKHCoordinator, container: EQContain band_count is measured on the KH 120 II and the KH 750 only; for any other model the device's own array is the better source. """ - enabled = coordinator.value(container.path + ("enabled",)) + enabled = coordinator.value(container.leaf(EQ_ENABLED)) if isinstance(enabled, list) and enabled: return len(enabled) return container.band_count @@ -77,14 +74,14 @@ def __init__( self._attr_translation_placeholders = { "container": container.label_for(coordinator.hass.config.language) } - self._path = container.path + ("enabled",) + self._path = container.leaf(EQ_ENABLED) @property def is_on(self) -> bool | None: values = self.coordinator.value(self._path) if not isinstance(values, list) or not values: return None - return any(bool(v) for v in values) + return any(bool(value) for value in values) async def _async_set(self, value: bool) -> None: array = [value] * _reported_band_count(self.coordinator, self._container) @@ -136,25 +133,9 @@ async def async_press(self) -> None: # lock: a backup reading while this writes mixes the old curve with # the new one and then replaces the last good snapshot. async with self.coordinator.claim_device(): - try: - await self.coordinator.client.set(self._container.path + ("gain",), zero) - await self.coordinator.client.set(self._container.path + ("boost",), zero) - except SSCLocalModeError as err: - raise HomeAssistantError( - translation_domain=DOMAIN, translation_key="local_mode" - ) from err - except SSCDeviceError as err: - raise HomeAssistantError( - translation_domain=DOMAIN, - translation_key="eq_reset_rejected", - translation_placeholders={"error": str(err)}, - ) from err - except (SSCConnectionError, SSCTimeoutError) as err: - raise HomeAssistantError( - translation_domain=DOMAIN, - translation_key="device_unreachable", - translation_placeholders={"error": str(err)}, - ) from err + async with device_errors(rejected_key="eq_reset_rejected"): + await self.coordinator.client.set(self._container.leaf(EQ_GAIN), zero) + await self.coordinator.client.set(self._container.leaf(EQ_BOOST), zero) await self.coordinator.async_persist() # No refresh afterwards: gain and boost are never polled (only the # container's "enabled" array is), so there is no cached value a diff --git a/custom_components/neumann_kh/eq_containers.py b/custom_components/neumann_kh/eq_containers.py index aabd436..7327aab 100644 --- a/custom_components/neumann_kh/eq_containers.py +++ b/custom_components/neumann_kh/eq_containers.py @@ -27,6 +27,10 @@ class EQContainer: label_en: str label_de: str + def leaf(self, name: str) -> tuple[str, ...]: + """Path of one leaf of this container (one of EQ_LEAVES).""" + return self.path + (name,) + def label_for(self, language: str | None) -> str: """Return the display label for the given HA language (de/en, fallback en).""" return localized(language, self.label_de, self.label_en) @@ -96,7 +100,10 @@ def label_for(self, language: str | None) -> str: # Leaves every EQ container exposes, each an array with one entry per band. # Verified by reading them off a KH 120 II (firmware 1_7_3); "bypass" and # "order" do not exist and answer 404. -EQ_LEAVES = ("enabled", "gain", "boost", "frequency", "q", "type") +EQ_ENABLED = "enabled" +EQ_GAIN = "gain" +EQ_BOOST = "boost" +EQ_LEAVES = (EQ_ENABLED, EQ_GAIN, EQ_BOOST, "frequency", "q", "type") def eq_leaf_paths(model: str | None) -> tuple[tuple[str, ...], ...]: @@ -107,9 +114,7 @@ def eq_leaf_paths(model: str | None) -> tuple[tuple[str, ...], ...]: them a restored EQ would not be the one that was saved. """ return tuple( - container.path + (leaf,) - for container in eq_containers_for_model(model) - for leaf in EQ_LEAVES + container.leaf(leaf) for container in eq_containers_for_model(model) for leaf in EQ_LEAVES ) diff --git a/custom_components/neumann_kh/export_actions.py b/custom_components/neumann_kh/export_actions.py index a7bce35..6620df7 100644 --- a/custom_components/neumann_kh/export_actions.py +++ b/custom_components/neumann_kh/export_actions.py @@ -23,7 +23,6 @@ ) from .const import ( CONF_MODEL, - CONF_SERIAL, CONTROL_MODE_LOCAL, DEVICE_ACTION_TIMEOUT_SECONDS, DOMAIN, @@ -32,7 +31,9 @@ PATH_OUTPUT_MUTE, PATH_UI_CONTROL_MODE, ) +from .coordinator import NeumannKHCoordinator from .discovery_export import DiscoveryTimeoutError, async_discover_all_values +from .entity import unique_id_base from .export_file import async_write_export from .identity import mask_serial from .ssc_client import ( @@ -47,6 +48,7 @@ KIND_BACKUP = "backup" KIND_DISCOVERY = "discovery" +KIND_RESTORE = "restore" def _notify(hass: HomeAssistant, entry: ConfigEntry, kind: str, title: str, body: str) -> None: @@ -85,7 +87,7 @@ def _notify_written(hass: HomeAssistant, entry: ConfigEntry, kind: str, path: st async def async_run_backup(hass: HomeAssistant, entry: ConfigEntry, client: SSCClient) -> str: """Read all known values, store them and write them out.""" - serial = entry.data.get(CONF_SERIAL) or entry.entry_id + serial = unique_id_base(entry) model = entry.data.get(CONF_MODEL) try: @@ -159,7 +161,7 @@ async def async_run_backup(hass: HomeAssistant, entry: ConfigEntry, client: SSCC async def async_run_discovery(hass: HomeAssistant, entry: ConfigEntry, client: SSCClient) -> str: """Run a full device discovery, store it and write it out.""" - serial = entry.data.get(CONF_SERIAL) or entry.entry_id + serial = unique_id_base(entry) model = entry.data.get(CONF_MODEL) try: @@ -234,7 +236,7 @@ async def async_check_restorable(hass: HomeAssistant, entry: ConfigEntry) -> dic both the model and the (masked) serial have to match before anything is written. """ - serial = entry.data.get(CONF_SERIAL) or entry.entry_id + serial = unique_id_base(entry) backup = await storage.async_get_backup(hass, serial) if not backup or not backup.get("values"): raise HomeAssistantError(translation_domain=DOMAIN, translation_key="restore_no_backup") @@ -301,7 +303,7 @@ def _notify_restored( _notify( hass, entry, - "restore", + KIND_RESTORE, localized( language, "Neumann Connect: Backup zurückgespielt", @@ -336,7 +338,7 @@ def _write_order(paths: list[tuple[str, ...]], values: dict[str, Any]) -> list[t async def async_run_restore( hass: HomeAssistant, entry: ConfigEntry, - coordinator: Any, + coordinator: NeumannKHCoordinator, backup: dict[str, Any], ) -> tuple[int, int, int]: """Write a stored backup back to the device. diff --git a/custom_components/neumann_kh/export_file.py b/custom_components/neumann_kh/export_file.py index d85212a..59bc72f 100644 --- a/custom_components/neumann_kh/export_file.py +++ b/custom_components/neumann_kh/export_file.py @@ -42,16 +42,16 @@ def _write(hass: HomeAssistant, filename: str, payload: dict[str, Any]) -> str: directory = hass.config.path(EXPORT_DIR_NAME) os.makedirs(directory, exist_ok=True) path = os.path.join(directory, filename) - tmp_path = f"{path}.tmp" + temporary_path = f"{path}.tmp" try: - with open(tmp_path, "w", encoding="utf-8") as file: + with open(temporary_path, "w", encoding="utf-8") as file: json.dump(payload, file, indent=2, ensure_ascii=False) file.flush() os.fsync(file.fileno()) - os.replace(tmp_path, path) + os.replace(temporary_path, path) except BaseException: with contextlib.suppress(OSError): - os.remove(tmp_path) + os.remove(temporary_path) raise return path diff --git a/custom_components/neumann_kh/identity.py b/custom_components/neumann_kh/identity.py index 89a27cf..5199b5a 100644 --- a/custom_components/neumann_kh/identity.py +++ b/custom_components/neumann_kh/identity.py @@ -27,6 +27,10 @@ def as_identity_text(value: Any) -> str | None: return text or None +# Enough to tell a pair of speakers apart in a support thread. +_VISIBLE_SERIAL_CHARACTERS = 3 + + def mask_serial(serial: Any) -> str: """Mask a serial number, leaving only the last 3 characters visible. @@ -36,9 +40,9 @@ def mask_serial(serial: Any) -> str: text = as_identity_text(serial) if text is None: return "no serial" - if len(text) <= 3: + if len(text) <= _VISIBLE_SERIAL_CHARACTERS: return text - return "x" * (len(text) - 3) + text[-3:] + return "x" * (len(text) - _VISIBLE_SERIAL_CHARACTERS) + text[-_VISIBLE_SERIAL_CHARACTERS:] def serial_matches(expected: Any, answered: Any) -> bool: diff --git a/custom_components/neumann_kh/number.py b/custom_components/neumann_kh/number.py index f4c3337..6d314e8 100644 --- a/custom_components/neumann_kh/number.py +++ b/custom_components/neumann_kh/number.py @@ -10,11 +10,13 @@ from typing import Any from homeassistant.components.number import ( + NumberDeviceClass, NumberEntity, NumberEntityDescription, NumberMode, ) from homeassistant.config_entries import ConfigEntry +from homeassistant.const import UnitOfTime from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.entity_platform import AddEntitiesCallback @@ -84,7 +86,8 @@ class NeumannKHNumberDescription(NumberEntityDescription): native_min_value=STANDBY_AUTO_TIME_MIN, native_max_value=STANDBY_AUTO_TIME_MAX, native_step=1, - native_unit_of_measurement="min", + device_class=NumberDeviceClass.DURATION, + native_unit_of_measurement=UnitOfTime.MINUTES, mode=NumberMode.BOX, ssc_path=PATH_STANDBY_AUTO_TIME, integer=True, diff --git a/custom_components/neumann_kh/select.py b/custom_components/neumann_kh/select.py index 7d87e96..1346ae4 100644 --- a/custom_components/neumann_kh/select.py +++ b/custom_components/neumann_kh/select.py @@ -30,7 +30,6 @@ class NeumannKHSelectDescription(SelectEntityDescription): translation_key="input_interface", icon="mdi:audio-input-stereo-minijack", options=list(INPUT_INTERFACE_OPTIONS), - entity_registry_enabled_default=True, ssc_path=PATH_INPUT_INTERFACE_TYPE, ) diff --git a/custom_components/neumann_kh/sensor.py b/custom_components/neumann_kh/sensor.py index 8b3f462..82c25a7 100644 --- a/custom_components/neumann_kh/sensor.py +++ b/custom_components/neumann_kh/sensor.py @@ -17,7 +17,7 @@ SensorStateClass, ) from homeassistant.config_entries import ConfigEntry -from homeassistant.const import EntityCategory, UnitOfTemperature +from homeassistant.const import EntityCategory, UnitOfTemperature, UnitOfTime from homeassistant.core import HomeAssistant from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.typing import StateType @@ -103,7 +103,8 @@ class NeumannKHSensorDescription(SensorEntityDescription): key="standby_countdown", translation_key="standby_countdown", icon="mdi:timer-sand", - native_unit_of_measurement="min", + device_class=SensorDeviceClass.DURATION, + native_unit_of_measurement=UnitOfTime.MINUTES, state_class=SensorStateClass.MEASUREMENT, entity_category=EntityCategory.DIAGNOSTIC, entity_registry_enabled_default=False, diff --git a/custom_components/neumann_kh/ssc_client.py b/custom_components/neumann_kh/ssc_client.py index 73af396..597d66d 100644 --- a/custom_components/neumann_kh/ssc_client.py +++ b/custom_components/neumann_kh/ssc_client.py @@ -25,6 +25,9 @@ _LOGGER = logging.getLogger(__name__) +_VISIBLE_HOST_CHARACTERS = 2 + + def mask_host(host: str) -> str: """Mask a host, leaving the network prefix and the last two characters. @@ -38,9 +41,10 @@ def mask_host(host: str) -> str: """ address, separator, scope = host.partition("%") prefix, delimiter, identifier = address.rpartition("::") - if len(identifier) <= 2: + if len(identifier) <= _VISIBLE_HOST_CHARACTERS: return host - return f"{prefix}{delimiter}xx{identifier[-2:]}{separator}{scope}" + visible = identifier[-_VISIBLE_HOST_CHARACTERS:] + return f"{prefix}{delimiter}xx{visible}{separator}{scope}" def _os_reason(err: OSError) -> str: @@ -60,6 +64,8 @@ def _os_reason(err: OSError) -> str: # Terminator per the SSC specification: CR+LF or LF alone are allowed. _MESSAGE_TERMINATOR = b"\n" +# What the devices send and expect; reading stops at the "\n" alone. +_SEND_TERMINATOR = b"\r\n" # Guard against excessively large/never-terminated responses: set as an explicit # StreamReader limit (readuntil raises LimitOverrunError above it, see @@ -243,9 +249,6 @@ def __init__( # reopen the connection of an entry that is already unloaded. self._closed = False self._lock = asyncio.Lock() - # Tells the poll loop a user action is waiting (see request(priority= - # True)); the poll then pauses before its next path. - self._priority_waiting = asyncio.Event() self._xids = itertools.count(1) # Whether the last answer on this connection was matched by path only. # Only then can a trailing line of it pass for the next answer, so @@ -254,11 +257,6 @@ def __init__( # Logical path -> path on this device (see use_path_aliases). self._path_aliases: dict[tuple[str, ...], tuple[str, ...]] = {} - @property - def priority_waiting(self) -> asyncio.Event: - """Event indicating that a user action is waiting on the lock.""" - return self._priority_waiting - @property def _connect_host(self) -> str: """Appends the scope ID for link-local addresses (fe80::/10, RFC 4291).""" @@ -331,7 +329,7 @@ async def _send_raw(self, payload: dict[str, Any]) -> None: # json.dumps writes NaN/Infinity by default, which is not JSON. The # number entity refuses them first; this catches every other # writer, a restore of a hand-edited backup included. - message = json.dumps(payload, allow_nan=False).encode("utf-8") + b"\r\n" + message = json.dumps(payload, allow_nan=False).encode("utf-8") + _SEND_TERMINATOR except ValueError as err: raise SSCDeviceError( f"Refused to send a non-finite number to {mask_host(self._host)}" @@ -521,28 +519,15 @@ async def _read_lines_until_settled( async def request( self, payload: dict[str, Any], - priority: bool = False, expect_path: tuple[str, ...] | None = None, ) -> dict[str, Any]: """Sends an SSC message and returns the merged result dict. - priority=True marks a user action. The poll takes the lock per path and - asyncio.Lock is FIFO, so the action waits one query at most either way; - the flag only makes the poll pause briefly before its next path. + The poll takes the lock per path and asyncio.Lock hands it over in FIFO + order, so a user action waits for one query of a running cycle at most. """ - if priority: - self._priority_waiting.set() - try: - async with self._lock: - if priority: - self._priority_waiting.clear() - return await self._locked_request(payload, expect_path) - finally: - # Cancellation while waiting for the lock would otherwise leave the - # event set forever, and the poll loop sleeps before every single - # path while it is (see coordinator._poll_all_paths). - if priority: - self._priority_waiting.clear() + async with self._lock: + return await self._locked_request(payload, expect_path) async def _locked_request( self, payload: dict[str, Any], expect_path: tuple[str, ...] | None @@ -609,12 +594,7 @@ async def get(self, path: tuple[str, ...]) -> Any: return extract(response, device_path) async def set(self, path: tuple[str, ...], value: Any) -> Any: - """Sets a value and returns the value confirmed by the device. - - Every write comes from a user action, so it always jumps the poll. - """ + """Sets a value and returns the value confirmed by the device.""" device_path = self._path_aliases.get(path, path) - response = await self.request( - build_nested(device_path, value), priority=True, expect_path=device_path - ) + response = await self.request(build_nested(device_path, value), expect_path=device_path) return extract(response, device_path) diff --git a/custom_components/neumann_kh/switch.py b/custom_components/neumann_kh/switch.py index 5ce70f2..51d55f2 100644 --- a/custom_components/neumann_kh/switch.py +++ b/custom_components/neumann_kh/switch.py @@ -133,12 +133,8 @@ def is_on(self) -> bool | None: return None return bool(value) - async def _async_set(self, value: bool) -> None: - """Sets the value; turns a device rejection into a clear HA error message.""" - await self._async_write(self.entity_description.ssc_path, value) - async def async_turn_on(self, **kwargs: Any) -> None: - await self._async_set(True) + await self._async_write(self.entity_description.ssc_path, True) async def async_turn_off(self, **kwargs: Any) -> None: - await self._async_set(False) + await self._async_write(self.entity_description.ssc_path, False) diff --git a/tests/README.md b/tests/README.md index 5ade7dd..9620387 100644 --- a/tests/README.md +++ b/tests/README.md @@ -76,7 +76,7 @@ the thing it prevents actually happened here. | `test_mutation_harness.py` | The mutation run fails loudly instead of reporting a breakage it never applied | | `test_resolve_phcc.py` | The latest-HA job picks the newest plugin release that pins a final Home Assistant, never a beta | | `test_sensor_states.py` | The not_assigned mapping and its translations stay in step | -| `test_translations.py` | strings.json, en.json and de.json carry the same keys | +| `test_translations.py` | strings.json, en.json and de.json carry the same keys, and every error key the code hands out has a text | | `test_guards.py` | Every guard is listed, scans a real directory, and is not pinned to one file | ## When a budget turns red diff --git a/tests/test_budgets.py b/tests/test_budgets.py index 3680854..3410abe 100644 --- a/tests/test_budgets.py +++ b/tests/test_budgets.py @@ -44,7 +44,8 @@ # Raised to 559 to skip the stale drain on a connection opened just now. # Raised to 608 for matching replies by osc/xid (token limit, late lines). # Raised to 620 so a split xid answer waits for its value (re-audit R1). - "ssc_client.py": 620, + # Raised to 626 for naming the visible host characters and the terminator. + "ssc_client.py": 626, } # SonarSource's default. Everything above needs a written entry. diff --git a/tests/test_ci_parity.py b/tests/test_ci_parity.py index f50d482..0f2d7ef 100644 --- a/tests/test_ci_parity.py +++ b/tests/test_ci_parity.py @@ -16,6 +16,7 @@ import re import subprocess +import pytest import yaml REPO = pathlib.Path(__file__).resolve().parents[1] @@ -23,7 +24,7 @@ CHECK_SCRIPT = REPO / ".github" / "scripts" / "check.sh" # Tools whose invocation belongs to check.sh alone. -OWNED_BY_THE_SCRIPT = ("ruff", "pytest", "mutate.py") +OWNED_BY_THE_SCRIPT = ("ruff", "pytest", "mypy", "compileall", "check_gitleaks_pin", "mutate.py") def _run_commands(workflow_text: str) -> list[str]: @@ -53,14 +54,18 @@ def test_the_workflow_calls_the_check_script(): ) -def test_the_workflow_does_not_invoke_the_tools_itself(): - offenders = [ +def _offenders(commands: list[str]) -> list[str]: + return [ step - for step in _steps() + for step in commands if "check.sh" not in step and any(re.search(rf"\b{tool}\b", step) for tool in OWNED_BY_THE_SCRIPT) ] + +def test_the_workflow_does_not_invoke_the_tools_itself(): + offenders = _offenders(_steps()) + assert not offenders, ( f"CI step(s) running a tool directly: {offenders}. Put the gate in " "check.sh instead, or the two lists drift and only one of them is the " @@ -142,6 +147,23 @@ def test_the_scripts_are_committed_executable(): ) +@pytest.mark.parametrize( + "command", + [ + "python -m mypy", + "python -m compileall -q custom_components", + "python .github/scripts/check_gitleaks_pin.py", + ], +) +def test_every_gate_check_sh_owns_counts_as_a_direct_call(command): + """mypy, compileall and the gitleaks pin are gates too. + + Called from a workflow step they passed the guard, against the decision + that CI runs check.sh and nothing else. + """ + assert _offenders([command]) == [command] + + def test_the_scan_reads_block_style_steps_too(): """Proof-of-red: the shape a line scan for "- run:" walks straight past.""" block_style = """ diff --git a/tests/test_config_flow.py b/tests/test_config_flow.py index 6f6a42d..6e519b7 100644 --- a/tests/test_config_flow.py +++ b/tests/test_config_flow.py @@ -918,3 +918,37 @@ async def test_reconfigure_takes_the_scope_id_out_of_the_host(hass, _custom_inte assert result["reason"] == "reconfigure_successful" assert entry.data[CONF_HOST] == "fe80::9" assert entry.data[CONF_INTERFACE] == "eth5" + + +async def test_the_discovery_card_asks_again_for_an_empty_name(hass, _custom_integration): + """Guards the flattened step: an empty name shows the form with an error.""" + card = await _run_zeroconf( + hass, _zeroconf_info(serial=_GOOD_IDENTITY.serial), identity=_GOOD_IDENTITY + ) + + result = await hass.config_entries.flow.async_configure(card["flow_id"], {CONF_NAME: " "}) + + assert result["type"] is FlowResultType.FORM + assert result["step_id"] == "zeroconf_confirm" + assert result["errors"] == {"base": "name_required"} + + +async def test_the_name_field_offers_the_name_the_speaker_had_before(hass, _custom_integration): + """Set up again after removing it: the name field is pre-filled. + + Both naming steps share the field now, so one unguarded line would lose the + suggestion for the discovery card and the scan alike. + """ + from custom_components.neumann_kh import storage + + await storage.async_remember_name(hass, _GOOD_IDENTITY.serial, "Left") + + card = await _run_zeroconf( + hass, _zeroconf_info(serial=_GOOD_IDENTITY.serial), identity=_GOOD_IDENTITY + ) + + suggested = { + str(key): (key.description or {}).get("suggested_value") + for key in card["data_schema"].schema + } + assert suggested[CONF_NAME] == "Left" diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index 5f73f7d..5cb7fb3 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -30,7 +30,6 @@ def __init__(self, values: dict[tuple[str, ...], Any]) -> None: self.values = values self.fail_connection = False self.rejected_paths: set[tuple[str, ...]] = set() - self.priority_waiting = asyncio.Event() self.asked: list[tuple[str, ...]] = [] async def get(self, path: tuple[str, ...]) -> Any: diff --git a/tests/test_discovery.py b/tests/test_discovery.py index 1f10994..71bf0d7 100644 --- a/tests/test_discovery.py +++ b/tests/test_discovery.py @@ -71,6 +71,7 @@ class _FakeInfo: def __init__(self, service_type, name): self.name = name + self.decoded_properties: dict[str, str | None] = {} async def async_request(self, zeroconf, timeout_ms): resolved.append(self.name) @@ -105,3 +106,58 @@ async def async_cancel(self): # skipcq: PYL-R0201 - a stand-in, not a design assert len(resolved) == expected, "the resolution pass was not bounded" assert len(speakers) == expected + + +@pytest.mark.parametrize( + ("properties", "expected"), + [ + ({"id": "SIM0001234"}, "SIM0001234"), + ({"id": " SIM0001234 "}, "SIM0001234"), + ({"id": None}, None), + ({"id": " "}, None), + ({}, None), + ], + ids=["serial", "padded", "none", "blank", "missing"], +) +async def test_the_announced_serial_reaches_the_candidate(properties, expected): + """Relocate takes a candidate by the serial its TXT record announces. + + Read from the wrong key, or left out, it silently fell back to a TCP probe + per speaker - and a blank id kept the speaker from being probed at all. + """ + from unittest.mock import AsyncMock, patch + + from custom_components.neumann_kh import discovery + + class _FakeInfo: + port = 45 + + def __init__(self, service_type, name): + self.decoded_properties = properties + + async def async_request(self, zeroconf, timeout_ms): # skipcq: PYL-R0201 + return True + + def parsed_scoped_addresses(self): # skipcq: PYL-R0201 - a stand-in, not a design + return ["fe80::1%2"] + + class _FakeBrowser: + def __init__(self, *args, **kwargs): + (kwargs.get("handlers") or args[2])[0]( + zeroconf=None, + service_type="_ssc._tcp.local.", + name="Right._ssc._tcp.local.", + state_change=discovery.ServiceStateChange.Added, + ) + + async def async_cancel(self): # skipcq: PYL-R0201 - a stand-in, not a design + return None + + with ( + patch.object(discovery, "AsyncServiceBrowser", _FakeBrowser), + patch.object(discovery, "AsyncServiceInfo", _FakeInfo), + patch.object(discovery.ha_zeroconf, "async_get_async_instance", AsyncMock()), + ): + speakers = await discovery.async_scan_for_speakers(AsyncMock(), duration=0) + + assert [speaker.serial for speaker in speakers] == [expected] diff --git a/tests/test_discovery_export.py b/tests/test_discovery_export.py index 2baa0a0..a9f0352 100644 --- a/tests/test_discovery_export.py +++ b/tests/test_discovery_export.py @@ -186,3 +186,21 @@ async def test_the_depth_budget_counts_levels_descended(monkeypatch): await _async_discover_via_schema(client) assert client.limits_asked == [], client.limits_asked + + +async def test_a_connection_lost_mid_walk_keeps_what_was_already_collected(): + """Sibling of the timeout: the walk is best-effort either way.""" + from custom_components.neumann_kh.ssc_client import SSCConnectionError + + class _DropsOnTheSecondLeaf(_SchemaClient): + def __init__(self) -> None: + super().__init__({"first": None, "second": None}) + + async def request(self, payload, expect_path=None): + if "limits" in payload["osc"] and self._path_of(payload) == ("second",): + raise SSCConnectionError("line dropped") + return await super().request(payload, expect_path) + + result = await _async_discover_via_schema(_DropsOnTheSecondLeaf()) + + assert result == {"first": {"type": "Number"}}, result diff --git a/tests/test_entity_writes.py b/tests/test_entity_writes.py index 6f7b40f..5d05f51 100644 --- a/tests/test_entity_writes.py +++ b/tests/test_entity_writes.py @@ -367,3 +367,24 @@ async def _save_fails(): with pytest.raises(HomeAssistantError): await _switch(coordinator).async_turn_on() assert coordinator.confirmed == [(("audio", "out", "mute"), True)] + + +@pytest.mark.parametrize( + ("error", "key"), + [ + (SSCDeviceError("refused here"), "change_rejected"), + (SSCConnectionError("line dropped"), "device_unreachable"), + ], + ids=["rejected", "unreachable"], +) +async def test_a_write_error_tells_the_user_what_the_device_said(error, key): + """The message quotes the device's reason; without it a refusal reads the + same whatever the speaker answered.""" + from custom_components.neumann_kh.entity import device_errors + + with pytest.raises(HomeAssistantError) as err: + async with device_errors(rejected_key="change_rejected"): + raise error + + assert err.value.translation_key == key + assert err.value.translation_placeholders == {"error": str(error)} diff --git a/tests/test_eq_reset.py b/tests/test_eq_reset.py index 31deb7b..bb85b5d 100644 --- a/tests/test_eq_reset.py +++ b/tests/test_eq_reset.py @@ -19,9 +19,10 @@ from custom_components.neumann_kh.coordinator import NeumannKHCoordinator from custom_components.neumann_kh.eq import NeumannKHEQResetButton +from custom_components.neumann_kh.eq_containers import EQContainer from custom_components.neumann_kh.ssc_client import SSCDeviceError, SSCLocalModeError -_CONTAINER = SimpleNamespace(path=("audio", "out", "eq2"), band_count=2) +_CONTAINER = EQContainer(path=("audio", "out", "eq2"), band_count=2, label_en="EQ", label_de="EQ") class _Client: @@ -126,3 +127,15 @@ async def test_a_reset_writes_as_many_bands_as_the_device_reports(): await NeumannKHEQResetButton.async_press(button) assert [len(payload) for payload in button.coordinator.client.payloads] == [3, 3] + + +async def test_a_reset_that_loses_the_speaker_says_so(): + from custom_components.neumann_kh.ssc_client import SSCConnectionError + + button = _button(_Client(reject="boost", error=SSCConnectionError)) + + with pytest.raises(HomeAssistantError) as err: + await NeumannKHEQResetButton.async_press(button) + + assert err.value.translation_key == "device_unreachable" + assert button.coordinator.persisted == 0 diff --git a/tests/test_export_actions.py b/tests/test_export_actions.py index 0cb7142..fb751f7 100644 --- a/tests/test_export_actions.py +++ b/tests/test_export_actions.py @@ -16,7 +16,7 @@ from homeassistant.exceptions import HomeAssistantError -from custom_components.neumann_kh import backup_export, export_actions +from custom_components.neumann_kh import backup_export, discovery_export, export_actions from custom_components.neumann_kh._util import build_nested, deep_merge from custom_components.neumann_kh.backup_export import ( restorable_paths_for_model, @@ -782,3 +782,95 @@ async def test_an_unconfirmed_value_is_logged_and_reported(monkeypatch, caplog): if record.levelname == "INFO" ), "the unconfirmed path was not logged" assert "1 unconfirmed" in notified[0] + + +async def test_a_restore_cut_off_mid_way_keeps_what_landed_and_rereads_the_rest(): + """A lost connection mid-restore: the value in flight may have landed. + + Without the re-read that path showed its pre-restore value until the next + slow cycle; without the count the user got a bare error on a half-written + speaker. + """ + from custom_components.neumann_kh.ssc_client import SSCConnectionError + + class _DropsOnThirdSet(_FakeClient): + async def set(self, path, value): + if len(self.written) == 2: + self.written.append((path, value)) + raise SSCConnectionError("line dropped") + return await super().set(path, value) + + client = _DropsOnThirdSet(answers_none=set()) + coordinator = _FakeCoordinator(client) + + with pytest.raises(HomeAssistantError) as err: + await async_run_restore( + _FakeHass(), _FakeEntry(_KH_120_II), coordinator, _backup_covering(_KH_120_II) + ) + + assert err.value.translation_key == "restore_interrupted" + assert err.value.translation_placeholders["written"] == "2" + assert coordinator.reread == [client.written[-1][0]] + assert coordinator.published == 1, "the two confirmed values were never published" + + +async def test_a_value_the_device_adjusts_is_counted_as_adjusted(): + adjusted_path = restorable_paths_for_model(_KH_120_II)[0] + + class _Clamps(_FakeClient): + async def set(self, path, value): + confirmed = await super().set(path, value) + return confirmed + 1 if path == adjusted_path else confirmed + + paths = restorable_paths_for_model(_KH_120_II) + written, adjusted, skipped = await async_run_restore( + _FakeHass(), + _FakeEntry(_KH_120_II), + _FakeCoordinator(_Clamps(answers_none=set())), + _backup_covering(_KH_120_II), + ) + + assert (written, adjusted, skipped) == (len(paths) - 1, 1, 0) + + +@pytest.mark.parametrize( + ("error", "key"), + [ + (backup_export.BackupTimeoutError(), "backup_timed_out"), + (RuntimeError("anything else"), "backup_failed"), + ], + ids=["timeout", "other"], +) +async def test_a_backup_that_fails_to_read_says_why(monkeypatch, error, key): + saved, written = await _run_backup(None, monkeypatch, values={}) + + async def _fails(client, model): + raise error + + monkeypatch.setattr(export_actions, "async_build_backup", _fails) + hass, entry, client = _fakes() + with pytest.raises(HomeAssistantError) as err: + await export_actions.async_run_backup(hass, entry, client) + + assert err.value.translation_key == key + assert not saved and not written + + +@pytest.mark.parametrize( + ("error", "key"), + [ + (discovery_export.DiscoveryTimeoutError(), "discovery_timed_out"), + (RuntimeError("anything else"), "discovery_failed"), + ], + ids=["timeout", "other"], +) +async def test_a_discovery_that_fails_to_read_says_why(monkeypatch, error, key): + async def _fails(client, model): + raise error + + monkeypatch.setattr(export_actions, "async_discover_all_values", _fails) + hass, entry, client = _fakes() + with pytest.raises(HomeAssistantError) as err: + await export_actions.async_run_discovery(hass, entry, client) + + assert err.value.translation_key == key diff --git a/tests/test_guards.py b/tests/test_guards.py index ae5ecfa..5dc77c2 100644 --- a/tests/test_guards.py +++ b/tests/test_guards.py @@ -34,7 +34,7 @@ "test_resolve_phcc.py": "the latest-HA job never resolves to a beta Home Assistant", "test_sensor_states.py": "the not_assigned mapping and its translations stay in step", "test_log_privacy.py": "no log call or raised message hands out a serial or address in full", - "test_translations.py": "strings.json, en.json and de.json stay in step", + "test_translations.py": "the translation files stay in step and every error key in the code has a text", "test_guards.py": "the guards stay package-wide and stay present", } diff --git a/tests/test_offline_is_not_an_error.py b/tests/test_offline_is_not_an_error.py index 33cbffd..6250444 100644 --- a/tests/test_offline_is_not_an_error.py +++ b/tests/test_offline_is_not_an_error.py @@ -23,9 +23,6 @@ class UnreachableClient: """Answers nothing at all - the speaker is switched off.""" - def __init__(self) -> None: - self.priority_waiting = asyncio.Event() - async def get(self, path: tuple[str, ...]) -> object: raise SSCConnectionError("device offline (test)") @@ -33,9 +30,6 @@ async def get(self, path: tuple[str, ...]) -> object: class SlowClient: """Answers, but too late for the cycle's time limit.""" - def __init__(self) -> None: - self.priority_waiting = asyncio.Event() - async def get(self, path: tuple[str, ...]) -> object: await asyncio.sleep(0.2) return False @@ -44,9 +38,6 @@ async def get(self, path: tuple[str, ...]) -> object: class RejectingClient: """Answers every request - with an error.""" - def __init__(self) -> None: - self.priority_waiting = asyncio.Event() - async def get(self, path: tuple[str, ...]) -> object: raise SSCDeviceError("rejected (test)", code=400) diff --git a/tests/test_relocate.py b/tests/test_relocate.py index dc3c78f..d0a4307 100644 --- a/tests/test_relocate.py +++ b/tests/test_relocate.py @@ -361,3 +361,36 @@ async def close(self): # skipcq: PYL-R0201 - a stand-in, not a design assert await _async_relocate(hass, entry) is False assert peak > 1, "the candidates were queried one after another" + + +async def test_announced_serials_spare_the_connections(hass, _custom_integration): + """The TXT record carries the serial; asking every candidate over TCP cost a + connection per speaker on every failed setup retry. The next setup still + checks the serial against the device, so the announcement is not trusted + blindly. + """ + entry = _entry(hass) + opened: list[str] = [] + + def _client(host, **kwargs): + opened.append(host) + client = AsyncMock() + client.get = AsyncMock(return_value=None) + return client + + speakers = [ + DiscoveredSpeaker( + mdns_name="other._ssc._tcp.local.", host="fe80::9%2", port=45, serial="SIM0007500" + ), + DiscoveredSpeaker( + mdns_name="own._ssc._tcp.local.", host=_NEW_HOST, port=45, serial=_SERIAL + ), + ] + with ( + patch("custom_components.neumann_kh.async_scan_for_speakers", return_value=speakers), + patch("custom_components.neumann_kh.SSCClient", side_effect=_client), + ): + assert await _async_relocate(hass, entry) is True + + assert opened == [], f"connected to {opened} despite the announced serials" + assert entry.data[CONF_HOST] == _NEW_HOST diff --git a/tests/test_sensor.py b/tests/test_sensor.py index 02c84bc..64f88c0 100644 --- a/tests/test_sensor.py +++ b/tests/test_sensor.py @@ -153,3 +153,30 @@ def test_a_front_panel_setting_is_a_diagnostic_sensor(key): looks like a measurement. """ assert _description(key).entity_category is EntityCategory.DIAGNOSTIC + + +def test_the_standby_minutes_are_durations(): + """A minute count without the duration class gets no unit conversion or + duration display, and the free-text "min" was never validated.""" + from homeassistant.components.number import NumberDeviceClass + from homeassistant.components.sensor import SensorDeviceClass + from homeassistant.const import UnitOfTime + + from custom_components.neumann_kh import number, sensor + + countdown = next(d for d in sensor.COMMON_SENSOR_DESCRIPTIONS if d.key == "standby_countdown") + auto_time = next( + d + for value in vars(number).values() + for d in (value if isinstance(value, tuple) else (value,)) + if getattr(d, "key", None) == "standby_auto_time" + ) + + assert (countdown.device_class, countdown.native_unit_of_measurement) == ( + SensorDeviceClass.DURATION, + UnitOfTime.MINUTES, + ) + assert (auto_time.device_class, auto_time.native_unit_of_measurement) == ( + NumberDeviceClass.DURATION, + UnitOfTime.MINUTES, + ) diff --git a/tests/test_setup_cleanup.py b/tests/test_setup_cleanup.py index 8830464..aff2655 100644 --- a/tests/test_setup_cleanup.py +++ b/tests/test_setup_cleanup.py @@ -64,7 +64,6 @@ def __init__(self, **kwargs) -> None: self.reached_the_device = asyncio.Event() # The coordinator reads this one; the real client sets it while a user # action waits for the poll to yield. - self.priority_waiting = asyncio.Event() async def get(self, path): self.reached_the_device.set() @@ -111,7 +110,6 @@ class _AnsweringClient: def __init__(self, **kwargs) -> None: self.closed = False - self.priority_waiting = asyncio.Event() async def get(self, path): if path == PATH_IDENTITY_SERIAL: @@ -157,3 +155,26 @@ async def _never_finishes(config_entry, platforms): await setup assert client.closed, "the connection was left open when adding platforms was cancelled" + + +async def test_a_setup_whose_platforms_fail_closes_the_connection(hass, _custom_integration): + """The exception sibling of the cancel case above. + + A speaker takes about four connections. Every retry of a setup that failed + while adding platforms left one open, until the vendor app or the + integration itself could no longer connect. + """ + entry = _entry(hass) + client = _AnsweringClient() + + async def _fails(config_entry, platforms): + raise RuntimeError("platform setup failed") + + with ( + patch("custom_components.neumann_kh.SSCClient", return_value=client), + patch.object(hass.config_entries, "async_forward_entry_setups", _fails), + ): + await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + assert client.closed, "the connection was left open after the platforms failed" diff --git a/tests/test_ssc_client.py b/tests/test_ssc_client.py index 44afb36..e05dbe6 100644 --- a/tests/test_ssc_client.py +++ b/tests/test_ssc_client.py @@ -484,39 +484,6 @@ async def test_leftover_line_is_not_read_as_the_next_answer(server): await client.close() -async def test_cancelled_priority_request_does_not_wedge_the_poll_loop(server): - """A cancelled priority request must not leave the event set. - - The poll loop sleeps before every single path while priority_waiting is - set, so a stuck event slows down every cycle until another priority - request happens to complete. - """ - server.release = asyncio.Event() - client = _client(server.port) - try: - # Occupy the lock so the priority request has to wait for it: the - # blocker holds it until the server is released. - blocker = asyncio.create_task(client.get(("device", "name"))) - await asyncio.wait_for(server.request_received.wait(), timeout=5) - - waiter = asyncio.create_task(client.set(("audio", "out", "mute"), True)) - # request() sets the flag before it queues on the lock, so waiting for - # the event is exactly the state this test needs - no sleep required. - await asyncio.wait_for(client.priority_waiting.wait(), timeout=5) - - waiter.cancel() - with pytest.raises(asyncio.CancelledError): - await waiter - server.release.set() - server.release = None - with contextlib.suppress(Exception): - await blocker - - assert not client.priority_waiting.is_set(), "event stayed set after cancellation" - finally: - await client.close() - - def test_is_link_local(): assert SSCClient.is_link_local("fe80::1") assert SSCClient.is_link_local("FE80::1") @@ -1220,3 +1187,97 @@ async def _counting_drain(self) -> None: finally: await client.close() assert drains == 1 + + +# --- Faulty firmware: end of stream and odd lines ----------------------------- + + +async def test_a_speaker_closing_without_an_answer_is_a_connection_error(socket_enabled): + """A clean FIN (standby, reboot) after the request, not a reset. + + Turned into a silent None it would count as answered: every entity would + show unknown instead of unavailable, and the dead socket would stay. + """ + + async def _handle(reader, writer): + await reader.readline() + writer.close() + + raw_server, handled = await _serve(_handle) + client = _client(raw_server.sockets[0].getsockname()[1]) + try: + with pytest.raises(SSCConnectionError): + await client.get(("device", "name")) + assert client._writer is None + finally: + await client.close() + await _shutdown(raw_server, handled) + + +async def test_a_last_line_without_a_terminator_still_counts(socket_enabled): + async def _handle(reader, writer): + await reader.readline() + writer.write(json.dumps({"device": {"name": "x"}}).encode()) + await writer.drain() + writer.close() + + raw_server, handled = await _serve(_handle) + client = _client(raw_server.sockets[0].getsockname()[1]) + try: + assert await client.get(("device", "name")) == "x" + finally: + await client.close() + await _shutdown(raw_server, handled) + + +@pytest.mark.parametrize("not_an_object", [b"null", b"[1, 2]", b"42"]) +async def test_a_json_line_that_is_not_an_object_is_skipped(socket_enabled, not_an_object): + async def _handle(reader, writer): + await reader.readline() + writer.write(not_an_object + b"\r\n") + writer.write(json.dumps({"device": {"name": "ok"}}).encode() + b"\r\n") + await writer.drain() + await _stay_open() + + raw_server, handled = await _serve(_handle) + client = _client(raw_server.sockets[0].getsockname()[1]) + try: + assert await client.get(("device", "name")) == "ok" + finally: + await client.close() + await _shutdown(raw_server, handled) + + +async def test_a_write_during_a_poll_waits_for_one_query_at_most(server): + """The poll takes the lock per path, and asyncio.Lock hands it over in FIFO + order: a waiting write goes before the poll's next query. That carries user + actions ahead of a cycle - no priority flag is needed for it. + """ + server.response_delay = 0.02 + order: list[str] = [] + + def _record(request): + is_write = "mute" in request.get("audio", {}).get("out", {}) + order.append("write" if is_write else "poll") + return [request] + + server.responder = _record + client = _client(server.port) + three_polled = asyncio.Event() + + async def _poll() -> None: + for count in range(1, 21): + await client.get(("audio", "out", "level")) + if count == 3: + three_polled.set() + + try: + poll = asyncio.create_task(_poll()) + await three_polled.wait() + issued_at = len(order) + await client.set(("audio", "out", "mute"), True) + await poll + finally: + await client.close() + + assert order.index("write") - issued_at <= 1, order diff --git a/tests/test_stale_snapshot_overwrites_newer_state.py b/tests/test_stale_snapshot_overwrites_newer_state.py index 9c11c68..00c4bc8 100644 --- a/tests/test_stale_snapshot_overwrites_newer_state.py +++ b/tests/test_stale_snapshot_overwrites_newer_state.py @@ -51,7 +51,6 @@ def __init__(self) -> None: PATH_OUTPUT_MUTE: False, PATH_DEVICE_NAME: "Before", } - self.priority_waiting = asyncio.Event() self.after_set: dict[tuple[str, ...], Any] = {} self.hold_first_read_of: tuple[str, ...] | None = None self.release = asyncio.Event() diff --git a/tests/test_translations.py b/tests/test_translations.py index cf27345..3a645d3 100644 --- a/tests/test_translations.py +++ b/tests/test_translations.py @@ -7,6 +7,7 @@ from __future__ import annotations +import ast import json import pathlib @@ -34,3 +35,66 @@ def test_german_has_exactly_the_keys_english_has(): german = _keys(_load("translations/de.json")) assert not english - german, f"missing in de.json: {sorted(english - german)}" assert not german - english, f"only in de.json: {sorted(german - english)}" + + +# Exceptions Home Assistant translates: *Error, ConfigEntryNotReady, UpdateFailed. +_RAISED = ("Error", "NotReady", "Failed") + + +def _exception_keys_used(source: str) -> set[str]: + """Every literal key handed to an error, directly or on its way there. + + translation_key= on an exception call, rejected_key= (passed on to one), + the default of a rejected_key parameter and the first argument of _stop(). + Keys built at run time are not literals and are out of reach here. + """ + used: set[str] = set() + for node in ast.walk(ast.parse(source)): + if isinstance(node, ast.Call): + name = getattr(node.func, "id", getattr(node.func, "attr", "")) + for keyword in node.keywords: + literal = isinstance(keyword.value, ast.Constant) + if literal and keyword.arg == "rejected_key": + used.add(keyword.value.value) + if literal and keyword.arg == "translation_key" and name.endswith(_RAISED): + used.add(keyword.value.value) + if name == "_stop" and node.args and isinstance(node.args[0], ast.Constant): + used.add(node.args[0].value) + if isinstance(node, ast.arguments): + params = node.args[len(node.args) - len(node.defaults) :] + for param, default in zip(params, node.defaults, strict=True): + if param.arg == "rejected_key" and isinstance(default, ast.Constant): + used.add(default.value) + return used + + +def test_every_error_key_in_the_code_has_a_text(): + """Unit tests compare a key with a literal they repeat, never with strings.json. + + A typo, or a new key without an entry, stayed green everywhere and showed + the raw key to the user. + """ + known = set(_load("strings.json")["exceptions"]) + used = set().union( + *(_exception_keys_used(path.read_text(encoding="utf-8")) for path in COMPONENT.glob("*.py")) + ) + assert used, "the scan found no key at all - it went blind" + assert not used - known, f"error key(s) without a text: {sorted(used - known)}" + + +def test_the_key_scan_sees_each_way_a_key_travels(): + """Counter-check: an invented key in each form is found.""" + source = """ +raise HomeAssistantError(translation_domain=DOMAIN, translation_key="direct_key") +raise ConfigEntryNotReady(translation_domain=DOMAIN, translation_key="not_ready_key") +await self._async_write(path, value, rejected_key="passed_key") +raise _stop("stopped_key") +async def _async_write(self, path, value, rejected_key="default_key"): ... +""" + assert _exception_keys_used(source) == { + "direct_key", + "not_ready_key", + "passed_key", + "stopped_key", + "default_key", + } diff --git a/tests/test_wrong_device_at_stored_address.py b/tests/test_wrong_device_at_stored_address.py index 96ccdcd..4e3db6e 100644 --- a/tests/test_wrong_device_at_stored_address.py +++ b/tests/test_wrong_device_at_stored_address.py @@ -67,7 +67,6 @@ def __init__(self, serial, **kwargs) -> None: self.serial = serial self.closed = False self.asked_for_serial = False - self.priority_waiting = asyncio.Event() async def get(self, path): if path == PATH_IDENTITY_SERIAL: diff --git a/tools/ssc_simulator.py b/tools/ssc_simulator.py index 5970877..151568d 100644 --- a/tools/ssc_simulator.py +++ b/tools/ssc_simulator.py @@ -44,22 +44,22 @@ MODELS = (MODEL_KH_120_II, MODEL_KH_750) # Error codes as observed on real hardware. -_ERR_BAD_REQUEST = 400 -_ERR_AUTHORISATION_NEEDED = 401 -_ERR_NOT_FOUND = 404 -_ERR_NOT_ALLOWED = 405 -_ERR_NOT_ACCEPTABLE = 406 -_ERR_REQUEST_TOO_LONG = 413 -_ERR_PARAMETER_NOT_FOUND = 454 - -_ERR_DESCRIPTIONS = { - _ERR_BAD_REQUEST: "message not understood", - _ERR_AUTHORISATION_NEEDED: "authorisation needed", - _ERR_NOT_FOUND: "address not found", - _ERR_NOT_ALLOWED: "method not allowed", - _ERR_NOT_ACCEPTABLE: "not acceptable", - _ERR_REQUEST_TOO_LONG: "request too long", - _ERR_PARAMETER_NOT_FOUND: "parameter address not found", +_ERROR_BAD_REQUEST = 400 +_ERROR_AUTHORISATION_NEEDED = 401 +_ERROR_NOT_FOUND = 404 +_ERROR_NOT_ALLOWED = 405 +_ERROR_NOT_ACCEPTABLE = 406 +_ERROR_REQUEST_TOO_LONG = 413 +_ERROR_PARAMETER_NOT_FOUND = 454 + +_ERROR_DESCRIPTIONS = { + _ERROR_BAD_REQUEST: "message not understood", + _ERROR_AUTHORISATION_NEEDED: "authorisation needed", + _ERROR_NOT_FOUND: "address not found", + _ERROR_NOT_ALLOWED: "method not allowed", + _ERROR_NOT_ACCEPTABLE: "not acceptable", + _ERROR_REQUEST_TOO_LONG: "request too long", + _ERROR_PARAMETER_NOT_FOUND: "parameter address not found", } # EQ containers per model: (path prefix, band count), as measured on the @@ -444,13 +444,13 @@ def _introspection_path(value: Any, method: str) -> tuple[tuple[str, ...] | None 454, a tree ending in {} a 406. """ if not isinstance(value, list) or not value or not isinstance(value[0], dict) or not value[0]: - return None, _osc_error(method, _ERR_PARAMETER_NOT_FOUND) + return None, _osc_error(method, _ERROR_PARAMETER_NOT_FOUND) leaves = _collect_leaves(value[0]) if len(leaves) != 1: - return None, _error(_ERR_BAD_REQUEST) + return None, _error(_ERROR_BAD_REQUEST) path, end = leaves[0] if end is not None: - return None, _osc_error(method, _ERR_NOT_ACCEPTABLE) + return None, _osc_error(method, _ERROR_NOT_ACCEPTABLE) return path, None @@ -468,12 +468,12 @@ def _json_tokens(value: Any) -> int: def _error(code: int) -> dict[str, Any]: """A flat SSC error: the recorded 413, and the never recorded cases (400).""" - return {"osc": {"error": [code, {"desc": _ERR_DESCRIPTIONS[code]}]}} + return {"osc": {"error": [code, {"desc": _ERROR_DESCRIPTIONS[code]}]}} def _error_entry(node: tuple[str, ...], code: int) -> dict[str, Any]: """One osc/error entry nested at `node`, the format recorded from the devices.""" - return _build_nested(node, [code, {"desc": _ERR_DESCRIPTIONS[code]}]) + return _build_nested(node, [code, {"desc": _ERROR_DESCRIPTIONS[code]}]) def _same_kind(current: Any, value: Any) -> bool: @@ -543,17 +543,17 @@ def _answer_leaf( if bundled: # Recorded: inside a bundle the 404 moves up to the parent. node = node[:-1] or node - return None, _error_entry(node, _ERR_NOT_FOUND) + return None, _error_entry(node, _ERROR_NOT_FOUND) if value is None: return _build_nested(path, current), None # Recorded on LOCAL: a read-only leaf keeps its 405, only a writable # one gets 401 - the device checks writability before the switch. if path in self.read_only: - return None, _error_entry(path, _ERR_NOT_ALLOWED) + return None, _error_entry(path, _ERROR_NOT_ALLOWED) if self._local_mode(): - return None, _error_entry(path, _ERR_AUTHORISATION_NEEDED) + return None, _error_entry(path, _ERROR_AUTHORISATION_NEEDED) if not _same_kind(current, value): - return None, _error_entry(path, _ERR_NOT_ACCEPTABLE) + return None, _error_entry(path, _ERROR_NOT_ACCEPTABLE) self._apply_set(path, value) return _build_nested(path, value), None @@ -586,7 +586,7 @@ def _osc_schema(self, value: Any) -> dict[str, Any]: if not found: # Never recorded for an unknown address; the code of the malformed # requests is the closest measured answer. - return _osc_error("schema", _ERR_PARAMETER_NOT_FOUND) + return _osc_error("schema", _ERROR_PARAMETER_NOT_FOUND) if not isinstance(node, dict): # A leaf has no sub-tree of its own. return {"osc": {"schema": [_build_nested(path, None)]}} @@ -598,7 +598,7 @@ def _osc_limits(self, value: Any) -> dict[str, Any]: return refusal found, node = _lookup(self._address_tree(), path) if not found or isinstance(node, dict): - return _osc_error("limits", _ERR_PARAMETER_NOT_FOUND) + return _osc_error("limits", _ERROR_PARAMETER_NOT_FOUND) # Only the type is reproduced; the devices' other fields (desc, units, # min, max, ...) are not, and none is invented in their place. description: dict[str, Any] = {} @@ -611,16 +611,16 @@ def handle(self, request: dict[str, Any]) -> dict[str, Any]: # Recorded on both models: refused before anything is read, so a # request over the limit gets a flat 413 and no xid. if _json_tokens(request) > _MAX_REQUEST_TOKENS: - return _error(_ERR_REQUEST_TOO_LONG) + return _error(_ERROR_REQUEST_TOO_LONG) # osc/schema and osc/limits are optional per the specification; both # tested firmwares answer them. Without them, the reply is unrecorded. if "osc" in request: osc = request["osc"] if not isinstance(osc, dict): - return _error(_ERR_BAD_REQUEST) + return _error(_ERROR_BAD_REQUEST) if "schema" in osc or "limits" in osc: if not self.enable_schema: - return _error(_ERR_BAD_REQUEST) + return _error(_ERROR_BAD_REQUEST) if "schema" in osc: return self._osc_schema(osc["schema"]) return self._osc_limits(osc["limits"]) @@ -629,11 +629,11 @@ def handle(self, request: dict[str, Any]) -> dict[str, Any]: # Recorded with an xid: an unknown method is a 404 at the osc # node (two keys, so the bundle rule moves it up), xid echoed. return { - "osc": {"error": [_error_entry(("osc",), _ERR_NOT_FOUND)], "xid": osc["xid"]} + "osc": {"error": [_error_entry(("osc",), _ERROR_NOT_FOUND)], "xid": osc["xid"]} } methods = _osc_method_answer(osc) if methods is None: - return _error(_ERR_BAD_REQUEST) + return _error(_ERROR_BAD_REQUEST) values = {key: value for key, value in request.items() if key != "osc"} answer = self._answer_values(values) if values else {} _merge(answer, {"osc": methods}) @@ -644,7 +644,7 @@ def _answer_values(self, request: dict[str, Any]) -> dict[str, Any]: """Gets and sets of device values, single or bundled.""" leaves = _collect_leaves(request) if not leaves: - return _error(_ERR_BAD_REQUEST) + return _error(_ERROR_BAD_REQUEST) answer: dict[str, Any] = {} errors: list[dict[str, Any]] = [] for path, value in leaves: @@ -688,7 +688,7 @@ async def _handle_client( continue if not isinstance(request, dict): - response = _error(_ERR_BAD_REQUEST) + response = _error(_ERROR_BAD_REQUEST) else: response = simulator.handle(request)