diff --git a/custom_components/nvidia_shield/config_flow.py b/custom_components/nvidia_shield/config_flow.py index fdaec45..2486f4b 100644 --- a/custom_components/nvidia_shield/config_flow.py +++ b/custom_components/nvidia_shield/config_flow.py @@ -231,6 +231,8 @@ class NvidiaShieldConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): self._user_data = {} self._protocol_client = None self._reauth_entry = None + # audit G5 / t35 F1: the entry a "Reconfigure" visit targets + self._reconfigure_entry = None # Zeroconf discovery state (P4-1) self._discovered_host: str | None = None self._discovered_name: str | None = None @@ -542,6 +544,11 @@ class NvidiaShieldConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): # say so instead of pretending the re-pair worked self._reauth_entry = None return self.async_abort(reason="reauth_skipped") + if self._reconfigure_entry is not None: + # same for a Reconfigure visit: nothing changed, the entry keeps + # its options and can be reconfigured again at any time + self._reconfigure_entry = None + return self.async_abort(reason="reconfigure_skipped") return self._create_entry() async def _async_restart_protocol_pairing(self, errors: dict): @@ -578,9 +585,11 @@ class NvidiaShieldConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): typed_host = str(user_input.get(CONF_PROTOCOL_HOST) or "").strip() if typed_host and typed_host != (self._protocol_host() or ""): - # A host typed HERE is authoritative — the user step is gone, and - # the session started against the old address is void. + # A host typed HERE is authoritative — the user step is gone, so + # the session started against the old address is dropped and the + # Shield is asked for a NEW code at the corrected address (t35 F1). self._user_data[CONF_PROTOCOL_HOST] = typed_host + self._discard_protocol_client() code = str(user_input.get(CONF_PAIRING_CODE, "") or "").strip() if self._protocol_client is None or not code: @@ -602,11 +611,19 @@ class NvidiaShieldConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): with contextlib.suppress(Exception): client.disconnect() self._protocol_client = None - if mac: + target = self._reauth_entry or self._reconfigure_entry + if mac and mac != getattr(target, "unique_id", None): + # A MAC that is already configured must never be adopted silently. + # A reauth/reconfigure flow already OWNS an entry, and + # _abort_if_unique_id_configured looks the domain up by unique_id — + # it would abort on that very entry when the MAC is unchanged + # (t35 F1: re-pairing an entry used to abort as "already_configured"). await self.async_set_unique_id(mac) self._abort_if_unique_id_configured() if self._reauth_entry is not None: return self._finish_reauth() + if self._reconfigure_entry is not None: + return self._finish_reconfigure(mac) return self._create_entry() async def async_step_import(self, data): @@ -687,17 +704,68 @@ class NvidiaShieldConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): step_id="reauth_confirm", data_schema=vol.Schema({}) ) + async def async_step_reconfigure(self, user_input=None): + """Re-run protocol pairing for an existing entry (audit G5, t35 F1). + + Home Assistant offers this as the entry's "Reconfigure" action — the + discoverable way back for an entry whose protocol transport is off + (typically the "continue without protocol pairing" outcome): the + address is editable, the Shield shows a PIN, and the entry's options + are updated + reloaded. + """ + entry = self._reconfigure_entry or self._get_reconfigure_entry() + if entry is None: + # HA always links the entry; a missing one gets a plain abort + # instead of an exception + return self.async_abort(reason="reconfigure_entry_missing") + self._reconfigure_entry = entry + # Seed the flow from the entry so the pairing step offers the address + # the entry already knows (and _create_entry is never reached here). + self._user_data = { + "name": entry.title, + CONF_ANDROIDTV_ENTITY: entry.data.get(CONF_ANDROIDTV_ENTITY, ""), + CONF_ADB_ENABLED: entry.options.get(CONF_ADB_ENABLED, False), + CONF_ADB_HOST: entry.options.get(CONF_ADB_HOST, ""), + CONF_ADB_PORT: entry.options.get(CONF_ADB_PORT, DEFAULT_ADB_PORT), + CONF_PROTOCOL_ENABLED: True, + CONF_PROTOCOL_HOST: entry.options.get(CONF_PROTOCOL_HOST, ""), + } + return await self.async_step_protocol_pair() + + def _finish_reconfigure(self, mac: str | None): + """Reconfigure pairing done: update + reload + abort (G3a contract). + + The protocol host persists as the address that just paired, so the + entry no longer depends on the ADB-host fallback for its transport. + HA derives the abort reason ("reconfigure_successful") from the flow + source, so this needs no integration string. + """ + entry = self._reconfigure_entry or self._get_reconfigure_entry() + updates: dict = { + "options": { + **entry.options, + CONF_PROTOCOL_ENABLED: True, + CONF_PROTOCOL_HOST: self._protocol_host() or "", + }, + } + if mac: + # a swapped device keeps the entry's identity honest + updates["unique_id"] = mac + self._reconfigure_entry = None + return self.async_update_reload_and_abort(entry, **updates) + def _finish_reauth(self): """Reauth pairing done: update + reload + abort via the current API (G3a). The deprecated update-entry + schedule-reload boomerang listener is gone (2026.12 treats it as an error); the current config-entries contract is async_update_reload_and_abort with _get_reauth_entry. + Core's kwarg is `options` (REPLACE) — there is no `options_updates`. """ entry = self._reauth_entry or self._get_reauth_entry() self.async_update_reload_and_abort( entry, - options_updates={ + options={ **entry.options, CONF_PROTOCOL_ENABLED: True, CONF_PROTOCOL_HOST: self._user_data.get(CONF_PROTOCOL_HOST, ""), diff --git a/custom_components/nvidia_shield/strings.json b/custom_components/nvidia_shield/strings.json index 3c4fb49..4b17e55 100644 --- a/custom_components/nvidia_shield/strings.json +++ b/custom_components/nvidia_shield/strings.json @@ -57,7 +57,7 @@ "data_description": { "pairing_code": "The code the Shield displays while this step is open.", "protocol_host": "The Shield's IP address. Pre-filled when the Shield was discovered \u2014 change it if the Shield moved.", - "skip_pairing": "Finishes setup now without the remote-protocol transport. Power, availability and the app push then come from the other state sources only." + "skip_pairing": "Finishes setup with the remote protocol off \u2014 it stays off until you pair it. Pair later from this integration's entry menu: choose Reconfigure." } }, "reauth_confirm": { @@ -80,6 +80,8 @@ "reauth_successful": "Re-paired successfully.", "reauth_skipped": "Pairing skipped. The saved certificate is still not trusted, so the remote protocol stays unavailable until you pair again.", "reauth_entry_missing": "This entry no longer exists.", + "reconfigure_skipped": "Nothing changed \u2014 the remote protocol stays off until you pair it. Choose Reconfigure from this integration's entry menu when you are ready.", + "reconfigure_entry_missing": "This entry no longer exists, so there is nothing to reconfigure.", "zeroconf_no_host": "The discovery event carried no usable address.", "yaml_seed_only": {}, "yaml_seed": { diff --git a/custom_components/nvidia_shield/translations/en.json b/custom_components/nvidia_shield/translations/en.json index 3c4fb49..4b17e55 100644 --- a/custom_components/nvidia_shield/translations/en.json +++ b/custom_components/nvidia_shield/translations/en.json @@ -57,7 +57,7 @@ "data_description": { "pairing_code": "The code the Shield displays while this step is open.", "protocol_host": "The Shield's IP address. Pre-filled when the Shield was discovered \u2014 change it if the Shield moved.", - "skip_pairing": "Finishes setup now without the remote-protocol transport. Power, availability and the app push then come from the other state sources only." + "skip_pairing": "Finishes setup with the remote protocol off \u2014 it stays off until you pair it. Pair later from this integration's entry menu: choose Reconfigure." } }, "reauth_confirm": { @@ -80,6 +80,8 @@ "reauth_successful": "Re-paired successfully.", "reauth_skipped": "Pairing skipped. The saved certificate is still not trusted, so the remote protocol stays unavailable until you pair again.", "reauth_entry_missing": "This entry no longer exists.", + "reconfigure_skipped": "Nothing changed \u2014 the remote protocol stays off until you pair it. Choose Reconfigure from this integration's entry menu when you are ready.", + "reconfigure_entry_missing": "This entry no longer exists, so there is nothing to reconfigure.", "zeroconf_no_host": "The discovery event carried no usable address.", "yaml_seed_only": {}, "yaml_seed": { diff --git a/tests/entity_stublib.py b/tests/entity_stublib.py index d2d6a16..2c355f0 100644 --- a/tests/entity_stublib.py +++ b/tests/entity_stublib.py @@ -173,6 +173,11 @@ def install() -> None: return entry return None # the caller's cached entry is used + def _get_reconfigure_entry(self, entry=None): + if entry is not None: + return entry + return None # the caller's cached entry is used + # NOTE: the real HA signature is async_update_reload_and_abort(entry, ...) # which updates, schedules the reload and aborts; the stub records both. diff --git a/tests/test_config_flow_protocol.py b/tests/test_config_flow_protocol.py index 91bbe92..268c0f5 100644 --- a/tests/test_config_flow_protocol.py +++ b/tests/test_config_flow_protocol.py @@ -158,12 +158,24 @@ def _install_stubs() -> None: ConfigFlow._get_reauth_entry = lambda self, entry=None: entry def _update_reload_and_abort(self, entry, **kwargs): - updates = kwargs.get("options_updates") or {} + # `options` REPLACES the entry options (the real signature); the merge + # form is still accepted for older callers. + updates = dict(kwargs.get("options_updates") or {}) + updates.update(kwargs.get("options") or {}) options = entry.options for key, value in updates.items(): options[key] = value + if kwargs.get("unique_id"): + entry.unique_id = kwargs["unique_id"] self.hass.config_entries.async_schedule_reload(entry.entry_id) - return self.async_abort(reason="reauth_successful") + reason = kwargs.get("reason") + if reason is None: + reason = ( + "reconfigure_successful" + if getattr(self, "source", None) == "reconfigure" + else "reauth_successful" + ) + return self.async_abort(reason=reason) ConfigFlow.async_update_reload_and_abort = _update_reload_and_abort diff --git a/tests/test_config_flow_ux.py b/tests/test_config_flow_ux.py index 11e8c26..1c6876e 100644 --- a/tests/test_config_flow_ux.py +++ b/tests/test_config_flow_ux.py @@ -120,11 +120,42 @@ def _install_stubs() -> None: def _get_reauth_entry(self, entry=None): return entry + def _get_reconfigure_entry(self, entry=None): + """Real HA reads the entry from the flow context (SOURCE_RECONFIGURE).""" + if entry is not None: + return entry + return self.hass.config_entries.async_get_entry( + (getattr(self, "context", None) or {}).get("entry_id") + ) + + @property + def source(self): + """Real HA exposes the flow's source through the context.""" + return (getattr(self, "context", None) or {}).get("source") + def async_update_reload_and_abort(self, entry, **kwargs): + """Real-HA shape: update, schedule the reload, then abort. + + ``options`` REPLACES the entry's options (core has no + ``options_updates``); the merge form is still accepted so the older + reauth call and this one can share the stub. HA picks the abort + reason from the flow source unless one is passed. + """ + for key, value in (kwargs.get("options") or {}).items(): + entry.options[key] = value for key, value in (kwargs.get("options_updates") or {}).items(): entry.options[key] = value + if kwargs.get("unique_id"): + entry.unique_id = kwargs["unique_id"] self.hass.config_entries.async_schedule_reload(entry.entry_id) - return self.async_abort(reason="reauth_successful") + reason = kwargs.get("reason") + if reason is None: + reason = ( + "reconfigure_successful" + if self.source == "reconfigure" + else "reauth_successful" + ) + return self.async_abort(reason=reason) class OptionsFlow: def async_show_form( @@ -423,7 +454,6 @@ class TestVoluptuousMirrorContract: with pytest.raises(AssertionError): vol_mirror.declared_fields({CONF_ADB_HOST: str}) - def test_marker_equality_is_field_only_like_voluptuous(self): """t35 F2: real ``Marker.__eq__`` is ``self.schema == other``. @@ -874,6 +904,123 @@ class TestNoDeadEndPairing: assert entry.options[CONF_PROTOCOL_ENABLED] is True # untouched +class TestProtocolReconfigure: + """F1: an entry whose protocol transport is off can pair LATER (audit G5).""" + + def _entry(self, **overrides): + options = { + CONF_PROTOCOL_ENABLED: False, # the "continue without pairing" outcome + CONF_PROTOCOL_HOST: "", + CONF_ADB_ENABLED: True, + CONF_ADB_HOST: "10.0.0.9", + CONF_ADB_PORT: 5555, + } + options.update(overrides) + return SimpleNamespace( + entry_id="e1", title="mdarcy", data={}, options=options, + unique_id=None, + ) + + def _reconfigure_flow(self, module, tmp_path, entry=None): + flow, hass = _new_flow(module, tmp_path) + if entry is not None: + hass.config_entries.by_id["e1"] = entry + flow.context = {"entry_id": "e1", "source": "reconfigure"} + return flow, hass + + async def test_reconfigure_pairs_a_protocol_disabled_entry( + self, tmp_path, monkeypatch + ): + """Host editable → PIN step → options updated → entry reloaded.""" + module = _form_module() + client = _patch_protocol(monkeypatch, module) + entry = self._entry() + flow, hass = self._reconfigure_flow(module, tmp_path, entry) + + form = await flow.async_step_reconfigure() + + assert form["step_id"] == "protocol_pair" # the PIN step + assert client.pairing_started is True + fields = vol_mirror.declared_fields(form["data_schema"]) + assert CONF_PROTOCOL_HOST in fields # the address is editable here + assert CONF_PAIRING_CODE in fields + + result = await flow.async_step_protocol_pair( + {CONF_PAIRING_CODE: FAKE_PAIRING_CODE} + ) + + assert result["reason"] == "reconfigure_successful" + # the transport is ON and carries the address that just paired + assert entry.options[CONF_PROTOCOL_ENABLED] is True + assert entry.options[CONF_PROTOCOL_HOST] == "10.0.0.9" + # the rest of the entry's options survive the update + assert entry.options[CONF_ADB_ENABLED] is True + assert entry.options[CONF_ADB_HOST] == "10.0.0.9" + assert entry.unique_id == FAKE_MAC + assert hass.config_entries.reloaded_entry_ids == ["e1"] + assert client.finished_codes == [FAKE_PAIRING_CODE] + + async def test_reconfigure_host_correction_reopens_the_pin_step( + self, tmp_path, monkeypatch + ): + """The editable host wins: a corrected address starts a NEW session.""" + module = _form_module() + from nsh.connection import ShieldConnectionManager + + first = FakeProtocolClient() + _patch_protocol(monkeypatch, module, first) + entry = self._entry() + flow, _hass = self._reconfigure_flow(module, tmp_path, entry) + await flow.async_step_reconfigure() + assert first.pairing_started is True + + second = FakeProtocolClient() + monkeypatch.setattr( + ShieldConnectionManager, "build_protocol_client", lambda *a, **k: second + ) + form = await flow.async_step_protocol_pair( + {CONF_PROTOCOL_HOST: "10.0.0.77", CONF_PAIRING_CODE: "0000"} + ) + + assert form["step_id"] == "protocol_pair" # asks for the NEW code + assert first.disconnects == 1 # the stale session was dropped + assert second.pairing_started is True # pairing followed the new address + assert first.finished_codes == [] # the stale code was never used + + result = await flow.async_step_protocol_pair( + {CONF_PAIRING_CODE: FAKE_PAIRING_CODE} + ) + + assert result["reason"] == "reconfigure_successful" + assert entry.options[CONF_PROTOCOL_HOST] == "10.0.0.77" + + async def test_reconfigure_skip_leaves_the_entry_untouched( + self, tmp_path, monkeypatch + ): + module = _form_module() + _patch_protocol(monkeypatch, module) + entry = self._entry() + flow, hass = self._reconfigure_flow(module, tmp_path, entry) + await flow.async_step_reconfigure() + + result = await flow.async_step_protocol_pair({CONF_SKIP_PAIRING: True}) + + assert result == {"type": "abort", "reason": "reconfigure_skipped"} + assert entry.options[CONF_PROTOCOL_ENABLED] is False # unchanged + assert hass.config_entries.reloaded_entry_ids == [] + + async def test_reconfigure_without_a_linked_entry_aborts( + self, tmp_path, monkeypatch + ): + module = _form_module() + _patch_protocol(monkeypatch, module) + flow, _hass = self._reconfigure_flow(module, tmp_path, entry=None) + + result = await flow.async_step_reconfigure() + + assert result == {"type": "abort", "reason": "reconfigure_entry_missing"} + + class TestCopyContract: """F-C: plain, self-consistent copy with no unreachable-step pointers.""" @@ -889,6 +1036,15 @@ class TestCopyContract: assert "host" in text, key assert "discovered" in text, key + def test_protocol_skip_copy_names_the_later_pairing_path(self): + """F1b: the transport stays OFF until paired, and the way back is named.""" + for file in (_strings(), _en_strings()): + text = file["config"]["step"]["protocol_pair"]["data_description"][ + "skip_pairing" + ] + assert "stays off until you pair it" in text + assert "Reconfigure" in text + def test_every_error_and_abort_key_the_flow_raises_exists_in_both_files(self): source = (BASE / "config_flow.py").read_text(encoding="utf-8") raised = set(re.findall(r'errors\["base"\] = "([a-z_]+)"', source))