From 81f70585cd56e88d2a459aa0b317da1c4e8814be Mon Sep 17 00:00:00 2001 From: James Mackay Date: Sun, 11 Oct 2026 06:19:36 +0100 Subject: [PATCH] =?UTF-8?q?feat(config)!:=20migrate=20entries=20to=20v3=20?= =?UTF-8?q?=E2=80=94=20one=20address,=20no=20androidtv=20key=20(t41)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- custom_components/nvidia_shield/__init__.py | 65 ++++++++++--- .../nvidia_shield/config_flow.py | 3 +- custom_components/nvidia_shield/const.py | 5 + tests/test_config_flow_protocol.py | 94 ++++++++++++++++--- tests/test_standalone_setup.py | 66 +++++++++---- 5 files changed, 187 insertions(+), 46 deletions(-) diff --git a/custom_components/nvidia_shield/__init__.py b/custom_components/nvidia_shield/__init__.py index 1f569d9..b6f9007 100644 --- a/custom_components/nvidia_shield/__init__.py +++ b/custom_components/nvidia_shield/__init__.py @@ -24,11 +24,14 @@ from .const import ( CONF_ADB_ENABLED, CONF_ADB_HOST, CONF_ADB_PORT, + CONF_ANDROIDTV_ENTITY, CONF_HOST, CONF_PROTOCOL_ENABLED, + CONF_PROTOCOL_HOST, DEFAULT_ADB_PORT, DEFAULT_NAME, DOMAIN, + ENTRY_VERSION, SERVICE_ADB_COMMAND, ) from .helpers import ( @@ -206,26 +209,62 @@ async def _build_protocol_settings(hass: HomeAssistant, entry: ConfigEntry): async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: - """Migrate v1 entries to v2 (adds the remote-protocol fields). + """Migrate an older config entry to the current version. - ADB options carry over untouched; the ADB-host discovery fallback keeps - serving as the protocol host until pairing populates it. Downgrades - (version > 2) are rejected so HA offers a rollback. + v1 → v2 added the remote-protocol fields (ADB options carried over + untouched). v2 → v3 (t41) mirrors the two changes in the stored shape: + + * the duplicated address fields collapse into the single CONF_HOST — the + protocol host wins, else the ADB host, since both name the same Shield; + * the androidtv-entity key goes. This integration owns the whole stack now: + the entity it used to configure is neither a state source nor an app-name + or power tier, so keeping the value would only mislead. + + Downgrades (version > ENTRY_VERSION) are rejected so HA offers a rollback. """ - if config_entry.version > 2: + if config_entry.version > ENTRY_VERSION: return False + if config_entry.version >= ENTRY_VERSION: + return True + + options = {**config_entry.options} + data = {**config_entry.data} + if config_entry.version < 2: - options = {**config_entry.options} options.setdefault(CONF_PROTOCOL_ENABLED, False) - # t41: the entry's address is the single CONF_HOST field; a v1 entry - # carried it under the legacy adb_host key. - if not options.get(CONF_HOST): - options[CONF_HOST] = options.get(CONF_ADB_HOST) or "" - hass.config_entries.async_update_entry( - config_entry, options=options, version=2 + + if not options.get(CONF_HOST): + options[CONF_HOST] = ( + options.get(CONF_PROTOCOL_HOST) + or options.get(CONF_ADB_HOST) + or data.get(CONF_PROTOCOL_HOST) + or data.get(CONF_ADB_HOST) + or "" ) + + dropped_entity = options.pop(CONF_ANDROIDTV_ENTITY, None) + dropped_entity = data.pop(CONF_ANDROIDTV_ENTITY, None) or dropped_entity + for legacy_key in (CONF_PROTOCOL_HOST, CONF_ADB_HOST): + options.pop(legacy_key, None) + data.pop(legacy_key, None) + + hass.config_entries.async_update_entry( + config_entry, data=data, options=options, version=ENTRY_VERSION + ) + if dropped_entity: _LOGGER.info( - "Migrated %s config entry to version 2", config_entry.title + "%s: config entry migrated to version %d — dropped the androidtv " + "entity %s (this integration reads the Shield directly; the entry " + "needs no external media_player)", + config_entry.title, + ENTRY_VERSION, + dropped_entity, + ) + else: + _LOGGER.info( + "Migrated %s config entry to version %d", + config_entry.title, + ENTRY_VERSION, ) return True diff --git a/custom_components/nvidia_shield/config_flow.py b/custom_components/nvidia_shield/config_flow.py index ce44770..cc67c26 100644 --- a/custom_components/nvidia_shield/config_flow.py +++ b/custom_components/nvidia_shield/config_flow.py @@ -33,6 +33,7 @@ from .const import ( DEFAULT_ADB_PORT, DEFAULT_NAME, DOMAIN, + ENTRY_VERSION, ) from .helpers import entry_host, protocol_cert_paths, seed_adb_enabled, seed_host @@ -190,7 +191,7 @@ def _build_selectors(): class NvidiaShieldConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Handle a config flow for Nvidia Shield.""" - VERSION = 2 + VERSION = ENTRY_VERSION def __init__(self): self._user_data = {} diff --git a/custom_components/nvidia_shield/const.py b/custom_components/nvidia_shield/const.py index c197e67..42c1075 100644 --- a/custom_components/nvidia_shield/const.py +++ b/custom_components/nvidia_shield/const.py @@ -55,6 +55,11 @@ STATE_STORE_TTL = 60 # ADB-sourced fields older than this are stale DEFAULT_NAME = "Nvidia Shield" +# Config-entry schema version (t41): v1 → v2 added the remote-protocol fields, +# v2 → v3 collapsed the two address fields into CONF_HOST and dropped the +# androidtv-entity key. One constant so the flow and the migration cannot drift. +ENTRY_VERSION = 3 + # Emotiva DAG attribute we read to get available inputs ATTR_MEDIA_DAG_INPUT_MAP = "media_dag_input_map" diff --git a/tests/test_config_flow_protocol.py b/tests/test_config_flow_protocol.py index 17a973f..a62d2d0 100644 --- a/tests/test_config_flow_protocol.py +++ b/tests/test_config_flow_protocol.py @@ -15,6 +15,7 @@ import pytest from conftest import _SHIELD_DIR, _load_module from nsh.const import ( CONF_ADB_ENABLED, + CONF_ADB_HOST, CONF_ADB_PORT, CONF_ADB_STORE_APP_INFO, CONF_HOST, @@ -22,6 +23,7 @@ from nsh.const import ( CONF_PAIRING_CODE, CONF_POWER_ENTITY, CONF_PROTOCOL_ENABLED, + CONF_PROTOCOL_HOST, CONF_RECEIVER_ENTITY, CONF_RECEIVER_INPUT, DEFAULT_ADB_PORT, @@ -392,12 +394,25 @@ class TestReauthFlow: class TestEntryMigration: - async def test_v1_gains_protocol_defaults_and_keeps_adb_options(self, tmp_path, monkeypatch): + """t41: v2 → v3 collapses the two address fields into CONF_HOST. + + The protocol host wins, else the ADB host (both name the same Shield); the + legacy keys are removed so nothing can read a stale duplicate. A v1 entry + travels the whole way in one call. + """ + + async def test_v1_keeps_its_adb_options_and_gains_both_defaults(self, tmp_path): module = _entry_module() hass = FakeHass(str(tmp_path)) entry = SimpleNamespace( version=1, - options={"adb_enabled": True, "adb_port": 5555, "jellyfin_entity": "jf.x"}, + data={}, + options={ + "adb_enabled": True, + "adb_port": 5555, + "jellyfin_entity": "jf.x", + "adb_host": "10.0.0.31", # legacy address + }, title="mdarcy", ) @@ -407,27 +422,82 @@ class TestEntryMigration: # ADB options carried over byte-compatibly assert entry.options["adb_enabled"] is True assert entry.options["adb_port"] == 5555 - # Protocol fields default; the entry keeps its own address (t41: the v1 - # adb_host is mapped onto CONF_HOST, empty here) + assert entry.options["jellyfin_entity"] == "jf.x" + # the v2 default, then the v3 address collapse assert entry.options[CONF_PROTOCOL_ENABLED] is False - assert entry.options[CONF_HOST] == "" - assert hass.config_entries.updating_entry_calls == [ - (entry, {"options": entry.options, "version": 2}) - ] + assert entry.options[CONF_HOST] == "10.0.0.31" + assert "adb_host" not in entry.options + assert entry.version == 3 - async def test_v2_is_a_noop(self, tmp_path, monkeypatch): + async def test_v2_prefers_the_protocol_host_over_the_adb_host(self, tmp_path): module = _entry_module() hass = FakeHass(str(tmp_path)) entry = SimpleNamespace( version=2, - options={CONF_PROTOCOL_ENABLED: True}, + data={}, + options={ + CONF_PROTOCOL_ENABLED: True, + CONF_PROTOCOL_HOST: "10.0.0.2", # the address that was paired + CONF_ADB_HOST: "10.0.0.1", + }, title="mdarcy", ) + + assert await module.async_migrate_entry(hass, entry) is True + + assert entry.options[CONF_HOST] == "10.0.0.2" + assert CONF_PROTOCOL_HOST not in entry.options + assert CONF_ADB_HOST not in entry.options + assert entry.options[CONF_PROTOCOL_ENABLED] is True # untouched + assert entry.version == 3 + + async def test_v2_falls_back_to_the_adb_host(self, tmp_path): + module = _entry_module() + hass = FakeHass(str(tmp_path)) + entry = SimpleNamespace( + version=2, + data={}, + options={CONF_PROTOCOL_ENABLED: False, CONF_ADB_HOST: "10.0.0.1"}, + title="mdarcy", + ) + + assert await module.async_migrate_entry(hass, entry) is True + + assert entry.options[CONF_HOST] == "10.0.0.1" + assert CONF_ADB_HOST not in entry.options + + async def test_v2_without_an_address_migrates_to_an_empty_one(self, tmp_path): + module = _entry_module() + hass = FakeHass(str(tmp_path)) + entry = SimpleNamespace( + version=2, + data={}, + options={CONF_PROTOCOL_ENABLED: False, CONF_ADB_HOST: "", CONF_PROTOCOL_HOST: ""}, + title="mdarcy", + ) + + assert await module.async_migrate_entry(hass, entry) is True + + assert entry.options[CONF_HOST] == "" + assert CONF_ADB_HOST not in entry.options + assert CONF_PROTOCOL_HOST not in entry.options + assert entry.version == 3 + + async def test_the_current_version_is_a_noop(self, tmp_path): + module = _entry_module() + hass = FakeHass(str(tmp_path)) + entry = SimpleNamespace( + version=3, + data={}, + options={CONF_PROTOCOL_ENABLED: True, CONF_HOST: "shield.local"}, + title="mdarcy", + ) + assert await module.async_migrate_entry(hass, entry) is True assert hass.config_entries.updating_entry_calls == [] - async def test_downgrade_rejected(self, tmp_path, monkeypatch): + async def test_downgrade_rejected(self, tmp_path): module = _entry_module() hass = FakeHass(str(tmp_path)) - entry = SimpleNamespace(version=3, options={}, title="x") + entry = SimpleNamespace(version=4, options={}, title="x") assert await module.async_migrate_entry(hass, entry) is False diff --git a/tests/test_standalone_setup.py b/tests/test_standalone_setup.py index 8596f90..8c1927d 100644 --- a/tests/test_standalone_setup.py +++ b/tests/test_standalone_setup.py @@ -9,6 +9,7 @@ migration validity for v2 entries without the field. import asyncio # noqa: F401 (async steps need the event loop) import gc +import logging import sys import types import warnings @@ -414,48 +415,73 @@ class TestAdbOnlyStandalone: assert result["errors"] == {"base": "no_state_source"} -class TestMigrationKeepsOrAllowsAbsence: - async def test_v1_to_v2_keeps_the_androidtv_field(self, tmp_path): +class TestMigrationDropsTheEntityKey: + """t41: the androidtv key is dropped from BOTH dicts with a logged note. + + An entry that still carries it keeps working until then (nothing reads it) — + the migration is what removes the last trace of the entity path. + """ + + async def test_v1_entry_reaches_v3_without_the_entity_key(self, tmp_path, caplog): module = _entry_module() hass = FakeHass(str(tmp_path)) entry = SimpleNamespace( version=1, data={CONF_ANDROIDTV_ENTITY: "media_player.shield"}, - options={}, + options={CONF_ANDROIDTV_ENTITY: "media_player.shield"}, title="mdarcy", ) - ok = await module.async_migrate_entry(hass, entry) + with caplog.at_level(logging.INFO): + ok = await module.async_migrate_entry(hass, entry) assert ok is True - assert entry.data[CONF_ANDROIDTV_ENTITY] == "media_player.shield" # kept - # options were rewritten with the protocol defaults; the data dict — - # including the androidtv field — was never touched - assert hass.config_entries.updating_entry_calls == [ - ( - entry, - { - "options": {CONF_PROTOCOL_ENABLED: False, CONF_HOST: ""}, - "version": 2, - }, - ) - ] + assert CONF_ANDROIDTV_ENTITY not in entry.data + assert CONF_ANDROIDTV_ENTITY not in entry.options + assert entry.options == {CONF_PROTOCOL_ENABLED: False, CONF_HOST: ""} + assert entry.data == {} + # the note names what was dropped, so the log is actionable + notes = [record.getMessage() for record in caplog.records] + assert any("media_player.shield" in note for note in notes), notes + assert hass.config_entries.updating_entry_calls[0][1]["version"] == 3 - async def test_v2_without_the_field_is_valid(self, tmp_path): - """Standalone entries born v2 must pass through migration untouched.""" + async def test_an_entry_without_the_key_logs_plainly(self, tmp_path, caplog): + module = _entry_module() + hass = FakeHass(str(tmp_path)) + entry = SimpleNamespace(version=2, data={}, options={}, title="mdarcy") + + with caplog.at_level(logging.INFO): + ok = await module.async_migrate_entry(hass, entry) + + assert ok is True + notes = [record.getMessage() for record in caplog.records] + assert any("Migrated mdarcy config entry to version 3" in note for note in notes), notes + + async def test_v2_entry_with_a_legacy_address_migrates_to_the_one_field(self, tmp_path): + """An entry written before the unification lands on CONF_HOST.""" module = _entry_module() hass = FakeHass(str(tmp_path)) entry = SimpleNamespace( version=2, data={}, - options={CONF_PROTOCOL_ENABLED: True, CONF_HOST: "shield.local"}, + options={CONF_PROTOCOL_ENABLED: True, "protocol_host": "shield.local"}, title="mdarcy", ) ok = await module.async_migrate_entry(hass, entry) assert ok is True - assert hass.config_entries.updating_entry_calls == [] # no rewrite needed + assert entry.options[CONF_HOST] == "shield.local" + assert "protocol_host" not in entry.options + + def test_the_stored_version_and_the_flow_cannot_drift(self, tmp_path): + # _form_module owns the scoped stub install/pop — never a bare + # entity_stublib.install() here: a shallow stub set leaks into the + # entity-module loader of later tests. + module = _form_module() + from nsh.const import ENTRY_VERSION + + assert module.NvidiaShieldConfigFlow.VERSION == ENTRY_VERSION == 3 def _cmd_player(modules):