fix(config_flow): F-A/F-A2/F-D — the discovered host and the 5555 port ride the schema KEY defaults (t33)
F-A (HIGH): _user_schema_dict re-declared vol.Optional(CONF_ADB_HOST) /
CONF_PROTOCOL_HOST inside `if "host" in defaults:` — a no-op, because
voluptuous keys compare by field name and the first (defaultless) key object
survives. The discovered address therefore never reached the form, and the
user submitted a blank host. The host keys are now declared WITH default=host
(_host_field_key), so a discovery-driven user step arrives pre-filled.
F-A2: a discovery-driven user step says the address is known
("{discovery_note}Discovered Shield at <host> — its address is already filled
in below. ") through description_placeholders, and the no_state_source
re-render keeps the prefill instead of rebuilding a blank schema.
F-D: the ADB port is declared with default=DEFAULT_ADB_PORT (5555), and a YAML
seed's port hint is carried into the form instead of being flattened to 5555.
The seed host now feeds _seed_host (same prefill, no discovery claim).
Reproduce-through-test: tests/voluptuous_mirror.py mirrors the real
voluptuous.Marker key contract (hash/eq by field name, default=UNDEFINED when
omitted), because voluptuous is not installed in either CI variant and the old
bare-key stubs could not tell a schema WITH a default from one without. The
pre-t33 discovery assertion only checked the stored attribute and stayed green
while the form rendered blank; it now reads the schema keys and fails on the
pre-fix code (8 tests reproduced red locally).
Verified: uvx --with pytest-asyncio pytest tests -q → 242 passed, 1 skipped;
uvx ruff check custom_components tests → clean.
This commit is contained in:
1 parent
bbd98ebc3f
commit
73e886dafe
6 files changed
+193
-37
No files matched your search
@@ -154,12 +154,31 @@ def _app_map_from_text(
|
||||
return app_map
|
||||
|
||||
|
||||
def _host_field_key(field: str, host: str):
|
||||
"""The host-field KEY, carrying the known address as its default.
|
||||
|
||||
t33 F-A: the default MUST ride on this key. Re-declaring a bare
|
||||
``vol.Optional(field)`` afterwards is a no-op that silently drops it —
|
||||
voluptuous keys compare by field name, so the first (defaultless) key
|
||||
object survives and the form renders blank.
|
||||
"""
|
||||
if host:
|
||||
return vol.Optional(field, default=host)
|
||||
return vol.Optional(field)
|
||||
|
||||
|
||||
def _user_schema_dict(sel, defaults: dict | None = None) -> dict:
|
||||
"""User-step schema — the androidtv entity is optional (standalone).
|
||||
|
||||
``defaults`` pre-fills name/hosts from a zeroconf discovery event.
|
||||
``defaults`` pre-fills name/hosts from a zeroconf discovery event (or a
|
||||
YAML seed hint); the ADB port is pre-filled with the standard 5555 so it
|
||||
reads as an advanced setting rather than required knowledge (t33 F-D).
|
||||
"""
|
||||
defaults = defaults or {}
|
||||
host = str(defaults.get("host") or "")
|
||||
port = defaults.get("port")
|
||||
if not isinstance(port, int):
|
||||
port = DEFAULT_ADB_PORT
|
||||
schema_dict = {
|
||||
vol.Required("name", default=defaults.get("name", DEFAULT_NAME)): str,
|
||||
vol.Optional(CONF_ANDROIDTV_ENTITY): sel["androidtv"],
|
||||
@@ -174,14 +193,13 @@ def _user_schema_dict(sel, defaults: dict | None = None) -> dict:
|
||||
schema_dict[vol.Optional(CONF_POWER_ENTITY)] = sel["power"]
|
||||
|
||||
schema_dict[vol.Optional(CONF_ADB_ENABLED)] = bool
|
||||
schema_dict[vol.Optional(CONF_ADB_STORE_APP_INFO)] = bool
|
||||
schema_dict[vol.Optional(CONF_ADB_HOST)] = str
|
||||
schema_dict[vol.Optional(CONF_ADB_PORT)] = int
|
||||
# the code's own fallback for this flag is True (async_step_user) — the
|
||||
# rendered checkbox must agree with it
|
||||
schema_dict[vol.Optional(CONF_ADB_STORE_APP_INFO, default=True)] = bool
|
||||
schema_dict[_host_field_key(CONF_ADB_HOST, host)] = str
|
||||
schema_dict[vol.Optional(CONF_ADB_PORT, default=port)] = int
|
||||
schema_dict[vol.Optional(CONF_PROTOCOL_ENABLED)] = bool
|
||||
schema_dict[vol.Optional(CONF_PROTOCOL_HOST)] = str
|
||||
if "host" in defaults:
|
||||
schema_dict[vol.Optional(CONF_ADB_HOST)] = str
|
||||
schema_dict[vol.Optional(CONF_PROTOCOL_HOST)] = str
|
||||
schema_dict[_host_field_key(CONF_PROTOCOL_HOST, host)] = str
|
||||
return schema_dict
|
||||
|
||||
|
||||
@@ -215,6 +233,9 @@ class NvidiaShieldConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
|
||||
# Zeroconf discovery state (P4-1)
|
||||
self._discovered_host: str | None = None
|
||||
self._discovered_name: str | None = None
|
||||
# YAML-seed prefill hint (t33 F-A): feeds the same user-step prefill as
|
||||
# a discovery, without claiming the address was discovered.
|
||||
self._seed_host: str | None = None
|
||||
|
||||
async def async_step_user(self, user_input=None):
|
||||
"""Step 1: Basic setup.
|
||||
@@ -236,12 +257,7 @@ class NvidiaShieldConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
|
||||
)
|
||||
if not has_source:
|
||||
errors["base"] = "no_state_source"
|
||||
sel = _build_selectors()
|
||||
return self.async_show_form(
|
||||
step_id="user",
|
||||
data_schema=vol.Schema(_user_schema_dict(sel)),
|
||||
errors=errors,
|
||||
)
|
||||
return self._show_user_form(errors)
|
||||
|
||||
# t29 G2: the androidtv entity_id is a renameable reference —
|
||||
# never a unique_id. A sqlite/registry MAC is preferred (the
|
||||
@@ -301,21 +317,48 @@ class NvidiaShieldConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
|
||||
|
||||
return self._create_entry()
|
||||
|
||||
return self._show_user_form(errors)
|
||||
|
||||
sel = _build_selectors()
|
||||
def _user_defaults(self) -> dict:
|
||||
"""Defaults for the user step: whatever setup already knows.
|
||||
|
||||
defaults = {}
|
||||
A zeroconf discovery (or a YAML seed hint) supplies the address; the
|
||||
port keeps a seed's hint instead of being flattened to the standard
|
||||
value. This is the ONLY place the known address enters the form — the
|
||||
schema keys carry these values as their defaults (t33 F-A).
|
||||
"""
|
||||
defaults = {
|
||||
"name": self._discovered_name
|
||||
or self._user_data.get("name")
|
||||
or DEFAULT_NAME,
|
||||
}
|
||||
host = self._discovered_host or self._seed_host
|
||||
if host:
|
||||
defaults["host"] = host
|
||||
port = self._user_data.get(CONF_ADB_PORT)
|
||||
if isinstance(port, int):
|
||||
defaults["port"] = port
|
||||
return defaults
|
||||
|
||||
def _show_user_form(self, errors=None):
|
||||
"""Render the user step with the known address actually applied.
|
||||
|
||||
t33 F-A2: a discovery-driven form says so in its description, so a
|
||||
pre-filled host can never be misread as "unknown".
|
||||
"""
|
||||
note = ""
|
||||
if self._discovered_host:
|
||||
defaults = {
|
||||
"name": self._discovered_name or DEFAULT_NAME,
|
||||
"host": self._discovered_host,
|
||||
}
|
||||
schema_dict = _user_schema_dict(sel, defaults)
|
||||
|
||||
note = (
|
||||
f"Discovered Shield at {self._discovered_host} — its address "
|
||||
"is already filled in below. "
|
||||
)
|
||||
return self.async_show_form(
|
||||
step_id="user",
|
||||
data_schema=vol.Schema(schema_dict),
|
||||
errors=errors,
|
||||
data_schema=vol.Schema(
|
||||
_user_schema_dict(_build_selectors(), self._user_defaults())
|
||||
),
|
||||
errors=errors or {},
|
||||
description_placeholders={"discovery_note": note},
|
||||
)
|
||||
|
||||
async def async_step_zeroconf(self, discovery_info):
|
||||
@@ -530,7 +573,10 @@ class NvidiaShieldConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
|
||||
# protocol prefill hints ONLY (pairing stays interactive)
|
||||
CONF_PROTOCOL_HOST: block.get(CONF_PROTOCOL_HOST, ""),
|
||||
}
|
||||
self._discovered_host = self._user_data[CONF_PROTOCOL_HOST] or self._user_data[
|
||||
# t33 F-A: the seed host feeds the same user-step prefill as a
|
||||
# zeroconf discovery, but WITHOUT the discovery announcement — the
|
||||
# address came from configuration.yaml, not from the network.
|
||||
self._seed_host = self._user_data[CONF_PROTOCOL_HOST] or self._user_data[
|
||||
CONF_ADB_HOST
|
||||
]
|
||||
return await self.async_step_user(None)
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
"step": {
|
||||
"user": {
|
||||
"title": "NVIDIA Shield",
|
||||
"description": "Set up your NVIDIA Shield. Link the Android TV remote, Jellyfin, receiver, and power monitoring.",
|
||||
"description": "{discovery_note}Set up your NVIDIA Shield. Link the Android TV remote, Jellyfin, receiver, and power monitoring.",
|
||||
"data": {
|
||||
"name": "Device Name",
|
||||
"androidtv_entity": "The media_player entity from the Android TV Remote integration. Optional for standalone setups that pair with the Nvidia Remote Protocol instead.",
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
"step": {
|
||||
"user": {
|
||||
"title": "NVIDIA Shield",
|
||||
"description": "Set up your NVIDIA Shield. Link the Android TV remote, Jellyfin, receiver, and power monitoring.",
|
||||
"description": "{discovery_note}Set up your NVIDIA Shield. Link the Android TV remote, Jellyfin, receiver, and power monitoring.",
|
||||
"data": {
|
||||
"name": "Device Name",
|
||||
"androidtv_entity": "The media_player entity from the Android TV Remote integration. Optional for standalone setups that pair with the Nvidia Remote Protocol instead.",
|
||||
|
||||
+10
-4
@@ -11,6 +11,8 @@ import enum
|
||||
import sys
|
||||
import types
|
||||
|
||||
import voluptuous_mirror
|
||||
|
||||
ENTITIES_STUB_NAMES = (
|
||||
"voluptuous",
|
||||
"homeassistant",
|
||||
@@ -115,9 +117,9 @@ def install() -> None:
|
||||
return
|
||||
|
||||
vol = types.ModuleType("voluptuous")
|
||||
vol.Required = lambda key, default=None: key
|
||||
vol.Optional = lambda key, default=None: key
|
||||
vol.Schema = lambda schema: schema
|
||||
# t33 F-A: the faithful mirror keeps schema defaults visible — a bare-key
|
||||
# stub cannot tell a schema WITH a default from one without.
|
||||
voluptuous_mirror.install(vol)
|
||||
sys.modules["voluptuous"] = vol
|
||||
|
||||
ha = types.ModuleType("homeassistant")
|
||||
@@ -148,12 +150,16 @@ def install() -> None:
|
||||
# t32 F2 (audited helper): recording no-op at hub scope
|
||||
pass
|
||||
|
||||
def async_show_form(self, *, step_id, data_schema=None, errors=None):
|
||||
def async_show_form(
|
||||
self, *, step_id, data_schema=None, errors=None,
|
||||
description_placeholders=None,
|
||||
):
|
||||
return {
|
||||
"type": "form",
|
||||
"step_id": step_id,
|
||||
"errors": dict(errors or {}),
|
||||
"data_schema": data_schema,
|
||||
"description_placeholders": dict(description_placeholders or {}),
|
||||
}
|
||||
|
||||
def async_create_entry(self, *, title, data, options=None):
|
||||
|
||||
@@ -15,6 +15,7 @@ from types import SimpleNamespace
|
||||
from unittest import mock
|
||||
|
||||
import pytest
|
||||
import voluptuous_mirror as vol_mirror
|
||||
from conftest import _SHIELD_DIR, _load_module
|
||||
from nsh.const import (
|
||||
CONF_ADB_ENABLED,
|
||||
@@ -54,9 +55,10 @@ def _install_stubs() -> None:
|
||||
return
|
||||
|
||||
vol = types.ModuleType("voluptuous")
|
||||
vol.Required = lambda key, default=None: key
|
||||
vol.Optional = lambda key, default=None: key
|
||||
vol.Schema = lambda schema: schema
|
||||
# t33 F-A: a faithful mirror — the default stays ON the key, so a schema
|
||||
# whose host default was dropped FAILS here instead of looking identical
|
||||
# to one that carries it (the old bare-key stub hid exactly that bug).
|
||||
vol_mirror.install(vol)
|
||||
sys.modules["voluptuous"] = vol
|
||||
|
||||
ha = types.ModuleType("homeassistant")
|
||||
@@ -71,12 +73,16 @@ def _install_stubs() -> None:
|
||||
kwargs.pop("domain", None)
|
||||
super().__init_subclass__(**kwargs)
|
||||
|
||||
def async_show_form(self, *, step_id, data_schema=None, errors=None):
|
||||
def async_show_form(
|
||||
self, *, step_id, data_schema=None, errors=None,
|
||||
description_placeholders=None,
|
||||
):
|
||||
return {
|
||||
"type": "form",
|
||||
"step_id": step_id,
|
||||
"errors": dict(errors or {}),
|
||||
"data_schema": data_schema,
|
||||
"description_placeholders": dict(description_placeholders or {}),
|
||||
}
|
||||
|
||||
def async_create_entry(self, *, title, data, options=None):
|
||||
@@ -942,6 +948,11 @@ class TestZeroconfDiscovery:
|
||||
)
|
||||
|
||||
async def test_discovery_prefills_the_user_step(self, tmp_path):
|
||||
"""t33 F-A: the discovered host is the DEFAULT OF THE SCHEMA KEYS.
|
||||
|
||||
The old check (the attribute was stored) stayed green while the
|
||||
rendered form was blank — this one reads the schema itself.
|
||||
"""
|
||||
module = _form_module()
|
||||
flow = module.NvidiaShieldConfigFlow()
|
||||
flow.hass = FakeHass(str(tmp_path))
|
||||
@@ -950,9 +961,13 @@ class TestZeroconfDiscovery:
|
||||
|
||||
assert result["type"] == "form"
|
||||
assert result["step_id"] == "user"
|
||||
# pre-fill landed: the discovered host is recorded for the schema
|
||||
assert flow._discovered_host == "192.168.1.50"
|
||||
assert result["data_schema"] is not None
|
||||
defaults = vol_mirror.defaults_by_key(result["data_schema"])
|
||||
assert defaults[CONF_ADB_HOST] == "192.168.1.50"
|
||||
assert defaults[CONF_PROTOCOL_HOST] == "192.168.1.50"
|
||||
# ...and a discovery-driven form says the address is already known
|
||||
note = result["description_placeholders"]["discovery_note"]
|
||||
assert "Discovered Shield at 192.168.1.50" in note
|
||||
|
||||
async def test_discovery_without_address_aborts(self, tmp_path):
|
||||
module = _form_module()
|
||||
|
||||
@@ -0,0 +1,89 @@
|
||||
"""A voluptuous mirror faithful enough to SEE a dropped schema default.
|
||||
|
||||
The test environment never installs voluptuous (the config-flow tests import
|
||||
``config_flow`` against import-only stubs), so the harness has to model the two
|
||||
behaviours the flow schemas depend on:
|
||||
|
||||
* ``Optional(field, default=value)`` / ``Required(field, default=value)`` carry
|
||||
that default on the KEY object;
|
||||
* keys compare by FIELD NAME only — real ``voluptuous.Marker`` implements
|
||||
``__hash__ = hash(self.schema)`` and ``__eq__`` on class + schema. A later
|
||||
re-declaration of the same field therefore keeps the FIRST key object, so a
|
||||
default added by a second declaration is silently dropped.
|
||||
|
||||
That second property is exactly how the t33 F-A discovery-prefill bug hid from
|
||||
the suite: the old stubs reduced every key to its bare string, so a schema
|
||||
whose host key carried no default looked identical to one that carried the
|
||||
discovered address. ``TestVoluptuousMirrorContract`` in
|
||||
``tests/test_config_flow_ux.py`` pins both properties so the harness itself
|
||||
cannot regress into blindness.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
UNDEFINED = object()
|
||||
"""Sentinel for "this key declares no default" (real ``voluptuous.UNDEFINED``)."""
|
||||
|
||||
|
||||
class Marker:
|
||||
"""Stand-in for ``voluptuous.Marker`` (the ``Required``/``Optional`` base)."""
|
||||
|
||||
def __init__(self, schema, default=UNDEFINED):
|
||||
self.schema = schema
|
||||
self.default = default
|
||||
|
||||
def __hash__(self):
|
||||
# real Marker hashes the field name only
|
||||
return hash(self.schema)
|
||||
|
||||
def __eq__(self, other):
|
||||
# real Marker compares class + field name, never the default
|
||||
return type(other) is type(self) and other.schema == self.schema
|
||||
|
||||
def __repr__(self):
|
||||
return f"<{type(self).__name__} {self.schema!r} default={self.default!r}>"
|
||||
|
||||
|
||||
class Required(Marker):
|
||||
"""Stand-in for ``voluptuous.Required``."""
|
||||
|
||||
|
||||
class Optional(Marker):
|
||||
"""Stand-in for ``voluptuous.Optional``."""
|
||||
|
||||
|
||||
def install(vol) -> None:
|
||||
"""Wire the mirror onto a stub ``voluptuous`` module object."""
|
||||
vol.UNDEFINED = UNDEFINED
|
||||
vol.Marker = Marker
|
||||
vol.Required = Required
|
||||
vol.Optional = Optional
|
||||
vol.Schema = lambda schema: schema
|
||||
|
||||
|
||||
def defaults_by_key(schema_dict) -> dict:
|
||||
"""``{field: default}`` for a schema declared with mirror keys."""
|
||||
return {key.schema: key.default for key in declared_keys(schema_dict)}
|
||||
|
||||
|
||||
def declared_keys(schema_dict) -> list:
|
||||
"""The schema's marker keys, in declaration order.
|
||||
|
||||
Raises on a non-marker key: a bare string key means the harness (or the
|
||||
code under test) lost the KEY structure this mirror exists to check.
|
||||
Test-harness bookkeeping keys (``__suggested__`` and friends) are skipped.
|
||||
"""
|
||||
keys = []
|
||||
for key in schema_dict:
|
||||
if isinstance(key, Marker):
|
||||
keys.append(key)
|
||||
continue
|
||||
if isinstance(key, str) and key.startswith("__"):
|
||||
continue
|
||||
raise AssertionError(f"schema key {key!r} is not a voluptuous marker")
|
||||
return keys
|
||||
|
||||
|
||||
def declared_fields(schema_dict) -> list:
|
||||
"""The field names of a schema, in declaration order."""
|
||||
return [key.schema for key in declared_keys(schema_dict)]
|
||||
Reference in new issue
Block a user