fix(review): stop counting timeouts, notify on disable, drop WSLink indoor
parent
2c6b698ca9
commit
c33f9454f7
|
|
@ -10,6 +10,7 @@ from typing import Any, Literal
|
|||
from aiohttp import ClientError, ClientTimeout
|
||||
from py_typecheck.core import checked_or
|
||||
|
||||
from homeassistant.components import persistent_notification
|
||||
from homeassistant.config_entries import ConfigEntry
|
||||
from homeassistant.core import HomeAssistant
|
||||
from homeassistant.helpers.aiohttp_client import async_get_clientsession
|
||||
|
|
@ -91,13 +92,20 @@ class PocasiPush:
|
|||
return "unexpected_response"
|
||||
|
||||
async def _disable_pocasi(self, reason: str) -> None:
|
||||
"""Turn resending off and persist it, so it survives a restart."""
|
||||
"""Turn resending off and persist it, so it survives a restart.
|
||||
|
||||
Notifies the user as well - as `WindyPush._disable_windy` does. Forwarding
|
||||
switching itself off is otherwise invisible until someone notices that data
|
||||
stopped arriving upstream, since the only other trace is a log line.
|
||||
"""
|
||||
|
||||
self.last_error = reason
|
||||
|
||||
if not await update_options(self.hass, self.config, POCASI_CZ_ENABLED, False):
|
||||
_LOGGER.debug("Failed to set Pocasi Meteo options to false.")
|
||||
|
||||
persistent_notification.async_create(self.hass, reason, "Pocasi Meteo resending disabled.")
|
||||
|
||||
async def push_data_to_server(self, data: dict[str, Any], mode: PocasiMode):
|
||||
"""Forward a station payload to Pocasi Meteo.
|
||||
|
||||
|
|
@ -219,7 +227,20 @@ class PocasiPush:
|
|||
# TimeoutError is not a ClientError: an `async_timeout`/`asyncio` timeout would
|
||||
# otherwise escape into the webhook handler and answer the station with HTTP 500,
|
||||
# even though the measured data was already stored.
|
||||
except (ClientError, TimeoutError) as ex:
|
||||
#
|
||||
# It gets its own branch because it must not spend the retry budget: a slow
|
||||
# upstream is transient and self-healing, so the next push just tries again,
|
||||
# while `invalid_response_count` is meant for faults that will not fix
|
||||
# themselves. Caught first on purpose - aiohttp's ServerTimeoutError is both a
|
||||
# ClientError and a TimeoutError, and it belongs here.
|
||||
except TimeoutError as ex:
|
||||
self.last_status = "timeout"
|
||||
self.last_error = type(ex).__name__
|
||||
_LOGGER.warning(
|
||||
"Pocasi Meteo did not answer within %s seconds. Will try again on the next push.",
|
||||
FORWARD_TIMEOUT,
|
||||
)
|
||||
except ClientError as ex:
|
||||
self.last_status = "client_error"
|
||||
# Store only the exception class - last_error is surfaced via entity
|
||||
# attributes; str(ex) could embed the request URL.
|
||||
|
|
|
|||
|
|
@ -175,15 +175,13 @@ class WindyPush:
|
|||
indata["uv"] = indata.pop("t1uvi")
|
||||
if "t1solrad" in indata:
|
||||
indata["solarradiation"] = indata.pop("t1solrad")
|
||||
# Indoor readings mean nothing to Windy. Renaming them to their PWS spelling
|
||||
# (rather than dropping them here) leaves PURGE_DATA - which lists PWS names -
|
||||
# as the single place that decides what Windy never receives, exactly as on the
|
||||
# Ecowitt path. The values stay Celsius/percent, which is fine because they are
|
||||
# purged before the request is built.
|
||||
if "intem" in indata:
|
||||
indata["indoortempf"] = indata.pop("intem")
|
||||
if "inhum" in indata:
|
||||
indata["indoorhumidity"] = indata.pop("inhum")
|
||||
|
||||
# Indoor readings mean nothing to Windy, and there is no PWS name to put them
|
||||
# under: `indoortempf` is Fahrenheit while WSLink `intem` is Celsius. Dropping
|
||||
# them outright keeps the payload unit-correct on its own terms, rather than
|
||||
# relying on PURGE_DATA to discard a mislabelled value afterwards.
|
||||
indata.pop("intem", None)
|
||||
indata.pop("inhum", None)
|
||||
|
||||
return indata
|
||||
|
||||
|
|
@ -349,7 +347,20 @@ class WindyPush:
|
|||
# TimeoutError is not a ClientError: an `async_timeout`/`asyncio` timeout would
|
||||
# otherwise escape into the webhook handler and answer the station with HTTP 500,
|
||||
# even though the measured data was already stored.
|
||||
except (ClientError, TimeoutError) as ex:
|
||||
#
|
||||
# It gets its own branch because it must not spend the retry budget: a slow
|
||||
# upstream is transient and self-healing, so the next push just tries again,
|
||||
# while `invalid_response_count` is meant for faults that will not fix
|
||||
# themselves. Caught first on purpose - aiohttp's ServerTimeoutError is both a
|
||||
# ClientError and a TimeoutError, and it belongs here.
|
||||
except TimeoutError as ex:
|
||||
self.last_status = "timeout"
|
||||
self.last_error = type(ex).__name__
|
||||
_LOGGER.warning(
|
||||
"Windy did not answer within %s seconds. Will try again on the next push.",
|
||||
FORWARD_TIMEOUT,
|
||||
)
|
||||
except ClientError as ex:
|
||||
self.last_status = "client_error"
|
||||
# Store only the exception class - last_error is surfaced via entity
|
||||
# attributes; str(ex) could embed the request URL.
|
||||
|
|
|
|||
|
|
@ -121,6 +121,16 @@ def hass():
|
|||
return SimpleNamespace()
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def notify(monkeypatch):
|
||||
"""Capture the disable notification; the stub `hass` cannot serve the real one."""
|
||||
created = MagicMock()
|
||||
monkeypatch.setattr(
|
||||
"custom_components.sws12500.pocasti_cz.persistent_notification.async_create", created
|
||||
)
|
||||
return created
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def pusher(monkeypatch, hass):
|
||||
"""A PocasiPush wired to a recording session and allowed to send immediately."""
|
||||
|
|
|
|||
|
|
@ -101,6 +101,16 @@ def hass():
|
|||
return SimpleNamespace()
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def notify(monkeypatch):
|
||||
"""Capture the disable notification; the stub `hass` cannot serve the real one."""
|
||||
created = MagicMock()
|
||||
monkeypatch.setattr(
|
||||
"custom_components.sws12500.pocasti_cz.persistent_notification.async_create", created
|
||||
)
|
||||
return created
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_push_data_to_server_missing_api_id_returns_early(monkeypatch, hass):
|
||||
entry = _make_entry(api_id=None, api_key="key")
|
||||
|
|
@ -339,14 +349,15 @@ async def test_push_data_to_server_client_error_increments_and_disables_after_th
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_push_data_to_server_timeout_is_handled_like_client_error(
|
||||
monkeypatch, hass
|
||||
):
|
||||
async def test_push_data_to_server_timeout_is_caught_and_not_counted(monkeypatch, hass):
|
||||
"""A network timeout must not escape into the webhook handler.
|
||||
|
||||
`TimeoutError` is not a subclass of `ClientError`, so it used to propagate out
|
||||
of `push_data_to_server`, through the awaiting coordinator, and answer the
|
||||
station with HTTP 500 - even though the measured data was already stored.
|
||||
|
||||
It must not spend the retry budget either: a slow upstream is transient, so the
|
||||
next push simply tries again.
|
||||
"""
|
||||
entry = _make_entry()
|
||||
pp = PocasiPush(hass, entry)
|
||||
|
|
@ -366,12 +377,94 @@ async def test_push_data_to_server_timeout_is_handled_like_client_error(
|
|||
pp.next_update = dt_util.utcnow() - timedelta(seconds=1)
|
||||
await pp.push_data_to_server({"x": 1}, "WU")
|
||||
|
||||
assert pp.last_status == "client_error"
|
||||
assert pp.last_status == "timeout"
|
||||
assert pp.last_error == "TimeoutError"
|
||||
assert pp.invalid_response_count == 1
|
||||
assert pp.invalid_response_count == 0
|
||||
assert pp.enabled is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_repeated_timeouts_never_disable_pocasi(monkeypatch, hass, notify):
|
||||
"""Two minutes of a merely slow server must not turn forwarding off for good.
|
||||
|
||||
With a 30 second send interval, counting timeouts would spend the whole retry
|
||||
budget on a slowdown that recovers by itself.
|
||||
"""
|
||||
entry = _make_entry()
|
||||
pp = PocasiPush(hass, entry)
|
||||
|
||||
update_options = _write_through_update_options(entry)
|
||||
monkeypatch.setattr(
|
||||
"custom_components.sws12500.pocasti_cz.update_options", update_options
|
||||
)
|
||||
|
||||
session = _FakeSession(exc=TimeoutError("timed out"))
|
||||
monkeypatch.setattr(
|
||||
"custom_components.sws12500.pocasti_cz.async_get_clientsession",
|
||||
lambda _h: session,
|
||||
)
|
||||
|
||||
for _ in range(POCASI_CZ_MAX_RETRIES + 2):
|
||||
pp.next_update = dt_util.utcnow() - timedelta(seconds=1)
|
||||
await pp.push_data_to_server({"x": 1}, "WU")
|
||||
|
||||
assert pp.invalid_response_count == 0
|
||||
assert pp.enabled is True
|
||||
update_options.assert_not_awaited()
|
||||
notify.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_timeout_does_not_reset_an_existing_client_error_budget(monkeypatch, hass):
|
||||
"""A timeout is ignored by the counter, not a substitute for a successful send."""
|
||||
entry = _make_entry()
|
||||
pp = PocasiPush(hass, entry)
|
||||
pp.invalid_response_count = 2
|
||||
pp.next_update = dt_util.utcnow() - timedelta(seconds=1)
|
||||
|
||||
session = _FakeSession(exc=TimeoutError("timed out"))
|
||||
monkeypatch.setattr(
|
||||
"custom_components.sws12500.pocasti_cz.async_get_clientsession",
|
||||
lambda _h: session,
|
||||
)
|
||||
|
||||
await pp.push_data_to_server({"x": 1}, "WU")
|
||||
|
||||
assert pp.invalid_response_count == 2
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_disabling_pocasi_notifies_the_user(monkeypatch, hass, notify):
|
||||
"""Forwarding switching itself off must be visible, not just a log line.
|
||||
|
||||
Windy already raises a persistent notification; without one here the user only
|
||||
finds out when data stops arriving upstream.
|
||||
"""
|
||||
entry = _make_entry()
|
||||
pp = PocasiPush(hass, entry)
|
||||
pp.next_update = dt_util.utcnow() - timedelta(seconds=1)
|
||||
|
||||
session = _FakeSession(response=_FakeResponse("", status=401))
|
||||
monkeypatch.setattr(
|
||||
"custom_components.sws12500.pocasti_cz.async_get_clientsession",
|
||||
lambda _h: session,
|
||||
)
|
||||
monkeypatch.setattr("custom_components.sws12500.pocasti_cz.anonymize", lambda d: d)
|
||||
monkeypatch.setattr(
|
||||
"custom_components.sws12500.pocasti_cz.update_options",
|
||||
_write_through_update_options(entry),
|
||||
)
|
||||
monkeypatch.setattr("custom_components.sws12500.pocasti_cz._LOGGER.critical", MagicMock())
|
||||
|
||||
await pp.push_data_to_server({"x": 1}, "WU")
|
||||
|
||||
assert pp.enabled is False
|
||||
notify.assert_called_once()
|
||||
args = notify.call_args.args
|
||||
assert args[0] is hass
|
||||
assert POCASI_INVALID_KEY in args[1]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_push_data_to_server_bounds_the_request(monkeypatch, hass):
|
||||
"""The send must carry an explicit timeout.
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ from custom_components.sws12500.const import (
|
|||
PURGE_DATA,
|
||||
WINDY_ENABLED,
|
||||
WINDY_LOGGER_ENABLED,
|
||||
WINDY_MAX_RETRIES,
|
||||
WINDY_STATION_ID,
|
||||
WINDY_STATION_PW,
|
||||
WINDY_UNEXPECTED,
|
||||
|
|
@ -130,8 +131,6 @@ def test_covert_wslink_to_pws_maps_keys(hass):
|
|||
assert out["precip"] == "8"
|
||||
assert out["uv"] == "9"
|
||||
assert out["solarradiation"] == "10"
|
||||
assert out["indoortempf"] == "21.5"
|
||||
assert out["indoorhumidity"] == "48"
|
||||
assert out["other"] == "keep"
|
||||
for k in (
|
||||
"t1ws",
|
||||
|
|
@ -149,6 +148,11 @@ def test_covert_wslink_to_pws_maps_keys(hass):
|
|||
):
|
||||
assert k not in out
|
||||
|
||||
# Indoor readings are dropped, not renamed: WSLink sends Celsius, while the PWS
|
||||
# `indoortempf` spelling promises Fahrenheit.
|
||||
assert "indoortempf" not in out
|
||||
assert "indoorhumidity" not in out
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_push_data_to_windy_respects_initial_next_update(monkeypatch, hass):
|
||||
|
|
@ -462,14 +466,15 @@ async def test_push_data_to_windy_client_error_increments_and_disables_after_thr
|
|||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_push_data_to_windy_timeout_is_handled_like_client_error(
|
||||
monkeypatch, hass
|
||||
):
|
||||
async def test_push_data_to_windy_timeout_is_caught_and_not_counted(monkeypatch, hass):
|
||||
"""A network timeout must not escape into the webhook handler.
|
||||
|
||||
`TimeoutError` is not a subclass of `ClientError`, so it used to propagate out
|
||||
of `push_data_to_windy`, through the awaiting coordinator, and answer the
|
||||
station with HTTP 500 - even though the measured data was already stored.
|
||||
|
||||
It must not spend the retry budget either: a slow upstream is transient, so the
|
||||
next push simply tries again.
|
||||
"""
|
||||
entry = _make_entry()
|
||||
wp = WindyPush(hass, entry)
|
||||
|
|
@ -493,9 +498,61 @@ async def test_push_data_to_windy_timeout_is_handled_like_client_error(
|
|||
ok = await wp.push_data_to_windy({"a": "b"})
|
||||
|
||||
assert ok is True
|
||||
assert wp.last_status == "client_error"
|
||||
assert wp.last_status == "timeout"
|
||||
assert wp.last_error == "TimeoutError"
|
||||
assert wp.invalid_response_count == 1
|
||||
assert wp.invalid_response_count == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_repeated_timeouts_never_disable_windy(monkeypatch, hass):
|
||||
"""15 minutes of a merely slow Windy must not turn forwarding off for good.
|
||||
|
||||
Bounding the request made this branch reachable; counting it would let a
|
||||
transient slowdown burn the whole retry budget with nothing to recover it.
|
||||
"""
|
||||
entry = _make_entry()
|
||||
wp = WindyPush(hass, entry)
|
||||
|
||||
update_options = AsyncMock(return_value=True)
|
||||
monkeypatch.setattr("custom_components.sws12500.windy_func.update_options", update_options)
|
||||
notify = MagicMock()
|
||||
monkeypatch.setattr(
|
||||
"custom_components.sws12500.windy_func.persistent_notification.async_create", notify
|
||||
)
|
||||
|
||||
session = _FakeSession(exc=TimeoutError("timed out"))
|
||||
monkeypatch.setattr(
|
||||
"custom_components.sws12500.windy_func.async_get_clientsession",
|
||||
lambda _h: session,
|
||||
)
|
||||
|
||||
for _ in range(WINDY_MAX_RETRIES + 2):
|
||||
wp.next_update = dt_util.utcnow() - timedelta(seconds=1)
|
||||
assert await wp.push_data_to_windy({"a": "b"}) is True
|
||||
|
||||
assert wp.invalid_response_count == 0
|
||||
assert wp.enabled is True
|
||||
update_options.assert_not_awaited()
|
||||
notify.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_timeout_does_not_reset_an_existing_client_error_budget(monkeypatch, hass):
|
||||
"""A timeout is ignored by the counter, not a substitute for a successful send."""
|
||||
entry = _make_entry()
|
||||
wp = WindyPush(hass, entry)
|
||||
wp.invalid_response_count = 2
|
||||
wp.next_update = dt_util.utcnow() - timedelta(seconds=1)
|
||||
|
||||
session = _FakeSession(exc=TimeoutError("timed out"))
|
||||
monkeypatch.setattr(
|
||||
"custom_components.sws12500.windy_func.async_get_clientsession",
|
||||
lambda _h: session,
|
||||
)
|
||||
|
||||
await wp.push_data_to_windy({"a": "b"})
|
||||
|
||||
assert wp.invalid_response_count == 2
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
@ -608,8 +665,9 @@ async def test_push_data_to_windy_purges_after_converting(monkeypatch, hass):
|
|||
async def test_push_data_to_windy_drops_wslink_indoor_readings(monkeypatch, hass):
|
||||
"""WSLink `intem`/`inhum` must not reach Windy under either spelling.
|
||||
|
||||
PURGE_DATA lists the PWS names, so before these two were mapped in the converter
|
||||
they slipped through untouched and Windy received a raw `intem`/`inhum` pair.
|
||||
PURGE_DATA lists the PWS names, so these two used to slip through untouched and
|
||||
Windy received a raw `intem`/`inhum` pair. The converter drops them instead, which
|
||||
holds regardless of what PURGE_DATA contains.
|
||||
"""
|
||||
entry = _make_entry()
|
||||
wp = WindyPush(hass, entry)
|
||||
|
|
|
|||
Loading…
Reference in New Issue