fix(discovery): a sweep that could not run is not an answer (t62 F1)
The installed-apps sweep stamped an EMPTY answer on every failure path, so a Shield that was asleep at the entity's startup sweep was cached as "0 apps" for APP_LIST_TTL (6 h): `homeassistant.update_entity` then issued zero shells and the sensor kept reporting 0/"pending" until the TTL or the Configure row's force. A genuinely empty device was indistinguishable from it, so "ok" was unreachable for an empty list. Only a sweep that RAN stamps the cache now. Every failure records the outcome as `unknown` (`_apps_unavailable`), stores no table, and is retried by the next non-forced refresh; a sweep that reached the device and failed is spaced by APP_SWEEP_RETRY_SECONDS (30 s, force=True bypasses) so a silent socket is not hammered once per poll, while an unavailable transport costs nothing and is retried immediately. `installed_apps_status` exposes ok/unknown/pending, the sensor surfaces it, and "ok" + an empty table is now the honest empty answer. Docstrings, the README vocabulary and the docs on what a refresh actually re-runs are corrected; the "update_entity re-runs only without a fresh table, the Configure row forces one" wording stays true. Red proof (pre-fix sources, current tests): the five new/updated tests fail on HEAD, e.g. `installed_apps_stale` is False after the ADB-down startup sweep, and the sensor reports 'pending' where it must say 'unknown'. Same class already fixed for app labels: a fetch that could not run is not an answer. Also records that the `-3`/resolve-activity/pm-dump output shapes are documented, not observed.
This commit is contained in:
1 parent
ef42682250
commit
c05e6fa929
4 files changed
+217
-43
No files matched your search
@@ -224,10 +224,22 @@ class ShieldConnectionManager:
|
||||
# t55: packages whose label fetch is queued or in flight — a render
|
||||
# during the fetch must not enqueue the same package twice.
|
||||
self._app_label_inflight: set[str] = set()
|
||||
# t57: the discovered installed-apps table — (rows, stamp), or an empty
|
||||
# row list with a stamp when ADB cannot answer. APP_LIST_TTL, refreshed
|
||||
# on demand; never rebuilt on a state poll.
|
||||
# t57: the discovered installed-apps table — (rows, stamp), or None when
|
||||
# no sweep has produced a fresh answer. Only a sweep that RAN writes
|
||||
# here (t62 F1): a failed or unavailable sweep records itself as UNKNOWN
|
||||
# instead, because stamping an empty answer would serve "0 apps" — and
|
||||
# block every retry — for the whole APP_LIST_TTL. Refreshed on demand;
|
||||
# never rebuilt on a state poll.
|
||||
self._installed_apps: tuple[list[dict], float] | None = None
|
||||
# t62 F1: True when the last sweep could not deliver an answer (no
|
||||
# poller, ADB down, a shell that raised or answered nothing) — so an
|
||||
# empty installed_apps() is "unknown", not "this device has no apps".
|
||||
self._apps_unavailable = False
|
||||
# ...and the clock stamp of an ATTEMPTED failure (shells really went
|
||||
# out). A retry then costs two listing shells, so it is spaced by
|
||||
# APP_SWEEP_RETRY_SECONDS instead of firing on every state poll; an
|
||||
# unavailable transport costs nothing and retries immediately.
|
||||
self._apps_failed_at: float | None = None
|
||||
#: t57: the in-flight sweep, so concurrent callers share one sweep
|
||||
#: instead of stacking a second burst on the same connection.
|
||||
self._apps_sweep: asyncio.Task | None = None
|
||||
@@ -888,12 +900,22 @@ class ShieldConnectionManager:
|
||||
|
||||
# ── t57: installed-app discovery ───────────────────────────────────────
|
||||
|
||||
#: t62 F1: how long after an ATTEMPTED sweep failed before another one is
|
||||
#: issued. A retry costs two listing shells, so it waits out one state poll
|
||||
#: (30 s) rather than hammering a silent socket; ``force=True`` bypasses it.
|
||||
#: An unavailable transport (no poller, ADB down) costs nothing and is
|
||||
#: retried on the very next update.
|
||||
APP_SWEEP_RETRY_SECONDS: float = 30.0
|
||||
|
||||
def installed_apps(self) -> list[dict]:
|
||||
"""The discovered apps, ``[{package, name, icon}]``, cached (t57).
|
||||
|
||||
Returns the cached table whenever it is fresh, and an EMPTY list when
|
||||
there is nothing usable yet (ADB disabled, not connected, or the first
|
||||
sweep still running). A caller that wants the table filled calls
|
||||
there is no fresh answer: ADB disabled, not connected, the first sweep
|
||||
still running, or a sweep that could not run (t62 F1). Empty is
|
||||
therefore "nothing to show", never by itself "this device has no apps"
|
||||
— ask :meth:`installed_apps_status` which of the two it is. A caller
|
||||
that wants the table filled calls
|
||||
:meth:`async_refresh_installed_apps`; this accessor never blocks and
|
||||
never issues a shell itself, so a state poll stays free of discovery
|
||||
work.
|
||||
@@ -913,6 +935,23 @@ class ShieldConnectionManager:
|
||||
_rows, stamp = self._installed_apps
|
||||
return self._clock() - stamp > APP_LIST_TTL
|
||||
|
||||
@property
|
||||
def installed_apps_status(self) -> str:
|
||||
"""Why :meth:`installed_apps` may be empty, in one word (t62 F1).
|
||||
|
||||
* ``"ok"`` — a sweep RAN and its answer is fresh. An empty table with
|
||||
this status is a real answer (the device reported no launchable
|
||||
apps), which is the case the old stamped-empty cache could not say.
|
||||
* ``"unknown"`` — the last sweep could not deliver an answer (no
|
||||
poller, ADB down, a shell that raised or answered nothing). Nothing
|
||||
is known about the device, and the next update retries.
|
||||
* ``"pending"`` — no sweep has completed yet, or the last answer
|
||||
expired and a refresh is due.
|
||||
"""
|
||||
if not self.installed_apps_stale:
|
||||
return "ok"
|
||||
return "unknown" if self._apps_unavailable else "pending"
|
||||
|
||||
async def async_refresh_installed_apps(self, force: bool = False) -> list[dict]:
|
||||
"""(Re)build the installed-app table over ADB, bounded and shared (t57).
|
||||
|
||||
@@ -921,13 +960,26 @@ class ShieldConnectionManager:
|
||||
``force``. The sweep is deliberately NOT part of the watchdog: it is a
|
||||
handful of shells on demand, not a per-cycle cost.
|
||||
|
||||
Failure is normal and handled without noise — no poller, a disconnected
|
||||
transport, or a shell that answers nothing stores an EMPTY table with a
|
||||
stamp, so the caller degrades to "no discovered apps" and nothing
|
||||
retries in a loop. The next explicit refresh (or the TTL) tries again.
|
||||
Failure is not an answer (t62 F1). No poller, a disconnected transport,
|
||||
a shell that raises, or a sweep that delivered no output records
|
||||
``unknown`` and stores NO table, so the next non-forced call — an
|
||||
entity poll or the standard ``update_entity`` action — retries instead
|
||||
of serving a stamped empty answer for the whole APP_LIST_TTL. A sweep
|
||||
that reached the device and failed is retried at most once per
|
||||
``APP_SWEEP_RETRY_SECONDS`` so a silent socket is not hammered; a sweep
|
||||
that RAN stores its answer, including a genuinely empty list.
|
||||
"""
|
||||
if not force and not self.installed_apps_stale:
|
||||
return self.installed_apps()
|
||||
if (
|
||||
not force
|
||||
and self._apps_failed_at is not None
|
||||
and self._clock() - self._apps_failed_at < self.APP_SWEEP_RETRY_SECONDS
|
||||
):
|
||||
# The previous attempt really issued shells and failed: space the
|
||||
# retries by one poll interval instead of re-issuing them every
|
||||
# 30 s on a socket that is not answering (t62 F1).
|
||||
return self.installed_apps()
|
||||
if self._apps_sweep is not None and not self._apps_sweep.done():
|
||||
await asyncio.shield(self._apps_sweep)
|
||||
return self.installed_apps()
|
||||
@@ -941,14 +993,20 @@ class ShieldConnectionManager:
|
||||
return [dict(row) for row in rows]
|
||||
|
||||
async def _sweep_installed_apps(self) -> list[dict]:
|
||||
"""The sweep itself: list, label, icon — then stamp the cache."""
|
||||
"""The sweep itself: list, label, icon — then stamp the cache (t57).
|
||||
|
||||
Only a sweep that RAN stamps the cache (t62 F1): every failure path
|
||||
records ``unknown`` and leaves ``_installed_apps`` alone, so the table
|
||||
the entity serves is the last one that was real — never a fabricated
|
||||
empty answer that also blocks the retry for APP_LIST_TTL.
|
||||
"""
|
||||
poller = self._poller
|
||||
if poller is None or not self.connected:
|
||||
_LOGGER.debug(
|
||||
"installed-app discovery skipped (ADB unavailable) — the "
|
||||
"options page and the app list stay on what is already known"
|
||||
)
|
||||
self._installed_apps = ([], self._clock())
|
||||
self._note_apps_unavailable(attempted=False)
|
||||
return []
|
||||
try:
|
||||
user_output = await poller.shell("pm list packages -3")
|
||||
@@ -958,15 +1016,15 @@ class ShieldConnectionManager:
|
||||
)
|
||||
except Exception as err: # pragma: no cover - shell swallows its own
|
||||
_LOGGER.debug("installed-app discovery failed: %s", err)
|
||||
self._installed_apps = ([], self._clock())
|
||||
self._note_apps_unavailable(attempted=True)
|
||||
return []
|
||||
if user_output is None and launcher_output is None:
|
||||
# the transport delivered nothing at all: no list is an ANSWER
|
||||
# the transport delivered nothing at all: no list is NOT an answer
|
||||
_LOGGER.debug(
|
||||
"installed-app discovery got no output — will try again on the "
|
||||
"next refresh"
|
||||
"installed-app discovery got no output — will retry on the "
|
||||
"next update"
|
||||
)
|
||||
self._installed_apps = ([], self._clock())
|
||||
self._note_apps_unavailable(attempted=True)
|
||||
return []
|
||||
|
||||
packages = [p for p in installed_packages(user_output, launcher_output)
|
||||
@@ -981,9 +1039,20 @@ class ShieldConnectionManager:
|
||||
for package in packages
|
||||
]
|
||||
self._installed_apps = (rows, self._clock())
|
||||
self._apps_unavailable = False
|
||||
self._apps_failed_at = None
|
||||
_LOGGER.debug("installed-app discovery found %d apps", len(rows))
|
||||
return rows
|
||||
|
||||
def _note_apps_unavailable(self, *, attempted: bool) -> None:
|
||||
"""Record a sweep that could not deliver an answer (t62 F1).
|
||||
|
||||
``attempted`` says whether shells were actually issued: only then does
|
||||
the retry window apply, because only then does a retry cost anything.
|
||||
"""
|
||||
self._apps_unavailable = True
|
||||
self._apps_failed_at = self._clock() if attempted else None
|
||||
|
||||
async def _sweep_labels(self, packages: list[str]) -> dict[str, str]:
|
||||
"""Labels for a whole sweep, in a bounded burst (t57).
|
||||
|
||||
|
||||
@@ -167,10 +167,14 @@ class NvidiaShieldAppsSensor(NvidiaShieldSensorBase):
|
||||
Only ADB-enabled entries get one. Discovery is a handful of shells, so it
|
||||
runs once when the entity is added and then only when the table has gone
|
||||
stale: ``async_update`` goes through the manager's TTL cache, so a poll on a
|
||||
fresh table issues NO shell. On top of that the user can ask for a refresh
|
||||
with the standard ``homeassistant.update_entity`` action, and the Configure
|
||||
page runs the same sweep from its discovery row. With ADB off, or
|
||||
unreachable, the attributes say why and the state is 0 rather than raising.
|
||||
fresh table issues NO shell. A sweep that could not run is NOT an answer
|
||||
(t62 F1): it is recorded as ``unknown`` and retried on the next update
|
||||
instead of being frozen as a stamped-empty table for APP_LIST_TTL, while a
|
||||
sweep that ran and found nothing IS the answer ``ok`` with a count of 0. On
|
||||
top of that the user can ask for a refresh with the standard
|
||||
``homeassistant.update_entity`` action, and the Configure page runs the same
|
||||
sweep from its discovery row. With ADB off, or unreachable, the attributes
|
||||
say why and the state is 0 rather than raising.
|
||||
"""
|
||||
|
||||
_attr_translation_key = "installed_apps"
|
||||
@@ -178,7 +182,9 @@ class NvidiaShieldAppsSensor(NvidiaShieldSensorBase):
|
||||
# Polling is what makes the standard `homeassistant.update_entity` action
|
||||
# work for it — the user-facing "refresh the app list" trigger. The poll is
|
||||
# free while the table is fresh: the manager returns the cache and issues no
|
||||
# shell until APP_LIST_TTL expires.
|
||||
# shell until APP_LIST_TTL expires. A sweep that COULD NOT run leaves no
|
||||
# fresh table (t62 F1), so a later poll retries it — that retry is the
|
||||
# recovery the old stamped-empty answer blocked for up to six hours.
|
||||
_attr_should_poll = True
|
||||
|
||||
def __init__(self, hass: HomeAssistant, entry: ConfigEntry) -> None:
|
||||
@@ -203,7 +209,13 @@ class NvidiaShieldAppsSensor(NvidiaShieldSensorBase):
|
||||
self.async_write_ha_state()
|
||||
|
||||
async def async_update(self) -> None:
|
||||
"""Refresh the table on demand (explicitly requested only)."""
|
||||
"""Refresh the table when there is no fresh answer (t57, t62 F1).
|
||||
|
||||
HA's own poll (``_attr_should_poll``) and an explicit
|
||||
``homeassistant.update_entity`` both land here; the manager's TTL makes
|
||||
it a no-op while the table is fresh, and a retry while it is not —
|
||||
including after a sweep that could not run.
|
||||
"""
|
||||
manager = self._manager()
|
||||
if manager is None:
|
||||
return
|
||||
@@ -216,21 +228,28 @@ class NvidiaShieldAppsSensor(NvidiaShieldSensorBase):
|
||||
|
||||
@property
|
||||
def extra_state_attributes(self) -> dict:
|
||||
"""The table itself, plus why it may be empty (never a crash)."""
|
||||
"""The table itself, plus why it may be empty (never a crash).
|
||||
|
||||
``discovery`` is the honest reason (t62 F1): ``ok`` means the table IS
|
||||
the answer, including a genuinely empty one, while ``unknown`` means
|
||||
the last sweep could not run — so an empty list says nothing about the
|
||||
device and the next update retries.
|
||||
"""
|
||||
manager = self._manager()
|
||||
if manager is None:
|
||||
return {"installed_apps": [], "discovery": "unavailable"}
|
||||
apps = manager.installed_apps()
|
||||
attrs = {"installed_apps": apps}
|
||||
if not apps:
|
||||
if not self._entry.options.get(CONF_ADB_ENABLED, True):
|
||||
attrs["discovery"] = "adb_disabled"
|
||||
elif not manager.connected:
|
||||
attrs["discovery"] = "not_connected"
|
||||
else:
|
||||
attrs["discovery"] = "pending"
|
||||
else:
|
||||
if apps:
|
||||
attrs["discovery"] = "ok"
|
||||
elif not self._entry.options.get(CONF_ADB_ENABLED, True):
|
||||
attrs["discovery"] = "adb_disabled"
|
||||
elif not manager.connected:
|
||||
attrs["discovery"] = "not_connected"
|
||||
else:
|
||||
# "ok" here is a real empty answer, "unknown" a sweep that could
|
||||
# not run, "pending" one that has not completed yet.
|
||||
attrs["discovery"] = manager.installed_apps_status
|
||||
return attrs
|
||||
|
||||
|
||||
|
||||
Reference in new issue
Block a user