Merge pull request #391 from JayChickenK/observe-silent-href-subpolls
Keep asking the appliance for readings that never send live updates
This commit is contained in:
Marc Billow
@@ -991,22 +991,31 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
|
||||
# ------------------------------------------------------------------
|
||||
|
||||
async def _run_subpolls(self, force: bool = False) -> None:
|
||||
"""Poll hot/warm hrefs in the gaps between summary polls. No-op in
|
||||
observe-primary mode (those hrefs are already covered by push)
|
||||
unless `force` is set -- set when this cycle's sweep found the
|
||||
cache disagreeing with a still-live observe session (see
|
||||
log_sweep_discrepancies): a bounded fallback for a channel gone
|
||||
silent without a reconnect."""
|
||||
"""Poll hot/warm hrefs in the gaps between summary polls.
|
||||
|
||||
In observe-primary mode this is a no-op for hrefs the device is
|
||||
actually pushing, unless `force` is set (sweep disagreed with the
|
||||
cache -- see log_sweep_discrepancies). Hrefs that were subscribed
|
||||
but stayed silent through the grace period (issue #92) stay on
|
||||
the hot/warm cadence via `fallback_hrefs`.
|
||||
"""
|
||||
if self._observe.mode == MODE_OBSERVE and not force:
|
||||
return
|
||||
hot = self._hot_hrefs
|
||||
warm = self._warm_hrefs
|
||||
silent = self._observe.fallback_hrefs
|
||||
if not silent:
|
||||
return
|
||||
hot = [h for h in self._hot_hrefs if h in silent]
|
||||
warm = [h for h in self._warm_hrefs if h in silent]
|
||||
else:
|
||||
hot = self._hot_hrefs
|
||||
warm = self._warm_hrefs
|
||||
if not hot and not warm:
|
||||
return
|
||||
step = self._SUBPOLL_STEP_S
|
||||
for i in range(1, 10): # slots 1..9 (T+3 s … T+27 s)
|
||||
await asyncio.sleep(step)
|
||||
hrefs = list(hot) + (list(warm) if i % 2 == 0 else [])
|
||||
if not hrefs:
|
||||
continue
|
||||
async with self._session_lock:
|
||||
try:
|
||||
await self.hass.async_add_executor_job(self._poll_hrefs_blocking, hrefs)
|
||||
|
||||
@@ -95,8 +95,15 @@ class ObserveManager:
|
||||
self._notified: set[str] = set()
|
||||
self._last_notify_ts: float | None = None
|
||||
# Wakes try_enter_observe_mode's grace wait early once enough hrefs
|
||||
# have notified. Guards only `_notified` mutations + the `wait_for`.
|
||||
# have notified. Guards `_notified` mutations, the `wait_for`, and
|
||||
# fallback_hrefs (enter_observe_mode assignment, on_notification
|
||||
# discard).
|
||||
self._notify_cond = threading.Condition()
|
||||
# Idle while polling, except after downgrade_to_poll (every href
|
||||
# that was subscribed). While in observe mode this is the set of
|
||||
# subscribed hrefs that have not yet notified (issue #92) -- they
|
||||
# stay on the hot/warm sub-poll cadence. on_notification discards
|
||||
# an href once it pushes, so a late first notify self-corrects.
|
||||
self.fallback_hrefs: set[str] = set()
|
||||
self._on_applied: Callable[[str, dict, str], None] | None = None
|
||||
self._refresh_task: ObserveRefreshTask | None = None
|
||||
@@ -205,6 +212,10 @@ class ObserveManager:
|
||||
return
|
||||
with self._notify_cond:
|
||||
self._notified.add(href)
|
||||
# Snapshot in enter_observe_mode is at the 80% quorum, not the
|
||||
# full grace period -- a late first push (blockwise refetch)
|
||||
# must drop the href so we don't keep GET-polling a live one.
|
||||
self.fallback_hrefs.discard(href)
|
||||
self._last_notify_ts = time.monotonic()
|
||||
self._notify_cond.notify_all()
|
||||
self.log.debug("observe notify: %s", href)
|
||||
@@ -270,6 +281,13 @@ class ObserveManager:
|
||||
#294) -- committing against a session a reconnect already replaced
|
||||
would claim observe mode with nothing left to notice it's dead."""
|
||||
self.subscribed_hrefs = set(subscribed)
|
||||
# Issue #92: subscribed-but-silent hrefs are counted as covered by
|
||||
# push if we drop this, but they never emit a notify. Keep them on
|
||||
# the poll cadence via fallback_hrefs (otherwise idle in observe).
|
||||
# Same lock as on_notification's discard so a notify in this window
|
||||
# cannot land on a set object that is about to be replaced.
|
||||
with self._notify_cond:
|
||||
self.fallback_hrefs = set(subscribed) - self._notified
|
||||
self._set_mode(MODE_OBSERVE)
|
||||
self.start_refresh_task(session)
|
||||
|
||||
|
||||
@@ -2,6 +2,8 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import contextlib
|
||||
import time
|
||||
from datetime import timedelta
|
||||
from unittest.mock import AsyncMock, patch
|
||||
@@ -1050,6 +1052,110 @@ async def test_sweep_mismatch_forces_subpolls_on_a_live_observe_session(
|
||||
mock_subpolls.assert_called_once_with(force=True)
|
||||
|
||||
|
||||
async def _cancel_background_subpolls(coordinator: LocalThingsCoordinator) -> None:
|
||||
"""Setup starts `_run_subpolls` as a background task. Tests that drive
|
||||
it directly have to cancel that one first, or a patched
|
||||
`_poll_hrefs_blocking` also captures its batches."""
|
||||
task = coordinator._subpoll_task
|
||||
if task is None:
|
||||
return
|
||||
task.cancel()
|
||||
coordinator._subpoll_task = None
|
||||
with contextlib.suppress(asyncio.CancelledError):
|
||||
await task
|
||||
|
||||
|
||||
async def test_observe_mode_subpolls_only_silent_hrefs(
|
||||
hass: HomeAssistant, mock_entry, mock_coordinator_observe_session
|
||||
) -> None:
|
||||
"""Issue #92: subscribed-but-silent hrefs keep the hot/warm cadence
|
||||
instead of waiting for the 30s sweep."""
|
||||
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]
|
||||
await _cancel_background_subpolls(coordinator)
|
||||
assert coordinator._hot_hrefs
|
||||
silent = coordinator._hot_hrefs[0]
|
||||
coordinator._observe.mode = MODE_OBSERVE
|
||||
coordinator._observe.fallback_hrefs = {silent}
|
||||
|
||||
polled: list[list[str]] = []
|
||||
|
||||
def _capture(hrefs):
|
||||
polled.append(list(hrefs))
|
||||
|
||||
with (
|
||||
patch.object(coordinator, "_poll_hrefs_blocking", side_effect=_capture),
|
||||
patch(
|
||||
"custom_components.localthings.coordinator.asyncio.sleep",
|
||||
new_callable=AsyncMock,
|
||||
),
|
||||
):
|
||||
await coordinator._run_subpolls()
|
||||
|
||||
assert polled
|
||||
for batch in polled:
|
||||
assert set(batch) == {silent}
|
||||
|
||||
|
||||
async def test_observe_mode_skips_subpolls_when_nothing_is_silent(
|
||||
hass: HomeAssistant, mock_entry, mock_coordinator_observe_session
|
||||
) -> None:
|
||||
"""The observe-mode no-op stays in place when every subscribed href
|
||||
actually notified -- issue #92 only keeps the silent ones on poll."""
|
||||
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]
|
||||
await _cancel_background_subpolls(coordinator)
|
||||
coordinator._observe.mode = MODE_OBSERVE
|
||||
coordinator._observe.fallback_hrefs = set()
|
||||
|
||||
with (
|
||||
patch.object(coordinator, "_poll_hrefs_blocking") as mock_poll,
|
||||
patch(
|
||||
"custom_components.localthings.coordinator.asyncio.sleep",
|
||||
new_callable=AsyncMock,
|
||||
) as mock_sleep,
|
||||
):
|
||||
await coordinator._run_subpolls()
|
||||
|
||||
mock_poll.assert_not_called()
|
||||
mock_sleep.assert_not_called()
|
||||
|
||||
|
||||
async def test_observe_mode_skips_empty_subpoll_slots(
|
||||
hass: HomeAssistant, mock_entry, mock_coordinator_observe_session
|
||||
) -> None:
|
||||
"""Once hot is empty, odd slots would otherwise take the session lock
|
||||
and dispatch a no-op executor job. Skip those."""
|
||||
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]
|
||||
await _cancel_background_subpolls(coordinator)
|
||||
coordinator._observe.mode = MODE_OBSERVE
|
||||
coordinator._hot_hrefs = []
|
||||
coordinator._warm_hrefs = ["/warm/vs/0"]
|
||||
coordinator._observe.fallback_hrefs = {"/warm/vs/0"}
|
||||
|
||||
polled: list[list[str]] = []
|
||||
|
||||
def _capture(hrefs):
|
||||
polled.append(list(hrefs))
|
||||
|
||||
with (
|
||||
patch.object(coordinator, "_poll_hrefs_blocking", side_effect=_capture),
|
||||
patch(
|
||||
"custom_components.localthings.coordinator.asyncio.sleep",
|
||||
new_callable=AsyncMock,
|
||||
),
|
||||
):
|
||||
await coordinator._run_subpolls()
|
||||
|
||||
assert polled
|
||||
for batch in polled:
|
||||
assert batch == ["/warm/vs/0"]
|
||||
|
||||
|
||||
async def test_write_marks_href_pending_before_post(
|
||||
hass: HomeAssistant, mock_entry, mock_coordinator_observe_session
|
||||
) -> None:
|
||||
|
||||
@@ -190,6 +190,7 @@ def test_try_enter_observe_mode_succeeds_when_all_hrefs_notify():
|
||||
assert entered is True
|
||||
assert mgr.mode == "observe"
|
||||
assert mgr.subscribed_hrefs == set(hrefs)
|
||||
assert mgr.fallback_hrefs == set()
|
||||
finally:
|
||||
mgr.close()
|
||||
|
||||
@@ -218,6 +219,41 @@ def test_try_enter_observe_mode_falls_back_when_subscribe_fails_for_all():
|
||||
assert mgr.mode == "poll"
|
||||
|
||||
|
||||
def test_enter_observe_mode_keeps_silent_hrefs_on_fallback():
|
||||
"""Issue #92: subscribed hrefs that never notified stay on the poll
|
||||
cadence via fallback_hrefs, rather than being treated as push-covered."""
|
||||
mgr = _manager()
|
||||
session = _FakeSession()
|
||||
subscribed = {"/a/vs/0", "/b/vs/0", "/c/vs/0"}
|
||||
mgr.on_notification("/a/vs/0", cbor2.dumps({"x": 1}))
|
||||
mgr.on_notification("/b/vs/0", cbor2.dumps({"x": 1}))
|
||||
try:
|
||||
mgr.enter_observe_mode(session, subscribed)
|
||||
assert mgr.mode == "observe"
|
||||
assert mgr.subscribed_hrefs == subscribed
|
||||
assert mgr.fallback_hrefs == {"/c/vs/0"}
|
||||
finally:
|
||||
mgr.close()
|
||||
|
||||
|
||||
def test_on_notification_drops_href_from_fallback():
|
||||
"""A late first notify (after the 80% snapshot) self-corrects
|
||||
fallback_hrefs so a slow-but-pushing href is not polled for the rest
|
||||
of the session -- multi-block resources being the reliable victim."""
|
||||
mgr = _manager()
|
||||
session = _FakeSession()
|
||||
subscribed = {"/a/vs/0", "/b/vs/0", "/c/vs/0"}
|
||||
mgr.on_notification("/a/vs/0", cbor2.dumps({"x": 1}))
|
||||
mgr.on_notification("/b/vs/0", cbor2.dumps({"x": 1}))
|
||||
try:
|
||||
mgr.enter_observe_mode(session, subscribed)
|
||||
assert mgr.fallback_hrefs == {"/c/vs/0"}
|
||||
mgr.on_notification("/c/vs/0", cbor2.dumps({"x": 1}))
|
||||
assert mgr.fallback_hrefs == set()
|
||||
finally:
|
||||
mgr.close()
|
||||
|
||||
|
||||
def test_try_enter_observe_mode_meets_success_fraction_with_partial_notifies():
|
||||
mgr = _manager()
|
||||
session = _FakeSession()
|
||||
@@ -243,6 +279,7 @@ def test_try_enter_observe_mode_meets_success_fraction_with_partial_notifies():
|
||||
|
||||
assert entered is True
|
||||
assert mgr.mode == "observe"
|
||||
assert mgr.fallback_hrefs == {hrefs[3]}
|
||||
finally:
|
||||
mgr.close()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user