Not-Expected-Yet/ha-nvidia-shield
2
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 7 Wiki Activity

fix(adb): ADB-off is asked by every device path, not inferred from a missing poller (t23)

Browse Source 
The t22 review's O1, and the same bug class as t21's F1 — so it gets the same
structural treatment instead of a note.

WHAT WAS MEASURED. With a poller injected into an entry whose own option said
`adb_enabled: False`, the installed-apps sweep sent 4 real shells (the reviewer;
3 on this test's double, which yields one launchable package) and a label probe
sent 1 more. Both gated on `self._poller is None` — and the poller's absence is a
property of the SETUP code (`_build_poller_factory` returns None when ADB is off),
which is precisely the indirect guarantee the t21 repair had already shown to be
insufficient. Production was unreachable only through that setup path.

THE FIX. `_adb_disabled()` is now asked directly by every path in the manager
that can reach the device, and `_adb_active()` (option ON *and* a poller) is the
shared predicate for the ones that also need a transport. The `available` property
deliberately keeps its transport meaning (diagnostics reads it as "ADB is
configured"); a new docstring says so, because overloading it is how the gate got
lost in the first place.

Gated paths, and what each asks now:
  * intent scan — the option (t21, unchanged) AND the private shell site itself
  * installed-apps sweep — the option first, then poller+connected
  * installed-apps refresh (public entry) — returns the last table, starts nothing
  * app-label lookup / drain / the ONE probe site (`_dump_label`) — the option; a
    lookup returns None and queues NOTHING (no request, no task, no cache write)
  * ADB text entry, media keys, bring-to-foreground, digit and enter keys — `_adb_active()`
  * the heartbeat `_refresh` — the option: no wake probe, no media dump, no
    running-apps poll, no foreground fallback, no label drain, no logcat ensure
  * the watchdog — the option BEFORE the poller factory, so not even a factory can
    build a transport; and `_watchdog_cycle` refuses a direct call too (a TCP
    connect is device traffic)
  * the logcat stream — the option at the ensure gate, the option in
    `_open_logcat_stream`, and the ladder exits WITHOUT counting a failure (a local
    option must not auto-disable the option for the session)
  * the public `adb_poller` accessor — None while the option is off, which closes
    the shell paths that live OUTSIDE this module (the `adb_command` service, the
    screen-capture image and the intent shell) without reaching into them: all
    three already refuse on None with their own message
  * `test_connect` (the pairing probe) stays ungated on purpose: it runs BEFORE an
    entry exists, and the setup menu only offers that step when the ADB option is
    on (config_flow.py). `async_stop`/`_note_failure` disconnect calls are teardown,
    not traffic.

PROOF, BY TRAFFIC. tests/test_adb_off_gates_every_device_path.py drives every
audited path — as a parametrized AUDIT table plus one full cycle on one manager —
with `adb_enabled: False` and a double that WOULD answer everything, and asserts
the poller's complete traffic list (shells, polls, commands, connects) is EMPTY,
plus that nothing was cached (no intent answer, no app table, no stamp, no queue,
no store field, no wake reading). It also proves the watchdog never builds a
poller and never connects, that a direct cycle does not connect either, that the
option is a CHECK and not a latch (the same manager resumes the sweep, the scan,
the label queue, the media key, the digit key and the accessor when the option
flips back on), and that ADB-off and no-poller stay told apart.

PRE-FIX PROOF: the same file against `git archive HEAD` FAILS 16 of 23 tests, and
the failures print the measured traffic — the sweep's 3 shells, the label probe's
`pm dump … | grep …`, `input text hello`, `media:play`, `foreground:<pkg>`,
`digit:5`, `enter`, and 4 polls per heartbeat cycle.

HARNESS TRUTHFULNESS. The suites' managers were built with `entry=None`, which
means ADB is OFF (fail closed, matching `__init__.py`). Those harnesses now pass a
realistic entry whose option says ADB is on — they model production instead of the
one combination the gates block. No production behaviour changed for an ADB-on
entry: 923 passed / 5 skipped, up from 900 by this file.

Floors re-derived from the new walk (1661 blocks → 1627, same 2% band): raised,
because that is what the re-measurement says. Drift exit 0. Strings files
byte-identical.

NOT VERIFIED HERE: the device side is a scripted double, so what is proven is that
NOTHING WAS SENT, not what firmware would do; no real Shield was involved.
This commit is contained in:
James Mackay committed 2026-10-12 13:32:34 +01:00
1 parent 8f064b3247
commit de1c34777d
8 files changed
+651 -37
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

custom_components/nvidia_shield/connection.py
+137 -14
View File
@@ -617,7 +617,13 @@ class ShieldConnectionManager:
@property
def available(self) -> bool:
"""True when ADB is active for this entry (a poller exists)."""
"""True when a poller exists for this entry — a TRANSPORT fact (t23/O1).
Deliberately NOT the permission to talk to the device: the option is (see
:meth:`_adb_active`). Diagnostics reads this as "ADB is configured", and the
t22 review's scenario — a poller injected into an entry whose own option
says ADB is off — must not be able to turn it into traffic.
"""
return self._poller is not None
@property
@@ -804,8 +810,21 @@ class ShieldConnectionManager:
@property
def adb_poller(self):
"""The ADB poller (or None) — public accessor replacing the
``_poller`` reach-in in the adb_command service handler (F6)."""
"""The ADB poller this entry MAY use, or None — the public accessor (F6).
Read by the ``adb_command`` service, the screen-capture image and the
intent shell instead of reaching into ``_poller``. It answers the same
question as every other device path in this class: may this entry talk to
the device? So it is None while the entry's option says ADB is off (t23/O1)
— and those callers already treat None as "ADB is not available" and refuse
with their own message, which is what closes the shell paths that live
OUTSIDE this module without this module having to reach into them.
The private ``_poller`` stays the transport object for internal bookkeeping
(connect/disconnect/state), which is not traffic the user asked for.
"""
if self._adb_disabled():
return None
return self._poller
@property
@@ -937,7 +956,8 @@ class ShieldConnectionManager:
import shlex
poller = self._poller
if poller is None or not self.connected:
# t23/O1: the OPTION gates the shell, not the poller's existence.
if poller is None or not self.connected or self._adb_disabled():
return False
try:
result = await poller.shell(f"input text {shlex.quote(text)}")
@@ -982,6 +1002,17 @@ class ShieldConnectionManager:
# so there is no "unresolved" shape to fall through on.
return label or None
self._app_labels.pop(package, None) # stale — refetch on demand
if self._adb_disabled():
# t23/O1: ADB off means there is nothing to wait for — no request (so no
# task and no shell later) and no cache write. The name falls back to the
# user map / curated map / package id exactly as a failed fetch does, and
# because nothing was cached, the next render after ADB is turned back on
# asks again.
_LOGGER.debug(
"app label for %s not queued (ADB is switched off for this entry)",
package,
)
return None
if package not in self._app_label_requests and (
package not in self._app_label_inflight
):
@@ -1169,10 +1200,12 @@ class ShieldConnectionManager:
package,
)
poller = self._poller
if poller is None or not self.connected:
# t23/O1: the OPTION is asked at the ONE place a label dump is issued (the
# sweep's fan-out comes through here too), so this gate covers both paths.
if poller is None or not self.connected or self._adb_disabled():
_LOGGER.debug(
"app label for %s not fetched (ADB not connected) — will "
"retry on the next render",
"app label for %s not fetched (ADB not connected or switched off) "
"— will retry on the next render",
package,
)
self._app_label_failed.add(package)
@@ -1275,6 +1308,12 @@ class ShieldConnectionManager:
material with the poller, never the poller's socket.
"""
poller = self._poller
if self._adb_disabled():
# t23/O1: ADB off is not a poller problem, and this stream opens its OWN
# connection to the device — the option gates it like every other path.
raise RuntimeError(
"ADB is switched off for this entry — logcat stream cannot start"
)
if poller is None:
raise RuntimeError("poller gone — logcat stream cannot start")
@@ -1362,6 +1401,11 @@ class ShieldConnectionManager:
"""
delay = ADB_RECONNECT_BACKOFF_START
while not self._stopping and not self._logcat_disabled:
if not self._adb_active():
# t23/O1: ADB off ends the stream WITHOUT counting a failure — the
# device is not at fault for a local option, and five of those would
# auto-disable the stream for the session.
break
try:
await self._logcat_pass()
except asyncio.CancelledError:
@@ -1378,7 +1422,9 @@ class ShieldConnectionManager:
"""Start the consumer when the option is on, connected, enabled."""
if self._logcat_disabled or not self._entry_options_logcat():
return
if self._poller is None or not self.connected:
# t23/O1: option AND transport — a poller that exists while the option says
# ADB is off must not start a device stream.
if not self._adb_active() or not self.connected:
return
if self._logcat_task is None or self._logcat_task.done():
self._logcat_task = self._create_task(
@@ -1480,6 +1526,14 @@ class ShieldConnectionManager:
``APP_SWEEP_RETRY_SECONDS`` so a silent socket is not hammered; a sweep
that RAN stores its answer, including a genuinely empty list.
"""
if self._adb_disabled():
# t23/O1: no sweep is STARTED, and not even the retry-spacing bookkeeping
# below runs — nothing was attempted, so nothing is owed a retry. The last
# known table is returned as-is (empty when none was ever built).
_LOGGER.debug(
"installed-app refresh skipped (ADB is switched off for this entry)"
)
return self.installed_apps()
if not force and not self.installed_apps_stale:
return self.installed_apps()
if (
@@ -1512,6 +1566,16 @@ class ShieldConnectionManager:
empty answer that also blocks the retry for APP_LIST_TTL.
"""
poller = self._poller
# t23/O1: the OPTION is asked FIRST. The review measured this path sending 4
# real shells with a poller injected into an entry whose option said ADB is
# off: the poller's absence was doing the gating, and that is setup code.
if self._adb_disabled():
_LOGGER.debug(
"installed-app discovery skipped (ADB is switched off for this "
"entry) — the options page and the app list stay on what is known"
)
self._note_apps_unavailable(attempted=False)
return []
if poller is None or not self.connected:
_LOGGER.debug(
"installed-app discovery skipped (ADB unavailable) — the "
@@ -1629,7 +1693,9 @@ class ShieldConnectionManager:
def _drain_app_label_requests(self) -> None:
"""Start at most one label fetch per watchdog cycle."""
if not self._app_label_requests or self._poller is None:
# t23/O1: option AND poller. A queue that cannot be served stays queued (it is
# served once ADB is turned back on) instead of starting a task that cannot run.
if not self._app_label_requests or not self._adb_active():
return
package = min(self._app_label_requests)
self._app_label_requests.discard(package)
@@ -1654,15 +1720,34 @@ class ShieldConnectionManager:
return self._intents.state(package)
def _adb_disabled(self) -> bool:
"""Is ADB switched OFF for this entry (t16)?
"""Is ADB switched OFF for this entry (t16)? — the gate for EVERY device path (t23).
The config option is the honest signal: no poller can mean "off" OR
"enabled but not connected yet", and those are different statuses with
different fixes (turn ADB on / pair it, versus wait or reconnect).
t23/O1: every path in this class that can reach the device ASKS this, rather
than inferring it from a poller being absent. The two are not the same
guarantee: ``_build_poller_factory`` returns None when ADB is off, which
makes the poller's absence a property of the SETUP code, and the review
measured what that costs — with a poller injected into an entry whose own
option said ADB is off, the installed-apps sweep sent 4 real shells and a
label probe sent 1 more. ``available`` and ``connected`` keep describing the
TRANSPORT; this describes PERMISSION. An entry with no options at all is
treated as OFF (fail closed, matching ``__init__.py``).
"""
options = getattr(self.entry, "options", None) or {}
return not options.get(CONF_ADB_ENABLED, False)
def _adb_active(self) -> bool:
"""May this entry talk to the device right now (t23/O1)?
The two-part predicate the shared/background paths use: the OPTION says ADB
is on, and a poller exists to talk through. ``available`` alone answers the
transport half of that question, which is why it cannot be the gate.
"""
return self.available and not self._adb_disabled()
async def async_scan_intents(
self, package: str, force: bool = False
) -> IntentScanResult:
@@ -1753,6 +1838,17 @@ class ShieldConnectionManager:
why ``has_activity_resolver_table`` is asked before the parser is
believed.
"""
if not self._adb_active():
# t23/O1: the SHELL SITE itself asks the option (the public caller asked
# first), so no route reaches the device — the t21 repair, one layer down.
return IntentScanResult(
package=package,
status=(
INTENT_SCAN_ADB_DISABLED
if self._adb_disabled()
else INTENT_SCAN_ADB_UNAVAILABLE
),
)
poller = self._poller
if poller is None: # pragma: no cover - the caller checked this
return IntentScanResult(package=package, status=INTENT_SCAN_ADB_UNAVAILABLE)
@@ -1845,12 +1941,16 @@ class ShieldConnectionManager:
async def send_media_command(self, command: str) -> bool:
"""Send a media key command over the persistent socket."""
if not self.available:
# t23/O1: the option AND a poller — `available` alone is the transport half
# of the question, and a poller can exist while the option says ADB is off.
if not self._adb_active():
return False
return await self._poller.send_media_command(command)
async def bring_to_foreground(self, package: str) -> bool:
if not self.available:
# t23/O1: the option AND a poller — `available` alone is the transport half
# of the question, and a poller can exist while the option says ADB is off.
if not self._adb_active():
return False
return await self._poller.bring_to_foreground(package)
@@ -1898,13 +1998,17 @@ class ShieldConnectionManager:
async def send_digit_key(self, digit: str) -> bool:
"""Channel digit via the ADB keyevent path (fallback for §7)."""
if not self.available:
# t23/O1: the option AND a poller — `available` alone is the transport half
# of the question, and a poller can exist while the option says ADB is off.
if not self._adb_active():
return False
return await self._poller.send_digit_keyevent(digit)
async def send_enter_key(self) -> bool:
"""ENTER via the ADB keyevent path (KEYCODE_ENTER = 66, §7 fallback)."""
if not self.available:
# t23/O1: the option AND a poller — `available` alone is the transport half
# of the question, and a poller can exist while the option says ADB is off.
if not self._adb_active():
return False
return await self._poller.send_enter_keyevent()
@@ -1963,6 +2067,13 @@ class ShieldConnectionManager:
recreates it when options change (full reload).
"""
try:
if self._adb_disabled():
# t23/O1: ADB off means there is nothing to keep alive, and this sits
# BEFORE the factory — so even a factory that would build a poller
# cannot put traffic on the device. The entry reloads on an options
# change, which is what brings the loop back.
_LOGGER.debug("ADB watchdog not started (ADB is off for this entry)")
return
if self._poller is None:
if self._poller_factory is None:
return
@@ -1992,6 +2103,11 @@ class ShieldConnectionManager:
awake) media sessions + running apps, all stored with timestamps.
The current backoff sleeps after failures; success resets it."""
if self._adb_disabled():
# t23/O1: a DIRECT call must not connect either — a TCP connect is device
# traffic. The option is read live on every cycle, so a later flip is picked
# up (a check, never a latch).
return ADB_POLL_INTERVAL
try:
if not self.connected and not await self._poller.connect():
await self._note_failure(backoff)
@@ -2040,6 +2156,13 @@ class ShieldConnectionManager:
async def _refresh(self) -> None:
"""Heartbeat: pull all ADB-derived state into the store (with stamps)."""
if self._adb_disabled():
# t23/O1: the ADB half of the heartbeat — wake probe, media dump, running
# apps, foreground fallback, the label drain and the logcat ensure — does
# not run at all. (The protocol identity capture further down is protocol
# work, and it is reached only while the watchdog is running, which needs
# ADB on.)
return
poller = self._poller
awake = await poller.is_awake()
if awake is None: