diff --git a/README.md b/README.md index 609361a..ae8ccbb 100644 --- a/README.md +++ b/README.md @@ -448,6 +448,12 @@ not launchable apps — with the one exception that matters: the active launcher is a system app yet is exactly what the user sees. Nothing else is filtered, so an app is missing only if Android itself does not report it. +*(Every output shape above is the **documented** Android one, not an observed +capture: no real Shield was available when this was written, so +`pm list packages -3`, `resolve-activity --brief` and `pm dump` are exercised from +fixtures — the same "inferred, not observed" mark the state=6 buffering value +carries in the source ladder above.)* + Names come from the same cached path the Current App sensor uses: a `pm dump` per package, parsed for `application-label`, cached for 24 h. A full sweep runs the dumps in a small burst (4 at a time, 50 ms apart), which finishes a typical Shield in @@ -466,16 +472,22 @@ and `mdi:television` otherwise. The icon never enters your **icon column**: that is a picture URL or a `camera.*` entity and feeds `entity_picture`, so writing an `mdi:` name there would put a value the media player cannot use into your own map. -Discovery never runs on a state poll: it happens when you press the row, and the result -is cached for 6 h. With ADB off, or unreachable, the table simply shows the rows you -wrote yourself and the menu says why. +Discovery does not run on the watchdog: the Configure row runs it, the dashboard +sensor refreshes the table when it has no fresh one, and the result is cached for 6 h. +With ADB off, or unreachable, the table simply shows the rows you wrote yourself and +the menu says why. A sweep that **could not run** is not cached as an answer: it is +recorded as `unknown` and retried, so a Shield that was asleep at startup is picked up +by the next refresh instead of reporting an empty device for six hours. **For dashboards** the same table is a sensor: `sensor._installed_apps`, whose state is the app **count** and whose `installed_apps` attribute is the list of -`{package, name, icon}`. Its `discovery` attribute says `ok`, `pending`, `not_connected` -or `adb_disabled`. It only exists for ADB-enabled entries, and it is cheap to refresh — -`homeassistant.update_entity` on it re-runs the sweep only when the cache has expired, -while the Configure row forces one. +`{package, name, icon}`. Its `discovery` attribute says `ok`, `pending`, `unknown`, +`not_connected` or `adb_disabled`: `ok` means the table IS the answer — including a +genuinely empty one — and `unknown` means the last sweep could not run (an empty list +then says nothing about the device, and the next refresh retries). It only exists for +ADB-enabled entries, and it is cheap to refresh — `homeassistant.update_entity` on it +re-runs the sweep only when there is no fresh table (the cache expired, or the last +sweep could not run), while the Configure row forces one. ```yaml # every installed app, with the name this integration would show diff --git a/custom_components/nvidia_shield/connection.py b/custom_components/nvidia_shield/connection.py index d1a38a4..9bdc1df 100644 --- a/custom_components/nvidia_shield/connection.py +++ b/custom_components/nvidia_shield/connection.py @@ -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). diff --git a/custom_components/nvidia_shield/sensor.py b/custom_components/nvidia_shield/sensor.py index c3957ad..9c046da 100644 --- a/custom_components/nvidia_shield/sensor.py +++ b/custom_components/nvidia_shield/sensor.py @@ -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 diff --git a/tests/test_installed_apps.py b/tests/test_installed_apps.py index e3b6bc2..efbfc3f 100644 --- a/tests/test_installed_apps.py +++ b/tests/test_installed_apps.py @@ -393,7 +393,11 @@ class TestDiscoverySweep: class TestDiscoveryDegradesGracefully: - """ADB off, down or mute: an empty list and a note, never a crash or a loop.""" + """ADB off, down or mute: an empty list and a note, never a crash or a loop. + + t62 F1: "could not ask" and "asked, nothing there" are different answers. + Only the second one is cached; the first is retried. + """ async def test_no_poller_is_an_empty_list(self): manager = ShieldConnectionManager(hass=None, entry=None, poller=None) @@ -416,19 +420,64 @@ class TestDiscoveryDegradesGracefully: assert rows == [] assert poller.shell_calls == [] # nothing was even attempted - async def test_a_mute_transport_reports_empty_without_raising(self): + async def test_a_mute_transport_is_spaced_but_not_stamped(self): + """t62 F1: a failed sweep is retried — but not once per state poll.""" + now = [1000.0] poller = _DiscoveryPoller(failure=True) - manager = _manager(poller) + manager = ShieldConnectionManager( + hass=None, entry=None, poller=poller, clock=lambda: now[0] + ) rows = await manager.async_refresh_installed_apps() assert rows == [] assert poller.shell_calls # it tried - # and a second call does NOT re-shell in a loop: the empty answer is - # stamped, and only an explicit force/TTL asks again + assert manager.installed_apps_status == "unknown" # NOT an empty answer + # the next poll inside the window does NOT re-shell (no hammering).. calls = len(poller.shell_calls) assert await manager.async_refresh_installed_apps() == [] assert len(poller.shell_calls) == calls + # ..but nothing was stamped as an answer, so once the window passes the + # sweep runs again. Pre-fix it waited out APP_LIST_TTL (6 h) instead. + now[0] += manager.APP_SWEEP_RETRY_SECONDS + 1 + assert await manager.async_refresh_installed_apps() == [] + assert len(poller.shell_calls) > calls + + async def test_a_sweep_that_could_not_run_is_not_a_fresh_answer(self): + """t62 F1 (red proof): startup with ADB down must not BE the answer. + + The entity's one startup sweep runs with the poller down. Once ADB comes + back, ``homeassistant.update_entity`` (``sensor.async_update``) must + recover the table. Pre-fix the startup sweep stamped an empty answer, so + this call issued ZERO shells and kept reporting 0/"pending" for + APP_LIST_TTL = 6 h. + """ + poller = _DiscoveryPoller(dumps={"ar.tvplayer.tv": DUMP_TIVIMATE}) + poller.connected = False + manager = _manager(poller) + + assert await manager.async_refresh_installed_apps() == [] # the startup + assert poller.shell_calls == [] # it was not even attempted + assert manager.installed_apps_stale is True # ...so it is not an answer + assert manager.installed_apps_status == "unknown" + + poller.connected = True # ADB is back + rows = await manager.async_refresh_installed_apps() + + assert rows, "update_entity must recover the table once ADB returns" + assert "pm list packages -3" in poller.shell_calls + assert manager.installed_apps_status == "ok" + + async def test_a_genuinely_empty_answer_is_ok(self): + """t62 F1: 'ok' must be reachable for a really empty device.""" + poller = _DiscoveryPoller(packages="", launcher="") + manager = _manager(poller) + + assert await manager.async_refresh_installed_apps() == [] + + assert poller.shell_calls # the commands RAN and answered + assert manager.installed_apps_stale is False # stamped: a real answer + assert manager.installed_apps_status == "ok" async def test_the_accessor_never_shells(self): poller = _DiscoveryPoller() @@ -534,6 +583,31 @@ class TestInstalledAppsSensor: assert attrs["installed_apps"] == [] assert attrs["discovery"] in ("pending", "not_connected", "adb_disabled") + async def test_an_empty_discovery_is_ok_not_pending(self): + """t62 F1: a sweep that RAN and found nothing is the answer 'ok'.""" + poller = _DiscoveryPoller(packages="", launcher="") + manager = _manager(poller) + await manager.async_refresh_installed_apps() + sensor = self._sensor(manager, {"adb_enabled": True}) + + assert sensor.native_value == 0 + assert sensor.extra_state_attributes == { + "installed_apps": [], + "discovery": "ok", + } + + async def test_a_sweep_that_could_not_run_is_unknown_not_pending(self): + """t62 F1: 'pending' used to cover both "running" and "could not run".""" + poller = _DiscoveryPoller(failure=True) + manager = _manager(poller) + await manager.async_refresh_installed_apps() + sensor = self._sensor(manager, {"adb_enabled": True}) + + assert sensor.extra_state_attributes == { + "installed_apps": [], + "discovery": "unknown", + } + def test_adb_disabled_is_reported_as_such(self): poller = _DiscoveryPoller() manager = _manager(poller)