From b5eb4440155ce09e126361eb0046f200225df4d5 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 3 May 2026 20:01:51 -0500 Subject: [PATCH] [dashboard] Stabilize device-builder dashboard backend's API surface (#16206) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../components/dashboard_import/__init__.py | 11 + esphome/components/esp32/__init__.py | 12 + esphome/components/esp8266/__init__.py | 12 + esphome/components/libretiny/__init__.py | 12 + esphome/components/libretiny/const.py | 8 + esphome/components/rp2040/__init__.py | 12 + esphome/dashboard/util/text.py | 18 +- esphome/helpers.py | 24 +- esphome/storage_json.py | 28 +++ esphome/zeroconf.py | 41 +++ tests/unit_tests/test_dashboard_import.py | 203 +++++++++++++++ tests/unit_tests/test_helpers.py | 45 ++++ tests/unit_tests/test_zeroconf.py | 237 ++++++++++++++++++ 13 files changed, 656 insertions(+), 7 deletions(-) create mode 100644 tests/unit_tests/test_dashboard_import.py create mode 100644 tests/unit_tests/test_zeroconf.py diff --git a/esphome/components/dashboard_import/__init__.py b/esphome/components/dashboard_import/__init__.py index dbe5532902..30b3394165 100644 --- a/esphome/components/dashboard_import/__init__.py +++ b/esphome/components/dashboard_import/__init__.py @@ -89,6 +89,17 @@ def import_config( network: str = CONF_WIFI, encryption: bool = False, ) -> None: + """Materialise a dashboard-imported device's YAML on disk. + + Used by: + - esphome.dashboard (legacy dashboard) + - device-builder (esphome/device-builder) — called from the + ``devices/import`` WS handler to seed the YAML for an adopted + factory firmware. Coordinate before changing the kwargs or the + generated YAML's top-level keys; both consumers depend on the + output shape (``esphome.name`` / ``packages:`` import url) to + route subsequent compile + flash operations. + """ p = Path(path) if p.exists(): diff --git a/esphome/components/esp32/__init__.py b/esphome/components/esp32/__init__.py index 9a9ee8fb08..b60dab3634 100644 --- a/esphome/components/esp32/__init__.py +++ b/esphome/components/esp32/__init__.py @@ -489,6 +489,18 @@ def get_board(core_obj=None): def get_download_types(storage_json): + """Binary-download entries for a built ESP32 firmware. + + Used by: + - esphome.dashboard (legacy "Download .bin" button) + - device-builder (esphome/device-builder) — same dispatch via + ``importlib.import_module(f"esphome.components.{platform}")`` + then ``module.get_download_types(storage)``. The contract is + "returns ``list[dict]`` with at least ``title`` / + ``description`` / ``file`` / ``download`` keys"; please keep + the shape stable so the new dashboard's download panel + doesn't have to special-case per-platform schemas. + """ return [ { "title": "Factory format (Previously Modern)", diff --git a/esphome/components/esp8266/__init__.py b/esphome/components/esp8266/__init__.py index 34540bd48d..3c2806a307 100644 --- a/esphome/components/esp8266/__init__.py +++ b/esphome/components/esp8266/__init__.py @@ -94,6 +94,18 @@ def set_core_data(config): def get_download_types(storage_json): + """Binary-download entries for a built ESP8266 firmware. + + Used by: + - esphome.dashboard (legacy "Download .bin" button) + - device-builder (esphome/device-builder) — same dispatch via + ``importlib.import_module(f"esphome.components.{platform}")`` + then ``module.get_download_types(storage)``. The contract is + "returns ``list[dict]`` with at least ``title`` / + ``description`` / ``file`` / ``download`` keys"; please keep + the shape stable so the new dashboard's download panel + doesn't have to special-case per-platform schemas. + """ return [ { "title": "Standard format", diff --git a/esphome/components/libretiny/__init__.py b/esphome/components/libretiny/__init__.py index 74ac51d200..40fb773784 100644 --- a/esphome/components/libretiny/__init__.py +++ b/esphome/components/libretiny/__init__.py @@ -156,6 +156,18 @@ def only_on_family(*, supported=None, unsupported=None): def get_download_types(storage_json: StorageJSON = None): + """Binary-download entries for a built LibreTiny firmware. + + Used by: + - esphome.dashboard (legacy "Download .bin" button) + - device-builder (esphome/device-builder) — same dispatch via + ``importlib.import_module(f"esphome.components.{platform}")`` + then ``module.get_download_types(storage)``. The contract is + "returns ``list[dict]`` with at least ``title`` / + ``description`` / ``file`` / ``download`` keys"; please keep + the shape stable so the new dashboard's download panel + doesn't have to special-case per-platform schemas. + """ types = [ { "title": "UF2 package (recommended)", diff --git a/esphome/components/libretiny/const.py b/esphome/components/libretiny/const.py index 5de4a164b5..0119a0db3f 100644 --- a/esphome/components/libretiny/const.py +++ b/esphome/components/libretiny/const.py @@ -54,6 +54,14 @@ COMPONENT_LN882X = "ln882x" COMPONENT_RTL87XX = "rtl87xx" # COMPONENTS - end +# Note for ``generate_components.py`` maintainers: the +# ``FAMILY_COMPONENT`` map below is also consumed externally — +# device-builder (esphome/device-builder) derives the set of +# ``target_platform`` values that should route to the ``libretiny`` +# component for the dashboard's ``get_download_types`` lookup from +# ``FAMILY_COMPONENT.values()``. New chip families added by the +# generator are picked up automatically; please don't repurpose +# the public ``FAMILY_COMPONENT`` name without coordinating. # FAMILIES - auto-generated! Do not modify this block. FAMILY_BK7231N = "BK7231N" FAMILY_BK7231Q = "BK7231Q" diff --git a/esphome/components/rp2040/__init__.py b/esphome/components/rp2040/__init__.py index ed246416c9..79ed00cb41 100644 --- a/esphome/components/rp2040/__init__.py +++ b/esphome/components/rp2040/__init__.py @@ -69,6 +69,18 @@ def set_core_data(config): def get_download_types(storage_json): + """Binary-download entries for a built RP2040 firmware. + + Used by: + - esphome.dashboard (legacy "Download .bin" button) + - device-builder (esphome/device-builder) — same dispatch via + ``importlib.import_module(f"esphome.components.{platform}")`` + then ``module.get_download_types(storage)``. The contract is + "returns ``list[dict]`` with at least ``title`` / + ``description`` / ``file`` / ``download`` keys"; please keep + the shape stable so the new dashboard's download panel + doesn't have to special-case per-platform schemas. + """ return [ { "title": "UF2 factory format", diff --git a/esphome/dashboard/util/text.py b/esphome/dashboard/util/text.py index 2a3b9042e6..bdf9abfdb9 100644 --- a/esphome/dashboard/util/text.py +++ b/esphome/dashboard/util/text.py @@ -1,9 +1,15 @@ +"""Back-compat shim for ``friendly_name_slugify``. + +The function moved to :mod:`esphome.helpers` so it survives the legacy +dashboard's eventual removal — see the +``esphome.helpers.friendly_name_slugify`` docstring. This module +re-exports the name so existing +``from esphome.dashboard.util.text import friendly_name_slugify`` +imports keep working while downstream consumers migrate. +""" + from __future__ import annotations -from esphome.helpers import slugify +from esphome.helpers import friendly_name_slugify - -def friendly_name_slugify(value: str) -> str: - """Convert a friendly name to a slug with dashes instead of underscores.""" - # First use the standard slugify, then convert underscores to dashes - return slugify(value).replace("_", "-") +__all__ = ["friendly_name_slugify"] diff --git a/esphome/helpers.py b/esphome/helpers.py index f41bec357d..bb1984e17c 100644 --- a/esphome/helpers.py +++ b/esphome/helpers.py @@ -120,6 +120,24 @@ def slugify(value: str) -> str: return "".join(c for c in value if c in ALLOWED_NAME_CHARS) +def friendly_name_slugify(value: str) -> str: + """Convert a friendly name to a slug with dashes instead of underscores. + + Used by: + - esphome.dashboard.web_server (legacy dashboard) + - device-builder (esphome/device-builder) — slugifies friendly names + into the YAML filename / device name during adoption + wizard flows. + + Lives here rather than in ``esphome.dashboard.util.text`` so it + survives the legacy dashboard's eventual removal. + The dashboard module re-exports this name as a back-compat shim. + Coordinate with the device-builder team before changing the + slugification rules — the mapping must stay stable so existing + on-disk filenames keep matching across releases. + """ + return slugify(value).replace("_", "-") + + def indent_all_but_first_and_last(text, padding=" "): lines = text.splitlines(True) if len(lines) <= 2: @@ -370,7 +388,11 @@ def rmtree(path: Path | str) -> None: os.chmod(path, stat.S_IWUSR | stat.S_IRUSR) func(path) - shutil.rmtree(path, onerror=_onerror) + # ``onerror`` is deprecated in 3.12 in favour of ``onexc`` (different + # callable signature); keep the existing handler shape for now and + # silence the lint locally so this PR doesn't bundle an unrelated + # migration. + shutil.rmtree(path, onerror=_onerror) # pylint: disable=deprecated-argument def walk_files(path: Path): diff --git a/esphome/storage_json.py b/esphome/storage_json.py index d5423ab1c7..c6df16ce78 100644 --- a/esphome/storage_json.py +++ b/esphome/storage_json.py @@ -21,6 +21,14 @@ def storage_path() -> Path: def ext_storage_path(config_filename: str) -> Path: + """Path to the per-config StorageJSON sidecar. + + Used by: + - device-builder (esphome/device-builder) — locates the sidecar + to read board / framework / firmware-bin / loaded_integrations + info for the dashboard. Coordinate before changing the path + shape; device-builder reads the same file on disk. + """ return CORE.data_dir / "storage" / f"{config_filename}.json" @@ -29,6 +37,14 @@ def esphome_storage_path() -> Path: def ignored_devices_storage_path() -> Path: + """Path to the dashboard's ignored-devices list. + + Used by: + - device-builder (esphome/device-builder) — reads the same + ``ignored-devices.json`` so the new dashboard's "ignore" toggle + stays compatible with the legacy one. Don't change the file + shape without coordinating. + """ return CORE.data_dir / "ignored-devices.json" @@ -46,6 +62,18 @@ def _to_path_if_not_none(value: str | None) -> Path | None: class StorageJSON: + """Persisted device metadata sidecar. + + Used by: + - esphome.dashboard (legacy dashboard) + - device-builder (esphome/device-builder) — reads/writes the same + JSON file as the legacy dashboard so a single config_dir can be + shared between the two during the transition. The schema + (``storage_version``, field names, types) must stay backwards + compatible — coordinate with the device-builder team before + adding required fields or changing semantics of existing ones. + """ + def __init__( self, storage_version: int, diff --git a/esphome/zeroconf.py b/esphome/zeroconf.py index 6f5d33c808..5d922ea911 100644 --- a/esphome/zeroconf.py +++ b/esphome/zeroconf.py @@ -60,6 +60,18 @@ TXT_RECORD_VERSION = b"version" @dataclass class DiscoveredImport: + """An importable device discovered via mDNS ``_esphomelib._tcp.local.``. + + Used by: + - esphome.dashboard (legacy dashboard) + - device-builder (esphome/device-builder) — surfaces these as + "discovered devices" on the new dashboard's adoption flow. + + Fields are populated from TXT records on the broadcast service + info (see :class:`DashboardImportDiscovery`). Coordinate before + adding/removing fields — both consumers persist them. + """ + friendly_name: str | None device_name: str package_import_url: str @@ -73,6 +85,22 @@ class DashboardBrowser(AsyncServiceBrowser): class DashboardImportDiscovery: + """Track importable devices announcing on ``_esphomelib._tcp.local.``. + + Used by: + - esphome.dashboard (legacy dashboard) + - device-builder (esphome/device-builder) — wired up alongside + the dashboard's own ``ServiceBrowser`` to populate the + "Discovered devices" panel and the adoption flow. + + The class maintains ``import_state: dict[str, DiscoveredImport]`` + keyed by the mDNS service name. ``on_update`` is invoked with + ``(name, info | None)`` for additions and removals; update events + refresh ``import_state`` without firing the callback. + Coordinate before changing the callback signature or the keys + of ``import_state`` — device-builder reads both directly. + """ + def __init__( self, on_update: Callable[[str, DiscoveredImport | None], None] | None = None ) -> None: @@ -232,6 +260,19 @@ async def async_resolve_hosts( class AsyncEsphomeZeroconf(AsyncZeroconf): + """ESPHome-tuned ``AsyncZeroconf`` with a hostname-resolve helper. + + Used by: + - esphome.dashboard (legacy dashboard) + - device-builder (esphome/device-builder) — drives both the live + mDNS browser and the per-sweep ``async_resolve_host`` fallback + for non-API devices that don't broadcast esphomelib. + + Coordinate before adding required constructor args or changing + the ``async_resolve_host`` signature — device-builder calls it + on every ping cycle. + """ + async def async_resolve_host( self, host: str, timeout: float = DEFAULT_TIMEOUT ) -> list[str] | None: diff --git a/tests/unit_tests/test_dashboard_import.py b/tests/unit_tests/test_dashboard_import.py new file mode 100644 index 0000000000..427bee0f86 --- /dev/null +++ b/tests/unit_tests/test_dashboard_import.py @@ -0,0 +1,203 @@ +"""Unit tests for ``esphome.components.dashboard_import.import_config``. + +Locks the YAML shape that ``import_config`` materialises on disk for +adopted factory firmware. Both the legacy dashboard and the new +device-builder backend (esphome/device-builder) call this function +during the adoption flow and depend on the output's ``esphome.name`` +/ ``packages:`` keys to route subsequent compile + flash operations. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest +import yaml as pyyaml + +from esphome.components.dashboard_import import import_config + + +def _load_plain_yaml(path: Path) -> dict: + """Load YAML without invoking ESPHome's ``CORE``-aware loader. + + ``esphome.yaml_util.load_yaml`` resolves ``!include`` / + ``!secret`` against ``CORE.config_path`` which isn't set in + these tests. We're only asserting on plain key/value structure, + so ``pyyaml.load`` with a custom loader subclassing + ``pyyaml.SafeLoader`` (and empty fallbacks for the secret/include + tags) is enough. + """ + + class _Loader(pyyaml.SafeLoader): + pass + + _Loader.add_constructor("!secret", lambda loader, node: f"!secret {node.value}") + _Loader.add_constructor("!include", lambda loader, node: f"!include {node.value}") + + return pyyaml.load(path.read_text(encoding="utf-8"), Loader=_Loader) + + +def test_basic_import_writes_expected_yaml_shape(tmp_path: Path) -> None: + """A minimal Wi-Fi import emits the substitutions / packages / esphome triad. + + These three top-level blocks are the contract: substitutions + holds the device-specific name, packages pulls in the upstream + firmware via the import URL, and esphome.name interpolates from + substitutions. Anything that depends on this output (frontend + config viewer, follow-up edits, version checks) reads those + keys directly. + """ + yaml_path = tmp_path / "kitchen.yaml" + + import_config( + path=str(yaml_path), + name="kitchen", + friendly_name="Kitchen", + project_name="acme.kitchen-light", + import_url="github://acme/firmware/kitchen.yaml@main", + ) + + assert yaml_path.exists() + config = _load_plain_yaml(yaml_path) + + assert config["substitutions"] == { + "name": "kitchen", + "friendly_name": "Kitchen", + } + assert config["packages"] == { + "acme.kitchen-light": "github://acme/firmware/kitchen.yaml@main" + } + assert config["esphome"] == { + "name": "${name}", + "name_add_mac_suffix": False, + "friendly_name": "${friendly_name}", + } + + +def test_import_appends_wifi_config_when_network_is_wifi(tmp_path: Path) -> None: + """Wi-Fi devices get a ``wifi:`` block templated with secrets references. + + Adopted Wi-Fi devices need a ``wifi:`` section so they can + actually connect on the user's LAN — the boilerplate references + ``!secret wifi_ssid`` / ``!secret wifi_password`` so the + user's existing secrets file plugs in. Devices on other + networks (Ethernet) shouldn't get the Wi-Fi block. + """ + yaml_path = tmp_path / "kitchen.yaml" + import_config( + path=str(yaml_path), + name="kitchen", + friendly_name=None, + project_name="acme.kitchen-light", + import_url="github://acme/firmware/kitchen.yaml@main", + ) + contents = yaml_path.read_text() + assert "wifi:" in contents + assert "!secret wifi_ssid" in contents + assert "!secret wifi_password" in contents + + +def test_import_omits_wifi_block_for_ethernet_network(tmp_path: Path) -> None: + """Ethernet devices get no ``wifi:`` block — caller wires Ethernet separately. + + The ``network`` parameter exists specifically so non-Wi-Fi + devices (PoE / Ethernet, etc.) skip the Wi-Fi templating — + otherwise their generated YAML would carry an unused ``wifi:`` + section the user has to clean up by hand. + """ + yaml_path = tmp_path / "olimex-poe.yaml" + import_config( + path=str(yaml_path), + name="olimex-poe", + friendly_name=None, + project_name="acme.poe-monitor", + import_url="github://acme/firmware/poe.yaml@main", + network="ethernet", + ) + contents = yaml_path.read_text() + assert "wifi:" not in contents + + +def test_import_with_encryption_writes_api_key(tmp_path: Path) -> None: + """``encryption=True`` generates a fresh Noise PSK in the api block. + + Used during the adoption flow when the device-builder UI + explicitly opts the new device into encrypted API. Each + invocation must produce a fresh 32-byte PSK base64-encoded into + the YAML; subsequent compiles and the dashboard's encryption + indicator both read it from there. + """ + yaml_path_1 = tmp_path / "a.yaml" + yaml_path_2 = tmp_path / "b.yaml" + + import_config( + path=str(yaml_path_1), + name="a", + friendly_name=None, + project_name="acme.dev", + import_url="github://acme/firmware/dev.yaml@main", + encryption=True, + ) + import_config( + path=str(yaml_path_2), + name="b", + friendly_name=None, + project_name="acme.dev", + import_url="github://acme/firmware/dev.yaml@main", + encryption=True, + ) + + config_1 = _load_plain_yaml(yaml_path_1) + config_2 = _load_plain_yaml(yaml_path_2) + assert "api" in config_1 and "encryption" in config_1["api"] + key_1 = config_1["api"]["encryption"]["key"] + key_2 = config_2["api"]["encryption"]["key"] + # Fresh per-call PSK, not a hardcoded value. + assert key_1 != key_2 + # Base64-encoded 32 bytes → length 44 with one trailing `=`. + assert len(key_1) == 44 + + +def test_import_without_friendly_name_omits_friendly_substitution( + tmp_path: Path, +) -> None: + """``friendly_name=None`` skips the friendly_name substitution. + + Some imported configs don't carry a friendly name. The output + shouldn't pretend they do — the substitutions block must omit + ``friendly_name`` so the dashboard renders blank rather than + the literal substitution token. + """ + yaml_path = tmp_path / "noname.yaml" + import_config( + path=str(yaml_path), + name="noname", + friendly_name=None, + project_name="acme.dev", + import_url="github://acme/firmware/dev.yaml@main", + ) + config = _load_plain_yaml(yaml_path) + assert config["substitutions"] == {"name": "noname"} + assert "friendly_name" not in config["esphome"] + + +def test_import_refuses_to_overwrite_existing_yaml(tmp_path: Path) -> None: + """An already-present file raises rather than clobbering the user's edits. + + Both the legacy dashboard and device-builder rely on the + ``FileExistsError`` to surface a "config already exists" message + instead of silently destroying user data. + """ + yaml_path = tmp_path / "existing.yaml" + yaml_path.write_text("# user's hand-edited config\n", encoding="utf-8") + + with pytest.raises(FileExistsError): + import_config( + path=str(yaml_path), + name="existing", + friendly_name=None, + project_name="acme.dev", + import_url="github://acme/firmware/dev.yaml@main", + ) + # Original content survives unchanged. + assert yaml_path.read_text() == "# user's hand-edited config\n" diff --git a/tests/unit_tests/test_helpers.py b/tests/unit_tests/test_helpers.py index 159d3230ab..f2faf3ba8f 100644 --- a/tests/unit_tests/test_helpers.py +++ b/tests/unit_tests/test_helpers.py @@ -90,6 +90,51 @@ def test_cpp_string_escape(string, expected): assert actual == expected +@pytest.mark.parametrize( + "value, expected", + ( + # Basic underscore→dash conversion. + ("Living Room Sensor", "living-room-sensor"), + # Already-slugified input passes through with dash output. + ("kitchen_light", "kitchen-light"), + # Accents are stripped (matches the underlying ``slugify``). + ("Café Caché", "cafe-cache"), + # Mixed casing + multiple separators collapse correctly. + ("Foo Bar__Baz", "foo-bar-baz"), + # Empty input yields empty output. + ("", ""), + # Numbers survive intact. + ("Sensor 42", "sensor-42"), + ), +) +def test_friendly_name_slugify(value, expected): + """Friendly-name → URL-safe dash-slug. + + Stable mapping is part of the cross-tool contract + (legacy dashboard + device-builder both depend on it for + filename → device-name routing). Lock the cases here so a + refactor can't accidentally change a slug shape and break + on-disk filenames in already-deployed installs. + """ + assert helpers.friendly_name_slugify(value) == expected + + +def test_friendly_name_slugify_back_compat_shim(): + """``esphome.dashboard.util.text`` keeps re-exporting for back-compat. + + The function moved to ``esphome.helpers`` so the new + device-builder dashboard backend can import it without depending + on the legacy dashboard package, but downstream code that still + imports from the old path keeps working until the dashboard + module is removed. + """ + from esphome.dashboard.util.text import ( + friendly_name_slugify as legacy_friendly_name_slugify, + ) + + assert legacy_friendly_name_slugify is helpers.friendly_name_slugify + + @pytest.mark.parametrize( "host", ( diff --git a/tests/unit_tests/test_zeroconf.py b/tests/unit_tests/test_zeroconf.py new file mode 100644 index 0000000000..e325eb1e26 --- /dev/null +++ b/tests/unit_tests/test_zeroconf.py @@ -0,0 +1,237 @@ +"""Unit tests for ``esphome.zeroconf`` device-discovery primitives. + +Covers ``DashboardImportDiscovery`` (state transitions for adoption / +import flows) and ``DiscoveredImport`` (TXT-record parse shape). Both +are part of the cross-tool contract between the legacy dashboard and +the new device-builder backend (esphome/device-builder); changes to +the callback signature, the ``import_state`` dict shape, or the +``DiscoveredImport`` field set will break downstream consumers. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +from zeroconf import ServiceStateChange + +from esphome.zeroconf import ( + ESPHOME_SERVICE_TYPE, + DashboardImportDiscovery, + DiscoveredImport, +) + + +def _make_service_info( + package_import_url: str = "github://esphome/example/example.yaml", + project_name: str = "esphome.example", + project_version: str = "1.0.0", + network: str | None = "wifi", + friendly_name: str | None = "Living Room", + version: str | None = "2025.1.0", +) -> MagicMock: + """Build a fake ``AsyncServiceInfo`` with the TXT records we care about. + + The real callback path resolves a service via zeroconf and then + reads ``info.properties`` (a ``dict[bytes, bytes | None]``). Mock + that shape so we can drive ``_process_service_info`` directly + without spinning up a real zeroconf instance. + """ + info = MagicMock() + properties: dict[bytes, bytes | None] = { + b"package_import_url": package_import_url.encode(), + b"project_name": project_name.encode(), + b"project_version": project_version.encode(), + } + if network is not None: + properties[b"network"] = network.encode() + if friendly_name is not None: + properties[b"friendly_name"] = friendly_name.encode() + if version is not None: + properties[b"version"] = version.encode() + info.properties = properties + info.load_from_cache.return_value = True + return info + + +def test_added_service_populates_import_state_and_fires_callback() -> None: + """An ADD with the required TXT records lands a ``DiscoveredImport`` and notifies. + + Mirrors what both the legacy dashboard and device-builder rely + on — the callback is the only signal that an importable device + has appeared on the LAN, and ``import_state`` is the snapshot + they read on demand. + """ + on_update = MagicMock() + discovery = DashboardImportDiscovery(on_update=on_update) + + info = _make_service_info() + name = f"living-room.{ESPHOME_SERVICE_TYPE}" + discovery._process_service_info(name, info) + + assert name in discovery.import_state + entry = discovery.import_state[name] + assert isinstance(entry, DiscoveredImport) + assert entry.device_name == "living-room" + assert entry.package_import_url == "github://esphome/example/example.yaml" + assert entry.project_name == "esphome.example" + assert entry.project_version == "1.0.0" + assert entry.network == "wifi" + assert entry.friendly_name == "Living Room" + on_update.assert_called_once_with(name, entry) + + +def test_added_service_without_required_txt_is_ignored() -> None: + """A device that doesn't carry ``package_import_url`` etc. isn't importable. + + The dashboard browser also fires for plain ``_esphomelib._tcp`` + services that happen to match the type but aren't dashboard + imports. Those must not land in ``import_state`` or fire the + update callback — otherwise the dashboard would surface every + API-enabled device on the LAN as "ready to adopt". + """ + on_update = MagicMock() + discovery = DashboardImportDiscovery(on_update=on_update) + + info = MagicMock() + # Empty TXT records — no import URL, no version. ``version``-only + # services hit a separate ``update_device_mdns`` path that talks + # to ``StorageJSON``; that's covered elsewhere. + info.properties = {} + info.load_from_cache.return_value = True + + discovery._process_service_info(f"plain.{ESPHOME_SERVICE_TYPE}", info) + + assert discovery.import_state == {} + on_update.assert_not_called() + + +def test_repeated_add_does_not_re_fire_callback() -> None: + """Re-resolving the same service doesn't spam the on_update callback. + + The dashboard re-resolves periodically; without the ``is_new`` + guard, every refresh would fire ``IMPORTABLE_DEVICE_ADDED`` and + the dashboard's UI would re-render endlessly. + """ + on_update = MagicMock() + discovery = DashboardImportDiscovery(on_update=on_update) + + info = _make_service_info() + name = f"living-room.{ESPHOME_SERVICE_TYPE}" + discovery._process_service_info(name, info) + discovery._process_service_info(name, info) + + on_update.assert_called_once() + + +def test_removed_service_clears_state_and_fires_none_callback() -> None: + """A ServiceStateChange.Removed pops the entry and notifies with ``None``. + + Both consumers rely on the ``(name, None)`` callback shape to + distinguish "device gone" from "device updated". Coordinate + before changing the second-arg semantics. + """ + on_update = MagicMock() + discovery = DashboardImportDiscovery(on_update=on_update) + + info = _make_service_info() + name = f"living-room.{ESPHOME_SERVICE_TYPE}" + discovery._process_service_info(name, info) + on_update.reset_mock() + + discovery.browser_callback( + zeroconf=MagicMock(), + service_type=ESPHOME_SERVICE_TYPE, + name=name, + state_change=ServiceStateChange.Removed, + ) + + assert name not in discovery.import_state + on_update.assert_called_once_with(name, None) + + +def test_remove_for_unknown_service_does_not_fire_callback() -> None: + """A spurious Removed for a service we never tracked is a silent no-op. + + The browser can fire Removed for any matching service type, + not just the importable ones we're tracking. Don't let those + confuse the callback consumer. + """ + on_update = MagicMock() + discovery = DashboardImportDiscovery(on_update=on_update) + + discovery.browser_callback( + zeroconf=MagicMock(), + service_type=ESPHOME_SERVICE_TYPE, + name=f"never-seen.{ESPHOME_SERVICE_TYPE}", + state_change=ServiceStateChange.Removed, + ) + + on_update.assert_not_called() + + +def test_updated_service_for_unknown_name_is_ignored() -> None: + """Updates without a prior Add don't seed ``import_state``. + + The dashboard counts on Add to introduce the device and Update + to refresh it. Letting Update silently introduce new state would + let an unrelated TXT change bypass the Add-time validation. + """ + on_update = MagicMock() + discovery = DashboardImportDiscovery(on_update=on_update) + + discovery.browser_callback( + zeroconf=MagicMock(), + service_type=ESPHOME_SERVICE_TYPE, + name=f"living-room.{ESPHOME_SERVICE_TYPE}", + state_change=ServiceStateChange.Updated, + ) + + assert discovery.import_state == {} + on_update.assert_not_called() + + +def test_network_defaults_to_wifi_when_txt_absent() -> None: + """Older firmware that doesn't broadcast ``network`` defaults to ``wifi``. + + The TXT record was added in a later release; pre-existing + factory firmwares advertise without it. ``DiscoveredImport`` + has to default cleanly so adoption flows can still produce a + valid YAML for those devices. + """ + discovery = DashboardImportDiscovery() + info = _make_service_info(network=None) + name = f"older.{ESPHOME_SERVICE_TYPE}" + discovery._process_service_info(name, info) + + assert discovery.import_state[name].network == "wifi" + + +def test_friendly_name_optional() -> None: + """``friendly_name`` may be ``None`` if the device doesn't broadcast it. + + Both consumers handle the ``None`` case (rendering the device + name as fallback in the UI). Locking this in keeps the + optionality explicit so a future refactor doesn't accidentally + coerce it into an empty string. + """ + discovery = DashboardImportDiscovery() + info = _make_service_info(friendly_name=None) + name = f"no-friendly.{ESPHOME_SERVICE_TYPE}" + discovery._process_service_info(name, info) + + assert discovery.import_state[name].friendly_name is None + + +def test_callback_is_optional() -> None: + """``on_update=None`` lets ``import_state`` track silently. + + Used by callers that read the dict directly rather than + subscribing to events. + """ + discovery = DashboardImportDiscovery(on_update=None) + info = _make_service_info() + name = f"silent.{ESPHOME_SERVICE_TYPE}" + discovery._process_service_info(name, info) + + # No callback to assert against; just verify state landed. + assert name in discovery.import_state