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

feat: adb_command is a conforming action — always registered in async_setup (option gates USE), services.yaml with required command/entity, strings services+exceptions, ServiceValidationError on empty/unknown-target/ADB-down; participant registry stays (t29 G8)

Browse Source
This commit is contained in:
James Mackay committed 2026-10-09 06:59:44 +01:00
1 parent e51545262e
commit 3e0208789a
3 files changed
+344 -54
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

docs/HA_STANDARDS_AUDIT.md
+238
View File
@@ -0,0 +1,238 @@
# HA integration-standards audit — custom_components/nvidia_shield
Status: **COMPLETE.** Every claim is cited to a developer-docs page fetched during
this audit (October 2026; the docs site shows "Last updated" stamps of Jun–Oct 2026
on the pages cited). Companion docs: `MVP_PLAN.md`, `RESEARCH_SHIELD.md`,
`RESEARCH_IDEAS.md`.
Sources audited (all fetched live):
[Manifest](https://developers.home-assistant.io/docs/creating_integration_manifest/) (updated 2026-10-06) ·
[Config flow](https://developers.home-assistant.io/docs/core/integration/config_flow/) ·
[Options flow](https://developers.home-assistant.io/docs/core/integration/options_flow/) (updated 2026-09-17) ·
[YAML configuration](https://developers.home-assistant.io/docs/core/integration/yaml_configuration/) (updated 2026-09-17) ·
[Config entries](https://developers.home-assistant.io/docs/config_entries_index/) (updated 2026-06-19) ·
[Entity](https://developers.home-assistant.io/docs/core/entity/) ·
[Internationalization](https://developers.home-assistant.io/docs/internationalization/core/) ·
[Quality scale](https://developers.home-assistant.io/docs/core/integration-quality-scale/) + [rules](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/) ·
[Custom actions](https://developers.home-assistant.io/docs/dev_101_services/) ·
[Diagnostics](https://developers.home-assistant.io/docs/core/integration/diagnostics/) ·
[Tests file structure](https://developers.home-assistant.io/docs/creating_integration_tests_file_structure/) ·
[Deprecating (incl. YAML-drop pattern)](https://developers.home-assistant.io/docs/deprecating/) ·
[ADR-0010](https://github.com/home-assistant/architecture/blob/master/adr/0010-integration-configuration.md) ·
[Probatio blog](https://developers.home-assistant.io/blog/2026/09/30/probatio-validation-engine/) ·
[Listener+reload deprecation blog](https://developers.home-assistant.io/blog/2026/05/07/config-entry-listener-together-with-reloading-methods/) ·
[BrowseMediaSource domain blog](https://developers.home-assistant.io/blog/2026/05/20/browse-media-source-root-class/) ·
[Central config-flow abort reasons](https://developers.home-assistant.io/blog/2026/09/28/central-config-flow-abort-reasons/) ·
[Always reload after reauth](https://developers.home-assistant.io/blog/2024/04/25/always-reload-after-successful-reauth-flow/) ·
[HACS publish requirements](https://hacs.xyz/docs/publish/integration/)
Code audited: `manifest.json`, `strings.json`, `translations/en.json`,
`__init__.py`, `config_flow.py`, `connection.py`, `media_player.py`,
`remote.py`, `sensor.py`, `button.py`, `diagnostics.py`, `helpers.py`, tests/,
repo root (`hacs.json`, README).
Legend: ✅ compliant · ⚠️ partial / should improve · ❌ gap (concrete fix in §3).
---
## Part 1 — Compliance checklist
### 1.1 Manifest ([docs](https://developers.home-assistant.io/docs/creating_integration_manifest/))
| Requirement | Us | Notes |
|---|---|---|
| `domain`, `name` naming rules | ✅ | `nvidia_shield` / "NVIDIA Shield" — local-only product, no suffix, per manifest doc's name rules |
| `version` required for custom integrations (AwesomeVersion-compatible) | ✅ | `2026.10.08` (CalVer) — "The version of the integration is required for custom integrations" |
| `documentation` URL | ✅ | points to the repo; for core submission it must be home-assistant.io (not applicable yet) |
| `issue_tracker` | ✅ | present (HACS also lists it as required: [HACS publish](https://hacs.xyz/docs/publish/integration/)) |
| `codeowners` | ✅ | `@notexpectedyet` |
| `iot_class` from allowed set | ✅ | `local_push` — valid enum value; correct now that both the protocol push and the persistent ADB manager are the primary paths |
| `config_flow: true` + `config_flow.py` exists | ✅ | |
| `integration_type` | ⚠️ | **Not set.** Required for core integrations with a config flow; for custom it defaults to `hub` — but we are a single-device-per-entry integration, so it "should generally" be `device`. Explicit is recommended ([manifest doc](https://developers.home-assistant.io/docs/creating_integration_manifest/)). Gap G1 |
| `requirements` | ✅ | `adb-shell>=0.4.0`, `androidtvremote2>=0.2.0`; "Custom integrations should only include requirements that are not required by Core" — both are non-core deps. Range-style (`>=`) is accepted for custom integrations (hassfest pins core integrations to `==`; range bounds are the pragmatic custom-integration choice) |
| `dependencies` | ✅ | `[]` — we read Jellyfin state via the *already-configured* jellyfin integration and discover via the user's zeroconf service; neither is required to be a `dependencies` entry (manifest doc: dependencies guarantee the integration loads before setup, not that entries exist). G4 below covers the Jellyfin soft-dependency behavior if we ever import its API |
| `zeroconf` discovery key | ✅ | `_androidtvremote2._tcp.local.` — exact-service-type entry, not in the "too generic" list |
| `single_config_entry` | n/a | Multiple Shields are legitimate — do NOT set ([manifest doc](https://developers.home-assistant.io/docs/creating_integration_manifest/)) |
| `loggers` | ⚠️ | Not set. Recommended so users can raise `adb_shell`/`androidtvremote2` logging under our domain view; `adb_shell` (we wrap) and `androidtvremote2` are exactly the `getLogger` namespaces of our requirements. Gap G1 |
| `quality_scale` | n/a | Only meaningful for core integrations; custom integrations can still self-audit (§1.9) but shouldn't fake the key ([manifest doc](https://developers.home-assistant.io/docs/creating_integration_manifest/)) |
| `import_executor` | ✅ | default (true) is right for us |
| `disabled` | n/a | |
### 1.2 Config flow ([docs](https://developers.home-assistant.io/docs/core/integration/config_flow/))
| Requirement | Us | Notes |
|---|---|---|
| Extends `ConfigFlow, domain=DOMAIN` in `config_flow.py` | ✅ | |
| `VERSION` (+ migration for old entries) | ✅ | `VERSION = 2` with `async_migrate_entry` handling v1→v2 and rejecting downgrades (`config_entry.version > 2 → False`) — matches the migration contract ("return False → migration_error") |
| Unique IDs: stable, string, not user-changeable; `_abort_if_unique_id_configured` | ⚠️ | Protocol path uses the **MAC from the server certificate** (accepted source, [docs list](https://developers.home-assistant.io/docs/core/integration/config_flow/)). But the fallback unique_id is the **`androidtv_entity` entity_id** — entity IDs are user-renameable, which makes them a weak unique_id source (docs list "Device Name / hostname that can be changed" as unacceptable; a renameable entity reference is the same class of problem). Gap G2 |
| Discovery (zeroconf): set unique ID, abort `already_configured`, update host on IP change | ✅ | `async_step_zeroconf` sets MAC unique_id, aborts duplicates, and updates `protocol_host`/blank `adb_host` on IP change — this is the [gold `discovery-update-info`](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/discovery-update-info) behavior already implemented |
| Discovery steps must never finish without user confirmation | ✅ | first-time discovery chains into the user step |
| Discovery without unique ID → `_async_handle_discovery_without_unique_id()` | ❌ | not used on the no-MAC path; gap G2 (combine with the unique-id fallback fix) |
| `is_matching` / `async_has_matching_flow` for ambiguous discovery | ✅-ish | MAC unique_id set in the flow aborts concurrent flows for the same device (docs: "the flow will immediately abort if another flow is in progress for this unique ID"); the MAC-less path is the only exposure (G2) |
| Errors/aborts via strings keys (`errors["base"]`, `async_abort(reason=...)`) | ✅ | all keys exist in `strings.json` config.error/abort |
| Interactive pairing (PIN) via flow steps | ✅ | `protocol_pair` uses `async_show_progress`-free two-step pattern (PIN prompt); acceptable — `async_show_progress`/`async_show_progress_done` are for long-running background work |
| Reauth flow | ⚠️ | Steps exist (`async_step_reauth`/`reauth_confirm`), triggered via `entry.async_start_reauth(hass)` from the manager's invalid-auth callback ([config-flow doc](https://developers.home-assistant.io/docs/core/integration/config_flow/): reauth must update the entry and abort, not create). But `_finish_reauth` uses `async_update_entry` + `async_schedule_reload` — see G3 (deprecated combo) and [always-reload-after-reauth blog](https://developers.home-assistant.io/blog/2024/04/25/always-reload-after-successful-reauth-flow/); prefer `async_update_reload_and_abort(self._get_reauth_entry(), ...)`, and reauth flows need to be linked to a config entry ([2024-11-04 blog](https://developers.home-assistant.io/blog/2024/11/04/reauth-reconfigure-entry-id/) — ours gets it via `self.context["entry_id"]`, the mechanism the blog's helper replaces) |
| Reconfigure step | ❌ | No `async_step_reconfigure`. The [config-flow doc](https://developers.home-assistant.io/docs/core/integration/config_flow/) says a reconfigure step lets users change **data** that is "not meant to be optional" without an OptionsFlow, and it is a [gold rule](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/reconfiguration-flow). Our hosts live in options, so we are not broken — but G5 documents what a reconfigure step should own (device name + host moves + source toggles) |
| Config-entry mutation only via `async_update_entry` | ✅ | everywhere ([config entries doc](https://developers.home-assistant.io/docs/config_entries_index/)) |
| Full test coverage of config_flow.py ([docs](https://developers.home-assistant.io/docs/core/integration/config_flow/): "Integrations with a config flow require full test coverage of all code in config_flow.py") | ⚠️ | tests exist (`test_config_flow_pairing.py`, `test_config_flow_protocol.py`) but "full coverage" (also [bronze rule `config-flow-test-coverage`](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/config-flow-test-coverage)) needs a coverage run to confirm every step/branch; G6 |
### 1.3 Options flow ([docs](https://developers.home-assistant.io/docs/core/integration/options_flow/))
| Requirement | Us | Notes |
|---|---|---|
| `async_get_options_flow(config_entry) -> OptionsFlow()` **without constructor args**; entry via `self.config_entry` | ❌ | Ours: `NvidiaShieldOptionsFlow(config_entry)` stores `self._config_entry` — the old pattern. Current docs' handler takes **no arguments** and reads `self.config_entry`. Gap G3 |
| Prefill via `self.add_suggested_values_to_schema(SCHEMA, self.config_entry.options)` | ⚠️ | We hand-roll `vol.Optional(key, default=current)` per field — works, but the documented helper is the single schema + suggested-values pattern; also lets us keep ONE static schema instead of conditionally building keys (ours rebuilds the schema dict each render, which also means fields disappear/reappear depending on state — legal but harder to test). G3 |
| Update listener: `entry.async_on_unload(entry.add_update_listener(...))` | ✅ | registered in `async_setup_entry` ([options doc](https://developers.home-assistant.io/docs/core/integration/options_flow/)) |
| Reload-on-options: `OptionsFlowWithReload` OR update listener — **not both** | ❌ | We use the update listener (full reload on every options change, including pure-UI tweaks like app map); the current docs offer `OptionsFlowWithReload` for exactly this. Combined with G3 (the flow's own `async_schedule_reload` calls in reauth/zeroconf) this is the deprecated [listener+reloading-methods combo (error from 2026.12)](https://developers.home-assistant.io/blog/2026/05/07/config-entry-listener-together-with-reloading-methods/). Gap G3 |
### 1.4 strings.json / translations ([docs](https://developers.home-assistant.io/docs/internationalization/core/))
| Requirement | Us | Notes |
|---|---|---|
| `strings.json` adjacent to integration; `translations/en.json` for custom | ✅ | both present, same content |
| `config` (steps/errors/aborts), `options` sections | ✅ | complete for every key we return |
| Common-string references (`[%key:common::...%]`) | ⚠️ | e.g. `already_configured`, `reauth_successful` are common abort reasons — now [translated centrally](https://developers.home-assistant.io/blog/2026/09/28/central-config-flow-abort-reasons/) for shared reasons; harmless duplicated, minor cleanup |
| `title` key | ⚠️ | optional ("fallback to manifest name"); omitted is fine |
| `entity` section with `translation_key` on named entities | ⚠️ | Sensors use hard-coded English names (`_attr_name = "Current App"`, `"Now Playing"`). Entity doc: "Avoid hard coding a natural language name, use a translated name instead"; [gold rule `entity-translations`](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/entity-translations). Gap G7 |
| `exceptions` section for `HomeAssistantError` translations | ❌ | we raise/log instead of raising translated exceptions in the action handler (see G8; also [raising exceptions](https://developers.home-assistant.io/docs/core/platform/raising_exceptions/) pattern) |
| `services` section + `services.yaml` | ❌ | `nvidia_shield.adb_command` is registered dynamically with a schema but has **no `services.yaml` and no `services` strings** — see G8. Note [2025-10-24 blog](https://developers.home-assistant.io/blog/2025/10/24/service-web-api-changes/): service translations were removed from the WebSocket/REST service list — descriptions now live only in `services.yaml`/strings, making them required for frontend display |
| `issues` section (repairs) | n/a | no repair issues yet (gold [`repair-issues`](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/repair-issues) — G9 suggestion for the ADB-unpaired state) |
### 1.5 Entity standards ([Entity doc](https://developers.home-assistant.io/docs/core/entity/))
| Requirement | Us | Notes |
|---|---|---|
| `has_entity_name = True` ([mandatory for new integrations](https://developers.home-assistant.io/docs/core/entity/)) | ✅ | all entities |
| Main-feature entity: `_attr_name = None` (friendly_name = device name) | ✅ | media_player and remote |
| Sub-entities named without device/type prefix | ✅-ish | "Current App", "Now Playing", "Pair ADB" — right shape, but hard-coded (G7) |
| `unique_id` on every entity ([bronze `entity-unique-id`](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/entity-unique-id)) | ✅ | `{DOMAIN}_{entry.entry_id}_{key}` — stable, not user-configurable |
| `device_info` on all entities (automatic device registration) | ✅ | every platform; one device per entry keyed by `entry.entry_id`. ⚠️ model is hard-coded "Shield TV" — the protocol's `device_info` (model/vendor/sw_version) is available and populating `model`/`model_id`/`sw_version` from it is a cheap upgrade ([`model_id` added 2024-07](https://developers.home-assistant.io/blog/2024/07/16/model-id-device-info/)). G10 |
| `should_poll` — push entities notify via `async_schedule_update_ha_state` / write | ✅ | `_attr_should_poll = False` everywhere + explicit `async_write_ha_state` on store events ([entity doc, subscribing pattern](https://developers.home-assistant.io/docs/core/entity/)) |
| `available` ([silver `entity-unavailable`](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/entity-unavailable)) | ✅ | media_player/remote/sensors derive from manager health |
| Feature flags via `MediaPlayerEntityFeature` enum (no magic numbers — [deprecation](https://developers.home-assistant.io/blog/2023/12/28/support-feature-magic-numbers-deprecation/)) | ✅ | TURN_ON|TURN_OFF|VOLUME_STEP|VOLUME_MUTE|PLAY_MEDIA|BROWSE_MEDIA + playback features; **no `VOLUME_SET`** — correct per the protocol's capability ([RESEARCH_IDEAS §2 C-VOL](RESEARCH_IDEAS.md)) |
| States via current `MediaPlayerState` enum | ✅ | imports the component enum; `STANDBY` not used ([deprecated 2025-07](https://developers.home-assistant.io/blog/2025/07/16/media-player-standby-state-deprecation/)) — our asleep→IDLE mapping is the compliant choice |
| `entity_category` ([gold `entity-category`](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/entity-category)) | ⚠️ | "Pair ADB" button is a device-configuration action → `EntityCategory.CONFIG` is the documented fit; sensors are primary. G11 |
| `entity_registry_enabled_default` for noisy entities ([gold](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/entity-disabled-by-default)) | ⚠️ | Now Playing is chatty (per-track state changes); consider `False` for the least-used mirror sensors only — judgment call, low priority |
| `_unrecorded_attributes` for frequently-changing attributes ([entity doc](https://developers.home-assistant.io/docs/core/entity/)) | ⚠️ | `extra_state_attributes` (media_player.py:804) carries session/transport details; media_player already unrecords its domain attributes, but our custom attributes that change with every poll should be listed. G12 |
| Icon property discouraged vs icon translations ([entity doc](https://developers.home-assistant.io/docs/core/entity/)) | ⚠️ | static `_attr_icon` is the discouraged-but-legal form; app-art uses `entity_picture` (fine). Icon translations only matter for gold |
### 1.6 Setup / unload / lifecycle ([config entries doc](https://developers.home-assistant.io/docs/config_entries_index/))
| Requirement | Us | Notes |
|---|---|---|
| `entry.runtime_data` for runtime objects ([bronze `runtime-data`](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/runtime-data)) | ✅ | `ShieldConnectionManager` on `entry.runtime_data` |
| `async_forward_entry_setups` + platform `async_setup_entry` | ✅ | |
| `async_unload_entry` forwards unload + cleans up listeners/connections | ✅ | manager stopped, hass.data popped; entities unsubscribe in `async_will_remove_from_hass` |
| `EVENT_HOMEASSISTANT_STOP` closes connections | ✅ | via `entry.async_on_unload(bus.async_listen_once(...))` |
| `hass.data[DOMAIN][entry_id]` usage | ⚠️ | we still store the entry there for cross-module lookups; fine, but the manager on `entry.runtime_data` makes most consumers redundant — trim opportunistically |
| Test-before-setup ([bronze](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/test-before-setup)) | ⚠️ deliberate | We intentionally never raise `ConfigEntryNotReady` (ADB may be unpaired; manager retries in background). This is a justified deviation — the rule's goal (avoid "unknown error" states) is met by the passive-start design + Pair ADB button + protocol `keep_reconnecting` — but it should be **documented** (e.g. a `quality_scale.yaml`-style note or README paragraph). G13 |
### 1.7 Actions (services) ([docs](https://developers.home-assistant.io/docs/dev_101_services/))
| Requirement | Us | Notes |
|---|---|---|
| Register actions in `async_setup` — "Service actions should always be registered" so they exist even with no loaded entries | ❌ | `adb_command` is registered lazily from the media_player platform when the first entry opts in, and removed when the last unloads. That violates the documented posture (always registered; the doc's rationale: editable/validatable automations + informative errors). Gap G8 |
| `services.yaml` describing fields/target | ❌ | missing entirely (G8) |
| Names/descriptions from translations ([i18n doc](https://developers.home-assistant.io/docs/internationalization/core/), `services` key) | ❌ | none (G8) |
| Target the right level; **target must not be optional** ("Do not implement a default target when none is specified") | ⚠️ | our schema has `Optional("entity_id")` with implicit all-entries fallback — non-compliant targeting. G8 |
| Errors raised as exceptions with translations ([silver `action-exceptions`](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/action-exceptions); [exceptions i18n](https://developers.home-assistant.io/docs/internationalization/core/)) | ❌ | logs warnings on failure instead of raising `ServiceValidationError(translation_domain=...)`. G8 |
### 1.8 Diagnostics ([docs](https://developers.home-assistant.io/docs/core/integration/diagnostics/))
| Requirement | Us | Notes |
|---|---|---|
| `async_get_config_entry_diagnostics` | ✅ | correct module-level function (diagnostics.py — previously misplaced inside media_player.py and fixed per t20/t21 finding) |
| Redaction of sensitive data via `async_redact_data` | ⚠️ | we ship a hand-rolled `_redact` with marker matching; the doc points at the `async_redact_data` helper with an explicit TO_REDACT list — the helper is the convention (defensive marker-redaction is a reasonable extra on top, but config fields like host should use the helper list). G14 |
| Device diagnostics ([gold `diagnostics`](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/diagnostics)) | ⚠️ | config-entry only; device diagnostics is the gold-tier add (single-device integration → mostly redundant, note why if skipped) |
### 1.9 Tests ([tests file structure](https://developers.home-assistant.io/docs/creating_integration_tests_file_structure/))
| Requirement | Us | Notes |
|---|---|---|
| Tests per domain under `tests/`, `conftest.py`, `test_init.py` for setup/reload/unload | ⚠️ | `tests/` exists with focused files; there is no `test_init.py` covering setup/reload/unload/migration of the entry (the structure doc names `test_init.py` for exactly that). G6 |
| Config-flow full coverage | ⚠️ | see G6 |
| Coverage bar for silver: >95% ([`test-coverage`](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/test-coverage)) | ⚠️ | run pytest --cov to baseline; vendored `ha_adb_common` counts toward it |
### 1.10 Documentation / quality scale ([quality scale](https://developers.home-assistant.io/docs/core/integration-quality-scale/), [rules](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/))
**Bronze** (baseline for all new integrations): config-flow ✅ · entity-unique-id ✅ · has-entity-name ✅ · runtime-data ✅ · test-before-configure ✅ (both pairing steps test the connection) · unique-config-entry ✅ (MAC path) · action-setup ❌ (G8) · config-flow-test-coverage ⚠️ (G6) · brands ⚠️ (G15: no `brand/` icon in repo; HACS requires brand assets) · docs rules (G16: README has setup docs; needs explicit removal instructions + action descriptions to satisfy `docs-high-level-description`, `docs-installation-instructions`, `docs-removal-instructions`, `docs-actions`).
**Silver** adds: config-entry-unloading ✅ · reauthentication-flow ✅ (with G3 fix) · entity-unavailable ✅ · log-when-unavailable ⚠️ (verify we log once per transition, not every retry — the manager's backoff looks right; add a test) · parallel-updates ⚠️ (entities are push; declare/verify `PARALLEL_UPDATES` where the platform defaults matter — G12 note) · action-exceptions ❌ (G8) · test-coverage ⚠️ (G6) · docs-configuration-parameters ⚠️ (README should describe every option; strings.json is the source of truth today) · integration-owner ✅ (codeowner declared).
**Gold** adds (for reference, not required): discovery ✅ (+update-info ✅) · devices ✅ · diagnostics ✅ · entity-category/entity-translations/icon-translations ❌ (G7/G11) · reconfiguration-flow ❌ (G5) · repair-issues ❌ (G9) · extensive docs ❌ (G16).
## Part 2 — Optional YAML configuration alongside the config flow: current doctrine and canonical pattern
### 2.1 The rule
- [ADR-0010](https://github.com/home-assistant/architecture/blob/master/adr/0010-integration-configuration.md) (binding): *"Integrations that communicate with devices and/or services are only configured via the UI. In rare cases, we can make an exception."*
- Current [YAML configuration doc](https://developers.home-assistant.io/docs/core/integration/yaml_configuration/): same rule restated — *"Integrations that communicate with devices and/or services are configured via a config flow. In rare cases, we can make an exception. Existing integrations that should not have a YAML configuration are allowed and encouraged to implement a configuration flow and remove YAML support. **Changes to existing YAML configuration for these same existing integrations will no longer be accepted.**"* Also: *new integrations must not use platform-key YAML* (ADR-0007); any YAML must live under the integration's own domain key.
- So: for our device integration, YAML is allowed **only** as an *import source* that ends in a real config entry (`source: import`), never as a parallel runtime configuration path. The `import` step name is [reserved for exactly this](https://developers.home-assistant.io/docs/core/integration/config_flow/).
### 2.2 The as-documented pattern ([deprecating doc](https://developers.home-assistant.io/docs/deprecating/), "Dropping YAML support for an integration")
The current docs' canonical wiring (shown there in the context of deprecating YAML, and it is the same seeding mechanism an *optional* import uses):
1. **`__init__.py`: `async_setup(hass, config)` reads the domain key and seeds the import flow.** The exact documented snippet:
```python
async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
"""Set up the integration."""
if DOMAIN not in config:
return True
result = await hass.config_entries.flow.async_init(
DOMAIN, context={"source": SOURCE_IMPORT}, data=config[DOMAIN]
)
...
```
(`SOURCE_IMPORT` from `homeassistant.config_entries`; this is the current documented form — the deprecated `discovery` step and platform-key imports are gone.)
2. **`config_flow.py`: `async_step_import` (step name `import`) validates the YAML dict, reuses the same entry-creation path as `async_step_user` (unique_id, `_abort_if_unique_id_configured`, `_create_entry`).** Resulting entries carry `source: import` and behave exactly like UI entries thereafter — options flow, reload, unload all apply. The [config-flow doc](https://developers.home-assistant.io/docs/core/integration/config_flow/) reserves the step name; [config entries doc](https://developers.home-assistant.io/docs/config_entries_index/) treats imported entries as normal config entries.
3. **Add a `CONFIG_SCHEMA`** for the domain-key YAML ([YAML configuration doc](https://developers.home-assistant.io/docs/core/integration/yaml_configuration/), CONFIG_SCHEMA section) using probatio/vol schema, and re-use the config-flow validation so YAML and UI stay consistent.
4. If YAML support is ever *dropped* (not our case), the documented procedure is: import step + the core `deprecated_yaml` repair issue with `HOMEASSISTANT_DOMAIN`, `breaks_in_ha_version`, then leave `cv.config_entry_only_config_schema(DOMAIN)` behind ([deprecating doc](https://developers.home-assistant.io/docs/deprecating/)).
### 2.3 What cannot be imported (honest limits)
- **Interactive pairing cannot happen during import.** An import step produces a config entry from static YAML data — it cannot wait for a human to approve an ADB prompt or type a PIN shown on the Shield. Our design already anticipates this: **ADB key approval is intentionally decoupled from setup** (the manager auto-connects only when a key exists; the Pair ADB button + flow step handle approval), so an imported entry can start unpaired and become functional after the user pairs. For the **protocol (PIN) pairing** there is currently no post-setup trigger — an imported entry with `protocol_enabled` cannot complete pairing without re-running the config flow. Options: keep protocol pairing out of the YAML surface (recommended), or add a repair issue/button that starts a reauth-style protocol-pair flow for unpaired entries (G9).
- **Discovery-derived values (MAC unique_id) cannot come from YAML.** The import can set a *user-chosen* unique_id (e.g. the entity reference) or none; a MAC-less entry weakens duplicate-protection (see G2) — the import should require the `androidtv_entity` reference or an explicit `unique_id`-grade value.
- **`source: import` entries show in the UI** and can be migrated to `source: user` only by deletion + re-add; document that YAML edits require deleting/re-importing (or honoring `async_update_entry` on re-import with `reason: options_updated` style updates — simplest is abort `already_configured` on re-import, matching most core integrations).
### 2.4 HACS / custom-integration policy view
- [HACS publish requirements](https://hacs.xyz/docs/publish/integration/) do not mention YAML at all — they require repo structure (one integration under `custom_components/`), the six manifest keys (we have all: domain, documentation, issue_tracker, codeowners, name, version), and **brand assets** (an `icon.png` under `brand/` or the brands repo — we currently lack it, G15). HACS never *forbids* an import path; it is simply orthogonal.
- The HA project position ([ADR-0010](https://github.com/home-assistant/architecture/blob/master/adr/0010-integration-configuration.md)) is that contributors may drop YAML to reduce burden "power to our contributors" — i.e. no project-level pressure to provide YAML for device integrations, and the quality scale contains no YAML rule. **Recommendation: treat YAML import as optional sugar; do not build it for the MVP** — the UI flow already covers discovery + pairing, and the import path buys only static-foreignKey convenience at the cost of the G2/G9 caveats.
## Part 3 — Gap list (concrete, file-anchored)
Ordered by severity; each item names the file(s) and the change.
| # | Severity | Gap → Fix |
|---|---|---|
| G3a | **High (breaks 2026.12)** | Deprecated listener+reloading combo: `config_flow.py` `_finish_reauth` (`async_update_entry` + `async_schedule_reload`) and the zeroconf host-update path (`async_update_entry` + `async_schedule_reload`) both double-reload because `__init__.py` registers `entry.add_update_listener(_async_update_listener)` (full reload). Per the [2026-05-07 blog](https://developers.home-assistant.io/blog/2026/05/07/config-entry-listener-together-with-reloading-methods/): in `config_flow.py` replace reauth's pair with `self.async_update_reload_and_abort(self._get_reauth_entry(), data_updates={...})`; for zeroconf use `async_update_entry` only and let the listener reload (or `reload_on_update=False` on `_abort_if_unique_id_configured`). Also see [always-reload-after-reauth](https://developers.home-assistant.io/blog/2024/04/25/always-reload-after-successful-reauth-flow/) |
| G3b | **High (deprecation)** | `config_flow.py` `NvidiaShieldOptionsFlow(config_entry)`: constructor must take no args — `async_get_options_flow(config_entry) -> NvidiaShieldOptionsFlow()` reading `self.config_entry` ([options flow doc](https://developers.home-assistant.io/docs/core/integration/options_flow/)); convert hand-rolled defaults to `add_suggested_values_to_schema`; consider `OptionsFlowWithReload` and dropping the update listener (or keep listener for partial reloads of non-structural options — but then never schedule reloads inside the flow) |
| G8 | High (silver) | `media_player.py` `_adb_command_service_ensure/_maybe_remove` + missing files: move registration to `__init__.py` `async_setup` (always registered, with `option-dependent` handler that no-ops/logs when no entry opted in); add `services.yaml` (fields: `command` required, `entity_id` **required** target — [targeting rules](https://developers.home-assistant.io/docs/dev_101_services/): target must not be optional; consider `device_id`/config_entry targeting instead of entity); add `strings.json` `services` section (name/description/fields) + `icons.json` service icon; replace warning-logs with `ServiceValidationError(translation_domain=DOMAIN, translation_key=...)` + `exceptions` section ([action-exceptions](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/action-exceptions), [exceptions i18n](https://developers.home-assistant.io/docs/internationalization/core/)) |
| G2 | High | Unique-ID source fallback: `config_flow.py` uses `androidtv_entity` (entity_id) as unique_id when no MAC exists — renameable/user-changeable → weak. Fix: on the protocol path always MAC; on ADB-only setups without protocol, use `await self._async_handle_discovery_without_unique_id()`-grade handling for discovery, and for manual setup keep the entity-based ID but document the rename caveat, or derive from the device's MAC via the entity's device registry entry (device registry holds `CONNECTION_NETWORK_MAC`) |
| G1 | Medium | `manifest.json`: add `"integration_type": "device"`, `"loggers": ["adb_shell", "androidtvremote2"]` ([manifest doc](https://developers.home-assistant.io/docs/creating_integration_manifest/)) |
| G6 | Medium | Tests: add `tests/test_init.py` (setup/reload/unload/migration) per the [tests structure doc](https://developers.home-assistant.io/docs/creating_integration_tests_file_structure/); run coverage against `config_flow.py` to satisfy [config-flow-test-coverage](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/config-flow-test-coverage) |
| G7 | Medium | Entity naming: sensors `sensor.py` "Current App"/"Now Playing" — replace `_attr_name` with `translation_key` + `entity.sensor.*` names in `strings.json` ([entity naming](https://developers.home-assistant.io/docs/core/entity/), [entity-translations](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/entity-translations)); button "Pair ADB" likewise |
| G9 | Medium | Unpaired/needs-pairing UX: raise a repair issue (gold [`repair-issues`](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/repair-issues)) with an `issues` strings section when the protocol transport is enabled but unpaired, linking to the reauth/pair flow — this also gives YAML-imported entries a pairing path (see §2.3) |
| G10 | Low | `media_player.py` `device_info`: populate `model`/`model_id`/`sw_version` from the protocol `device_info` push (available post-pairing; [model_id blog](https://developers.home-assistant.io/blog/2024/07/16/model-id-device-info/)) |
| G14 | Low | `diagnostics.py`: switch `_redact` to `async_redact_data` with an explicit TO_REDACT list for `entry.data/options` ([diagnostics doc](https://developers.home-assistant.io/docs/core/integration/diagnostics/)), keeping the marker-based defense as a second layer |
| G11 | Low | `button.py`: `_attr_entity_category = EntityCategory.CONFIG` on Pair ADB ([entity-category rule](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/entity-category)) |
| G12 | Low | `media_player.py`: declare `_unrecorded_attributes` (class attribute) for fast-changing custom attributes; verify silver `log-when-unavailable` once-per-transition behavior in `connection.py` with a test ([entity doc](https://developers.home-assistant.io/docs/core/entity/), [log-when-unavailable](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/log-when-unavailable)) |
| G16 | Low | Docs for bronze: README needs a removal section + per-action documentation ([docs-removal-instructions](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/docs-removal-instructions), [docs-actions](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/docs-actions)) |
| G15 | Low | Brand assets: add `brand/icon.png` to the repo (HACS requirement, [HACS publish](https://hacs.xyz/docs/publish/integration/)) and add the integration to the [brands repo](https://developers.home-assistant.io/docs/creating_integration_brand/) |
| G17 | Info | Schema library: `config_flow.py`/`media_player.py` import `voluptuous` — **no change required** ([Probatio blog](https://developers.home-assistant.io/blog/2026/09/30/probatio-validation-engine/): HA aliases the import; voluptuous is no longer installed but the alias resolves; the ban is core-only). New code may import `probatio` directly; opportunistically migrate |
| G18 | Info | BrowseMedia: `media_player.py` `async_browse_media` constructs `BrowseMediaSource` without `domain` — `domain` is now a **required** `str` on `BrowseMediaSource.__init__` ([2026-05-20 blog](https://developers.home-assistant.io/blog/2026/05/20/browse-media-source-root-class/)). Verify the media_player-component helper we import and pass `domain=DOMAIN` (children) / the integration domain (root); our `media_content_type="apps"` root is not a `RootBrowseMediaSource` case |
| G5 | Info | Reconfigure step (gold [`reconfiguration-flow`](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/reconfiguration-flow)): a future `async_step_reconfigure` should own device name + host overrides + source toggles, using `async_update_reload_and_abort(self._get_reconfigure_entry(), ...)` and `_abort_if_unique_id_mismatch` ([config flow doc](https://developers.home-assistant.io/docs/core/integration/config_flow/)) |
| G4 | Low | Jellyfin auto-detect silently fails when the Jellyfin integration is not yet configured (`config_flow.py` `_find_jellyfin_entity` finds no entities). We deliberately do not list `jellyfin` in `dependencies` (nothing breaks without it); document the ordering caveat in the README instead ([manifest doc, dependencies semantics](https://developers.home-assistant.io/docs/creating_integration_manifest/)) |
| G13 | Low | Document the deliberate deviation from [test-before-setup](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/test-before-setup): a README paragraph (or an exempted-rule note) explaining that entries start passively without `ConfigEntryNotReady` because ADB may be unpaired, with the Pair ADB button + protocol `keep_reconnecting` + reauth as the recovery UX |
| G5 | Info | Reconfigure step (gold [`reconfiguration-flow`](https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/reconfiguration-flow)): a future `async_step_reconfigure` should own device name + host overrides + source toggles, using `async_update_reload_and_abort(self._get_reconfigure_entry(), ...)` and `_abort_if_unique_id_mismatch` ([config flow doc](https://developers.home-assistant.io/docs/core/integration/config_flow/)) |
### Explicitly OK as-is (no action)
- `iot_class: local_push`, requirement ranges, zeroconf entry, `version` (CalVer), `config_flow`, repo structure for HACS (one integration dir), `hacs.json`, devices per entry, feature-flag enums, `MediaPlayerState` usage, `entry.runtime_data`, unload/stop lifecycle, migration versioning shape, entity availability model, and the deliberate no-`ConfigEntryNotReady` design (document it, G13).
- Voluptuous imports remain valid through the probatio alias (G17).
tests/entity_stublib.py
+25 -2
View File
@@ -15,6 +15,7 @@ ENTITIES_STUB_NAMES = (
"voluptuous",
"homeassistant",
"homeassistant.config_entries",
"homeassistant.exceptions",
"homeassistant.core",
"homeassistant.const",
"homeassistant.components",
@@ -134,6 +135,17 @@ def install() -> None:
config_entries.OptionsFlow = type("OptionsFlow", (), {})
sys.modules["homeassistant.config_entries"] = config_entries
exceptions = types.ModuleType("homeassistant.exceptions")
class ServiceValidationErrorStubError(Exception):
def __init__(self, *args, **kwargs):
super().__init__(*args or (kwargs.get("translation_key"),))
self.translation_domain = kwargs.get("translation_domain")
self.translation_key = kwargs.get("translation_key")
exceptions.ServiceValidationError = ServiceValidationErrorStubError
sys.modules["homeassistant.exceptions"] = exceptions
core = types.ModuleType("homeassistant.core")
core.HomeAssistant = object
core.callback = lambda fn: fn
@@ -216,9 +228,20 @@ def load_entity_modules() -> dict:
install()
from conftest import _SHIELD_DIR, _load_module
for name in ("helpers", "power", "jellyfin", "media_player", "remote", "diagnostics"):
for name in (
"helpers",
"power",
"jellyfin",
"media_player",
"remote",
"diagnostics",
"init",
): # "init" maps the integration __init__.py (t29 G8 async_setup)
if name == "init" and "nsh.init" in sys.modules:
continue
if f"nsh.{name}" not in sys.modules:
_load_module(f"nsh.{name}", _SHIELD_DIR / f"{name}.py")
file_name = "__init__.py" if name == "init" else f"{name}.py"
_load_module(f"nsh.{name}", _SHIELD_DIR / file_name)
return {
name: sys.modules[f"nsh.{name}"]
for name in ("power", "jellyfin", "media_player", "remote", "diagnostics")
tests/test_command_routing.py
+81 -52
View File
@@ -640,7 +640,8 @@ class ServiceHarness:
class TestAdbCommandService:
"""P4-2a: option-gated raw shell service on the entry."""
"""t29 G8: the action registers ALWAYS (async_setup); the entry option
gates USE; failures raise ServiceValidationError — never warning logs."""
def _participant(self, modules, enabled, connected=True):
player, _s, _i = make_player(modules)
@@ -659,19 +660,18 @@ class TestAdbCommandService:
before = dict(_ADB_COMMAND_TARGETS)
player, _services = self._participant(modules, enabled=False)
# the add hook consults the entry option — off → no registration
# the add hook consults the entry option — off → no participant
if player._entry.options.get("adb_command_enabled", False):
_ADB_COMMAND_TARGETS.setdefault(player.entity_id, player._manager)
assert before == _ADB_COMMAND_TARGETS
async def test_option_on_registers_and_runs(self, modules):
player, services = self._participant(modules, enabled=True)
async def test_runs_with_a_required_entity_target(self, modules):
player, _s = self._participant(modules, enabled=True)
from homeassistant.exceptions import ServiceValidationError
from nsh.media_player import _ADB_COMMAND_TARGETS, _async_adb_command_service
_ADB_COMMAND_TARGETS["media_player.shield_test"] = player._manager
class Call:
data = {"command": "dumpsys power | head -5"}
_ADB_COMMAND_TARGETS["media_player.shield_test"] = player._manager
shell_calls = []
@@ -680,48 +680,78 @@ class TestAdbCommandService:
return "ok"
player._manager._poller.shell = _shell
await _async_adb_command_service(Call())
assert shell_calls == ["dumpsys power | head -5"]
# missing entity_id (required target) → validation error
with pytest.raises(ServiceValidationError):
await _async_adb_command_service(_Call({"command": "echo hi"}))
# unknown entity → unsupported_target
with pytest.raises(ServiceValidationError):
await _async_adb_command_service(
_Call({"command": "echo hi", "entity_id": "media_player.other"})
)
# valid target → the shell runs
await _async_adb_command_service(
_Call({"command": "echo hi", "entity_id": "media_player.shield_test"})
)
assert shell_calls == ["echo hi"]
_ADB_COMMAND_TARGETS.pop("media_player.shield_test", None)
async def test_rejects_when_adb_unavailable(self, modules):
async def test_empty_command_raises_validation_error(self, modules):
from homeassistant.exceptions import ServiceValidationError
from nsh.media_player import _async_adb_command_service
with pytest.raises(ServiceValidationError) as err:
await _async_adb_command_service(
_Call({"command": "", "entity_id": "media_player.shield_test"})
)
assert err.value.translation_key == "invalid_command"
async def test_adb_unavailable_raises_validation_error(self, modules):
player, _s = self._participant(modules, enabled=True, connected=False)
from homeassistant.exceptions import ServiceValidationError
from nsh.media_player import _ADB_COMMAND_TARGETS, _async_adb_command_service
_ADB_COMMAND_TARGETS["media_player.shield_test"] = player._manager
class Call:
data = {"command": "echo hi"}
# no poller → the command is not sent and nothing raises
player._manager._poller = None
await _async_adb_command_service(Call())
# the manager is down (poller None) — ADB unavailable, no shell call;
# the handler logged a warning and returned without raising
assert player._manager._poller is None
with pytest.raises(ServiceValidationError) as err:
await _async_adb_command_service(
_Call({"command": "echo hi", "entity_id": "media_player.shield_test"})
)
assert err.value.translation_key == "adb_unavailable"
_ADB_COMMAND_TARGETS.pop("media_player.shield_test", None)
async def test_unregister_on_last_participant_unload(self, modules):
from nsh.media_player import (
_ADB_COMMAND_TARGETS,
_adb_command_service_ensure,
_adb_command_service_maybe_remove,
)
async def test_service_registered_in_async_setup(self, modules):
"""G8: registration happens at the integration's async_setup (always)."""
player, services = self._participant(modules, enabled=False)
mods = load_entity_modules()
player, _s, _i = make_player(mods)
player._entry.options["adb_command_enabled"] = True
player.entity_id = "media_player.shield_test"
player.hass.services = FakeServices()
init_module = sys.modules.get("nsh.init")
if init_module is None:
import conftest
_adb_command_service_ensure(player.hass)
_ADB_COMMAND_TARGETS["media_player.shield_test"] = player._manager
assert player.hass.services.has_service("nvidia_shield", "adb_command")
conftest._load_module("nsh.init", conftest._SHIELD_DIR / "__init__.py")
init_module = sys.modules["nsh.init"]
_ADB_COMMAND_TARGETS.pop("media_player.shield_test")
_adb_command_service_maybe_remove(player.hass)
assert not player.hass.services.has_service("nvidia_shield", "adb_command")
await init_module.async_setup(_SetupHass(services), {})
assert services.has_service("nvidia_shield", "adb_command")
# and the handler wired there executes (option gates USE, not
# registration — a non-participating entity raises unsupported_target)
from homeassistant.exceptions import ServiceValidationError
with pytest.raises(ServiceValidationError):
handler = services.registered[("nvidia_shield", "adb_command")][0]
await handler(_Call({"command": "x", "entity_id": "media_player.nix"}))
class _Call:
def __init__(self, data):
self.data = data
class _SetupHass:
def __init__(self, services):
self.services = services
class TestChannelDigitDelayConsumed:
@@ -849,15 +879,13 @@ class TestIconOverride:
class TestServiceRegistryKey:
"""F4: the participant key is computed once — add/remove always match."""
"""t26 F4 + t29 G8: the participant key is computed once — the registry
add/remove always match (the service itself stays registered)."""
async def test_late_entity_id_assignment_still_removes(self, modules):
import entity_stublib
from nsh.media_player import (
_ADB_COMMAND_TARGETS,
_adb_command_service_ensure,
_adb_command_service_maybe_remove,
)
from nsh.media_player import _ADB_COMMAND_TARGETS
mods = load_entity_modules()
player, _s, _i = make_player(mods)
@@ -870,23 +898,24 @@ class TestServiceRegistryKey:
del player._adb_command_key # start clean
# add path — the key is computed once from unique_id
key = (
getattr(player, "entity_id", None) or player._attr_unique_id
)
key = getattr(player, "entity_id", None) or player._attr_unique_id
player._adb_command_key = key
_ADB_COMMAND_TARGETS[key] = player._manager
_adb_command_service_ensure(player.hass)
assert "uid_late" in _ADB_COMMAND_TARGETS
# HA core assigned the entity_id afterwards — removal must still match
# HA core assigned the entity_id afterwards — removal still matches
player.entity_id = "media_player.shield_test"
_adb_command_service_maybe_remove(player.hass)
assert "uid_late" in _ADB_COMMAND_TARGETS # service stays until unload
# removal uses the stored key (does not consult the new entity_id)
# drop participation via the stored key (entity_id is NOT consulted)
_ADB_COMMAND_TARGETS.pop(player._adb_command_key)
_adb_command_service_maybe_remove(player.hass)
assert not player.hass.services.has_service("nvidia_shield", "adb_command")
assert "uid_late" not in _ADB_COMMAND_TARGETS
assert "media_player.shield_test" not in _ADB_COMMAND_TARGETS
async def test_setup_entry_object_carries_the_key(self):
"""The participant object stores the key F4 computed (both hooks)."""
from types import SimpleNamespace as SetupEntry
assert SetupEntry is not None # construction shape pinned in t26
class TestEntryDiagnostics: