afc3326fc3fe5975c3b255f82b5da1f060a920dd
179
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 engineer
|
afc3326fc3 |
docs(discovery): precise wording on the watchdog, the empty state, recovery
Three comments this repair touches were still loose: "never rebuilt on a state poll" (the discovery table, not the watchdog — the sensor's own poll does refresh it when it has no fresh answer), and the class docstring's "recorded as unknown" now says where a reader actually sees it, since the sensor reports not_connected while the transport is down and unknown once it is up but the sweep delivered nothing. |
||
|
engineer
|
1c414c2f70 |
docs(discovery): mark the command shapes documented, not observed (t62 nit c)
The "no real Shield was available, so this output shape is Android's documentation rather than a capture" mark lived only in the test module, where a reader of the discovery code never looks. It now sits in the module docstring next to the commands it qualifies, and in the README's app-discovery section next to the command table, matching the state=6 buffering inference mark in the source ladder. |
||
|
engineer
|
93d0afcdcf |
test(drift): flag unreachable statements and raise the call-argument floor (t62 F2)
Two gate gaps let the t62 findings ship, and both are closed here. A pure-AST unreachable-statement pass (scan(): pass 6, entry point scan_unreachable()): in every statement block, a statement after return/raise/continue/break can never run. Per-block on purpose — an `else`, an except handler or a nested function is a different block and not dead, so the pass stays signal. Red proof: the pre-fix connection.py, saved from HEAD, is flagged as `connection.py:1113: unreachable-statement: return at line 1110 ends this block`; the live tree is clean. A new counter (statement_blocks, floor 500 of ~1102) keeps the "no findings" verdict from being a pass that stopped parsing. The call-argument floor was 20 while the walk counted 46, so deleting a flow-display owner from HA_CALL_OWNERS did not trip it. It is 44 now, and the suite proves the trip: removing async_show_menu yields 40 checked arguments and `only 40 call_args (floor: 44)`. Measured removals: show_form 25, abort 38, show_menu 40, create_entry 41, update_entry 41 all violate; the single-call-site async_update_reload_and_abort (45) is the one owner a count floor cannot watch, which is why the floor sits two below the count. Also (t62 nit a): the two comments that pinned the t40 entity-attribute finding to ``media_player.py:828`` now name the file and the attribute instead — the line had rotted to an unrelated property. |
||
|
engineer
|
0ef72a1fbb |
fix(connection): delete the unreachable duplicated label-drain body (t62 F2)
`_sweep_labels` ended with `return cached`, and the whole body of `_drain_app_label_requests` — its docstring, the queue check, the fetch task — followed it at the same indent level. Valid Python, never executed, and invisible to every gate: the sweep, ruff and the stub suite all pass over it. This is the second shipped artefact of its kind (an earlier review found a duplicated statement), so the mechanism lands in the drift sweep next; here the dead copy goes. The `; echo __NSH_EXIT:$?` marker in `_dump_label` is documented while it is touched (t62 nit b): it sits after a `head` pipeline, Android's sh has no PIPESTATUS, so `$?` is head's status and never the dump's. Nothing reads its VALUE — the parser strips the line — and the property that matters is that a command which RAN leaves non-blank output, which is what separates "ran, no label" (a cached negative) from "the transport delivered nothing" (a retry). |
||
|
engineer
|
c05e6fa929 |
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. |
||
|
James Mackay
|
ef42682250 |
fix(media-player): ON means no power or playback fact reported (t58 UNIT 0)
The t59 F2 ruling, implemented. The ladder said ON was for "no source reported
anything at all" but tested for something looser — no transport data — so a
source that ANSWERED "not playing" still fell through to ON. A source that
answers is not a source that is silent, and `on` is the "we cannot know" state.
The ladder now tracks an explicit `reported` flag, set by every source that gave
us a power or playback fact, and the last tier is `IDLE if reported else ON`.
Each case the ruling names now lands on IDLE:
* a configured Jellyfin entity in ANY state — idle, off, standby, unavailable,
or even absent from the state machine. It reports "not playing" without an
early return, so a silent Jellyfin still cannot mask a live media session
below it (both directions pinned);
* a live media session that carries no playback flag — the app publishes a
session but is not playing, paused or buffering;
* a bare foreground app: either transport's answer counts (the protocol's
current_app push or the ADB mResumedActivity parse);
* a configured power entity, whatever it currently says;
* protocol is_on / ADB awake, plugged or not.
`on` survives only for an entry where none of those exists. The t55 rules are
untouched: an off power entity still outranks a playing session, and no source
may mask playing/paused/buffering.
The module docstring's tier 6, the `state` docstring and the README's ladder
(moving "no data" to the precise wording) all say the same thing now, and the
README's one-line state table was aligned with it.
RED PROOF (the new tests against the parent tree): five of the reporting cases
read `on` where they must read `idle` — `assert 'on' == 'idle'` — plus the
"ON is unreachable with any source configured" shape test.
SIMULATED vs EXECUTED: no real Shield was available, so these are the documented
source shapes, not captures; the state=6 → buffering mapping stays marked
inferred. No browser exists either, so the README pins are textual and the
production behaviour is exercised against real HA's state machine in
tests/test_ha_api_drift.py.
|
||
|
James Mackay
|
ff8dd1c454 |
fix(discovery): a cached negative is an answer, not a miss (t57)
The sweep asked `app_label()` whether a package still needed a label, and that accessor answers "what should be displayed" — so a definitive negative (a package whose pm dump ran and carried no label) came back as None, indistinguishable from "never asked". Every sweep therefore re-dumped every label-less package, throwing away the 24 h cache the label path exists for, and a second sweep cost a full dump round for the whole app list. `_known_label(package) -> (known, label)` is the accessor that keeps the two apart: known-and-empty means the device already answered, so the row falls back to its package id with ZERO further round trips. A forced re-sweep now issues only the two list commands. Pinned by tests: a second sweep dumps nothing at all (negatives included), and a package whose fetch FAILED (no output — the UNIT 0 rule) stays uncached and is retried by the next sweep, recovering its label. |
||
|
James Mackay
|
364d4b7da3 |
test(discovery): pin that the discovered list is a superset
The live complaint in one assertion: every app the user has, plus the curated four, is offered — TiviMate, Wholphin, Netflix, YouTube, Kodi, Plex, Spotify and an app only this device knows, with the launcher added and no system app invented that `pm list packages -3` never reported. |
||
|
James Mackay
|
95d8d20571 |
docs(readme): the setup menu's rows and the sweep's socket behaviour
The setup flow's menu rows are named the way Home Assistant dispatches them (Receiver / Connection / Set up ADB / Pair the remote protocol / Create the entry), a skip leaves its pairing row while a success removes it, and the discovery burst shares the watchdog's connection with adb-shell serialising the I/O. |
||
|
James Mackay
|
3a55519b74 |
fix(config-flow): the setup menu was unreachable under real HA (t57 UNIT 0a)
t61 F1 (BLOCKER) + F2 + F3 + F5, all reproduced against a real FlowManager
before fixing. The OPTIONS flow was verified good and is untouched by this
commit; the SETUP flow was broken in three independent ways.
F1 — `_show_setup_menu` passed `errors=` to `async_show_menu`, which has no
such parameter (HA 2026.10.1: step_id / menu_options / sort /
description_placeholders). Every path that went near the menu therefore
raised `TypeError` the moment the first form was submitted or ADB pairing
was skipped. The path map, driven end to end: FOUR of the five ordinary
paths died before create_entry ("ADB skip", "ADB paired + protocol off",
"ADB skip + protocol skip", "pair ADB"); only "protocol only" survived, and
it never showed the menu — so a new entry could only be created on that one
path and the sub-screens were unreachable.
F2 — the menu's rows broke the rule this repo documents: HA dispatches a row
to `async_step_<row id>` AND dispatches a submitted form by its OWN step_id,
so a row named `adb` with a handler called `async_step_adb_pair` is an
UnknownStep, and a row named `receiver` rendering `step_id="setup_receiver"`
is a form that cannot be submitted. The rows are now their handler names
(`receiver`, `connection`, `adb_pair`, `protocol_pair`, `finish`) and the
two sub-screens render their row id. There is also a `_paired_adb` guard
now, so a completed ADB pairing stops offering that row — the rule the
protocol row already followed.
F3 — both menus keyed their row labels under a bare `menu`, which core does
not read (56 core strings files use `menu_options`), so every row fell back
to its raw id in the browser. Renamed in both strings files.
F5 — the per-command script merge is field-wise now: a `script_<command>`
key that is present and empty removes that command, one with a value sets
it, and a key the submission does not carry is untouched — the same rule as
every other option. The wholesale rebuild depended on the page always being
complete, so a partial submission silently wiped the commands it omitted.
THE SYSTEMIC HALF (F1), closed as asked: `ha_api_drift.HA_CALL_OWNERS` now
covers the flow-display APIs (`async_show_menu`, `async_show_progress`,
`async_show_progress_done`, plus the existing form/abort/entry owners), so
the drift engine resolves their REAL signatures and flags an unknown
keyword. `tests/test_ha_api_drift.py` carries a pre-fix source using
`errors=` to prove the new owner is non-vacuous. Every harness stub now
mirrors the real signature (no `errors` on a menu) and a stub-level test
pins that parameter set, so a stub cannot go back to accepting anything.
On the pre-fix tree the drift engine reports exactly this:
config_flow.py:898: unknown-kwarg: async_show_menu() has no 'errors'
parameter; real signature: (self, *, step_id=None, menu_options, sort,
description_placeholders) -> _FlowResultT
TESTS: tests/test_config_flow_real_ha.py gains a SETUP section that drives
the real flow — the first form, the ADB skip, the pairing guard, EVERY menu
row (dispatch AND re-submittable step_id), an accepted sub-screen
submission, a full connection+receiver+finish walk merging into one entry,
the menu's pairing note, and the five-path map as a parametrised test.
11 of those fail on the pre-fix tree; the drift engine failure above is the
12th. The stub suite additionally pins the menu stub's signature and the
`menu_options` key in both strings files.
Also in this commit (t57 F4): the real-HA file claimed entity-valued pages
could not be driven there, which was false (it has always submitted
`switch.soundbar`). The claim is gone and the scripts page is driven through
SET (two scripts selected, real entity ids) → CLEAR (one selector emptied)
→ SAVE → REOPEN, so the SET half is pinned in the real flow and not only
against the selector objects.
|
||
|
James Mackay
|
5eefd0c6d0 |
test(discovery): fixtures for the parsers, the bounded sweep and the surfaces
tests/test_installed_apps.py is new and is where the discovery claims are pinned. NO REAL SHIELD WAS AVAILABLE, so the command output is fixture-based in the documented Android shapes — `pm list packages` lines, the verbose `resolve-activity` header followed by `package/activity`, and `pm dump` carrying `application-label` — including a localized label (the default locale wins), a label-less app (a definitive negative, and still a row) and the launcher (a system app that must be present). Covered: both parsers plus their tolerances (noise, duplicates, blank, a bare `package/activity` line, the settings entry point that is not a launcher), the user+launcher merge and its de-duplication, the icon heuristic and that it is always a string, the sweep's commands, its concurrency bound (peak <= APP_SWEEP_CONCURRENCY and > 1, so it is bounded AND genuinely concurrent), cache reuse with zero shells, the TTL, shared sweeps for concurrent callers, and every degradation path (no poller, disconnected, mute transport — empty list, no loop, no raise). The sensor's count/table/reason and the "a failed refresh is a no-op, not an exception" behaviour are pinned too. The config-flow side is in tests/test_config_flow_ux.py: the table lists every discovered app, a discovered app the map never mentions is appended, the user's name wins for the same package, an `mdi:` value never reaches the icon column, rendering issues no shell, the Discover row runs one forced sweep and returns to the menu, a failed sweep still renders, and the merged table round-trips into the saved options. Also: tests/test_config_flow_real_ha.py learns the new row, the standalone stub accepts `description_placeholders` (which the apps form now passes), and the state=6 wording in media_session.py/media_player.py is the t57 UNIT 0c reword. |
||
|
James Mackay
|
a56ec38170 |
feat(config-flow,sensor): the discovered apps reach the UI and dashboards (t57)
SURFACES
- The Configure menu gains a **Discover installed apps** row. A row rather
than a form step, because a menu choice is dispatched with no user input:
it awaits the manager's bounded sweep and returns to the menu with a line
saying what was found. Discovery is therefore always explicit — never a
per-poll cost — and a failed sweep still renders the menu.
- The Apps table opens on the REAL app list: the stored rows first (kept
position, name and icon untouched) and then every discovered app, named
by its device label or its package id when the label fetch did not answer.
The user's entry for a package always wins over the device label.
- `sensor.<name>_installed_apps` (ADB-enabled entries only): state = app
count, `installed_apps` attribute = the same table, `discovery` attribute
= ok / pending / not_connected / adb_disabled. It polls through the
manager's TTL cache, so a poll on a fresh table issues no shell, and the
standard `homeassistant.update_entity` action refreshes it.
The discovered icon is deliberately NOT written into the icon column: that
column is documented as a picture URL or a `camera.*` entity feeding
`entity_picture`, and an `mdi:` name there would be a value the media
player cannot use. The icons are offered in the installed-apps attribute,
and the table's description names them.
strings.json + translations/en.json: the `installed_apps` entity name, the
Discover row label, and the menu description now carries {discovery}, so the
page says how many apps were found (or why none were).
README: an "App discovery" section documenting both commands, how system
and launcher entries are treated, the label/caching semantics, the icon
rule and its limits, the sensor surface and the fallback behaviour — and
that the source-list fallback stays the user's map, not the discovery.
|
||
|
James Mackay
|
ee3f2845af |
feat(discovery): enumerate the apps installed on the Shield (t57)
The live complaint ("it doesn't find all the apps") came from the curated
list: only four packages were ever offered, so anything the user had
installed elsewhere was invisible. The device knows its own apps.
ha_adb_common/installed_apps.py parses the two queries the sweep makes:
* `pm list packages -3` — every third-party (user-installed) package;
* `cmd package resolve-activity --brief -c android.intent.category.HOME`
— the ACTIVE launcher, a system app that is nonetheless what the user
opens, so it is appended rather than filtered away.
System apps are otherwise not enumerated (a Shield ships a few hundred and
they are not launchable apps); the parsers are tolerant of the noise a real
adb shell can prepend and of duplicate lines.
ShieldConnectionManager gains the sweep:
`async_refresh_installed_apps(force=False)` returns a cached
`[{package, name, icon}]` table, and `installed_apps()` is a pure read that
never issues a shell — so a state poll costs nothing.
Naming stays on the one path: the sweep calls `_dump_label`, the same
`pm dump` + parse + 24 h cache the entity path uses, now extracted so the
two cannot drift, including the t57 UNIT 0 rule that a fetch that returned
nothing is NOT cached. Labels are fetched APP_SWEEP_CONCURRENCY (4) at a
time with APP_SWEEP_SPACING (50 ms), so a typical Shield finishes in
seconds rather than the minutes a one-per-watchdog-cycle drain would take
while the socket still sees a trickle.
Failure is a normal outcome: no poller, a disconnected transport or a mute
shell stores an EMPTY table with a stamp and returns [], so nothing retries
in a loop and every surface degrades to "no discovered apps".
Icons: `pm dump` does NOT expose an app's icon resource — an extracted
APK's icon is a bitmap only its own code resolves — so this is honestly a
heuristic: APP_ICON_FALLBACKS (a curated package -> mdi: map) with
APP_ICON_GENERIC (mdi:television) for anything unknown, and always a string.
|
||
|
James Mackay
|
9461d69469 |
fix(connection): a label dump that returned nothing is a FAILED fetch (t57 UNIT 0)
t59 F1 (HIGH). `ShieldAdbPoller.shell` swallows its error and answers None
(adb_client.py:144-155), and `_fetch_app_label` read that as "the dump
ran and found no label" → `cache_app_label(pkg, None)` → `('', stamp,
resolved=True)`. That is a DEFINITIVE negative for APP_LABEL_TTL = 24 h,
so after ONE transient failure `app_label()` never re-enqueued, exactly
one `pm dump` was ever attempted, and a later good dump could not
recover the label — `sensor.native_value` and `media_player.app_name`
stayed on the package id for every later render.
The dump now carries an exit marker (`; echo __NSH_EXIT:$?`), so the
three outcomes are distinguishable:
* output with an `application-label` → cache the label;
* output that ran and carried none (the marker is still there) → cache
the empty string, the definitive negative that keeps t24 F1's
zero-round-trip rule for a genuinely label-less package;
* no output at all (`None`, or blank) → cache NOTHING and let the next
render re-enqueue, logged at debug.
The `resolved` flag goes with it: `cache_app_label` was its only writer
and always passed True, so an entry in `_app_labels` is now simply "a
fetch RAN" and the dead third field (`(label, stamp, True)`) is gone.
Tests drive the FakePoller-returns-None AND returns-'' shapes through
the sensor and through `media_player.app_name`, plus the label-less
package that must still be negatively cached. The stub poller now models
both real outcomes of a dump (an answer, or a transport that delivered
nothing) instead of conflating them — which is why the old tests could
only pin the raising-shell branch that already worked. 4 of the new
tests fail on the pre-fix tree, showing `('', stamp, True)` stamped.
Also in this commit:
- t59 F3: running_apps.py set `app["state"] = "paused"` twice — one line.
- t59 F4: no captured fixture contains state=6, so the "the Shield's own
sessions report 6 like stock Android" wording in README and the
media_session/media_player docstrings is reworded as INFERRED from the
`state=PlaybackState {state=N, …}` line shape. A `git notes` entry on
|
||
|
James Mackay
|
ca3fdec218 |
test(config-flow): drive the menus, pin the tables, probe real HA
The flow tests drove `async_step_init` as if it were the one form, so they
had to learn the menu: a shared helper picks the sub-screen that owns a
payload's keys out of the flow's own `menu_options` (no test re-implements
the routing), and a `save=True` flag walks to "Save and close" so the
assertions read as they did before.
New coverage for this task:
- the menu offers every sub-screen and every row has a handler named
after it, with the rendered step_id equal to the row id (the invariant
real HA enforces and the first draft broke);
- edits on one sub-screen are not lost by another, and a page that is
never saved never writes the entry;
- the apps table: add / edit / remove a row, the icon column round-trips,
the empty state renders without data, a blank field clears the map, and
the suggested value is TEXT (the `[object Object]` layer);
- a receiver the selector offers `switch` for, stored and pre-filled;
- the setup menu's rows, their handlers and a connection+receiver merge.
tests/test_config_flow_real_ha.py is new and is the empirical half: it
drives the real `FlowManager`, real probatio validation and the real
frontend serialiser (`to_field_list` + `cv.custom_serializer`) — the call
the HTTP flow view makes — so the menu dispatch, the cleared-selector
shapes and the wire serialisation are established rather than assumed.
It skips when homeassistant is absent; CI runs it with:
uvx --with homeassistant --with adb-shell pytest tests/test_config_flow_real_ha.py
53 of the stub tests and 11 of the real-HA ones fail on the pre-fix tree.
|
||
|
James Mackay
|
b0adbd2dfd |
docs(readme): the configure menu, the apps table and the switch receiver
The Configure page is a menu of sub-screens now, the Apps row is an editable one-row-per-app table, and a receiver may be a `switch.*`. |
||
|
James Mackay
|
cd8f528a27 |
feat(config-flow): menu of sub-screens on setup and options
Both configure pages were one long form: a field you wanted to change meant re-submitting everything, and the options page rendered the apps map as `[object Object]`. Verified against a real Home Assistant install (not assumed): the HTTP flow view serialises a field's suggested value verbatim, so a dict handed to a `type: string` field reaches the browser as a non-string; and real HA validates a submitted page against the schema the step returned, where the built-in EntitySelector rejects `''` and `None`. - The options flow is a MENU (Main setup / Receiver / Apps / Scripts / Save and close). Each sub-screen owns a disjoint set of keys and merges into one pending dict under a single rule: a key the submission CARRIES is authoritative (empty clears), a key it does not carry is untouched. The per-command script dict is rebuilt from the submitted script fields, so unsetting one persists instead of surviving as a stale key. - The sub-screen step_id IS the menu row id. Real HA dispatches a menu choice and a submitted form by that same name, so a mismatch is an UnknownStep or an unsubmittable page (the setup menu first tried a "setup_menu" step id and dead-ended in real HA). - The apps map reaches the suggestion layer serialized to the documented `package=name[|icon]` lines, and parses back to both maps. - The setup flow keeps its first form and gains a menu after it; pairing completion returns to the menu, and the "ADB then protocol" sequence still happens automatically. - Receiver accepts a `switch.*` as well as a media_player. Tests: 522 pass on stubs, and tests/test_config_flow_real_ha.py drives the real FlowManager + probatio + to_field_list; 53 + 11 of those fail on the pre-fix tree. |
||
|
James Mackay
|
74e0549196 |
docs(readme): the t55 state ladder and the label tier's retry rule
The State ladder section still described tier 7 as "app open → on" and tier 8 as "On — powered, awake, nothing more specific", which the t55 ladder change makes wrong: an open app with nothing playing is `idle`, `buffering` is a state, and `on` now means "no source reported anything at all". The retired-piece table gets the same vocabulary, and the app-name row records that the manifest label is fetched for the app on screen and retried after a failure instead of being cached as a miss. |
||
|
James Mackay
|
85e96f3c60 |
docs(sensor): name where the Current App name chain comes from (t55)
The Current App sensor already resolved through the shared `resolve_app_name` chain, so the package-id live symptom was never a missing tier — it was the label path being silenced (fix in the previous commit). Its docstrings now say so explicitly: `_package` is the app on screen (protocol push → ADB mResumedActivity → recents as a last resort) and `native_value` resolves exactly that package's map entry or cached device label, which is why the label fetch must be driven off it. The behaviour is unchanged; the comment is what was missing. |
||
|
James Mackay
|
0c7e405108 |
fix(connection): a failed app-label fetch is retried, not cached as a miss (t55)
The Current App sensor and media_player.app_name showed a package id in
the live run. Root cause was in the manifest-label tier: `_fetch_app_label`
returned early (no poller, ADB down) or logged and returned on a shell
error WITHOUT touching the cache, but `app_label` had already put the
package in `_app_label_requests`; the drain then threw the request away,
so a render between the failure and the next cycle re-enqueued — while
nothing distinguished "the device answered, this package has no label"
from "we never got an answer". Nothing was visibly retried, and the label
tier could stay silent indefinitely.
The cache is now (label, stamp, resolved) and records the FETCH, not the
wishful result:
* a fetch that RAN caches its outcome for APP_LABEL_TTL — the label, or
the empty string for a genuinely label-less package. Definitive: no
re-enqueue, so the t24 F1 per-heartbeat round-trip tax stays closed.
* a fetch that could NOT run stamps nothing, logs at debug (so a
package id in the UI is diagnosable), and the next render retries.
* a queued or in-flight fetch is not enqueued twice, so a burst of
renders still costs one `pm dump` per watchdog cycle.
`cache_app_label()` is the one public writer of the cache (tests and any
future out-of-band reader no longer poke the private tuple), and
`app_label_requests`/`app_label_inflight` expose the queue for tests.
The old 2-tuple seeds in the label tests move to `cache_app_label`.
|
||
|
James Mackay
|
ccde3cff71 |
fix(media_player): the state ladder is idle/playing/buffering/off; ON means no data (t55)
The live report: the card showed "On" where the user expected Idle,
Playing, Buffering or Off, and "On" appeared to override playing states.
Root cause: `state` returned MediaPlayerState.ON for two different
situations — "powered on with an app open and nothing playing" (tier 3.5)
and "we have no idea" (the fallthrough) — while the power gate above the
media tiers returned IDLE whenever power resolved to None, which is
exactly the case in which Jellyfin a media session DO know what is
playing. Media truth was therefore lost whenever the protocol push and
the ADB wake poll had not reported yet.
The ladder is now:
1. OFF — the power entity says off, or the protocol is_on push is
False. Explicit power outranks media, so a stale Jellyfin
session cannot resurrect playback on a device that is off.
2. intent — a just-sent play/pause/stop.
3. Jellyfin — playing / paused.
4. session — the ADB media session: buffering / playing / paused.
5. IDLE — ADB standby (powered but asleep), or the device is on
(protocol is_on / ADB awake / the plug) with nothing
playing. "Powered on, app open, nothing playing" is IDLE.
6. ON — nothing reported anything at all: no transport, no power
entity, no Jellyfin, no session. ON never masks a
playing/paused/buffering source and never appears while
any source reported a power or media state.
Buffering also carries the playback controls: a buffering session
advertises PLAY/PAUSE/STOP exactly like a playing one, and the
now-playing dict names it "buffering" instead of collapsing it into
"playing" and contradicting the entity state.
The module docstring documents the ladder and the ON-only-with-no-data
rule, and the new TestStateModelT55 / TestCurrentAppNameEndToEndT55 pin
one test per source (Jellyfin, intent, media session, power entity,
protocol push, ADB wake) that a reporting source outranks ON/IDLE.
Red-proof against the previous tree (new tests, old source): 21 failed /
17 passed, with the three headline symptoms visible as literal asserts —
"on" where "idle" belongs, "idle" where "on" belongs (no data), and
"playing" lost because power was unknown.
|
||
|
James Mackay
|
07682ff082 |
feat(media_session): map Android PlaybackState 6 to buffering (t55)
The media session parser only understood states 2 (paused) and 3 (playing), so a session refilling its buffer (Android state 6, which the Shield's own ExoPlayer/MediaSession sessions report exactly like stock Android) read as "idle" or vanished into the foreground tier. MediaSessionInfo gains an independent `buffering` flag (playing/paused/ buffering are now three flags off one integer), the session sort puts buffering right behind playing — it is mid-playback, so it outranks a paused session when the ladder picks one — and the running-apps enrichment names a buffering live/last-known session "buffering" instead of falling into "idle". The MediaPlayerState stub gains BUFFERING and STANDBY so an in-suite reference to a member real HA does not have fails here (real HA 2026.10 exposes off/on/idle/playing/paused/standby/buffering). |
||
|
gitea-actions
|
49bc23ae34 | chore(release): v2026.10.11.2 [skip ci] v2026.10.11.2 | ||
|
James Mackay
|
0357f47284 |
fix(media_player): dedupe source_list case-insensitively (t53 F1)
t52 F1: the dropdown collapsed exact duplicates but not names that differ only by case.
With a map of {com.a: 'Dup', com.b: 'dup'} the pre-fix tree offered ['Dup', 'dup']
(two rows) while the reverse lookup — case-insensitive by design since t47 — resolved
BOTH spellings to the first entry. So row two was a row that LIED: picking it launched
com.a, the app row one already names. Editor-reachable (two valid lines), unlike the
earlier hand-edit-only edges, which is why it is fixed rather than documented.
source_list now dedupes on name.casefold(), keeping the first spelling in the user's
typed map order. One row per name means a row can only launch the app it shows. The
shadowed package is not lost: the Browse tree still lists it under its own package id
(the t51 rule — both entries kept, each reachable by id) and select_source accepts that
id, pinned by test. The t51 policy is otherwise untouched: names stripped once, blank
name/package entries dropped, exact duplicates already collapsed, well-formed maps
byte-identical to the Browse titles with inner spaces preserved.
README: a short 'Known limits (the Apps map)' note records the two accepted boundaries
so they are known rather than latent — names differing only by case offer one row (the
shadowed app stays reachable by id), and a hand-padded map value is shown cleaned in
the dropdown and the Browse tree while app_name/source/Current App sensor display it
verbatim, so `source` can be a string that is not in source_list (a blank value falls
through to the raw package).
RED PROOF (previous tree swapped in, then restored): 4 failed / 5 passed pre-fix for
the case tests, all green after. The literal pre-fix symptom:
`assert ['Dup', 'dup'] == ['Dup']` — "Left contains one more item: 'dup'"; the
wrong-app half is pinned by `_source_package("dup") == "com.a"`, which holds on BOTH
trees and is exactly the chain that made row two launch row one's app. The new
no-duplicate invariant fails pre-fix with len(['dup', 'dup']) == 2 versus 1.
Verified: 471 passed / 2 skipped, 472 / 1 with androidtvremote2, ruff clean, drift
engine exit 0 under real HA (23 modules, 58 imports, 164 relative imports, 36 call
arguments, 7 awaited HA calls); README YAML blocks still parse (5) and the t47/t48/t51
rig is green.
|
||
|
James Mackay
|
dc3cfbbe3e |
fix(media_player): normalize the app directory in one place (t51)
t49's two residuals share one cause: `_app_directory` handed the raw options map to
its consumers, so the Browse tree showed a child with a blank title and the reverse
lookup compared the user's padded name against itself. The map is hand-editable, so
the normalization belongs in that one accessor — the browse tree, source_list and the
reverse lookup then cannot disagree.
Policy, now documented in `_app_directory` (the single place):
* names are stripped as the directory is built, so display and match key are one
string: `com.x= TiviMate ` shows as "TiviMate" AND resolves as "TiviMate";
* an entry whose NAME or whose PACKAGE is blank/whitespace-only is dropped, so no
surface offers a child with no title or no package behind it (the t48 invariant,
now extended to the Browse tree);
* two packages normalizing to the same name are BOTH KEPT — the tree reaches each by
its own package id, so neither app is hidden — while the FIRST in map order wins the
name: that is the single entry source_list offers and the package select_source
resolves. Map order is the user's order, so the winner does not move between
rebuilds; reversing the map reverses the winner, which is pinned.
* a well-formed map (what the editor and the curated default produce) is byte-identical
in and out: stripping touches the EDGES only, so "TiviMate Pro" keeps its inner space.
source_list now just dedupes the normalized values (its own t49 filter moved into the
accessor) and `_source_package` documents the same first-wins rule.
RED PROOFS, both residuals, against the previous tree (HEAD
|
||
|
James Mackay
|
97615408c0 |
fix(media_player): keep blank app names out of the source list (t49 F1)
t48 F1: source_list stringified the app map's values without filtering, so a
hand-edited map with a blank name ("com.x=") put an empty string in the dropdown.
That entry is worse than useless — it is a dropdown item whose only possible outcome
is an error, because select_source has to reject an empty name as an unknown source.
source_list now drops blank and whitespace-only names. The values themselves are
untouched: no name is rewritten or trimmed here, because the reverse lookup matches on
them, so for a well-formed map the list stays byte-equal to the Browse tree's child
titles (same _app_directory accessor, same order) — pinned by comparing the two
directly, for the user's map and for the curated default. The reverse lookup is
unchanged.
Tests (tests/test_media_sources.py, 3 new; tests/test_standalone_setup.py, 1 new):
a hand-edited map with "" and " " entries offers only the real names; the offered
list equals the Browse tree titles for well-formed maps (both map and curated
default); the surviving entries still resolve AND launch through select_source, and a
blank name still resolves to nothing; and the editor keeps rejecting "com.x="
(_app_map_from_text returns None, the page answers invalid_app_map and stores nothing
half-way), so a blank name cannot get in that way either.
RED PROOF: with the filter removed, exactly the two blank-name tests fail — the blank
appears in the list, and the entry has no package to resolve to (1 passed / 2 failed;
the byte-equality test passes either way, which is the "well-formed list unchanged"
half of the claim) — while the rest of the file is green; restored, 20 pass.
Verified: 458 passed / 2 skipped, 459 / 1 with androidtvremote2, ruff clean, drift
engine exit 0 under real HA (23 modules, 58 imports, 164 relative imports, 36 call
arguments, 7 awaited HA calls).
Residual, deliberately NOT touched here (out of this repair's scope, and pre-existing):
a blank map entry still reaches the Browse tree as an empty child title, and a PADDED
name (" TiviMate ", hand-edited only — the editor strips) still fails the reverse
lookup because the map value is matched unstripped. Both are hand-edited-map edges;
say the word and either can be a follow-up.
|
||
|
James Mackay
|
1c6c223f27 |
docs(readme): the app list as sources, the remote command script, current versioning (t47 D)
t46 F2: the status line still cited `v2026.10.08`. It now describes the scheme (`vYYYY.MM.DD`, plus `vYYYY.MM.DD.N` for a second release in the same day) and links to the Releases section, so it cannot go stale again. The other dated reference stays: "v2026.10.09 shipped that way" is the historical example that motivated the write-back, and it is accurate. New "Sources (the app list in a dropdown)" section: source_list from the Apps map (with the built-in fallback named, so a card is never empty), `source` resolved exactly like the Current App sensor, and `media_player.select_source` by name — accepting a raw package id and any case, erroring on an unknown name — with a copy-pasteable service call. It also points at the Select source script as the override for the "more than launching an app" case. New "Remote command script" subsection: the four variables in a table (command, num_repeats, delay_secs, hold_secs) with their meanings and defaults, and a copy-pasteable script that forwards the call to an IR bridge. It states that all four are always present including defaults, and that unset changes nothing. Corrected the two claims UNIT A invalidated: the command table no longer says select_source has no native path, and the paragraph no longer says it is offered only while a script is configured. Every fenced YAML block in the README still parses with a real parser (5 now), and tests/test_readme.py is green. |
||
|
James Mackay
|
c3e4c7855c |
feat(remote): optional command script for remote.send_command (t47 B)
One optional 'Remote command script' on the Configure page. Configured, it replaces
the whole key path for remote.send_command; unset, repeats, long presses and delays go
out as protocol keys exactly as before (the existing send_command tests pass
untouched).
A remote command is a SEQUENCE — repeats, a START_LONG/END_LONG pair, an inter-repeat
delay — so the escape hatch is one script for the call rather than per-key overrides:
an IR bridge, a macro, or a second device needs the call, not individual keys. The
script receives the service's own parameters as variables, named as they are on the
service and documented in the README and pinned in tests:
command the list of key commands as submitted (e.g. ["HOME", "KEYCODE_DPAD_CENTER"])
num_repeats the repeat count (default 1)
delay_secs the inter-repeat delay (default 0.0)
hold_secs the long-press duration, START_LONG/END_LONG handled by the script
(default 0.0) — the script owns the key semantics, so it decides what
"hold" means
All four are always present, including defaults, so a script can rely on the shape.
A raising script surfaces a translated error rather than a silent no-op. That made the
t44 script-failure mechanism consistent in the process: async_call_script() is now the
one caller for both the media player's per-command scripts and the remote's, raising
ServiceValidationError(translation_domain="nvidia_shield",
translation_key="command_script_failed") with {action, script, error} placeholders
instead of a bare HomeAssistantError — the t44 test was updated to assert the
translation contract (domain, key, placeholders) rather than a message string, which is
stronger evidence and gives the user real copy. The new copy lives in BOTH strings
files, as does the unknown_source key from UNIT A.
Option handling reuses the t47 C work: the field is an entity selector (so a cleared
value is tolerated by the schema), the stored value is trimmed, and a blank,
hand-edited or non-string value means "not configured" rather than a call to nowhere.
Verified: 454 passed / 2 skipped, ruff clean, drift engine exit 0 under real HA
(23 modules, 58 imports, 164 relative imports, 36 call arguments, 7 awaited HA calls).
|
||
|
James Mackay
|
b2aab15ebf |
feat(media_player): source list + native select_source from the app list (t47 A)
The last piece a universal-media-player replacement needs: a dropdown of the apps,
an answer to what is on screen, and a way to pick one — all reading ONE app directory
(the user's Apps map, else DEFAULT_APP_DIRECTORY), so the dropdown can never offer a
name the launch path cannot resolve. That invariant is pinned by a test that walks
source_list and resolves every entry.
* `source_list` — friendly names in directory order (the user's map order, or the
curated list), from a new `_app_directory` accessor that the browse tree now shares.
* `source` — the foreground app's resolved name via the existing `resolve_app_name`
chain (map → cached device-manifest label → raw package), i.e. exactly `app_name`,
so the media player and the Current App sensor cannot disagree.
* `select_source(name)` — a configured select_source script still takes precedence
(t44, unchanged: selecting a source can be more than launching an app); without one
the name is resolved back to its package and launched over the shared
`play_media("app", …)` path, so the native semantics are the ones play_media already
had. Accepts a raw package id (what the browse tree publishes) and matches names
case-insensitively; an unknown or empty name raises
ServiceValidationError(translation_domain="nvidia_shield",
translation_key="unknown_source", translation_placeholders={"source": …}) — a
translated error, never a silent no-op — with new copy in BOTH strings files.
* SELECT_SOURCE is advertised when EITHER a select_source script is configured OR
source_list is non-empty; VOLUME_SET stays script-only (the protocol has no
absolute-volume key), and the two flags are pinned as independent.
Two t44 tests were updated because this is a deliberate behaviour change: "no native
path" is now only true of volume_set, and SELECT_SOURCE is advertised by default
because the curated directory always offers something.
Also: rewriting the strings files re-indented the seven script labels added in t44
(they were valid JSON but sat two levels shallow); the diff there is that fix plus
the new key.
Verified: 441 passed / 2 skipped, ruff clean, drift engine exit 0 under real HA
(23 modules, 59 imports, 157 relative imports, 36 call arguments, 7 awaited HA calls).
|
||
|
James Mackay
|
b3f897c36e |
fix(config_flow): clearing an entity field no longer fails validation (t47 C)
t46 F1, reproduced against real Home Assistant: submitting the options page with a
cleared entity field (script_turn_on='') produced "Schema validation failed at
script_turn_on". HA validates the submitted page against the schema the flow last
returned, and real EntitySelector rejects BOTH '' and None — "Entity is neither a
valid entity ID nor a valid UUID" (verified directly against homeassistant's
EntitySelector). Clearing therefore depended on the frontend OMITTING the key, on
every entity field of the page, not only the script ones t44 added.
_ClearingEntitySelector subclasses EntitySelector and returns '' for None or an
empty/whitespace string, delegating everything else to the parent — so a real entity
id is still domain-checked, and the class is still an EntitySelector: its serialized
form is byte-identical to the real selector's
({"selector": {"entity": {"domain": ["script"], "multiple": false, "reorder": false}}}),
so the frontend keeps rendering an entity picker.
The step also normalizes a submitted None to '' before it acts: an explicit empty
value clears the setting (the script dict drops it, the patched options store '' which
every reader already treats as unset), while an OMITTED key still means "the page did
not touch it" for the patched options — and still means "cleared" for the script dict,
which is rebuilt from the page (t44's editor-is-the-truth rule, unchanged).
The test harness now models the trap instead of hiding it: EntitySelector is a CLASS
(entity_stublib.EntitySelectorStub) that rejects ''/None like real HA, defined once and
shared by all five stub installers through install_selector_faces(), so the stubs
cannot drift apart.
Tests (tests/test_config_flow_ux.py::TestClearingAnEntityField, 8): the schema accepts
omitted/None/'' while passing a real entity id through; the plain selector is pinned as
rejecting the same values (the trap itself); '' and None each clear a script AND a
pre-existing entity field (jellyfin, the older instance of the pattern) while
untouched fields survive; an omitted script field is cleared; an omitted entity field
is kept; and a prefill→submit round trip changes nothing.
RED PROOF: with the five selector sites switched back to the plain EntitySelector,
exactly the five tolerance tests fail (5 failed, 4 passed) and the rest of the file
stays green; restored, 70 pass. Real-HA probe run separately (see the task output):
strict rejects '' and None, the subclass accepts both and returns '', and serialization
is unchanged.
Verified: 423 passed / 2 skipped, ruff clean, drift engine exit 0 under real HA.
|
||
|
James Mackay
|
0cfa045190 |
docs(readme): the release write-back and the command scripts (t44 3, R2)
Releases: the section described the manifest/date fallback only, and pointed at `.github/workflows/release.yml`, which does not exist. It now covers the same-day `vYYYY.MM.DD.N` suffix (lowest free N, gaps reused), the manifest write-back commit and WHY it exists (HACS compares against the default-branch manifest, so a tag-only release is offered to nobody — v2026.10.09 shipped that way), what `[skip ci]` means and what it does not (release commits only; Gitea's own handling can still skip an ordinary commit whose body mentions the marker), the refusal to publish when the default branch disagrees, and the branch-protection failure path. t44 R2, both halves of the tag's identity: with a write-back the tag names the bot commit, so `git describe --tags` on a developer commit reports the previous release (expected, not a missing tag); hand-bumping the manifest keeps the tag on the developer's own commit, because the workflow then has nothing to write back. Command scripts: a new section under Options, with the command/variable table (`level` for volume_set, `is_volume_muted` for volume_mute, `app_name` for select_source), the "instead of, never both" rule, the fact that an unset selector means the native path, and a copy-pasteable script block covering the three cases the user asked for — a speaker switch owning the volume, an AVR owning the input, a mute that hits the switch too. It says plainly that scripts are set on the Configure page and not in the YAML seed, which stays address + transport only (the seed example is unchanged and still the only `nvidia_shield:` block). Verified: 414 passed / 2 skipped, 415 / 1 with androidtvremote2, test_config_flow_ux 61, test_standalone_setup + test_config_flow_protocol 58, ruff clean, drift engine exit 0 under real HA, and every fenced YAML block in the README parses with pyyaml. |
||
|
James Mackay
|
b03cfbae13 |
fix(release): notes must not open with the bot commit; guard matches the subject (t44 3b/3c)
t45's review of the release versioning found three lows; two touch behaviour here (the
third, README wording, is the next commit).
R1 — with a write-back the tag points at the bot's own commit, so the notes range
`PREV..TAG` published "- chore(release): vX [skip ci]" as the first line of every
release. The range now ends at `${TAG}^` when BUMP=yes (the last commit a developer
wrote) and at `${TAG}` when it is not (there the tag IS the pushed commit, and it
belongs in the notes). `git describe` for the previous tag is unchanged: it already
started from `${TAG}^`, so it keeps naming the previous release.
R3 — the job guard matched the `[skip ci]` marker ANYWHERE in the commit message, so a
developer commit whose body merely mentioned it silently skipped its own release. The
guard is now anchored to the release commit's subject prefix
(`!startsWith(message, 'chore(release):')`), which is a structural property, not a
substring a body can trip. The marker stays in the commit message: it is the
convention Gitea itself may honour to skip the run outright, and the README now says
so, together with the fact that the job guard is the layer that holds when it does not.
3c — the resolver's tests used tag fixtures, so `refs/tags/X^{commit}` was never
exercised, and that dereference is the entire reason an annotated tag is recognised
(for one, `git rev-parse refs/tags/X` is the TAG OBJECT, not the commit, so a
comparison that forgot to dereference would miss a released commit). The resolver now
also runs inside real temporary repositories built with real annotated and lightweight
tags: released-on-HEAD both ways, a tag on an older commit falling back to the date,
no tags at all, the same-day scan walking real tags, and gap reuse. The workflow's own
run: scripts are executed too — extracted from the YAML by hand rather than with a
parser the fast suite does not have — covering the bump/verify/tag path (commit
subject, tag == HEAD, the tag's manifest, the pushed tag), the inconsistent-manifest
refusal, the rejected-push failure, and the notes range: with BUMP=yes the fixed range
yields "- feat: the real change" while the range it replaced demonstrably contains
"- chore(release):". The job-level `if:` itself cannot be executed here (no Gitea
runner) and stays a contract pin; the script-side layer that makes a bypassed guard
harmless is executed instead.
Verified: 414 passed / 2 skipped, ruff clean, release.yml parses.
|
||
|
James Mackay
|
89c05f1dcd |
feat(media_player): per-command script overrides (t44)
The user's remaining universal-media-player dependency: some commands need hardware this integration deliberately does not drive — a speaker switch that owns the volume, an AVR input that has to move with the Shield, a mute that has to hit two devices. One script per command is the escape hatch, and the rule is absolute: a configured command calls its script INSTEAD of the native protocol/ADB path, and with nothing configured every command behaves exactly as it did (the whole existing suite passes untouched, which is what "byte-identical" means here). * const.py owns the option: CONF_COMMAND_SCRIPTS (one dict, keyed by command), SCRIPT_COMMANDS (turn_on, turn_off, volume_up, volume_down, volume_set, volume_mute, select_source) and command_scripts(), the ONE normalizer of the stored shape — options round-trip through HA storage and can be hand-edited, so a value that is not a non-empty string for a known command is dropped instead of reaching the service layer. * media_player.py: async_run_command_script() calls script.turn_on with the script entity and its variables, blocking=True so a failure surfaces HERE — a mapped command that cannot run raises HomeAssistantError naming the command and script rather than falling back silently or doing nothing. Every command method checks its script first; volume_set passes `level` and select_source passes `app_name`. * volume_mute also passes `is_volume_muted`: without it a mute script cannot tell muting from unmuting, which is the case the user asked for. * volume_set and select_source have NO native path (C-VOL; app launching goes through play_media), so they are advertised only when configured — the feature flag is what makes HA offer the command at all. Unset leaves the feature set exactly as it was. * remote.py shares async_run_command_script for POWER: turn_on/turn_off is one configuration and the remote entity is the other route to it, so a script that owns the power sequence owns it on both. * config_flow.py: one entity selector per command (domain: script), stored as the single dict option, prefilled from the entry's options and cleared by saving without a value — the editor is the truth, as with the app map editor. Two things the harness caught, worth recording: `MediaPlayerEntityFeature` has no SET_VOLUME (the real member is VOLUME_SET — the stub enum and the drift engine disagree with a wrong name), and the stub `homeassistant.exceptions` had no HomeAssistantError, so the stub now mirrors the real inheritance (ServiceValidationError derives from it). tests/test_command_scripts.py covers each command mapped and unmapped, the exact variable payloads, the native path untouched when unmapped (protocol key, then the ADB fallthrough with the protocol down), the rejecting script's error, hand-edited option shapes, and the feature set. test_config_flow_ux.py covers the selectors, the stored shape, prefill and clearing. New tests leaked the harness into the next file at first (a missing per-file scope); that is fixed, and tests/test_harness_scoping.py now asserts that every file loading the harness scopes it, so the next such mistake fails where it is made. Verified: 401 passed / 2 skipped, 402 / 1 with androidtvremote2, reverse file order 401 / 2, ruff clean, and the drift engine exits 0 on a real-HA interpreter (23 modules, 59 imports, 157 relative imports, 36 call arguments, 6 awaited HA calls). |
||
|
James Mackay
|
2e1d162ef4 |
test(harness): make the stub scoping independent of file order (t44)
A subset of test files in an order the suite was not written for failed 3 test_init.py::TestImportStep tests with ModuleNotFoundError: voluptuous, while the full suite stayed green. Two order-only mechanisms, both now fixed at the source: 1. entity_stublib.install() guarded on ONE sentinel (`homeassistant.components.media_player`). A file whose teardown popped `voluptuous` but left the sentinel turned the next file's install() into a no-op, so importing config_flow failed. The guard now checks every face a file scopes on (`_STUB_FACES`, derived from ENTITIES_STUB_NAMES so the two cannot drift) and rebuilds the set when anything is missing. install() also registers the `homeassistant.helpers.selector` face itself, which the per-file installers had been adding separately — without it the derived list would never be complete. 2. Teardowns restored only the names their file had listed, so a module the file loaded (`nsh.diagnostics`, `nsh.media_player`) survived into the next file still holding stub objects — a stub exception class, the redact helper — that had since been replaced; comparisons against the new objects then failed (5 tests in test_command_routing on an unlucky order). Every file now ends through entity_stublib.snapshot_modules()/restore_modules(), which drops EVERYTHING the file created under the harness namespaces (homeassistant/voluptuous/nsh) and puts back everything it replaced. Hand-written lists were the bug, so there is none to keep in sync. The t7 no-leak discipline is unchanged: a file's stubs still never reach another file, and a file's loaded modules still do not survive it. A session-scoped baseline install was tried first and rejected: files whose own installer guards on "the stubs are already there" skipped installing their richer faces, which produced exactly the inconsistent state a baseline is meant to prevent (10 test_standalone_setup failures). tests/test_harness_scoping.py: the sentinel poisoning, the wholesale-rebuild invariant, a teardown that removes what the file loaded, the stale-object case (diagnostics vs the redact helper), and — because order is the thing under test — two subprocess runs of the reported orderings. Verified: the reported order (test_command_routing, test_protocol_transport, test_standalone_setup, test_init) 152 passed / 1 skipped; the minimal pair both ways; the previously failing 6-test order now 167 passed; every test file in REVERSE alphabetical order 369 passed / 2 skipped; two scrambled thirds green; full suite 369 / 2 and 370 / 1 with androidtvremote2; ruff clean. |
||
|
James Mackay
|
fbc72dfd9e |
docs: correct two stale claims about a tier and a red proof (t44 0b/0c)
media_player.py's module docstring still described the command path as
"protocol key push → ADB keyevent → external androidtv_remote", calling the last
tier "transitional; removed in Phase 4". t41 removed it, so the sentence
documented a path that no longer exists; it now names the two real tiers.
tests/test_module_coverage.py recorded the sweeps' red proof as the pre-t41 commit.
That commit cannot prove either sweep: at
|
||
|
James Mackay
|
3175582419 |
fix(config): degrade an unreachable Shield to the pairing form (t44 F1)
`_start_protocol_pairing` called androidtvremote2's cert generation and `async_start_pairing` with no try/except, so `CannotConnect` propagated straight out of `async_step_protocol_pair`. The caller already had the shape for a soft failure — `_async_begin_protocol_pairing()` returns `(client, None) | (None, error)` and `_async_restart_protocol_pairing` renders `errors["base"]` — the exception simply bypassed it, and the protocol_pair form never appeared: not the address field, not the "continue without pairing" escape. That made it a default-path crash rather than an edge case: t41 turned the protocol transport ON by default, so an asleep, rebooting or mis-addressed Shield (whenever someone adds the integration, or skips ADB pairing into this step) raised out of the flow instead of showing the form. The reviewer reproduced both under real HA. Now the step returns `(None, "protocol_unreachable")`, logs the address and the library error, discards the half-built client so a retry starts clean, and renders the same form the other failures render — with the address editable and the skip option intact. `protocol_unreachable` is documented in strings.json AND translations/en.json (a form error without copy shows a bare key in the UI). Tests (tests/test_config_flow_ux.py::TestUnreachableShieldDuringPairing): the fake client can now fail to start pairing, and five cases cover fresh setup, the ADB-skip route into the protocol step, reconfigure, reauth and the skip path that still finishes the entry — all as FORMS, never exceptions. Red proof on HEAD: the same drive raises CannotConnectStubError out of async_step_user. Verified: 361 passed / 2 skipped, 362 / 1 with androidtvremote2, ruff clean. |
||
|
James Mackay
|
7608518d97 |
feat(release): same-day versions + manifest write-back so HACS sees every release (t42)
The evidence this fixes: v2026.10.09 was published while master's manifest still read 2026.10.08, so HACS compared every install against a version that never moved and offered nobody that update. A second release in the same UTC day was also a silent no-op: the date tag already existed, the run resolved to the same version again, and it stopped at "already tagged". * scripts/determine-release-version.sh owns the decision, so it is tested rather than hoped for: the manifest version while it is untagged (bump it to force a release); else today's UTC date; else YYYY.MM.DD.N with the LOWEST untagged N >= 2 (a gap is reused, not burned). An already-released commit, and any version whose tag exists, are an explicit RELEASE=no instead of a failure. The script only ever reads the clock with `date -u`, and it emits VERSION/TAG/SOURCE/BUMP/RELEASE on stdout and into $GITHUB_OUTPUT. * the workflow writes the resolved version into the manifest whenever it differs, commits it as "chore(release): v<VERSION> [skip ci]" with the existing bot identity, pushes HEAD:master, and only then tags THAT commit — so the artifact, its tag and the default branch agree. A branch-protection rejection fails the job with an instruction to bump the manifest manually; nothing is tagged first. * post-push verification reads the manifest out of origin/master (the copy HACS fetches) and refuses to publish when it does not equal the released version. * no self-trigger loop: the conventional [skip ci] marker on the bot commit plus a job-level `if:` that re-checks github.event.head_commit.message. The guard is the load-bearing part — it holds even where a Gitea version ignores the marker. * preserved: manifest-wins precedence, the concurrency group without cancel-in-progress, notes from the commit range, zip asset upload and the 409-tolerant publish path. RELEASE=no replaces the old `exists` output, so an already-tagged version still ends as a clean no-op. Also fixes the data: master's manifest moves 2026.10.08 -> 2026.10.09, the latest released tag, so the repo stops contradicting its own releases immediately rather than only after the next successful run (which resolves to v2026.10.11 either way). tests/test_release_versioning.py drives all of it: the four scenarios, the lowest-free-suffix rule, UTC determinism under three timezones, the error paths, the $GITHUB_OUTPUT handoff, and the workflow's own contract — textual, because the fast suite has no YAML parser; the verify command parses the file with pyyaml. Verified: 356 passed / 2 skipped, 357 / 1 with androidtvremote2, ruff clean, release.yml valid, `bash -n` clean, and the script resolves the real repo to v2026.10.11 (date, bump=yes). The workflow itself cannot run here (no Gitea runner): its run: steps were extracted from the YAML and executed against scratch repos with a bare origin, covering the bump+verify+tag path (commit subject, tag target and tag manifest all agree), the inconsistent-manifest failure and the rejected-push failure. |
||
|
James Mackay
|
aea9f9f9d2 |
test(config): pin the Jellyfin name tiers and the address redaction (t41)
Two acceptance items that had no test of their own:
* the Jellyfin auto-detect tiers. t41 replaced the entity-derived search key
(the Android TV Remote entity's friendly_name) with the names setup already
knows — the name the user typed, then the discovery device name. Three tests
pin it, the third being the regression: a Jellyfin player matching the *atv*
friendly_name, with a serving state, used to be auto-detected; now only the
typed/discovered names search. Red proof on the pre-t41 tree (
|
||
|
James Mackay
|
130dee4bc6 |
test(ci): sweep every module for dangling imports (t41)
Two defects shipped with a green suite: button.py imported a helper that had been deleted (no harness ever loaded the file), and the drift sweep only judged `homeassistant.*`, so a relative import of a name that no longer exists was invisible to it. Both are now impossible to ship silently. * tests/test_module_coverage.py loads EVERY *.py file of the package, in a clean interpreter, under the stub harness — exhaustive by construction (a new platform is covered the moment the file exists; there is no list and no special case). It asserts the reported set equals the files on disk, so a silently skipped module fails too; * entity_stublib.load_entity_modules() now also loads button.py (the named gap); the sweep is what keeps that list from needing to be complete; * ha_api_drift gained pass 5: the package's own relative imports. Pure AST, no homeassistant required, with its own entry point `scan_package_imports()` for the fast suite — it checks the target file exists and defines every imported name (submodule imports still resolve), instead of trusting that the modules a harness happened to load are the ones that matter; * the file walk in both AST passes is recursive, and the non-vacuity floors moved with it (modules 10 → 20, relative_imports ≥ 100 — a shallow-glob regression trips the floor instead of looking green); * two permanent red proofs, both on a copy of the tree with the reverted `from .helpers import discover_adb_host` reference in button.py: the stub-harness sweep fails with "FAIL button.py", and the static pass fails with "button.py:1: broken-relative-import: … not defined in helpers.py". Manual runs confirm the engine's other direction on the real pre-fix tree ( |
||
|
James Mackay
|
151c49a0da |
docs(readme): replacement guide + a verified seed example (t41)
The README now answers the question a migrator actually has: what do I delete, and what covers it afterwards. * a new "Replacing androidtv / androidtv_remote + the glue" section: the back-up → remove-the-platform-YAML → remove-mediasession2mqtt → delete the old integrations → add this one → update automations checklist, plus a table mapping every retired piece to what serves it now. It notes that Wholphin publishes no MediaSession of its own and gets its now-playing from the ADB media session, so no bridge is needed for it either; * the setup steps describe the real flow (name + one address, both transports on by default, ADB pairing then PIN pairing, each with a skip); * the YAML seed section carries a complete, copy-pasteable block. It documents ONLY the keys `_yaml_seed_blocks` honours (name, host, adb_port, adb_enabled, and the still-accepted protocol_host/adb_host) and says explicitly that the rest lives on the Configure page — an example that lists ignored keys is worse than no example; * the state-ladder section, the discovery bullet and the options list follow the two-transport, one-address reality. `tests/test_readme.py` pins the doc/code contract: the example's keys are a subset of what the seed reads, the example uses the current `host` key, and the guide keeps naming the retired integrations and glues. Verified: 326 passed / 2 skipped, ruff clean. |
||
|
James Mackay
|
fb481c3c14 |
feat(config): enable both native transports by default (t41)
The transports are ours now, so setup offers them ON rather than OFF: a fresh form renders "Read state over ADB" and "Add the Nvidia Remote Protocol" checked, with the Shield's address pre-filled by discovery (or typed by the user). The schema default, the async_step_user fallback and the _create_entry fallback all carry True, so the checkbox the user sees and the entry that gets written cannot disagree — the t33 F-A lesson applied to the new defaults. Still a choice, not a mandate: * unchecking either toggle routes to the other transport's pairing step; * each pairing step keeps its "continue without pairing" escape (the transport stays off until paired), so a passive entry is still one click away; * no_state_source now means exactly "neither transport enabled" — and a payload that names neither is REJECTED rather than silently enabling both (fail closed; a real submission always carries the schema defaults). * the YAML seed keeps t32 F4's shape rule: an address turns ADB on, a protocol-only block leaves it off, an explicit adb_enabled wins. Verified: 323 passed / 2 skipped, 324 / 1 with androidtvremote2, ruff clean, drift engine exit 0. |
||
|
James Mackay
|
81f70585cd |
feat(config)!: migrate entries to v3 — one address, no androidtv key (t41)
ENTRY_VERSION = 3, declared once in const.py and reused as the flow's VERSION so the stored shape and the migration cannot drift. v2 → v3 mirrors the two shape changes: * the duplicated address fields collapse into CONF_HOST — the protocol host wins, else the ADB host (both name the same Shield), reading options first and data second so a hand-written seed is covered too; * the androidtv-entity key is dropped from data AND options with a logged note naming the dropped entity (the old value would only mislead now that nothing reads it). A v1 entry still travels the whole way in one call (the v1→v2 protocol defaults are applied, then the v3 collapse). Downgrades (version > 3) are rejected so HA can offer a rollback, and the current version is a no-op. Existing entries carrying the entity key keep working until the migration runs: nothing reads it anymore (the c1 commit), which is exactly why the migration can be a plain removal. Test note: my first version of the version-parity pin called a bare entity_stublib.install() and leaked the shallow stub set into the entity-module loader of later tests (STATE_PAUSED/entity_available import failures). It now uses the file's scoped harness, like every other test there. Verified: 316 passed / 2 skipped, 317 / 1 with androidtvremote2, ruff clean, drift engine exit 0 on HEAD (36 call arguments — one more awaited/checked call). |
||
|
James Mackay
|
75ebeb40b0 |
refactor(config): one Shield IP field for both transports (t41)
The two address fields were the same Shield spelled twice, and every bug in the zeroconf/IP-change path came from keeping them in sync. One field now: * const.CONF_HOST replaces CONF_ADB_HOST / CONF_PROTOCOL_HOST in the setup form, the options page, the pairing forms, the reconfigure/reauth seeding and the description copy. ADB port 5555 stays the only port field; * helpers.entry_host(entry) is the single resolver (options first, then data); resolve_adb_config and the protocol settings both go through it, so both transports read the same value at call time (the t32 F1 factory re-read keeps working: the NEXT built poller sees the new address); * helpers.seed_host / seed_adb_enabled own the YAML seed's shape rule. A block may use `host` or the legacy protocol_host/adb_host keys — t41 keeps the old spelling working — and t32 F4's outcome is unchanged: a protocol-only block leaves ADB off, an address (or an explicit adb_enabled) turns it on; * the zeroconf IP-change path collapses the t21-F2 per-host dance into one rule: the discovery address IS the entry's address, so the entry is updated when it differs and left alone when it matches. The blank-self-heal and deliberately-different-ADB-host cases are gone with the duplication — a second address field cannot express them anymore, and the tests now pin that (previously "an explicit override is never clobbered"); * the options page exposes the protocol toggle it never had (the page could turn ADB off but not the protocol); * button.py (granted to this task) reads the entry's own address instead of another integration's entity. The v3 entry migration that maps existing adb_host/protocol_host onto CONF_HOST and drops the androidtv key lands in the next commit; HA runs migrations before setup, so an existing entry is never read with the old shape. Verified: 311 passed / 2 skipped, 312 / 1 with androidtvremote2, ruff clean, drift engine exit 0 on HEAD. The UI-copy contract tests (every rendered field has a label in strings.json AND translations/en.json, key trees identical, host errors name the field) drove the string edits. |
||
|
James Mackay
|
86aa2827a2 |
refactor(config): drop the androidtv media_player entity path end-to-end (t41)
The user ruling: this integration REPLACES both old integrations, so nothing may depend on an androidtv/androidtv_remote entity anymore. Removed: * the setup-form field + its entity selector (CONF_ANDROIDTV_ENTITY is no longer offered, only read by the migration that drops it); * the entity-derived unique_id derivation (_unique_id_from_atv_device, t29 G2) and, with it, every registry lookup for another integration's devices — the user step now sets NO unique_id at all; the protocol MAC learned during pairing is the identity; * the Jellyfin auto-detect's second tier (the atv entity's friendly_name) — it now uses the entry name the user typed or the device name from discovery; * the app-name and app-id tiers (media_player.app_name/app_id + the shared resolve_app_name chain, the Current App sensor, the sensor's state-change listener and its `_androidtv_state` helper) — the manager's store is the only source; * the "transitional external entity" command tier in every path: `_cmd` and the pan-media_player service call are gone, and so is the fallback in _media_cmd, _remote_key_cmd, the §7 channel-digit sequence and the intent self-heal retry (an intent no transport served now surrenders to the truth instead of delegating); * the entity-derived power signal (`_on_source_change`'s "device off clears the intent" check no longer watches another integration's entity; the power LADDER — protocol is_on → ADB wake → power switch — is unchanged). turn_on/turn_off are native now: they send the protocol POWER key and, like the remote entity, send nothing when the known state already matches (no superfluous toggle). An ADB-only entry (protocol skipped) has no power write path — the manager owns the socket and exposes no device-keyevent, which is what the old delegation used to cover. helpers.discover_adb_host — the one helper that derived a host from another integration's config entry — is deleted; button.py (granted to this task) reads the entry's own address fields instead of the entity, so the Pair ADB button keeps working. Present-day entries that still carry the key are unaffected (nothing reads it); the v3 migration that drops it lands with the host unification. Verified: 309 passed / 2 skipped, 310 / 1 with androidtvremote2, ruff clean, drift engine exit 0 on HEAD (13 modules, 58 imports, 35 call arguments, 6 awaited HA calls) and 16 passed in the real-HA drift suite. The stub harness's legacy service recorder became a watchdog: any HA service call from the media player now fails the routing tests instead of being the expected outcome. |
||
|
James Mackay
|
60d884b76a |
fix(availability): F3 — any usable transport keeps the entities up (t40)
Reported from a live install: with ADB enabled but unreachable and a paired, healthy protocol transport, the media player went unavailable even though the protocol still served power/app/volume. The old rule was ``manager.connected or manager.awake is not None or not manager.available`` — i.e. "a configured ADB manager that is down ⇒ the whole entity is down" (t5-F2), which ignored transport B entirely. The ladder is now: * protocol transport wired and not reported unreachable — usable; * or ADB connected — usable; * or fresh wake data (the poller has spoken) — usable; * or a passive manager (no ADB configured at all) — usable, because nothing to transport is not a broken transport. Unavailable only when a CONFIGURED transport is actively down and no usable data exists. One implementation, ``entity_available`` (media_player.py), is used by the media player, the remote (whose ladder previously duplicated a weaker version of this rule) and the diagnostics payload. Diagnostics grew ``transport.entity_available`` plus ``adb.configured`` so a support dump answers "why is my entity unavailable?" — the ladder inputs and its verdict are now in the same place. Pinned by tests: the exact user case (ADB configured+down, protocol up ⇒ available), all-down (⇒ unavailable), wake-data rescue, connected ADB without a protocol, the passive manager, a configured-but-down transport with no data, the remote following the same ladder, and the diagnostics verdict. Verified: 310 passed / 2 skipped, 311 passed / 1 skipped (androidtvremote2), 16 passed in the HA drift suite, ruff clean. |
||
|
James Mackay
|
dd23d01a88 |
fix(sensor): F2 — the Current App sensor shows the resolved app name (t40)
Reported from a live install: the sensor showed ``ar.tvplayer.tv`` while the media player showed "TiviMate" — the same foreground app, two identities. The sensor's ``native_value`` returned ``manager.foreground_app`` (the raw package push) or ``running_apps[0]["package"]`` as-is, while the media player resolved names through apps map → cached manifest label → androidtv entity tier. Both now call ONE function, ``resolve_app_name`` (media_player.py), so the two entities cannot disagree again; the sensor keeps the raw package as its ``app_id`` attribute and ``app_name`` carries the shown name. Per the contract: * apps map wins, then the cached manifest label, then the androidtv entity's ``app_name``, then the package text itself; * a bare or unknown package falls back to the package text, NOT None (it used to be possible for the value to vanish while the app was plainly running); * pinned by tests: map tier, label tier, entity tier, the ADB-recents source, the unknown-package fallback, and sensor == media player for the same inputs. Verified: 302 passed / 2 skipped, ruff clean. |
||
|
James Mackay
|
be3abae3e7 |
test(ci): F4 — entity-contract pass in the drift sweep + real-HA state-write proof (t40)
The F1 blocker was invisible to every existing test: the stub harnesses call the
entity and replace ``async_write_ha_state``, so a non-property
``extra_state_attributes`` looked perfectly healthy. Two new gates close that
hole.
1. ``ha_api_drift.scan_entity_contracts()`` — a pure-AST pass (so it needs no
homeassistant, unlike passes 1-3) that walks every class inheriting from an
HA entity base, including our own intermediate ``NvidiaShieldSensorBase``, and
flags any HA-READ attribute (``extra_state_attributes``, ``available``,
``state``, ``native_value``, ``media_*``, …) declared as a plain method
instead of ``@property``. It is wired into ``scan()`` before the
homeassistant gate and exported for the fast suite, so ONE implementation
backs the sweep, the CLI red proof and tests/test_entity_contract.py (the t39
F1 lesson).
Red proof on the pre-fix tree:
media_player.py:828: entity-attribute-is-a-method:
NvidiaShieldMediaPlayer.extra_state_attributes is a plain method, but HA
reads it as an attribute (``attr |= self.<name>``) — declare it @property
…and HEAD is clean (13 modules, 60 imports, 35 call arguments, 7 awaited).
2. tests/test_ha_api_drift.py now builds each entity under REAL homeassistant
(real ``PlatformData``, real ``async_write_ha_state``) and asserts HA's own
``__async_calculate_state`` succeeds and that the media player's app_id/
app_name survive the merge — for the media player, both sensors, the remote
and the pairing button. A coverage test compares that matrix against the
sweep's classification, so a new entity class cannot quietly skip it.
Verified: 296 passed / 2 skipped (fast suite), 16 passed (drift suite under
Python 3.14 + homeassistant 2026.10.1), ruff clean; the entity-contract red
proof above reproduces exactly the reported line.
|
||
|
James Mackay
|
acc7bc3d01 |
fix(media_player): F1 — extra_state_attributes must be a property (t40 blocker)
Reported from a live install: the entity never rendered, and every state write
raised ``TypeError: 'method' object is not iterable``.
Root cause, from HA's own code (homeassistant/helpers/entity.py,
``__async_calculate_state``):
if extra_state_attributes := self.extra_state_attributes:
attr |= extra_state_attributes
``attr`` is a dict, so ``|=`` needs a mapping — and this was declared as a plain
method while the sensors' were properties. Reproduced against the pre-fix tree
with real HA: instantiating the entity and calling ``async_write_ha_state()``
prints ``REPRODUCED TypeError: 'method' object is not iterable``; with the fix
the same call writes the state and the extra attributes through.
The stub suites could never see it: their harness replaces
``async_write_ha_state``, so the method was simply called and returned a dict.
tests/test_ha_api_drift.py now drives HA's real state calculation for every
entity, and the drift sweep grew an entity-contract pass (next commit).
|
||
|
James Mackay
|
c588a7c66b |
docs(readme): record the MAC-less discovery contract (t37 D3, t39)
t37 made MAC-less zeroconf discovery honour Home Assistant's own `_async_handle_discovery_without_unique_id` contract — the helper is a coroutine and its dedup (abort `already_configured` when ANY entry exists for this domain, abort `already_in_progress` when another flow runs) only happens when it is awaited. That is stricter than the accidental host-equality guard it replaced, so it changes what a user can expect: * with a MAC, discovery still aborts duplicates and updates a moved device's hosts; * without a MAC, a second (or third) Shield must be added MANUALLY — discovery will abort as "already configured". Someone with two Shields would otherwise read that abort as a bug. The README now has a "Discovery (zeroconf)" section under Configuration stating the rule and the manual route, and the drift sweep's docstring carries the same note next to the D3 entry, so the decision is discoverable from wherever you meet it. Verified: 289 passed, 2 skipped; ruff clean.v2026.10.11 |
||
|
James Mackay
|
fd62d850b7 |
fix(drift): F1+F2 — the sweep refuses false greens, and CI provisions the real floor (t39)
t38 found two holes in the drift gate itself. F2 — the job could never run where it mattered. Home Assistant 2026.10.1 declares ``Requires-Python >=3.14.2`` (PyPI metadata), so the old guard let a 3.13 runner through and the ``homeassistant>=2026.10`` install then failed with a resolver error. The job now provisions its own interpreter (``actions/setup-python@v5`` with ``python-version: "3.14"``) and the guard runs against the REAL floor, read from ``ha_api_drift.HOMEASSISTANT_PYTHON_FLOOR`` — one source of truth, pinned by a test against the installed distribution's own ``Requires-Python`` metadata. Verified: on 3.13.16 the guard refuses with "Home Assistant needs Python >= 3.14.2, but this job runs 3.13.16 / Fix: raise actions/setup-python…"; on 3.14.8 it reports "interpreter ok"; and ``uvx --python 3.14 --with 'homeassistant>=2026.10'`` resolves 2026.10.1. F1 — the standalone entry point was a false green. It printed counters and exited 0 even when it had walked nothing, which is exactly how the manual red-proof tool gets misused. The non-vacuity floors now live in ``ha_api_drift.FLOORS`` and are enforced in ONE place, ``Report.floor_violations()``, used by the pytest wrapper and by ``__main__`` (so the constants cannot drift apart; a test asserts the wrapper holds no floor-like constants of its own). On top of that the sweep can no longer lie in either direction: * an empty or mis-targeted directory reports ``FLOOR:`` lines and ``drift sweep FAILED`` and exits 1 (a non-existent path is a ``bad-path`` finding, not a traceback and not a silent zero-module sweep); * with no homeassistant installed it returns a single ``no-homeassistant`` finding with zero counters — never a wall of phantom "broken-import"s, never a green exit. Verified in the CI-equivalent environment (Python 3.14.8 + homeassistant 2026.10.1): HEAD exits 0 with "no Home-Assistant API drift (13 modules, 60 imports, 35 call arguments, 7 awaited HA calls)"; the empty directory exits 1 with 4 FLOOR lines and no findings; a missing path exits 1 with the ``bad-path`` finding; and the full pre-fix tree (``git archive 5ede4fe``) exits 1 with exactly the three t36 drifts (media_player.py:965 BrowseMediaSource, diagnostics.py:93 homeassistant.util.diagnostic, config_flow.py:425 missing-await) and 0 floor violations. The fast ``test`` job and ``release.yml`` are untouched; the workflow is unchanged in shape except the new provisioning + guard steps, and parses as valid YAML. |