refactor(config): one Shield IP field for both transports (t41)
The two address fields were the same Shield spelled twice, and every bug in the zeroconf/IP-change path came from keeping them in sync. One field now: * const.CONF_HOST replaces CONF_ADB_HOST / CONF_PROTOCOL_HOST in the setup form, the options page, the pairing forms, the reconfigure/reauth seeding and the description copy. ADB port 5555 stays the only port field; * helpers.entry_host(entry) is the single resolver (options first, then data); resolve_adb_config and the protocol settings both go through it, so both transports read the same value at call time (the t32 F1 factory re-read keeps working: the NEXT built poller sees the new address); * helpers.seed_host / seed_adb_enabled own the YAML seed's shape rule. A block may use `host` or the legacy protocol_host/adb_host keys — t41 keeps the old spelling working — and t32 F4's outcome is unchanged: a protocol-only block leaves ADB off, an address (or an explicit adb_enabled) turns it on; * the zeroconf IP-change path collapses the t21-F2 per-host dance into one rule: the discovery address IS the entry's address, so the entry is updated when it differs and left alone when it matches. The blank-self-heal and deliberately-different-ADB-host cases are gone with the duplication — a second address field cannot express them anymore, and the tests now pin that (previously "an explicit override is never clobbered"); * the options page exposes the protocol toggle it never had (the page could turn ADB off but not the protocol); * button.py (granted to this task) reads the entry's own address instead of another integration's entity. The v3 entry migration that maps existing adb_host/protocol_host onto CONF_HOST and drops the androidtv key lands in the next commit; HA runs migrations before setup, so an existing entry is never read with the old shape. Verified: 311 passed / 2 skipped, 312 / 1 with androidtvremote2, ruff clean, drift engine exit 0 on HEAD. The UI-copy contract tests (every rendered field has a label in strings.json AND translations/en.json, key trees identical, host errors name the field) drove the string edits.
This commit is contained in:
1 parent
86aa2827a2
commit
75ebeb40b0
12 files changed
+302
-276
No files matched your search
@@ -31,12 +31,10 @@ import voluptuous_mirror as vol_mirror
|
||||
from conftest import _SHIELD_DIR, _load_module
|
||||
from nsh.const import (
|
||||
CONF_ADB_ENABLED,
|
||||
CONF_ADB_HOST,
|
||||
CONF_ADB_PORT,
|
||||
CONF_ANDROIDTV_ENTITY,
|
||||
CONF_HOST,
|
||||
CONF_PAIRING_CODE,
|
||||
CONF_PROTOCOL_ENABLED,
|
||||
CONF_PROTOCOL_HOST,
|
||||
CONF_SKIP_PAIRING,
|
||||
DEFAULT_ADB_PORT,
|
||||
)
|
||||
@@ -390,11 +388,9 @@ def _patch_adb(monkeypatch, module, connected=True):
|
||||
def _user_input(**overrides) -> dict:
|
||||
base = {
|
||||
"name": "mdarcy",
|
||||
CONF_ANDROIDTV_ENTITY: "media_player.shield",
|
||||
CONF_ADB_ENABLED: False,
|
||||
CONF_ADB_HOST: "",
|
||||
CONF_PROTOCOL_ENABLED: False,
|
||||
CONF_PROTOCOL_HOST: "",
|
||||
CONF_HOST: "", # t41: one address field for both transports
|
||||
}
|
||||
base.update(overrides)
|
||||
return base
|
||||
@@ -444,22 +440,22 @@ class TestVoluptuousMirrorContract:
|
||||
|
||||
def test_a_default_on_the_declaration_is_visible(self):
|
||||
vol = sys.modules["voluptuous"]
|
||||
schema = {vol.Optional(CONF_ADB_HOST, default=DISCOVERED_HOST): str}
|
||||
schema = {vol.Optional(CONF_HOST, default=DISCOVERED_HOST): str}
|
||||
|
||||
assert vol_mirror.defaults_by_key(schema)[CONF_ADB_HOST] == DISCOVERED_HOST
|
||||
assert vol_mirror.defaults_by_key(schema)[CONF_HOST] == DISCOVERED_HOST
|
||||
|
||||
def test_a_later_redeclaration_cannot_add_the_default(self):
|
||||
vol = sys.modules["voluptuous"]
|
||||
# this is the t33 shape: declare bare, then "fix" it by re-declaring.
|
||||
# Real voluptuous keeps the FIRST key object, so the default is dropped.
|
||||
schema = {vol.Optional(CONF_ADB_HOST): str}
|
||||
schema[vol.Optional(CONF_ADB_HOST, default=DISCOVERED_HOST)] = str
|
||||
schema = {vol.Optional(CONF_HOST): str}
|
||||
schema[vol.Optional(CONF_HOST, default=DISCOVERED_HOST)] = str
|
||||
|
||||
assert vol_mirror.defaults_by_key(schema)[CONF_ADB_HOST] is vol_mirror.UNDEFINED
|
||||
assert vol_mirror.defaults_by_key(schema)[CONF_HOST] is vol_mirror.UNDEFINED
|
||||
|
||||
def test_a_bare_string_key_is_rejected_loudly(self):
|
||||
with pytest.raises(AssertionError):
|
||||
vol_mirror.declared_fields({CONF_ADB_HOST: str})
|
||||
vol_mirror.declared_fields({CONF_HOST: str})
|
||||
|
||||
def test_marker_equality_is_field_only_like_voluptuous(self):
|
||||
"""t35 F2: real ``Marker.__eq__`` is ``self.schema == other``.
|
||||
@@ -468,12 +464,12 @@ class TestVoluptuousMirrorContract:
|
||||
"""
|
||||
vol = sys.modules["voluptuous"]
|
||||
|
||||
assert vol.Optional(CONF_ADB_HOST) == vol.Required(CONF_ADB_HOST)
|
||||
assert hash(vol.Optional(CONF_ADB_HOST)) == hash(
|
||||
vol.Required(CONF_ADB_HOST)
|
||||
assert vol.Optional(CONF_HOST) == vol.Required(CONF_HOST)
|
||||
assert hash(vol.Optional(CONF_HOST)) == hash(
|
||||
vol.Required(CONF_HOST)
|
||||
)
|
||||
# the real implementation compares the field name to the other value
|
||||
assert vol.Optional(CONF_ADB_HOST) == CONF_ADB_HOST
|
||||
assert vol.Optional(CONF_HOST) == CONF_HOST
|
||||
|
||||
def test_a_mixed_marker_redeclaration_drops_the_default(self):
|
||||
"""The case a class-comparing mirror masks (t35 F2).
|
||||
@@ -483,11 +479,11 @@ class TestVoluptuousMirrorContract:
|
||||
schema silently loses the default, exactly like the Optional case.
|
||||
"""
|
||||
vol = sys.modules["voluptuous"]
|
||||
schema = {vol.Optional(CONF_ADB_HOST): str}
|
||||
schema[vol.Required(CONF_ADB_HOST, default=DISCOVERED_HOST)] = str
|
||||
schema = {vol.Optional(CONF_HOST): str}
|
||||
schema[vol.Required(CONF_HOST, default=DISCOVERED_HOST)] = str
|
||||
|
||||
assert (
|
||||
vol_mirror.defaults_by_key(schema)[CONF_ADB_HOST]
|
||||
vol_mirror.defaults_by_key(schema)[CONF_HOST]
|
||||
is vol_mirror.UNDEFINED
|
||||
)
|
||||
|
||||
@@ -495,7 +491,7 @@ class TestVoluptuousMirrorContract:
|
||||
class TestDiscoveryPrefill:
|
||||
"""F-A / F-A2 / F-D: the discovered address reaches the rendered FORM."""
|
||||
|
||||
async def test_discovered_host_is_the_default_of_both_host_fields(self, tmp_path):
|
||||
async def test_discovered_host_is_the_default_of_the_host_field(self, tmp_path):
|
||||
module = _form_module()
|
||||
flow, _hass = _new_flow(module, tmp_path)
|
||||
|
||||
@@ -509,23 +505,22 @@ class TestDiscoveryPrefill:
|
||||
defaults = vol_mirror.defaults_by_key(result["data_schema"])
|
||||
# schema-level, not "the attribute was stored": this is precisely what
|
||||
# the old assertion missed while the form rendered blank
|
||||
assert defaults[CONF_ADB_HOST] == DISCOVERED_HOST
|
||||
assert defaults[CONF_PROTOCOL_HOST] == DISCOVERED_HOST
|
||||
assert defaults[CONF_HOST] == DISCOVERED_HOST
|
||||
|
||||
async def test_every_host_field_carries_the_discovered_default(self, tmp_path):
|
||||
"""The keys are declared WITH the default — never re-declared bare."""
|
||||
async def test_the_host_field_carries_the_discovered_default(self, tmp_path):
|
||||
"""The key is declared WITH the default — never re-declared bare."""
|
||||
module = _form_module()
|
||||
flow, _hass = _new_flow(module, tmp_path)
|
||||
|
||||
result = await flow.async_step_zeroconf(_discovery())
|
||||
|
||||
for field in (CONF_ADB_HOST, CONF_PROTOCOL_HOST):
|
||||
keys = [
|
||||
key for key in vol_mirror.declared_keys(result["data_schema"])
|
||||
if key.schema == field
|
||||
]
|
||||
assert len(keys) == 1, f"{field} is declared {len(keys)} times"
|
||||
assert keys[0].default == DISCOVERED_HOST
|
||||
keys = [
|
||||
key
|
||||
for key in vol_mirror.declared_keys(result["data_schema"])
|
||||
if key.schema == CONF_HOST
|
||||
]
|
||||
assert len(keys) == 1, f"the address field is declared {len(keys)} times"
|
||||
assert keys[0].default == DISCOVERED_HOST
|
||||
|
||||
async def test_discovery_step_announces_the_known_address(self, tmp_path):
|
||||
module = _form_module()
|
||||
@@ -544,7 +539,7 @@ class TestDiscoveryPrefill:
|
||||
|
||||
assert result["description_placeholders"]["discovery_note"] == ""
|
||||
assert (
|
||||
vol_mirror.defaults_by_key(result["data_schema"])[CONF_ADB_HOST]
|
||||
vol_mirror.defaults_by_key(result["data_schema"])[CONF_HOST]
|
||||
is vol_mirror.UNDEFINED
|
||||
)
|
||||
|
||||
@@ -556,7 +551,7 @@ class TestDiscoveryPrefill:
|
||||
|
||||
assert result["step_id"] == "user"
|
||||
defaults = vol_mirror.defaults_by_key(result["data_schema"])
|
||||
assert defaults[CONF_ADB_HOST] == "10.0.0.31"
|
||||
assert defaults[CONF_HOST] == "10.0.0.31"
|
||||
assert defaults["name"] == "Living"
|
||||
assert result["description_placeholders"]["discovery_note"] == ""
|
||||
|
||||
@@ -583,8 +578,7 @@ class TestDiscoveryPrefill:
|
||||
|
||||
assert result["errors"] == {"base": "no_state_source"}
|
||||
defaults = vol_mirror.defaults_by_key(result["data_schema"])
|
||||
assert defaults[CONF_ADB_HOST] == DISCOVERED_HOST
|
||||
assert defaults[CONF_PROTOCOL_HOST] == DISCOVERED_HOST
|
||||
assert defaults[CONF_HOST] == DISCOVERED_HOST
|
||||
|
||||
|
||||
class TestNoDeadEndPairing:
|
||||
@@ -604,9 +598,9 @@ class TestNoDeadEndPairing:
|
||||
assert CONF_SKIP_PAIRING in vol_mirror.declared_fields(form["data_schema"])
|
||||
|
||||
adb_fields = vol_mirror.declared_fields(adb_form["data_schema"])
|
||||
assert CONF_ADB_HOST in adb_fields # the field adb_no_host names
|
||||
assert CONF_HOST in adb_fields # the field adb_no_host names
|
||||
protocol_fields = vol_mirror.declared_fields(protocol_form["data_schema"])
|
||||
assert CONF_PROTOCOL_HOST in protocol_fields # protocol_no_host names this
|
||||
assert CONF_HOST in protocol_fields # protocol_no_host names this
|
||||
|
||||
async def test_the_skip_option_is_off_by_default(self, tmp_path, monkeypatch):
|
||||
module = _form_module()
|
||||
@@ -631,7 +625,7 @@ class TestNoDeadEndPairing:
|
||||
_user_input(
|
||||
**{
|
||||
CONF_PROTOCOL_ENABLED: True,
|
||||
CONF_PROTOCOL_HOST: DISCOVERED_HOST,
|
||||
CONF_HOST: DISCOVERED_HOST,
|
||||
}
|
||||
)
|
||||
)
|
||||
@@ -662,7 +656,7 @@ class TestNoDeadEndPairing:
|
||||
_user_input(
|
||||
**{
|
||||
CONF_ADB_ENABLED: True,
|
||||
CONF_ADB_HOST: DISCOVERED_HOST,
|
||||
CONF_HOST: DISCOVERED_HOST,
|
||||
}
|
||||
)
|
||||
)
|
||||
@@ -695,7 +689,7 @@ class TestNoDeadEndPairing:
|
||||
**{
|
||||
CONF_ADB_ENABLED: True,
|
||||
CONF_PROTOCOL_ENABLED: True,
|
||||
CONF_ADB_HOST: DISCOVERED_HOST,
|
||||
CONF_HOST: DISCOVERED_HOST,
|
||||
}
|
||||
)
|
||||
)
|
||||
@@ -717,7 +711,7 @@ class TestNoDeadEndPairing:
|
||||
**{
|
||||
CONF_ADB_ENABLED: True,
|
||||
CONF_PROTOCOL_ENABLED: True,
|
||||
CONF_ADB_HOST: DISCOVERED_HOST,
|
||||
CONF_HOST: DISCOVERED_HOST,
|
||||
}
|
||||
)
|
||||
)
|
||||
@@ -739,7 +733,7 @@ class TestNoDeadEndPairing:
|
||||
_user_input(
|
||||
**{
|
||||
CONF_PROTOCOL_ENABLED: True,
|
||||
CONF_PROTOCOL_HOST: DISCOVERED_HOST,
|
||||
CONF_HOST: DISCOVERED_HOST,
|
||||
}
|
||||
)
|
||||
)
|
||||
@@ -763,7 +757,7 @@ class TestNoDeadEndPairing:
|
||||
_user_input(
|
||||
**{
|
||||
CONF_ADB_ENABLED: True,
|
||||
CONF_ADB_HOST: DISCOVERED_HOST,
|
||||
CONF_HOST: DISCOVERED_HOST,
|
||||
}
|
||||
)
|
||||
)
|
||||
@@ -785,11 +779,11 @@ class TestNoDeadEndPairing:
|
||||
)
|
||||
|
||||
result = await flow.async_step_adb_pair(
|
||||
{CONF_ADB_HOST: "10.0.0.77", CONF_ADB_PORT: 5555}
|
||||
{CONF_HOST: "10.0.0.77", CONF_ADB_PORT: 5555}
|
||||
)
|
||||
|
||||
assert result["type"] == "create_entry"
|
||||
assert result["options"][CONF_ADB_HOST] == "10.0.0.77"
|
||||
assert result["options"][CONF_HOST] == "10.0.0.77"
|
||||
assert test_connect.await_args.args[0] == "10.0.0.77"
|
||||
|
||||
async def test_protocol_form_host_correction_restarts_pairing(self, tmp_path, monkeypatch):
|
||||
@@ -801,7 +795,7 @@ class TestNoDeadEndPairing:
|
||||
_user_input(
|
||||
**{
|
||||
CONF_PROTOCOL_ENABLED: True,
|
||||
CONF_PROTOCOL_HOST: DISCOVERED_HOST,
|
||||
CONF_HOST: DISCOVERED_HOST,
|
||||
}
|
||||
)
|
||||
)
|
||||
@@ -814,12 +808,12 @@ class TestNoDeadEndPairing:
|
||||
monkeypatch.setattr(
|
||||
ShieldConnectionManager, "build_protocol_client", lambda *a, **k: second
|
||||
)
|
||||
form = await flow.async_step_protocol_pair({CONF_PROTOCOL_HOST: "10.0.0.77"})
|
||||
form = await flow.async_step_protocol_pair({CONF_HOST: "10.0.0.77"})
|
||||
|
||||
assert form["step_id"] == "protocol_pair"
|
||||
assert first.disconnects == 1
|
||||
assert second.pairing_started is True
|
||||
assert flow._user_data[CONF_PROTOCOL_HOST] == "10.0.0.77"
|
||||
assert flow._user_data[CONF_HOST] == "10.0.0.77"
|
||||
|
||||
async def test_adb_no_host_error_keeps_the_user_on_a_fixable_form(
|
||||
self, tmp_path, monkeypatch
|
||||
@@ -834,7 +828,7 @@ class TestNoDeadEndPairing:
|
||||
result = await flow.async_step_adb_pair({CONF_ADB_PORT: 5555})
|
||||
|
||||
assert result["errors"] == {"base": "adb_no_host"}
|
||||
assert CONF_ADB_HOST in vol_mirror.declared_fields(result["data_schema"])
|
||||
assert CONF_HOST in vol_mirror.declared_fields(result["data_schema"])
|
||||
|
||||
async def test_adb_failed_error_keeps_the_user_on_the_form(self, tmp_path, monkeypatch):
|
||||
module = _form_module()
|
||||
@@ -844,7 +838,7 @@ class TestNoDeadEndPairing:
|
||||
_user_input(
|
||||
**{
|
||||
CONF_ADB_ENABLED: True,
|
||||
CONF_ADB_HOST: DISCOVERED_HOST,
|
||||
CONF_HOST: DISCOVERED_HOST,
|
||||
}
|
||||
)
|
||||
)
|
||||
@@ -873,7 +867,7 @@ class TestNoDeadEndPairing:
|
||||
flow._user_data = {
|
||||
"name": "mdarcy",
|
||||
CONF_PROTOCOL_ENABLED: True,
|
||||
CONF_PROTOCOL_HOST: DISCOVERED_HOST,
|
||||
CONF_HOST: DISCOVERED_HOST,
|
||||
}
|
||||
|
||||
result = await flow.async_step_protocol_pair(None)
|
||||
@@ -890,7 +884,7 @@ class TestNoDeadEndPairing:
|
||||
entry_id="e1",
|
||||
title="mdarcy",
|
||||
data={},
|
||||
options={CONF_PROTOCOL_ENABLED: True, CONF_PROTOCOL_HOST: DISCOVERED_HOST},
|
||||
options={CONF_PROTOCOL_ENABLED: True, CONF_HOST: DISCOVERED_HOST},
|
||||
)
|
||||
hass.config_entries.by_id["e1"] = entry
|
||||
flow.context = {"entry_id": "e1"}
|
||||
@@ -912,9 +906,8 @@ class TestProtocolReconfigure:
|
||||
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_HOST: "10.0.0.9", # t41: the entry's one address
|
||||
CONF_ADB_PORT: 5555,
|
||||
}
|
||||
options.update(overrides)
|
||||
@@ -944,7 +937,7 @@ class TestProtocolReconfigure:
|
||||
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_HOST in fields # the address is editable here
|
||||
assert CONF_PAIRING_CODE in fields
|
||||
|
||||
result = await flow.async_step_protocol_pair(
|
||||
@@ -954,10 +947,10 @@ class TestProtocolReconfigure:
|
||||
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"
|
||||
assert entry.options[CONF_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.options[CONF_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]
|
||||
@@ -990,7 +983,7 @@ class TestProtocolReconfigure:
|
||||
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"}
|
||||
{CONF_HOST: "10.0.0.77", CONF_PAIRING_CODE: "0000"}
|
||||
)
|
||||
|
||||
assert form["step_id"] == "protocol_pair" # asks for the NEW code
|
||||
@@ -1003,7 +996,7 @@ class TestProtocolReconfigure:
|
||||
)
|
||||
|
||||
assert result["reason"] == "reconfigure_successful"
|
||||
assert entry.options[CONF_PROTOCOL_HOST] == "10.0.0.77"
|
||||
assert entry.options[CONF_HOST] == "10.0.0.77"
|
||||
|
||||
async def test_reconfigure_skip_leaves_the_entry_untouched(
|
||||
self, tmp_path, monkeypatch
|
||||
|
||||
Reference in new issue
Block a user