a8041e14280c3637ed009ef73d1f1b759c9e55d2
104
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 James Mackay
|
2819ee5a49 |
fix(media_player,config_flow): close the batch review's O2 and O3 (t17)
Both observations from the t8-t12 batch review, settled by execution rather than
by argument.
O2 — WHICH STATES MAY CARRY A NOW-PLAYING ROW. The review read
`extra_state_attributes` and argued the row publishes the entity state verbatim,
so `now_playing_state: on` is reachable, while the README documents only
playing/paused/buffering/idle; t10's author had reported that `on` and `off`
publish no row at all. The probe settles both halves:
* the t10 account is true on the NATURAL ladder — `on` means no source reported a
power or playback fact, and every tier that can produce a reading sets
`reported = True` in the ladder, so the two cannot meet;
* the review is right about the RULE. It was "publish unless the state is off", an
assumption nothing enforced: forcing the ladder to `on` (and to `standby`, which
the README does not even mention) published `now_playing_state: on`/`standby`
beside a real title.
So the rule is now explicit — `NOW_PLAYING_STATE_VALUES = ("playing", "paused",
"buffering", "idle")` — and a reading is dropped whole when the entity state is
anything else, exactly as `off` always dropped it. Nothing legitimate is lost: a
tier only ever answers in one of those four (`idle` for "reported, nothing
playing", which is where the foreground tier's remembered-playback bridge lands).
The README says the same thing in the table row and in prose, and
tests/test_now_playing_publication.py walks EVERY `MediaPlayerState` member with a
reading present (the four publish themselves; on/off/standby publish nothing),
then reads the four values back OUT OF THE README and compares them to the
constant, so code and docs cannot drift again.
O3 — A REJECTED APP PASTE NAMES ITS BAD LINES. The bulk paste was all-or-nothing
and reported a generic `invalid_app_map`, so a 30-row paste with one typo had to
be bisected by hand. `_app_map_parse` now returns `(map | None, problems)` where
each problem carries the 1-based line NUMBER, the line as the user typed it, and
why (empty/space-bearing package, empty name, space in the icon), and the
rejection puts them in the form's `description_placeholders["invalid_lines"]`,
which the error copy interpolates — capped at 4 named lines with the remaining
count, so the message stays readable. The map is still None when anything is
wrong (never half-stored) and the detail is cleared as soon as a paste parses.
The single-row editor is untouched and keeps its own `invalid_app_row`; a test
asserts the new per-line report and `_valid_app_row` accept and refuse the same
rows, so a paste and a typed row cannot disagree.
Verify: 794 passed / 4 skipped, 795 passed / 3 skipped (with androidtvremote2),
ruff clean.
|
||
|
James Mackay
|
91d9fb3883 |
feat(intents): discover an app's intents from its activity resolver table (t16)
First half of the approved intents feature (docs/INTENTS_PLAN.md §2): the scan
that answers "what can this app actually be asked to do". No UI is wired here —
this is the discovery module, the manager path it rides on, and the tests.
THE DUMP IS FILTERED ON THE DEVICE. On the user's Shield `pm dump
com.netflix.ninja` is ~2241 lines of whole dumpsys for ONE app, so shipping it
over ADB to read a few dozen resolver lines repeats the mistake `| head -300`
made for app labels (t66 — where a truncation artefact was read as "this app has
no label"). The fetch now sends:
pm dump <pkg> 2>/dev/null | sed -n "/Activity Resolver Table/,/Service Resolver Table/p" | head -n 400
with a bounded `grep -A 400 -m 1 -F 'Activity Resolver Table:'` fallback for a
device whose toolbox cannot do sed ranges, tried only when the primary answer did
not contain the section. The package id is the only caller-supplied text in
either command and is always shlex.quote'd, and the sed script is built from two
constants — tests drive eight hostile package ids (semicolons, &&, backticks,
$(), pipes, both quote kinds, a newline) and assert that the shell sees the
package as exactly one verbatim token and that no extra command token appears.
WHAT A CANDIDATE CARRIES: component, action, categories and the data
(scheme/authority/path, plus the matcher kind — `/title.*` is a GLOB, not a
path, and a caller that treats it as literal builds the wrong URI). A filter
carrying several schemes/authorities/paths expands to one candidate per
combination, because each is a concrete match, and dedup collapses the
coincidences. MIME keys are deliberately NOT modelled (they are not one of §3's
fields) — such a filter is still reported through its component, action and
categories rather than being invented into a URI.
RANK, NEVER HIDE. The four families the plan names as internal-looking noise
(DEBUG_*, androidx.*, com.google.firebase.*, ConstraintProxy*) are FLAGGED, not
deleted: they sort last so the useful rows come first, and the user still
decides. Ordering is total (flag, action, scheme, authority, path, component,
categories) so a re-scan is byte-identical, and the 200-candidate cap is applied
after dedup and DISCLOSED through truncated/found — a capped answer never looks
complete.
THE CACHE KEEPS AN ANSWER AND A FAILURE APART, structurally: put() is its only
writer, only a scan that RAN calls it, and it derives the entry's state — an
empty candidate list is INTENT_ENTRY_EMPTY, a real ANSWER the device gave, while
a scan whose transport failed caches nothing and is retried. A stale entry is
dropped on read rather than served. The scan itself has one status per outcome
(fresh / fresh_empty / cached / adb_disabled / adb_unavailable / unreadable /
invalid_package) so "ADB is off", "the device did not answer" and "this app
advertises nothing" can never be confused, and the not-answered statuses carry no
candidates at all — an ADB-down scan cannot look like an app with no intents.
ADB disabled and ADB unreachable are told apart from the entry's own option.
Discovery is EXPLICIT and single: one package per request, never a sweep over the
installed apps, and a test drives a watchdog cycle and asserts it starts no scan.
Concurrent requests for the same package share one dump.
THE FIXTURE IS A TRANSCRIPTION, NOT IMAGINATION. tests/fixtures/
pm_dump_netflix_resolver.txt reproduces the user's 2026-10-12 capture structure
(whole-dumpsys envelope, the activity table with its Schemes/Non-Data Actions
groups, the receiver/service/provider tables that bound it) and the test file's
docstring states line by line what is VERBATIM from the ticket/plan and what is
reconstructed — no real Shield was available, and the raw capture is not in this
repository (the same limit t13 recorded). The capture's own entries are each
pinned by an assertion: the cast entry, http/https with the four categories and
the five www.netflix.com path globs, nflx:, netflix:, MAIN with LEANBACK_LAUNCHER
+ LAUNCHER, NETFLIX_KEY_START, DIAL_START and the two DEBUG_* noise actions —
18 candidates, deduplicated, with the receiver/provider tables proven unread.
Evidence: 59 new tests in tests/test_intents.py (parser against the transcribed
capture, degradation on shapes never seen, the command shape and quoting safety,
the cache states and TTL, and the manager: one shell, cached, retried on failure,
fallback recovery, ADB-off versus ADB-down, the cap disclosed, the watchdog
silent, concurrent sharing). No device was available: the parser is proven
against the transcription, and every "runs/does not run" claim is an assertion on
the poller's recorded commands rather than on intentions.
Verify: suites 778/4 and 779/3; ruff clean; drift exit 0 (26 modules, 76 imports,
242 relative imports, 1417 statement blocks, 57 call arguments) with the
relative-import floor raised 195 -> 235 and the statement-block floor 1300 ->
1400 by the new module rather than dodged; the real-HA glob still runs green
(56 passed, 0 skipped).
|
||
|
James Mackay
|
14c168c25e |
feat(remote): the app/activity picker, and a protocol transport that really connects (t15)
The user's remote displayed but offered no app picker, and the protocol transport was never actually switched on. Both halves are fixed here. REMOTE PARITY. The entity declares RemoteEntityFeature.ACTIVITY — exactly what core's androidtv_remote declares — and publishes activity_list, which is what makes the frontend render an activity picker at all. The list is the app directory (the user's apps map, else the curated table as a floor) in map order, then the apps ADB discovery found and something can name, in the one name chain (user map > device label > curated > package id). Nothing nameless is listed as a bare package id, and current_activity is None until something is known: no activity is invented. remote.turn_on honours activity= by waking the device when it reports itself off and then launching through the existing launch path; a listed name resolves case-insensitively, a package id or a detail link passes through, and anything else raises unknown_activity naming the value instead of silently doing nothing. An empty/absent activity keeps the previous behaviour exactly, async_turn_off is untouched, and a configured command script still owns the sequence. WHY THE TRANSPORT WAS DEAD, MEASURED. androidtvremote2's keep_reconnecting only starts _async_reconnect, whose whole body is `while self._remote_message_protocol:` — false on the first check with nothing connected, so the task returned immediately: no connect attempt, no retry, and the auth callback was never reachable. HA core connects FIRST and only then keeps it alive. Our _start_protocol called keep_reconnecting after generating a certificate and never called async_connect anywhere, so every send_key_command/send_launch_app_command raised ConnectionClosed internally, the helpers swallowed it into `return False` plus a debug line, and no push (is_available/is_on/current_app/volume_info) ever arrived. The fix is a background connect loop: async_connect under a bounded timeout, network failures retried with the ADB watchdog's jitter and backoff, InvalidAuth routed into the EXISTING reauth callback (entry.async_start_reauth), and keep_reconnecting only after a session is up. Setup never blocks on a transport. A send that cannot run is no longer silent: it is counted, its error recorded verbatim, and logged at WARNING with the transport state, and the remote publishes protocol_state / protocol_connected / protocol_ever_authenticated / protocol_last_error / protocol_failures / protocol_certificate plus sticky per-push *_seen flags — so "sent" and "never switched on" can never look alike again. CASE (b) IS FIXED, NOT DOCUMENTED: switching the transport on in the options flow for an entry that never paired now routes into the real async_step_protocol_pair, and "continue without pairing" persists the transport OFF. One marker carries the pairing (CONF_PROTOCOL_PAIRED), written by the setup pairing step, the reauth finisher and the reconfigure finisher, so no entry can sit enabled-but-unpaired. The certificate from the first pairing is reused by design, so a paired entry is asked for no new PIN — which is also why the marker, not certificate material, is the source of truth. ADB STAYS THE PRIMARY STATE SOURCE. Arming the protocol introduces pushes that were never live before, so the reading now names its tier: manager.foreground_source drives the media player's app_source and the remote's activity_source, and the new tests prove a full protocol push set leaves the media state, the now-playing reading and every ADB store field untouched, while an ADB-only entry still serves media player, sensors, apps, scripts and receiver with only the protocol surface degraded. Copy: unknown_activity, activity_launch_failed and protocol_unavailable in BOTH strings.json and translations/en.json, pinned by a test; the README documents the picker, activity resolution, how to read the transport-state attributes, the script bypass (a script-routed press is never evidence of pairing), the connect-then-keep-alive lifecycle and cert reuse versus the other integration's own pairing. Evidence: 6 new real-HA FlowManager tests in tests/test_config_flow_real_ha.py (unpaired -> pairs, PIN -> marker, skip -> off, paired -> not asked again, and the setup flow's marker on success and on skip), a real-HA entity contract test in tests/test_ha_api_drift.py (exact supported_features equality with core, activity_list/current_activity reaching the state machine, live _attr_ updates) and new suites in tests/test_protocol_transport.py (connect-first, keep_reconnecting alone is a no-op, retry, auth rejection, honest send failures, ADB-primary precedence) and tests/test_command_routing.py (the activity contract, turn_on(activity=), script ownership, the transport attribute states). No device and no browser exist here: device behaviour is inferred from the library and core source, and every UI-facing claim is proven at the entity/registry/metadata level under real Home Assistant. Verify: suites 719/4 and 720/3; real-HA glob (tests/test_adb_command_real_ha.py + tests/test_config_flow_real_ha.py) 56 passed, 0 skipped; ruff clean; drift exit 0 (25 modules, 76 imports, 206 relative imports, 1322 statement blocks, 57 call arguments) with the floors raised 40->70, 100->195, 500->1300 and 52->55. |
||
|
James Mackay
|
6aa3b6bfdd |
feat(adb_command): selectable target, strict resolution, and a real response (t14)
The user's live run could not select his Shield for
nvidia_shield.adb_command, so he ran his diagnostics through the deprecated
androidtv integration instead. Measured against the installed HA (2026.10.1),
the cause is a contract, not a bug in the handler:
* ServiceRegistry.async_register has NO target parameter, so a target cannot be
declared at registration;
* the frontend renders the target picker from the service DESCRIPTION, which
HA builds from services.yaml or from the runtime async_set_service_schema —
and the runtime description WINS.
So async_setup now registers with supports_response=OPTIONAL and publishes
ADB_COMMAND_SERVICE_DESCRIPTION (target: {entity: [{domain: media_player}]},
fields, prose mirrored in strings.json/translations/en.json and pinned against
both by a test). No services.yaml is needed, and none is added.
The target resolution is strict, because the failure mode of guessing is a raw
shell command on the wrong device: exactly one PARTICIPANT, told apart as
no_target (nothing selected), unsupported_target (not a participant / option
off / another integration's entity) and ambiguous_target (two selected — sent
to neither). Accepts what HA actually sends: entity_id as text or as the LIST a
picked target produces, and device_id (an automation targeting the DEVICE),
expanded only through this integration's own registry entries and only to media
players. The output now comes back in the response under adb_response — the key
the user's captures read — while an unreachable Shield stays adb_unavailable,
an error rather than an empty response, and a call that does not ask for a
response is unchanged.
Evidence: 12 real-HA tests (new tests/test_adb_command_real_ha.py) drive the
real service registry, the real get_services websocket payload (the exact JSON
bytes the browser parses: target present, response optional, both fields) and
real async_call for every shape above. t14's CI gate globs tests/test_*real_ha.py
so a new real-HA file cannot skip silently; the skip/empty-collection guards stay
and were executed. The real-HA probe also caught async_entries_for_device being
a MODULE-LEVEL helper (not a registry method) that the stub had hidden, so the
drift gate's owner map now covers async_register, async_set_service_schema and
async_entries_for_device, with the call-argument floor raised 44 -> 52.
Verify: suites 681/4 and 682/3; real-HA 38 + the new file 12 (50 with the glob);
ruff clean; drift exit 0 (54 call arguments).
|
||
|
James Mackay
|
7b32de789a |
t66: app names on a device that exposes none
The reported Shield names no app: `pm dump com.netflix.ninja` is ~2241 lines of
dumpsys and grep for `application-label` over the whole capture finds nothing.
The probe shipped `| head -300`, which cut the dump BEFORE the package section,
so "no label" was a truncation artefact; the sweep then named every app after
its own package id, and the apps editor pre-filled the `pkg=pkg` row the user
read as a bug.
* the probe greps ON THE DEVICE for the three shapes Android prints
(`application-label`, `nonLocalizedLabel`, `labelRes`); a quoted value is a
name, `null` and a resource id are not. The dump is never truncated.
* after APP_LABEL_PROBE_LIMIT consecutive "ran, no label" answers the device
is taken at its word: the sweep stops spending a shell per package
(APP_LABEL_TTL expiry re-checks, and a package whose probe FAILED still owes
its retry — t57 UNIT 0 survives, driven by the app on screen).
* ONE precedence chain in `ha_adb_common/app_label.resolve_package_name`, used
by the media player, the Current App sensor, the discovery table, the source
list and the apps editor: user map > device label > curated map > package id
(the id shown ONCE, `name` empty when nothing names the app).
* the curated table covers the user's 21 apps (community-maintained, not
device truth) and the Configure row says when the device named nothing.
* the real device shape is a test fixture (trimmed, and marked as such in the
file: the raw capture is not in the repo) pinning that no label is
extracted, that the probe count is bounded, and that `Netflix` surfaces.
Also drops the dead `ATTR_MEDIA_DAG_NOW_PLAYING` re-exports left by the t65
flattening and updates the RESEARCH_SHIELD now-playing note to the flat names.
No real Shield was available: the fetch/burst behaviour is exercised against the
documented Android output and the trimmed reproduction of the reported capture.
|
||
|
James Mackay
|
f29f62ddae |
feat(image): opt-in screen capture as an image entity, binary-safe over ADB
The user asked for "an image entity of the device screen capture? Can that be added". It is added as `image.<entry>_screen_capture`, and the three boundaries are the feature: - it NEVER drives state — the state is the capture timestamp, and the capture path reads the ADB poller and nothing else (pinned by a manager that raises on any other attribute, and by a real-HA run whose recorded state is the timestamp with no bytes anywhere in the attributes); - it is OPT-IN and OFF by default (`screen_capture_enabled`, Main setup row) — with the option absent `async_setup_entry` adds no entity at all; - it NEVER polls — HA's own `should_poll = False` for image entities is kept, so only an image fetch or `homeassistant.update_entity` captures; concurrent fetches share one shell and a capture is reused for 5 s. BINARY-SAFE CAPTURE: `screencap -p | base64` over the existing shell, decoded locally with `base64.b64decode` and gated on the PNG signature AND the IHDR chunk. `AdbDevice.shell()` returns `str`, so raw PNG bytes cannot travel through it; the alternative (a device temp file + the library's file-read path) needs the raw AdbDevice and writes to the Shield's shared storage, where a crash between write and delete leaves a screenshot behind. The base64 route leaves no device-side artifact — nothing to clean up — at the cost of ~33% more bytes on the wire. HONEST FAILURES: ADB off / ADB down / a capture that returned no PNG / a transport that raised all make the entity UNAVAILABLE with `capture_status` (`adb_disabled` / `adb_disconnected` / `capture_failed`) — never a blank or zero-byte image. `image_last_updated` moves only on success. Tests: 25 new cases (a real 2x1 PNG fixture on the wire — bytes, never a str; every non-PNG answer rejected; disconnected/failed/recovery paths; capture reuse, coalescing and the expiry window; opt-in default OFF and on) plus 4 config-flow cases (schema default False, off stays off, on is stored, and it can be turned back off). The stub harness gains the image platform and `util.dt` faces, the real-HA entity matrix gains this class, and the drift sweep's coverage test demanded exactly that. README documents the cost, the opt-in and the boundary. |
||
|
James Mackay
|
a1ce28996e |
feat(media-player): flatten now-playing into separate attributes and name the tier
The user reads attributes as YAML — "These could do with being separate attributes rather than an object in one?" — so the nested `media_dag_now_playing` object is gone and every field it carried is its own top-level attribute: now_playing_via, now_playing_state, now_playing_title, now_playing_artist, now_playing_album, now_playing_media_type, now_playing_series, now_playing_season, now_playing_episode, now_playing_duration, now_playing_position, now_playing_position_updated_at, now_playing_thumbnail, now_playing_media_info Rules the surface now holds to: - an unknown field is OMITTED, never published as None (`title: null` inside the object was exactly the noise this removes); 0 and False are real values - a device reading `off` publishes no now-playing row at all — a Jellyfin session that has gone stale must not put a title beside a device that is off — and `on` (nothing reported) publishes none either, because there is no reading - `now_playing_state` IS the entity's own state, so the ladder stays the single answer even while the foreground tier's last-known bridge serves a title - the tier indicator is `now_playing_via` (`jellyfin` / `media_session` / `app`), NOT `source`: on a media_player `source`/`source_list` mean the selected app input (`Netflix`, `TiviMate`, …) and they still do — a tier named `source` is what made the reading look like media metadata from somewhere else. The internal reading key is renamed `via` as well, and the Jellyfin tier now names itself too - every published name is declared unrecorded (G12): these change every poll COMPATIBILITY: `media_dag_now_playing` is removed, not deprecated, and no alias is kept — a nested copy would double every poll's payload. README documents the exact names, these rules, the sensor's own flat set and a one-line template migration. sensor.py: the Jellyfin now-playing sensor follows the same no-None rule (`title` and `media_type` used to be published as null there). Tests: 17 new cases pin the flattening, the tier rename, the five states, the off/on gate, the playing->idle transition, the last-known bridge, privacy mode and agreement with the Current App sensor; the tier-dict tests follow `via`. |
||
|
James Mackay
|
46738063aa |
fix(options): edit apps one row per step in a multiline field, and report discovery
The live Apps page showed the whole map as ONE long line with nowhere to add a
row. Cause: the field was a bare `vol.Optional(...): str`, and HA renders a
selector-less string as a SINGLE-LINE input (pre-fix wire field:
{"type": "string", "name": "app_name_map"} — no selector key). It is a
multiline TextSelector now: the real-HA test pins
selector.text.multiline on the wire and an N-line save/reopen round trip.
HA has no table selector, so the Apps sub-screen is a menu of row actions
(whole list as text / add / edit / remove) instead of one form:
- the EDIT row lists every app the user can act on: the rows they saved AND the
apps discovery found, marked `[detected]`, each labelled "name (package id)"
or "package id (no name yet)" — the user's Shield exposes no device labels at
all, so the unnamed case is the common one and it must not read as its own id
twice, and picking it is how a name gets set (saving stores the row)
- add: package + name + optional icon, refused by the SAME `_valid_app_row`
rule the text field uses, with an error on the page
- edit: PREFILLED from the stored row (suggested values — a voluptuous
`default=` never reaches the browser, so the form would have opened blank);
the device's mdi: icon is never prefilled into the icon column
- remove: pick a SAVED row, then an explicit confirmation menu; cancelling
keeps it (a detected app has no line to delete)
- edit/remove rows are offered only while something is stored
- every mutation writes ONE row through `_write_app_row`/`_drop_app_row`, so
untouched rows and their icons stay byte-for-byte as they were
The Discover row was a silent no-op ("seems to load then do nothing"): it now
returns a one-line verdict the menu shows — N apps found / the Shield reported
none / the sweep could not reach the Shield / ADB is not set up — while still
never raising and never caching a failed sweep as an answer.
Also fixes `async_step_menu_again`, which the real FlowManager dispatches with
user_input and which raised TypeError for every menu offering that row (only
reachable through a stub before, hence unnoticed).
Docs: README and BOTH strings files (byte-identical) describe the editor, the
one-per-line format, the detected-app dropdown and the four Discover verdicts.
|
||
|
engineer
|
eb62011ded |
fix(media-player): the browse root carries a real string id (t63)
The live report: "Browse media seems to spin and never finish loading". The cause is in what async_browse_media HANDS BACK, not in the method — it returns with no awaits, so nothing inside it can hang. The root node's media_content_id was None, and HA core never serves a node without a string id: across the 43 BrowseMedia constructions in the installed core media_player platforms, zero pass None (the roots use a literal/module constant — arcam_fmj's "root", "apps", BROWSE_ROOT, TUNER_PRESETS_ROOT — or "" for a pure container). The media-browser frontend keys the node it is loading on that id, so a null id left the dialog re-requesting the same node for ever. BROWSE_ROOT_ID = "root" is now what the entity serves for EVERY requested id — the root id, None, or anything a stale frontend asks for — so a repeat request is idempotent rather than a loop. The type was reassessed as well: MediaType.APPS (t61's choice) is what core pairs with children=[...] — a folder OF apps — so on a zero-child node it invited the frontend to treat it as an unloaded collection. No MediaType member means "a directory with nothing in it", so the node carries the class it already declares (MediaClass.DIRECTORY -> "directory"), which is exactly what can_expand=False and children=[] say. Verified over the real path the frontend uses — websocket_browse_media -> player.async_browse_media -> payload.as_dict() — by driving HA's own decorated handler against a real entity (tests/test_ha_api_drift.py, which the CI drift job runs with Home Assistant installed): the root is a genuine BrowseMedia, its as_dict() survives json.dumps, the id is the string "root", and browse(root_id) == browse(None) in shape. Red proof on the pre-fix source: the new real-HA test fails at `assert isinstance(payload["media_content_id"], str)` -> `isinstance(None, str)`. The frontend bundle is not installed here (no hass_frontend) and there is no browser to drive, so the rendered dialog is NOT exercised: this pins the backend half of that path, end to end through the handler's decorators. |
||
|
engineer
|
431b7cd8f4 |
feat(media-player): a switch Receiver is ensured on, and apps leave the browser (t61)
Two halves of the same live-run report.
RECEIVER AS A SWITCH. The user: "it would be nice if a receiver could just be a
switch, I don't need my volume scripts then because the receiver portion could
just make sure the receiver is on if it's a switch". `_ensure_receiver_on()`
runs at the top of the three playback entry points — turn_on, play_media,
select_source — and calls switch.turn_on only when the configured switch
receiver reports `off`: an `on` switch gets no call (idempotent), an
unavailable/unknown/missing state is left alone with a debug line rather than
guessed at, and a failing service call is logged, never raised — a receiver that
cannot be switched on must not stop the Shield from playing. Nothing else about
the switch is touched (no volume, no toggle), and nothing switches it from a
state read, a poll or a listener, so an off receiver stays off while the Shield
is idle. A media_player receiver keeps its behaviour exactly. select_source now
dispatches through the private `_dispatch_play_media`, so one user action looks
at the same switch once; `play_media` keeps its public signature and behaviour.
The Receiver field already accepted switch.* at the validation/strings layer
(t56, pinned by the real-HA tests); the strings copy now says what playback does
with it.
MEDIA BROWSER. The user: "Media Browser shows Apps. To me it should probably be
integrated with the home assistant media display, keep select sources for apps".
`async_browse_media` no longer publishes the app directory: it returns a valid,
standard BrowseMedia root (device title, DIRECTORY class, no content id, no
children) instead of a second, inconsistent copy of the list. Apps stay reachable
through source_list/source/select_source and play_media("app", <package>), which
is unchanged and complete; nothing else on this entity is browsable, so there is
no dead entry to select, and the drift gate's t37 D1 browse assertions still hold.
Tests: tests/test_receiver_switch.py pins all four promised states — off →
exactly one switch.turn_on; on → none; unavailable/unknown/missing/failing →
playback continues and nothing raises; media_player receiver → never switched —
plus the idle-Shield rule. The browse pins move from the removed tree to the
surviving surfaces (source_list, _source_package, _app_directory), and the harness
gains the real hass.states shape (FakeStates) and the real switch state constants.
README documents both changes; both strings files stay byte-identical.
|
||
|
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
|
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
|
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
|
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
|
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). |
||
|
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
|
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
|
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
|
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. |
||
|
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. |
||
|
James Mackay
|
824d6192a1 |
test(ci): D1/D2/D3 — repo-owned HA API-drift sweep with a CI job (t37)
The unit suite validates production modules against hand-written stubs, so a
call site that no longer matches real Home Assistant stays green — that is how
D1, D2 and D3 reached master. This adds the check that would have caught them.
`tests/ha_api_drift.py` (stdlib-only, documented inline) parses every module of
custom_components/nvidia_shield and resolves each Home-Assistant dependency
against an INSTALLED homeassistant:
1. every `homeassistant.*` import — module present, attribute present (or a
submodule of that name exists, which is what `from package import submodule`
means) — catches D1/D2 and any future move;
2. every keyword argument passed to a known HA API (HA_CALL_OWNERS: the
ConfigFlow/ConfigEntries APIs we call) must exist in the real signature —
catches the `options_updates` class of bug;
3. every `self.<method>()` call in a class that inherits from an HA class is
compared with the real definition: a coroutine must be awaited and an
awaited call must be a coroutine — catches D3.
`tests/test_ha_api_drift.py` runs it against the production tree, asserts the
sweep is not vacuous (module/import/argument/await counters), and pins the red
proof with the four pre-fix forms, plus the green proof for the fixed forms. It
skips cleanly when homeassistant is absent, so the stdlib-only jobs are
unaffected.
`.gitea/workflows/test.yml` gains the `ha-api-drift` job — the one place Home
Assistant is installed — guarded by an explicit Python >= 3.13 check (Home
Assistant 2026.x requires it) and the same PEP 668 retry the test job uses.
Red proof against the real pre-fix revisions (svn-style checkout of
`git show HEAD:…` into a temp dir, and the pre-t35 config_flow.py for the kwarg):
media_player.py:965: broken-import: from homeassistant.components.media_player
import BrowseMediaSource: …BrowseMediaSource does not exist
diagnostics.py:93: broken-import: from homeassistant.util.diagnostic import
async_redact_data: module 'homeassistant.util.diagnostic' is not importable
config_flow.py:425: missing-await: self._async_handle_discovery_without_unique_id()
is a coroutine on homeassistant.config_entries.ConfigFlow
config_flow.py:700: unknown-kwarg: async_update_reload_and_abort() has no
'options_updates' parameter (exit 1 on that tree, exit 0 on this one)
Verified: variant 1 289 passed / 2 skipped; variant 2 (androidtvremote2) 290
passed / 1 skipped; ruff clean; `UV_PYTHON=3.13 uvx --with homeassistant --with
pytest-asyncio pytest tests/test_ha_api_drift.py -q` → 4 passed (the bare form of
that command needs the pin here only because this sandbox's default Python is
3.15 and pycares has no wheel for it / no CMake to build it; CI pins the
interpreter explicitly).
|
||
|
James Mackay
|
60fb4f7557 |
fix(config_flow): D3 — await the discovery-dedup coroutine (t37)
`ConfigFlow._async_handle_discovery_without_unique_id` is an `async def` in core (verified: `inspect.iscoroutinefunction` is True) and core awaits it. t32 called it without `await`, so the coroutine body never ran: no `already_configured` abort when the handler already had entries, no `DEFAULT_DISCOVERY_UNIQUE_ID` claim, no `already_in_progress` abort — HA's discovery dedup was dead, Python warned "coroutine ... was never awaited", and the host-equality loop that used to sit here was the only accidental dedup. The call is awaited and the now-unreachable host loop is gone (the real helper is stricter: any entry for the domain, or any in-flight flow). The harness mirrors the real contract instead of a sync no-op: * `entity_stublib`'s hub stub makes the helper a coroutine (a sync stub would turn the production `await` into a TypeError); * `test_standalone_setup`'s stub models the real semantics — a coroutine raising `DiscoveryDedupAbortError` (core raises AbortFlow), driven by the shared holder's entry list and in-progress list — and its tests now assert all three outcomes: awaited-and-proceeds (counter + claimed discovery unique id), `already_configured` when entries exist, `already_in_progress` when another flow runs. A fourth test collects the un-awaited-coroutine warning, and the prefill test in the UX suite asserts the awaited counter. Red proof: with the pre-fix config_flow.py the repo-owned sweep reports `config_flow.py:425: missing-await: self._async_handle_discovery_without_unique_id()`. Verified: 289 passed, 2 skipped (D3 stage); ruff clean. |
||
|
James Mackay
|
bd72ae3170 |
fix(diagnostics): D2 — redact through homeassistant.helpers.redact (t37)
`homeassistant.util.diagnostic` does not exist — verified against a real install: `import homeassistant.util.diagnostic` raises ModuleNotFoundError, and the helper has always lived in `homeassistant.helpers.redact`. The import sat inside a `try/except ImportError` that set the helper to None, so the documented G14 pass — the one carrying `_TO_REDACT`, hosts included — silently did nothing; only the marker scan (pairing-/key-shaped names) masked anything, which is why the existing test's pairing-code assertion stayed green. The helper is now imported at MODULE scope from homeassistant.helpers.redact and applied unconditionally after the marker-name pass. A missing module is a loud import error at integration load instead of a silent downgrade. * the diagnostics test now asserts `adb_host` and `protocol_host` really are masked in the emitted payload (options and a nested data value); * TestFaithfulHaStubs asserts the module USES the helper from that path (`nsh.diagnostics.async_redact_data is homeassistant.helpers.redact.async_redact_data`), that no production module imports the non-existent module (AST), and that the try/except ImportError fallback is gone; * `entity_stublib` gains a faithful `homeassistant.helpers.redact` stub (same masking, same nested mapping/list recursion, same None/empty-string skip) so the harness meets the import the way a real install does. Verified: 287 passed, 2 skipped (D2 stage); ruff clean. Full gate re-run on the completed branch: 289 passed / 2 skipped and 290 passed / 1 skipped. |
||
|
James Mackay
|
37b2accfda |
fix(media_player): D1 — browse builds real BrowseMedia nodes, not BrowseMediaSource (t37)
`homeassistant.components.media_player` has not exported `BrowseMediaSource` for years: the class lives in `homeassistant.components.media_source.models`, where it exists to build `media_source://<domain>` ids and is the only place a `domain` parameter belongs (t30 G18's note was about THAT class). The local import therefore raised ImportError, and with it every real browse failed — while the suite stayed green because the harness stubbed a `BrowseMediaSource` that real HA does not expose from there. `async_browse_media` now builds the standard tree exactly as core's own androidtv_remote/media_player.py does: `BrowseMedia` imported from homeassistant.components.media_player at module scope, every node carrying an explicit media_content_id/media_content_type (MediaType.APP/APPS), `children_media_class=MediaClass.APP` on the directory, and no `domain`. Verified against a real install: the old import raises ImportError, the old `domain=` kwarg raises TypeError, and the new node constructs. The harness is now faithful so this cannot hide again: * `entity_stublib` exposes a `BrowseMedia` with the real keyword-only signature, `as_dict()` and `calculate_children_class()`, plus `MediaType`, and deliberately NOT `BrowseMediaSource` — so both the drifted import and the drifted kwarg fail in-suite; * the app-directory test asserts the standard shape (classes, explicit ids and types, children_media_class, the serialised payload) instead of the removed `domain` attribute; * TestFaithfulHaStubs pins the harness contract on the AST (no production module imports BrowseMediaSource) rather than on prose. Verified: 284 passed, 2 skipped (D1 stage); ruff clean. Full gate re-run on the completed branch: 289 passed / 2 skipped and 290 passed / 1 skipped. |