diff --git a/custom_components/localthings/config_flow.py b/custom_components/localthings/config_flow.py index f9936c3..183d6d7 100644 --- a/custom_components/localthings/config_flow.py +++ b/custom_components/localthings/config_flow.py @@ -164,19 +164,10 @@ def _fetch_samsung_uuid() -> str: def _normalize_pem(text: str) -> str: - """Clean up a pasted PEM blob before handing it to `cryptography`'s - parser (issue #291). - - A PEM copied out of a text editor can carry a few bytes `cryptography` - refuses outright: a UTF-8 BOM some Windows editors silently prepend, CR - line endings (or CRLF), and blank lines a paste can introduce between - the header/body/footer. Any of those surfaces as an opaque - `InvalidHeader` with no hint of what's actually wrong -- which is why - the same certificate pasted from `type` (no BOM, no stray blank lines) - loads fine while the editor's copy doesn't. None of the stripped bytes - are meaningful in PEM: the format is BOM-free, line-ending-agnostic, and - has no blank-line syntax of its own. - """ + """Strip a pasted PEM's BOM, CRLF endings, and blank lines before + `cryptography` sees it -- a text editor's copy carries all three and + fails with an opaque InvalidHeader, while the same file dumped via + `type` doesn't (issue #291).""" text = text.lstrip("\ufeff") text = text.replace("\r\n", "\n").replace("\r", "\n") lines = [line for line in text.split("\n") if line.strip()] @@ -742,12 +733,8 @@ class LocalThingsConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): if leaf_cert and leaf_key: existing_leaf = (leaf_cert, leaf_key) else: - # Normalized before the probe even runs (issue #291), not - # just before minting: the same pasted blob is also what - # gets stored in the config entry and reused to re-mint the - # leaf on a future reconfigure, so a raw copy with a BOM or - # CRLF line endings would keep failing every time it's read - # back, not just this once. + # Normalized here, not just before minting: this is also + # what gets stored and reused to re-mint the leaf later. self._ca_cert_pem = _normalize_pem(user_input[CONF_CA_CERT_PEM]) self._ca_key_pem = _normalize_pem(user_input[CONF_CA_KEY_PEM]) diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index c0484f6..d747406 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -834,18 +834,11 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): resources = await self.hass.async_add_executor_job(self._poll_once) except Exception as e2: self._log.error("poll failed after reconnect: %s", e2) - # The reconnect itself proved the old session is gone, so - # any OBSERVE subscription on it is too -- this cycle has - # no live session to resubscribe on, unlike the success - # branch below, so downgrade without setting - # just_downgraded_from_observe (that would attempt a - # resubscribe this same cycle against a session we just - # confirmed is unreachable). Without this, a device that - # drops off the network entirely leaves the connection-mode - # sensor reporting "Push" forever, since only the success - # path below ever changed it (issue #287) -- the poll-mode - # retry timer (_maybe_retry_observe_mode) still re-attempts - # observe once the device is actually reachable again. + # Without this, a fully unreachable device left the + # connection-mode sensor stuck on "Push" forever -- only + # the success branch below ever downgraded it (issue + # #287). No just_downgraded_from_observe here: there's no + # live session this cycle to resubscribe on. if self._observe.mode == MODE_OBSERVE: self._observe.downgrade_to_poll() snapshot = self._cache.snapshot() @@ -924,7 +917,8 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): # ------------------------------------------------------------------ async def async_send_command(self, bound_entity: BoundEntity, payload: Any) -> None: - """Write a value to the device. Fire-and-forget. + """Write a value to the device. Retries once on a dead session + (issue #294); raises HomeAssistantError if that retry fails too. A description-level validate_fn (SwitchDesc only, currently) rejects a write with a user-facing message ahead of write_fn's silent @@ -1034,17 +1028,9 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): code, _ = sess.post(path_segs, cbor2.dumps(body), timeout=self._POST_TIMEOUT_S) self._log.info("PUT %s → code %#04x", write_href, code) - # A session Samsung's firmware closed between polls (the same "known - # device behavior" _async_update_data reconnects around) previously - # sank the PUT here with nothing but a log line -- no retry, no - # user-facing error, so the command was silently lost until the next - # manual reload (issue #294). Mirror the poll path's own recovery: - # one reconnect, one retry, and this time raise on final failure so - # the user gets a red toast instead of nothing. - # - # Locked the same as the poll path so a write landing mid-reconnect - # doesn't tear down a session _async_update_data is simultaneously - # rebuilding (or vice versa). + # Mirrors the poll path's reconnect-and-retry (issue #294): a PUT + # landing on a session Samsung's firmware closed between polls used + # to be silently lost -- no retry, no user-facing error. async with self._session_lock: try: await self.hass.async_add_executor_job(_do_put) @@ -1061,6 +1047,15 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): translation_key="command_failed", translation_placeholders={"href": write_href, "error": str(e2)}, ) from e2 + # The retry's own reconnect pause + second PUT can eat well + # into the settle window armed above, leaving too little of + # it for the confirming poll below and reviving the + # revert-then-reapply symptom settle_s exists to prevent + # (issue #9). Re-arm it fresh now that the write actually + # landed. + self._observe.mark_write_pending( + write_href, settle_s=self._POST_TIMEOUT_S + self._POLL_TIMEOUT_S + ) await self.async_request_refresh() # ------------------------------------------------------------------ diff --git a/tests/localthings/test_coordinator.py b/tests/localthings/test_coordinator.py index 278e832..42cc435 100644 --- a/tests/localthings/test_coordinator.py +++ b/tests/localthings/test_coordinator.py @@ -1015,7 +1015,14 @@ async def test_send_command_reconnects_and_retries_after_socket_closed( """A command lost to a session Samsung's firmware closed between polls must not just vanish (issue #294): `_do_put` failing once is now followed by a reconnect and a single retry, mirroring the poll path's - own recovery in `_async_update_data`.""" + own recovery in `_async_update_data`. + + `mock_coordinator_observe_session` patches `_close_session` to a no-op, + which would leave `self._session` never actually going `None` -- and + with it, `_do_put`'s own `if self._session is None: self._connect_session()` + guard never exercised, so a broken reconnect could still pass. Overridden + here to actually drop the session, so the retry only succeeds if that + guard really rebuilds it.""" from custom_components.localthings.registry.discovery import BoundEntity from custom_components.localthings.registry.entities import NumberDesc @@ -1038,8 +1045,19 @@ async def test_send_command_reconnects_and_retries_after_socket_closed( raise ConnectionError("socket closed") return (0x44, b"") + def _drop_session(): + coordinator._session = None + + reconnects = {"n": 0} + + def _reconnect(): + reconnects["n"] += 1 + coordinator._session = fake + with ( patch.object(fake, "subscribe"), + patch.object(coordinator, "_close_session", side_effect=_drop_session), + patch.object(coordinator, "_connect_session", side_effect=_reconnect), patch( "custom_components.localthings.coordinator.asyncio.sleep", new=AsyncMock(), @@ -1048,6 +1066,8 @@ async def test_send_command_reconnects_and_retries_after_socket_closed( fake.post = _post await coordinator.async_send_command(bound, 5) + assert reconnects["n"] == 1 + assert calls["n"] == 2 assert coordinator._cache.get("/test/vs/0") == {"value": 5}