[core] Allow finding all devices as target that match mac suffix (#13135)

tests/unit_tests/test_address_cache.py

@@ -121,6 +121,26 @@ def test_get_addresses_auto_detection() -> None:
     assert cache.get_addresses("unknown.com") is None
 
 
+def test_add_mdns_addresses_stores_and_normalizes() -> None:
+    """add_mdns_addresses inserts entries under the normalized hostname."""
+    cache = AddressCache()
+    cache.add_mdns_addresses("Device.Local.", ["192.168.1.10", "192.168.1.11"])
+
+    assert cache.mdns_cache == {
+        normalize_hostname("Device.Local."): ["192.168.1.10", "192.168.1.11"]
+    }
+    # Overwrites on subsequent calls for the same host
+    cache.add_mdns_addresses("device.local", ["10.0.0.1"])
+    assert cache.mdns_cache[normalize_hostname("device.local")] == ["10.0.0.1"]
+
+
+def test_add_mdns_addresses_empty_is_noop() -> None:
+    """Passing an empty address list must not create an entry."""
+    cache = AddressCache()
+    cache.add_mdns_addresses("device.local", [])
+    assert cache.mdns_cache == {}
+
+
 def test_has_cache() -> None:
     """Test checking if cache has entries."""
     # Empty cache

tests/unit_tests/test_main.py

@@ -2,7 +2,7 @@
 
 from __future__ import annotations
 
-from collections.abc import Generator
+from collections.abc import Callable, Generator
 from dataclasses import dataclass
 import json
 import logging
@@ -12,16 +12,18 @@ import re
 import sys
 import time
 from typing import Any
-from unittest.mock import MagicMock, Mock, patch
+from unittest.mock import AsyncMock, MagicMock, Mock, patch
 
 import pytest
 from pytest import CaptureFixture
+from zeroconf import ServiceStateChange
 
 from esphome import platformio_api
 from esphome.__main__ import (
     Purpose,
     _get_configured_xtal_freq,
     _make_crystal_freq_callback,
+    _resolve_network_devices,
     choose_upload_log_host,
     command_analyze_memory,
     command_bundle,
@@ -36,6 +38,7 @@ from esphome.__main__ import (
     has_mqtt,
     has_mqtt_ip_lookup,
     has_mqtt_logging,
+    has_name_add_mac_suffix,
     has_non_ip_address,
     has_ota,
     has_resolvable_address,
@@ -48,6 +51,7 @@ from esphome.__main__ import (
     upload_using_picotool,
     upload_using_platformio,
 )
+from esphome.address_cache import AddressCache
 from esphome.bundle import BUNDLE_EXTENSION, BundleFile, BundleResult
 from esphome.components.esp32 import KEY_ESP32, KEY_VARIANT, VARIANT_ESP32
 from esphome.const import (
@@ -62,6 +66,7 @@ from esphome.const import (
     CONF_MDNS,
     CONF_MQTT,
     CONF_NAME,
+    CONF_NAME_ADD_MAC_SUFFIX,
     CONF_OTA,
     CONF_PASSWORD,
     CONF_PLATFORM,
@@ -79,6 +84,7 @@ from esphome.const import (
 )
 from esphome.core import CORE, EsphomeError
 from esphome.util import BootselResult
+from esphome.zeroconf import _await_discovery, discover_mdns_devices
 
 
 def strip_ansi_codes(text: str) -> str:
@@ -2218,6 +2224,509 @@ def test_has_resolvable_address() -> None:
     assert has_resolvable_address() is False
 
 
+def test_has_name_add_mac_suffix() -> None:
+    """Test has_name_add_mac_suffix function."""
+
+    # Test with name_add_mac_suffix enabled
+    setup_core(config={CONF_ESPHOME: {CONF_NAME_ADD_MAC_SUFFIX: True}})
+    assert has_name_add_mac_suffix() is True
+
+    # Test with name_add_mac_suffix disabled
+    setup_core(config={CONF_ESPHOME: {CONF_NAME_ADD_MAC_SUFFIX: False}})
+    assert has_name_add_mac_suffix() is False
+
+    # Test with name_add_mac_suffix not set (defaults to False)
+    setup_core(config={CONF_ESPHOME: {}})
+    assert has_name_add_mac_suffix() is False
+
+    # Test with no esphome config
+    setup_core(config={})
+    assert has_name_add_mac_suffix() is False
+
+    # Test with no config at all
+    CORE.config = None
+    assert has_name_add_mac_suffix() is False
+
+
+@pytest.fixture
+def mock_mdns_discovery() -> Generator[MagicMock]:
+    """Fixture to mock the async mDNS discovery infrastructure.
+
+    Patches ``AsyncEsphomeZeroconf``, ``AsyncServiceBrowser`` and
+    ``AddressResolver`` in ``esphome.zeroconf`` and exposes hooks for tests to
+    stage browser events and control resolution results. The default
+    ``AddressResolver`` stub simulates a cache hit returning no addresses, so
+    matched hosts appear in the discovery output with empty address lists
+    unless the test overrides ``_resolver_setup``.
+    """
+    with (
+        patch("esphome.zeroconf.AsyncEsphomeZeroconf") as mock_aiozc_class,
+        patch("esphome.zeroconf.AsyncServiceBrowser") as mock_browser_class,
+        patch("esphome.zeroconf.AddressResolver") as mock_resolver_class,
+    ):
+        mock_aiozc = MagicMock()
+        mock_aiozc.zeroconf = MagicMock()
+        mock_aiozc.async_close = AsyncMock(return_value=None)
+        mock_aiozc_class.return_value = mock_aiozc
+
+        mock_browser = MagicMock()
+        mock_browser.async_cancel = AsyncMock(return_value=None)
+
+        # Default: each host gets a fresh resolver that hits the cache and
+        # returns no addresses. Tests can override via ``_resolver_setup``.
+        def default_resolver_factory(name: str) -> MagicMock:
+            resolver = MagicMock()
+            resolver._name = name
+            resolver.load_from_cache.return_value = True
+            resolver.async_request = AsyncMock(return_value=True)
+            resolver.parsed_scoped_addresses.return_value = []
+            return resolver
+
+        mock_resolver_class.side_effect = default_resolver_factory
+
+        # Store references for test access
+        mock_aiozc._mock_browser_class = mock_browser_class
+        mock_aiozc._mock_browser = mock_browser
+        mock_aiozc._mock_class = mock_aiozc_class
+        mock_aiozc._mock_resolver_class = mock_resolver_class
+        yield mock_aiozc
+
+
+@pytest.mark.parametrize(
+    ("discovered_services", "base_name", "expected_hosts"),
+    [
+        # Matching devices; different-prefix device is filtered out
+        (
+            [
+                ("mydevice-abc123._esphomelib._tcp.local.", ServiceStateChange.Added),
+                ("mydevice-def456._esphomelib._tcp.local.", ServiceStateChange.Added),
+                (
+                    "otherdevice-abcdef._esphomelib._tcp.local.",
+                    ServiceStateChange.Added,
+                ),
+            ],
+            "mydevice",
+            ["mydevice-abc123.local", "mydevice-def456.local"],
+        ),
+        # No matches at all
+        (
+            [
+                (
+                    "otherdevice-abcdef._esphomelib._tcp.local.",
+                    ServiceStateChange.Added,
+                ),
+            ],
+            "mydevice",
+            [],
+        ),
+        # Deduplication (same device Added then Updated)
+        (
+            [
+                ("mydevice-abc123._esphomelib._tcp.local.", ServiceStateChange.Added),
+                ("mydevice-abc123._esphomelib._tcp.local.", ServiceStateChange.Updated),
+            ],
+            "mydevice",
+            ["mydevice-abc123.local"],
+        ),
+        # Suffix must be exactly 6 hex chars: wrong length and non-hex are rejected
+        (
+            [
+                # too short
+                ("mydevice-abcd._esphomelib._tcp.local.", ServiceStateChange.Added),
+                # too long
+                (
+                    "mydevice-abcdef1._esphomelib._tcp.local.",
+                    ServiceStateChange.Added,
+                ),
+                # non-hex
+                ("mydevice-xyz123._esphomelib._tcp.local.", ServiceStateChange.Added),
+                # valid
+                ("mydevice-012345._esphomelib._tcp.local.", ServiceStateChange.Added),
+            ],
+            "mydevice",
+            ["mydevice-012345.local"],
+        ),
+        # Prefix-collision: base "foo" must not match "foo-bar-abc123"
+        (
+            [
+                ("foo-abcdef._esphomelib._tcp.local.", ServiceStateChange.Added),
+                ("foo-bar-abcdef._esphomelib._tcp.local.", ServiceStateChange.Added),
+            ],
+            "foo",
+            ["foo-abcdef.local"],
+        ),
+    ],
+    ids=[
+        "matching_with_filter",
+        "no_matches",
+        "deduplication",
+        "hex_suffix_filter",
+        "prefix_collision",
+    ],
+)
+def test_discover_mdns_devices(
+    mock_mdns_discovery: MagicMock,
+    discovered_services: list[tuple[str, ServiceStateChange]],
+    base_name: str,
+    expected_hosts: list[str],
+) -> None:
+    """Test discover_mdns_devices filtering and deduplication."""
+    mock_browser = mock_mdns_discovery._mock_browser
+
+    def capture_callback(
+        zc: MagicMock,
+        service_type: str,
+        handlers: list[Callable[..., None]],
+    ) -> MagicMock:
+        callback = handlers[0]
+        for service_name, state_change in discovered_services:
+            callback(
+                mock_mdns_discovery.zeroconf, service_type, service_name, state_change
+            )
+        return mock_browser
+
+    mock_mdns_discovery._mock_browser_class.side_effect = capture_callback
+
+    # Each discovered host gets a resolver that returns a unique IP string
+    # derived from its server name so we can assert per-host.
+    def resolver_factory(name: str) -> MagicMock:
+        resolver = MagicMock()
+        resolver._name = name
+        resolver.load_from_cache.return_value = True
+        resolver.async_request = AsyncMock(return_value=True)
+        resolver.parsed_scoped_addresses.return_value = [f"10.0.0.1#{name}"]
+        return resolver
+
+    mock_mdns_discovery._mock_resolver_class.side_effect = resolver_factory
+
+    result = discover_mdns_devices(base_name, timeout=0)
+
+    assert sorted(result) == expected_hosts
+    # Resolved addresses should be stored for matched hosts. AddressResolver
+    # receives the fully-qualified name (``<device>.local.``).
+    for host in expected_hosts:
+        short = host.partition(".")[0]
+        assert result[host] == [f"10.0.0.1#{short}.local."]
+    mock_browser.async_cancel.assert_awaited_once()
+    mock_mdns_discovery.async_close.assert_awaited_once()
+
+
+def test_discover_mdns_devices_init_failure(caplog: pytest.LogCaptureFixture) -> None:
+    """If AsyncEsphomeZeroconf fails to init, return empty dict and log warning."""
+    with (
+        patch(
+            "esphome.zeroconf.AsyncEsphomeZeroconf",
+            side_effect=OSError("no network"),
+        ),
+        caplog.at_level(logging.WARNING, logger="esphome.zeroconf"),
+    ):
+        result = discover_mdns_devices("mydevice", timeout=0)
+
+    assert result == {}
+    assert "mDNS discovery failed to initialize" in caplog.text
+
+
+def test_discover_mdns_devices_resolution_failure(
+    mock_mdns_discovery: MagicMock,
+) -> None:
+    """If resolution raises, the host is still listed with an empty address list."""
+    mock_browser = mock_mdns_discovery._mock_browser
+
+    def capture_callback(
+        zc: MagicMock,
+        service_type: str,
+        handlers: list[Callable[..., None]],
+    ) -> MagicMock:
+        handlers[0](
+            mock_mdns_discovery.zeroconf,
+            service_type,
+            "mydevice-abc123._esphomelib._tcp.local.",
+            ServiceStateChange.Added,
+        )
+        return mock_browser
+
+    mock_mdns_discovery._mock_browser_class.side_effect = capture_callback
+
+    # Resolver misses the cache, then async_request raises.
+    def failing_resolver_factory(name: str) -> MagicMock:
+        resolver = MagicMock()
+        resolver.load_from_cache.return_value = False
+        resolver.async_request = AsyncMock(side_effect=OSError("boom"))
+        resolver.parsed_scoped_addresses.return_value = []
+        return resolver
+
+    mock_mdns_discovery._mock_resolver_class.side_effect = failing_resolver_factory
+
+    result = discover_mdns_devices("mydevice", timeout=0)
+
+    assert result == {"mydevice-abc123.local": []}
+
+
+def test_discover_mdns_devices_ignores_removed_state(
+    mock_mdns_discovery: MagicMock,
+) -> None:
+    """``Removed`` state changes are ignored and do not appear in the result."""
+    mock_browser = mock_mdns_discovery._mock_browser
+
+    def capture_callback(
+        zc: MagicMock,
+        service_type: str,
+        handlers: list[Callable[..., None]],
+    ) -> MagicMock:
+        handlers[0](
+            mock_mdns_discovery.zeroconf,
+            service_type,
+            "mydevice-abc123._esphomelib._tcp.local.",
+            ServiceStateChange.Removed,
+        )
+        return mock_browser
+
+    mock_mdns_discovery._mock_browser_class.side_effect = capture_callback
+
+    result = discover_mdns_devices("mydevice", timeout=0)
+
+    assert result == {}
+    # No AddressResolver should have been constructed since no host matched.
+    mock_mdns_discovery._mock_resolver_class.assert_not_called()
+
+
+def test_discover_mdns_devices_empty_resolution(
+    mock_mdns_discovery: MagicMock,
+) -> None:
+    """Host is listed with empty addresses when resolver returns no addresses."""
+    mock_browser = mock_mdns_discovery._mock_browser
+
+    def capture_callback(
+        zc: MagicMock,
+        service_type: str,
+        handlers: list[Callable[..., None]],
+    ) -> MagicMock:
+        handlers[0](
+            mock_mdns_discovery.zeroconf,
+            service_type,
+            "mydevice-abc123._esphomelib._tcp.local.",
+            ServiceStateChange.Added,
+        )
+        return mock_browser
+
+    mock_mdns_discovery._mock_browser_class.side_effect = capture_callback
+    # Default fixture resolver is a cache-hit with no addresses — simulates
+    # the "browse found it but no A/AAAA records are available" case.
+
+    result = discover_mdns_devices("mydevice", timeout=0)
+
+    assert result == {"mydevice-abc123.local": []}
+
+
+def test_resolve_network_devices_expands_cached_mdns_hosts(tmp_path: Path) -> None:
+    """Hostnames in ``CORE.address_cache`` are expanded to their cached IPs."""
+    setup_core(tmp_path=tmp_path)
+    CORE.address_cache = AddressCache(
+        mdns_cache={
+            "device-abc123.local": ["10.0.0.1", "10.0.0.2"],
+        }
+    )
+
+    result = _resolve_network_devices(
+        ["device-abc123.local", "192.168.1.50", "device-abc123.local"],
+        CORE.config,
+        MockArgs(),
+    )
+
+    # Cached hostname is replaced with its IPs (deduplicated across repeats)
+    # and the literal IP is preserved after.
+    assert result == ["10.0.0.1", "10.0.0.2", "192.168.1.50"]
+
+
+def test_resolve_network_devices_keeps_uncached_hosts(tmp_path: Path) -> None:
+    """Hostnames not in the cache pass through unchanged."""
+    setup_core(tmp_path=tmp_path)
+    CORE.address_cache = AddressCache()
+
+    result = _resolve_network_devices(
+        ["unknown.local", "192.168.1.50"],
+        CORE.config,
+        MockArgs(),
+    )
+
+    assert result == ["unknown.local", "192.168.1.50"]
+
+
+def test_await_discovery_timeout_returns_empty(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """If the discovery runner never sets its event, return {} and warn."""
+    stub = MagicMock()
+    stub.event.wait.return_value = False
+    stub.exception = None
+    stub.result = {"should_not_be_read": ["1.2.3.4"]}
+
+    with caplog.at_level(logging.WARNING, logger="esphome.zeroconf"):
+        result = _await_discovery(stub, timeout=0.01)
+
+    assert result == {}
+    assert "mDNS discovery timed out after 0.0s" in caplog.text
+    stub.event.wait.assert_called_once_with(timeout=pytest.approx(2.01))
+
+
+def test_await_discovery_propagates_exception_as_empty(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """If the coroutine raised, log and return {} rather than re-raise."""
+    stub = MagicMock()
+    stub.event.wait.return_value = True
+    stub.exception = RuntimeError("boom")
+    stub.result = None
+
+    with caplog.at_level(logging.WARNING, logger="esphome.zeroconf"):
+        result = _await_discovery(stub, timeout=5.0)
+
+    assert result == {}
+    assert "mDNS discovery failed: boom" in caplog.text
+
+
+@pytest.mark.usefixtures("mock_no_serial_ports")
+def test_choose_upload_log_host_discovers_mac_suffix_devices(tmp_path: Path) -> None:
+    """Interactive mode discovers MAC-suffixed devices and populates the cache."""
+    setup_core(
+        config={
+            CONF_ESPHOME: {CONF_NAME_ADD_MAC_SUFFIX: True},
+            CONF_OTA: [{CONF_PLATFORM: CONF_ESPHOME}],
+        },
+        address="mydevice.local",
+        tmp_path=tmp_path,
+        name="mydevice",
+    )
+    CORE.address_cache = None
+
+    discovered = {
+        "mydevice-abc123.local": ["10.0.0.1"],
+        "mydevice-def456.local": ["10.0.0.2"],
+    }
+    with (
+        patch(
+            "esphome.__main__.discover_mdns_devices", return_value=discovered
+        ) as mock_discover,
+        patch(
+            "esphome.__main__.choose_prompt", return_value="mydevice-abc123.local"
+        ) as mock_prompt,
+    ):
+        result = choose_upload_log_host(
+            default=None,
+            check_default=None,
+            purpose=Purpose.UPLOADING,
+        )
+
+    assert result == ["mydevice-abc123.local"]
+    mock_discover.assert_called_once_with("mydevice")
+    mock_prompt.assert_called_once_with(
+        [
+            ("Over The Air (mydevice-abc123.local)", "mydevice-abc123.local"),
+            ("Over The Air (mydevice-def456.local)", "mydevice-def456.local"),
+        ],
+        purpose=Purpose.UPLOADING,
+    )
+    # Resolved IPs should be cached so downstream resolution skips a second
+    # Zeroconf lookup.
+    assert CORE.address_cache is not None
+    assert CORE.address_cache.get_mdns_addresses("mydevice-abc123.local") == [
+        "10.0.0.1"
+    ]
+    assert CORE.address_cache.get_mdns_addresses("mydevice-def456.local") == [
+        "10.0.0.2"
+    ]
+
+
+@pytest.mark.usefixtures("mock_no_serial_ports")
+def test_choose_upload_log_host_mac_suffix_no_devices_found(
+    tmp_path: Path, caplog: pytest.LogCaptureFixture
+) -> None:
+    """When discovery finds nothing, no OTA option is offered and a warning logs."""
+    setup_core(
+        config={
+            CONF_ESPHOME: {CONF_NAME_ADD_MAC_SUFFIX: True},
+            CONF_OTA: [{CONF_PLATFORM: CONF_ESPHOME}],
+        },
+        address="mydevice.local",
+        tmp_path=tmp_path,
+        name="mydevice",
+    )
+
+    with (
+        patch("esphome.__main__.discover_mdns_devices", return_value={}),
+        caplog.at_level(logging.WARNING, logger="esphome.__main__"),
+        pytest.raises(EsphomeError),
+    ):
+        choose_upload_log_host(
+            default=None,
+            check_default=None,
+            purpose=Purpose.UPLOADING,
+        )
+
+    assert "No devices matching 'mydevice-<mac>.local'" in caplog.text
+
+
+def test_choose_upload_log_host_default_ota_discovers_mac_suffix(
+    tmp_path: Path,
+) -> None:
+    """``--device OTA`` also runs mDNS discovery when name_add_mac_suffix is on."""
+    setup_core(
+        config={
+            CONF_ESPHOME: {CONF_NAME_ADD_MAC_SUFFIX: True},
+            CONF_OTA: [{CONF_PLATFORM: CONF_ESPHOME}],
+        },
+        address="mydevice.local",
+        tmp_path=tmp_path,
+        name="mydevice",
+    )
+    CORE.address_cache = None
+
+    discovered = {
+        "mydevice-abc123.local": ["10.0.0.1"],
+        "mydevice-def456.local": ["10.0.0.2"],
+    }
+    with patch(
+        "esphome.__main__.discover_mdns_devices", return_value=discovered
+    ) as mock_discover:
+        result = choose_upload_log_host(
+            default="OTA",
+            check_default=None,
+            purpose=Purpose.UPLOADING,
+        )
+
+    # Both discovered hostnames are returned so aioesphomeapi / espota2 can
+    # try each in turn with the cached IPs.
+    assert result == ["mydevice-abc123.local", "mydevice-def456.local"]
+    mock_discover.assert_called_once_with("mydevice")
+    assert CORE.address_cache is not None
+    assert CORE.address_cache.get_mdns_addresses("mydevice-abc123.local") == [
+        "10.0.0.1"
+    ]
+
+
+def test_choose_upload_log_host_default_ota_no_suffix_discovery(
+    tmp_path: Path,
+) -> None:
+    """``--device OTA`` without name_add_mac_suffix uses CORE.address as-is."""
+    setup_core(
+        config={CONF_OTA: [{CONF_PLATFORM: CONF_ESPHOME}]},
+        address="192.168.1.100",
+        tmp_path=tmp_path,
+        name="mydevice",
+    )
+
+    with patch("esphome.__main__.discover_mdns_devices") as mock_discover:
+        result = choose_upload_log_host(
+            default="OTA",
+            check_default=None,
+            purpose=Purpose.UPLOADING,
+        )
+
+    assert result == ["192.168.1.100"]
+    # Discovery must NOT run when name_add_mac_suffix is disabled.
+    mock_discover.assert_not_called()
+
+
 def test_command_wizard(tmp_path: Path) -> None:
     """Test command_wizard function."""
     config_file = tmp_path / "test.yaml"

tests/unit_tests/test_resolver.py

@@ -4,7 +4,7 @@ from __future__ import annotations
 
 import re
 import socket
-from unittest.mock import patch
+from unittest.mock import MagicMock, patch
 
 from aioesphomeapi.core import ResolveAPIError, ResolveTimeoutAPIError
 from aioesphomeapi.host_resolver import AddrInfo, IPv4Sockaddr, IPv6Sockaddr
@@ -115,24 +115,21 @@ def test_async_resolver_generic_exception() -> None:
 
 
 def test_async_resolver_thread_timeout() -> None:
-    """Test timeout when thread doesn't complete in time."""
-    # Mock the start method to prevent actual thread execution
-    with (
-        patch.object(AsyncResolver, "start"),
-        patch("esphome.resolver.hr.async_resolve_host"),
-    ):
-        resolver = AsyncResolver(["test.local"], 6053)
-        # Override event.wait to simulate timeout (return False = timeout occurred)
-        with (
-            patch.object(resolver.event, "wait", return_value=False),
-            pytest.raises(
-                EsphomeError, match=re.escape("Timeout resolving IP address")
-            ),
-        ):
-            resolver.resolve()
+    """Test timeout when the runner thread doesn't complete in time."""
+    # Patch AsyncThreadRunner inside esphome.resolver so we never actually
+    # start a thread and can control the wait return value directly.
+    fake_runner = MagicMock()
+    fake_runner.start = MagicMock()
+    fake_runner.event.wait.return_value = False  # simulate timeout
 
-        # Verify thread start was called
-        resolver.start.assert_called_once()
+    with (
+        patch("esphome.resolver.AsyncThreadRunner", return_value=fake_runner),
+        patch("esphome.resolver.hr.async_resolve_host"),
+        pytest.raises(EsphomeError, match=re.escape("Timeout resolving IP address")),
+    ):
+        AsyncResolver(["test.local"], 6053).resolve()
+
+    fake_runner.start.assert_called_once()
 
 
 def test_async_resolver_ip_addresses(mock_addr_info_ipv4: AddrInfo) -> None: