diff --git a/custom_components/localthings/coordinator.py b/custom_components/localthings/coordinator.py index 692fa22..c0484f6 100644 --- a/custom_components/localthings/coordinator.py +++ b/custom_components/localthings/coordinator.py @@ -15,7 +15,7 @@ from typing import Any import cbor2 from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant, callback -from homeassistant.exceptions import ServiceValidationError +from homeassistant.exceptions import HomeAssistantError, ServiceValidationError from homeassistant.helpers import issue_registry as ir from homeassistant.helpers.device_registry import DeviceInfo from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed @@ -1026,18 +1026,42 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]): ) def _do_put(): + if self._session is None: + self._connect_session() sess = self._session if sess is None: raise RuntimeError("no session") code, _ = sess.post(path_segs, cbor2.dumps(body), timeout=self._POST_TIMEOUT_S) self._log.info("PUT %s → code %#04x", write_href, code) - try: - await self.hass.async_add_executor_job(_do_put) - except Exception as e: - self._log.error("command failed for %s: %s", write_href, e) - else: - await self.async_request_refresh() + # 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). + async with self._session_lock: + try: + await self.hass.async_add_executor_job(_do_put) + except Exception as e: + self._log.warning("command failed for %s, reconnecting: %s", write_href, e) + await self.hass.async_add_executor_job(self._close_session) + await asyncio.sleep(self._RECONNECT_PAUSE_S) + try: + await self.hass.async_add_executor_job(_do_put) + except Exception as e2: + self._log.error("command failed for %s after reconnect: %s", write_href, e2) + raise HomeAssistantError( + translation_domain=DOMAIN, + translation_key="command_failed", + translation_placeholders={"href": write_href, "error": str(e2)}, + ) from e2 + await self.async_request_refresh() # ------------------------------------------------------------------ # Debug raw write (issue #54): a power-user escape hatch for the diff --git a/custom_components/localthings/translations/cs.json b/custom_components/localthings/translations/cs.json index 83e5b72..82ccf6f 100644 --- a/custom_components/localthings/translations/cs.json +++ b/custom_components/localthings/translations/cs.json @@ -1356,6 +1356,9 @@ }, "intensive_unavailable_for_cycle": { "message": "Intenzivní není u vybraného cyklu k dispozici." + }, + "command_failed": { + "message": "Příkaz pro {href} selhal i po opětovném připojení: {error}" } } } diff --git a/custom_components/localthings/translations/en.json b/custom_components/localthings/translations/en.json index d403e24..dbf61ee 100644 --- a/custom_components/localthings/translations/en.json +++ b/custom_components/localthings/translations/en.json @@ -1356,6 +1356,9 @@ }, "intensive_unavailable_for_cycle": { "message": "Intensive isn't available on the selected cycle." + }, + "command_failed": { + "message": "The command to {href} failed even after reconnecting: {error}" } } } diff --git a/custom_components/localthings/translations/es.json b/custom_components/localthings/translations/es.json index ec8e6a0..d24b5a8 100644 --- a/custom_components/localthings/translations/es.json +++ b/custom_components/localthings/translations/es.json @@ -119,6 +119,9 @@ }, "intensive_unavailable_for_cycle": { "message": "El modo intensivo no está disponible en el ciclo seleccionado." + }, + "command_failed": { + "message": "El comando para {href} falló incluso después de reconectar: {error}" } }, "entity": { diff --git a/custom_components/localthings/translations/it.json b/custom_components/localthings/translations/it.json index be69d02..8e3e0e7 100644 --- a/custom_components/localthings/translations/it.json +++ b/custom_components/localthings/translations/it.json @@ -1356,6 +1356,9 @@ }, "intensive_unavailable_for_cycle": { "message": "Intensivo non è disponibile per il ciclo selezionato." + }, + "command_failed": { + "message": "Il comando per {href} è fallito anche dopo la riconnessione: {error}" } } } diff --git a/custom_components/localthings/translations/ko.json b/custom_components/localthings/translations/ko.json index 7a86ba4..736928f 100644 --- a/custom_components/localthings/translations/ko.json +++ b/custom_components/localthings/translations/ko.json @@ -1356,6 +1356,9 @@ }, "intensive_unavailable_for_cycle": { "message": "선택한 코스에서는 강력 세탁을 사용할 수 없습니다." + }, + "command_failed": { + "message": "재연결 후에도 {href} 명령이 실패했습니다: {error}" } } } diff --git a/custom_components/localthings/translations/nl.json b/custom_components/localthings/translations/nl.json index 09e15db..2623d3b 100644 --- a/custom_components/localthings/translations/nl.json +++ b/custom_components/localthings/translations/nl.json @@ -1356,6 +1356,9 @@ }, "intensive_unavailable_for_cycle": { "message": "Intensief is niet beschikbaar voor het geselecteerde programma." + }, + "command_failed": { + "message": "Het commando naar {href} is ook na opnieuw verbinden mislukt: {error}" } } } diff --git a/tests/localthings/test_coordinator.py b/tests/localthings/test_coordinator.py index 3338079..278e832 100644 --- a/tests/localthings/test_coordinator.py +++ b/tests/localthings/test_coordinator.py @@ -1007,6 +1007,89 @@ async def test_send_command_survives_stale_confirm_poll( assert coordinator._cache.get("/test/vs/0") == {"value": 5} +async def test_send_command_reconnects_and_retries_after_socket_closed( + hass: HomeAssistant, + mock_entry, + mock_coordinator_observe_session, +) -> None: + """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`.""" + from custom_components.localthings.registry.discovery import BoundEntity + from custom_components.localthings.registry.entities import NumberDesc + + fake = mock_coordinator_observe_session + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + coordinator: LocalThingsCoordinator = hass.data[DOMAIN][mock_entry.entry_id] + + def _write_fn(payload, rep, href=None): + return (["test", "vs", "0"], {"value": payload}) + + desc = NumberDesc(key="test", field="value", write_fn=_write_fn) + bound = BoundEntity(href="/test/vs/0", capability=coordinator.bound[0].capability, desc=desc) + + calls = {"n": 0} + + def _post(*args, **kwargs): + calls["n"] += 1 + if calls["n"] == 1: + raise ConnectionError("socket closed") + return (0x44, b"") + + with ( + patch.object(fake, "subscribe"), + patch( + "custom_components.localthings.coordinator.asyncio.sleep", + new=AsyncMock(), + ), + ): + fake.post = _post + await coordinator.async_send_command(bound, 5) + + assert calls["n"] == 2 + assert coordinator._cache.get("/test/vs/0") == {"value": 5} + + +async def test_send_command_raises_after_reconnect_retry_also_fails( + hass: HomeAssistant, + mock_entry, + mock_coordinator_observe_session, +) -> None: + """If the command still fails on the reconnected session, the user must + see it -- previously this was swallowed into a log line with no + feedback at all (issue #294).""" + from homeassistant.exceptions import HomeAssistantError + + from custom_components.localthings.registry.discovery import BoundEntity + from custom_components.localthings.registry.entities import NumberDesc + + fake = mock_coordinator_observe_session + await hass.config_entries.async_setup(mock_entry.entry_id) + await hass.async_block_till_done() + coordinator: LocalThingsCoordinator = hass.data[DOMAIN][mock_entry.entry_id] + + def _write_fn(payload, rep, href=None): + return (["test", "vs", "0"], {"value": payload}) + + desc = NumberDesc(key="test", field="value", write_fn=_write_fn) + bound = BoundEntity(href="/test/vs/0", capability=coordinator.bound[0].capability, desc=desc) + + def _post(*args, **kwargs): + raise ConnectionError("socket closed") + + with ( + patch( + "custom_components.localthings.coordinator.asyncio.sleep", + new=AsyncMock(), + ), + pytest.raises(HomeAssistantError), + ): + fake.post = _post + await coordinator.async_send_command(bound, 5) + + async def test_second_write_to_same_href_lands_during_first_writes_settle_window( hass: HomeAssistant, mock_entry,