docs: HA standards audit against current developer docs — deprecation gaps G3a/G3b/G8, YAML-import verdict, quality-scale notes (research)
This commit is contained in:
1 parent
e51545262e
commit
79faa32b68
1 file changed
+238
@@ -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).
|
||||
Reference in new issue
Block a user