Add a repair for ESPHome device conflicts (#142507)

This commit is contained in:
J. Nick Koston 2025-04-14 07:10:05 -10:00 committed by GitHub
parent 1463f05d46
commit 9ce44845fe
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 505 additions and 4 deletions

View File

@ -14,6 +14,7 @@ from homeassistant.const import (
)
from homeassistant.core import HomeAssistant
from homeassistant.helpers import config_validation as cv
from homeassistant.helpers.issue_registry import async_delete_issue
from homeassistant.helpers.typing import ConfigType
from .const import CONF_BLUETOOTH_MAC_ADDRESS, CONF_NOISE_PSK, DATA_FFMPEG_PROXY, DOMAIN
@ -23,7 +24,7 @@ from .domain_data import DomainData
# Import config flow so that it's added to the registry
from .entry_data import ESPHomeConfigEntry, RuntimeEntryData
from .ffmpeg_proxy import FFmpegProxyData, FFmpegProxyView
from .manager import ESPHomeManager, cleanup_instance
from .manager import DEVICE_CONFLICT_ISSUE_FORMAT, ESPHomeManager, cleanup_instance
CONFIG_SCHEMA = cv.config_entry_only_config_schema(DOMAIN)
@ -89,4 +90,7 @@ async def async_remove_entry(hass: HomeAssistant, entry: ESPHomeConfigEntry) ->
"""Remove an esphome config entry."""
if bluetooth_mac_address := entry.data.get(CONF_BLUETOOTH_MAC_ADDRESS):
async_remove_scanner(hass, bluetooth_mac_address.upper())
async_delete_issue(
hass, DOMAIN, DEVICE_CONFLICT_ISSUE_FORMAT.format(entry.entry_id)
)
await DomainData.get(hass).get_or_create_store(hass, entry).async_remove()

View File

@ -48,6 +48,7 @@ from homeassistant.exceptions import TemplateError
from homeassistant.helpers import (
config_validation as cv,
device_registry as dr,
entity_registry as er,
template,
)
from homeassistant.helpers.device_registry import format_mac
@ -80,6 +81,8 @@ from .domain_data import DomainData
# Import config flow so that it's added to the registry
from .entry_data import ESPHomeConfigEntry, RuntimeEntryData
DEVICE_CONFLICT_ISSUE_FORMAT = "device_conflict-{}"
if TYPE_CHECKING:
from aioesphomeapi.api_pb2 import ( # type: ignore[attr-defined]
SubscribeLogsResponse,
@ -418,7 +421,7 @@ class ESPHomeManager:
assert reconnect_logic is not None, "Reconnect logic must be set"
hass = self.hass
cli = self.cli
stored_device_name = entry.data.get(CONF_DEVICE_NAME)
stored_device_name: str | None = entry.data.get(CONF_DEVICE_NAME)
unique_id_is_mac_address = unique_id and ":" in unique_id
if entry.options.get(CONF_SUBSCRIBE_LOGS):
self._async_subscribe_logs(self._async_get_equivalent_log_level())
@ -448,12 +451,36 @@ class ESPHomeManager:
if not mac_address_matches and not unique_id_is_mac_address:
hass.config_entries.async_update_entry(entry, unique_id=device_mac)
issue = DEVICE_CONFLICT_ISSUE_FORMAT.format(entry.entry_id)
if not mac_address_matches and unique_id_is_mac_address:
# If the unique id is a mac address
# and does not match we have the wrong device and we need
# to abort the connection. This can happen if the DHCP
# server changes the IP address of the device and we end up
# connecting to the wrong device.
if stored_device_name == device_info.name:
# If the device name matches it might be a device replacement
# or they made a mistake and flashed the same firmware on
# multiple devices. In this case we start a repair flow
# to ask them if its a mistake, or if they want to migrate
# the config entry to the replacement hardware.
shared_data = {
"name": device_info.name,
"mac": format_mac(device_mac),
"stored_mac": format_mac(unique_id),
"model": device_info.model,
"ip": self.host,
}
async_create_issue(
hass,
DOMAIN,
issue,
is_fixable=True,
severity=IssueSeverity.ERROR,
translation_key="device_conflict",
translation_placeholders=shared_data,
data={**shared_data, "entry_id": entry.entry_id},
)
_LOGGER.error(
"Unexpected device found at %s; "
"expected `%s` with mac address `%s`, "
@ -475,6 +502,7 @@ class ESPHomeManager:
# flow.
return
async_delete_issue(hass, DOMAIN, issue)
# Make sure we have the correct device name stored
# so we can map the device to ESPHome Dashboard config
# If we got here, we know the mac address matches or we
@ -902,3 +930,40 @@ async def cleanup_instance(
await data.async_cleanup()
await data.client.disconnect()
return data
async def async_replace_device(
hass: HomeAssistant,
entry_id: str,
old_mac: str, # will be lower case (format_mac)
new_mac: str, # will be lower case (format_mac)
) -> None:
"""Migrate an ESPHome entry to replace an existing device."""
entry = hass.config_entries.async_get_entry(entry_id)
assert entry is not None
hass.config_entries.async_update_entry(entry, unique_id=new_mac)
dev_reg = dr.async_get(hass)
for device in dr.async_entries_for_config_entry(dev_reg, entry.entry_id):
dev_reg.async_update_device(
device.id,
new_connections={(dr.CONNECTION_NETWORK_MAC, new_mac)},
)
ent_reg = er.async_get(hass)
upper_mac = new_mac.upper()
old_upper_mac = old_mac.upper()
for entity in er.async_entries_for_config_entry(ent_reg, entry.entry_id):
# <upper_mac>-<entity type>-<object_id>
old_unique_id = entity.unique_id.split("-")
new_unique_id = "-".join([upper_mac, *old_unique_id[1:]])
if entity.unique_id != new_unique_id and entity.unique_id.startswith(
old_upper_mac
):
ent_reg.async_update_entity(entity.entity_id, new_unique_id=new_unique_id)
domain_data = DomainData.get(hass)
store = domain_data.get_or_create_store(hass, entry)
if data := await store.async_load():
data["device_info"]["mac_address"] = upper_mac
await store.async_save(data)

View File

@ -2,11 +2,95 @@
from __future__ import annotations
from typing import cast
import voluptuous as vol
from homeassistant import data_entry_flow
from homeassistant.components.assist_pipeline.repair_flows import (
AssistInProgressDeprecatedRepairFlow,
)
from homeassistant.components.repairs import RepairsFlow
from homeassistant.core import HomeAssistant
from homeassistant.core import HomeAssistant, callback
from homeassistant.helpers import issue_registry as ir
from .manager import async_replace_device
class ESPHomeRepair(RepairsFlow):
"""Handler for an issue fixing flow."""
def __init__(self, data: dict[str, str | int | float | None] | None) -> None:
"""Initialize."""
self._data = data
super().__init__()
@callback
def _async_get_placeholders(self) -> dict[str, str]:
issue_registry = ir.async_get(self.hass)
issue = issue_registry.async_get_issue(self.handler, self.issue_id)
assert issue is not None
return issue.translation_placeholders or {}
class DeviceConflictRepair(ESPHomeRepair):
"""Handler for an issue fixing device conflict."""
@property
def entry_id(self) -> str:
"""Return the config entry id."""
assert isinstance(self._data, dict)
return cast(str, self._data["entry_id"])
@property
def mac(self) -> str:
"""Return the MAC address of the new device."""
assert isinstance(self._data, dict)
return cast(str, self._data["mac"])
@property
def stored_mac(self) -> str:
"""Return the MAC address of the stored device."""
assert isinstance(self._data, dict)
return cast(str, self._data["stored_mac"])
async def async_step_init(
self, user_input: dict[str, str] | None = None
) -> data_entry_flow.FlowResult:
"""Handle the first step of a fix flow."""
return self.async_show_menu(
step_id="init",
menu_options=["migrate", "manual"],
description_placeholders=self._async_get_placeholders(),
)
async def async_step_migrate(
self, user_input: dict[str, str] | None = None
) -> data_entry_flow.FlowResult:
"""Handle the migrate step of a fix flow."""
if user_input is None:
return self.async_show_form(
step_id="migrate",
data_schema=vol.Schema({}),
description_placeholders=self._async_get_placeholders(),
)
entry_id = self.entry_id
await async_replace_device(self.hass, entry_id, self.stored_mac, self.mac)
self.hass.config_entries.async_schedule_reload(entry_id)
return self.async_create_entry(data={})
async def async_step_manual(
self, user_input: dict[str, str] | None = None
) -> data_entry_flow.FlowResult:
"""Handle the manual step of a fix flow."""
if user_input is None:
return self.async_show_form(
step_id="manual",
data_schema=vol.Schema({}),
description_placeholders=self._async_get_placeholders(),
)
self.hass.config_entries.async_schedule_reload(self.entry_id)
return self.async_create_entry(data={})
async def async_create_fix_flow(
@ -17,6 +101,8 @@ async def async_create_fix_flow(
"""Create flow."""
if issue_id.startswith("assist_in_progress_deprecated"):
return AssistInProgressDeprecatedRepairFlow(data)
if issue_id.startswith("device_conflict"):
return DeviceConflictRepair(data)
# If ESPHome adds confirm-only repairs in the future, this should be changed
# to return a ConfirmRepairFlow instead of raising a ValueError
raise ValueError(f"unknown repair {issue_id}")

View File

@ -130,6 +130,29 @@
"service_calls_not_allowed": {
"title": "{name} is not permitted to perform Home Assistant actions",
"description": "The ESPHome device attempted to perform a Home Assistant action, but this functionality is not enabled.\n\nIf you trust this device and want to allow it to perform Home Assistant action, you can enable this functionality in the options flow."
},
"device_conflict": {
"title": "Device conflict for {name}",
"fix_flow": {
"step": {
"init": {
"title": "Device conflict for {name}",
"description": "**The device `{name}` (`{model}`) at `{ip}` has reported a MAC address change from `{stored_mac}` to `{mac}`.**\n\nIf you have multiple devices with the same name, please rename or remove the one with MAC address `{mac}` to avoid conflicts.\n\nIf this is a hardware replacement, please confirm that you would like to migrate the Home Assistant configuration to the new device with MAC address `{mac}`.",
"menu_options": {
"migrate": "Migrate configuration to new device",
"manual": "Remove or rename device"
}
},
"migrate": {
"title": "Confirm device replacement for {name}",
"description": "Are you sure you want to migrate the Home Assistant configuration for `{name}` (`{model}`) at `{ip}` from `{stored_mac}` to `{mac}`?"
},
"manual": {
"title": "Remove or rename device {name}",
"description": "To resolve the conflict, either remove the device with MAC address `{mac}` from the network and restart the one with MAC address `{stored_mac}`, or re-flash the device with MAC address `{mac}` using a different name than `{name}`. Submit again once done."
}
}
}
}
}
}

View File

@ -33,6 +33,7 @@ from homeassistant.components.esphome.const import (
STABLE_BLE_URL_VERSION,
STABLE_BLE_VERSION_STR,
)
from homeassistant.components.esphome.manager import DEVICE_CONFLICT_ISSUE_FORMAT
from homeassistant.const import (
CONF_HOST,
CONF_PASSWORD,
@ -688,6 +689,7 @@ async def test_connection_aborted_wrong_device(
hass: HomeAssistant,
mock_client: APIClient,
caplog: pytest.LogCaptureFixture,
issue_registry: ir.IssueRegistry,
) -> None:
"""Test we abort the connection if the unique id is a mac and neither name or mac match."""
entry = MockConfigEntry(
@ -721,6 +723,82 @@ async def test_connection_aborted_wrong_device(
"with mac address `11:22:33:44:55:aa`, found `different` "
"with mac address `11:22:33:44:55:ab`" in caplog.text
)
# If its a different name, it means their DHCP
# reservations are missing and the device is not
# actually the same device, and there is nothing
# we can do to fix it so we only log a warning
assert not issue_registry.async_get_issue(
domain=DOMAIN, issue_id=DEVICE_CONFLICT_ISSUE_FORMAT.format(entry.entry_id)
)
assert "Error getting setting up connection for" not in caplog.text
mock_client.disconnect = AsyncMock()
caplog.clear()
# Make sure discovery triggers a reconnect
service_info = DhcpServiceInfo(
ip="192.168.43.184",
hostname="test",
macaddress="1122334455aa",
)
new_info = AsyncMock(
return_value=DeviceInfo(mac_address="1122334455aa", name="test")
)
mock_client.device_info = new_info
result = await hass.config_entries.flow.async_init(
"esphome", context={"source": config_entries.SOURCE_DHCP}, data=service_info
)
assert result["type"] is FlowResultType.ABORT
assert result["reason"] == "already_configured"
assert entry.data[CONF_HOST] == "192.168.43.184"
await hass.async_block_till_done()
assert len(new_info.mock_calls) == 2
assert "Unexpected device found at" not in caplog.text
@pytest.mark.usefixtures("mock_zeroconf")
async def test_connection_aborted_wrong_device_same_name(
hass: HomeAssistant,
mock_client: APIClient,
caplog: pytest.LogCaptureFixture,
issue_registry: ir.IssueRegistry,
) -> None:
"""Test we abort the connection if the unique id is a mac and the name matches."""
entry = MockConfigEntry(
domain=DOMAIN,
data={
CONF_HOST: "192.168.43.183",
CONF_PORT: 6053,
CONF_PASSWORD: "",
CONF_DEVICE_NAME: "test",
},
unique_id="11:22:33:44:55:aa",
)
entry.add_to_hass(hass)
disconnect_done = hass.loop.create_future()
async def async_disconnect(*args, **kwargs) -> None:
disconnect_done.set_result(None)
mock_client.disconnect = async_disconnect
mock_client.device_info = AsyncMock(
return_value=DeviceInfo(mac_address="1122334455ab", name="test")
)
await hass.config_entries.async_setup(entry.entry_id)
await hass.async_block_till_done()
async with asyncio.timeout(1):
await disconnect_done
assert (
"Unexpected device found at 192.168.43.183; expected `test` "
"with mac address `11:22:33:44:55:aa`, found `test` "
"with mac address `11:22:33:44:55:ab`" in caplog.text
)
# We should start a repair flow to help them fix the issue
assert issue_registry.async_get_issue(
domain=DOMAIN, issue_id=DEVICE_CONFLICT_ISSUE_FORMAT.format(entry.entry_id)
)
assert "Error getting setting up connection for" not in caplog.text
mock_client.disconnect = AsyncMock()

View File

@ -1,13 +1,258 @@
"""Test ESPHome repairs."""
from __future__ import annotations
import asyncio
from collections.abc import Awaitable, Callable
from unittest.mock import AsyncMock
from aioesphomeapi import (
APIClient,
BinarySensorInfo,
BinarySensorState,
DeviceInfo,
EntityInfo,
EntityState,
UserService,
)
import pytest
from homeassistant.components.esphome import repairs
from homeassistant.components.esphome.const import DOMAIN
from homeassistant.components.esphome.manager import DEVICE_CONFLICT_ISSUE_FORMAT
from homeassistant.const import STATE_ON
from homeassistant.core import HomeAssistant
from homeassistant.data_entry_flow import FlowResultType
from homeassistant.helpers import (
device_registry as dr,
entity_registry as er,
issue_registry as ir,
)
from .conftest import MockESPHomeDevice
from tests.common import MockConfigEntry
from tests.components.repairs import (
async_process_repairs_platforms,
get_repairs,
process_repair_fix_flow,
start_repair_fix_flow,
)
from tests.typing import ClientSessionGenerator, WebSocketGenerator
async def test_create_fix_flow_raises_on_unknown_issue_id(hass: HomeAssistant) -> None:
"""Test reate_fix_flow raises on unknown issue_id."""
"""Test create_fix_flow raises on unknown issue_id."""
with pytest.raises(ValueError):
await repairs.async_create_fix_flow(hass, "no_such_issue", None)
async def test_device_conflict_manual(
hass: HomeAssistant,
mock_client: APIClient,
hass_client: ClientSessionGenerator,
hass_ws_client: WebSocketGenerator,
mock_config_entry: MockConfigEntry,
issue_registry: ir.IssueRegistry,
caplog: pytest.LogCaptureFixture,
) -> None:
"""Test guided manual conflict resolution."""
disconnect_done = hass.loop.create_future()
async def async_disconnect(*args, **kwargs) -> None:
disconnect_done.set_result(None)
mock_client.disconnect = async_disconnect
mock_client.device_info = AsyncMock(
return_value=DeviceInfo(
mac_address="1122334455ab", name="test", model="esp32-iso-poe"
)
)
await hass.config_entries.async_setup(mock_config_entry.entry_id)
await hass.async_block_till_done()
async with asyncio.timeout(1):
await disconnect_done
assert "Unexpected device found" in caplog.text
issue_id = DEVICE_CONFLICT_ISSUE_FORMAT.format(mock_config_entry.entry_id)
issues = await get_repairs(hass, hass_ws_client)
assert issues
assert len(issues) == 1
assert any(True for issue in issues if issue["issue_id"] == issue_id)
await async_process_repairs_platforms(hass)
client = await hass_client()
data = await start_repair_fix_flow(client, DOMAIN, issue_id)
flow_id = data["flow_id"]
assert data["description_placeholders"] == {
"ip": "192.168.1.2",
"mac": "11:22:33:44:55:ab",
"model": "esp32-iso-poe",
"name": "test",
"stored_mac": "11:22:33:44:55:aa",
}
assert data["type"] == FlowResultType.MENU
assert data["step_id"] == "init"
data = await process_repair_fix_flow(
client, flow_id, json={"next_step_id": "manual"}
)
flow_id = data["flow_id"]
assert data["description_placeholders"] == {
"ip": "192.168.1.2",
"mac": "11:22:33:44:55:ab",
"model": "esp32-iso-poe",
"name": "test",
"stored_mac": "11:22:33:44:55:aa",
}
assert data["type"] == FlowResultType.FORM
assert data["step_id"] == "manual"
mock_client.device_info = AsyncMock(
return_value=DeviceInfo(
mac_address="11:22:33:44:55:aa", name="test", model="esp32-iso-poe"
)
)
caplog.clear()
data = await process_repair_fix_flow(client, flow_id)
assert data["type"] == FlowResultType.CREATE_ENTRY
await hass.async_block_till_done()
assert "Unexpected device found" not in caplog.text
assert issue_registry.async_get_issue(DOMAIN, issue_id) is None
async def test_device_conflict_migration(
hass: HomeAssistant,
mock_client: APIClient,
hass_client: ClientSessionGenerator,
hass_ws_client: WebSocketGenerator,
issue_registry: ir.IssueRegistry,
entity_registry: er.EntityRegistry,
device_registry: dr.DeviceRegistry,
caplog: pytest.LogCaptureFixture,
mock_esphome_device: Callable[
[APIClient, list[EntityInfo], list[UserService], list[EntityState]],
Awaitable[MockESPHomeDevice],
],
) -> None:
"""Test migrating existing configuration to new hardware."""
entity_info = [
BinarySensorInfo(
object_id="mybinary_sensor",
key=1,
name="my binary_sensor",
unique_id="my_binary_sensor",
is_status_binary_sensor=True,
)
]
states = [BinarySensorState(key=1, state=None)]
user_service = []
device: MockESPHomeDevice = await mock_esphome_device(
mock_client=mock_client,
entity_info=entity_info,
user_service=user_service,
states=states,
)
state = hass.states.get("binary_sensor.test_mybinary_sensor")
assert state is not None
assert state.state == STATE_ON
mock_config_entry = device.entry
ent_reg_entry = entity_registry.async_get("binary_sensor.test_mybinary_sensor")
assert ent_reg_entry
assert ent_reg_entry.unique_id == "11:22:33:44:55:AA-binary_sensor-mybinary_sensor"
entries = er.async_entries_for_config_entry(
entity_registry, mock_config_entry.entry_id
)
assert entries is not None
for entry in entries:
assert entry.unique_id.startswith("11:22:33:44:55:AA-")
disconnect_done = hass.loop.create_future()
async def async_disconnect(*args, **kwargs) -> None:
if not disconnect_done.done():
disconnect_done.set_result(None)
mock_client.disconnect = async_disconnect
new_device_info = DeviceInfo(
mac_address="11:22:33:44:55:AB", name="test", model="esp32-iso-poe"
)
mock_client.device_info = AsyncMock(return_value=new_device_info)
device.device_info = new_device_info
await hass.config_entries.async_reload(mock_config_entry.entry_id)
await hass.async_block_till_done()
async with asyncio.timeout(1):
await disconnect_done
assert "Unexpected device found" in caplog.text
issue_id = DEVICE_CONFLICT_ISSUE_FORMAT.format(mock_config_entry.entry_id)
issues = await get_repairs(hass, hass_ws_client)
assert issues
assert len(issues) == 1
assert any(True for issue in issues if issue["issue_id"] == issue_id)
await async_process_repairs_platforms(hass)
client = await hass_client()
data = await start_repair_fix_flow(client, DOMAIN, issue_id)
flow_id = data["flow_id"]
assert data["description_placeholders"] == {
"ip": "test.local",
"mac": "11:22:33:44:55:ab",
"model": "esp32-iso-poe",
"name": "test",
"stored_mac": "11:22:33:44:55:aa",
}
assert data["type"] == FlowResultType.MENU
assert data["step_id"] == "init"
data = await process_repair_fix_flow(
client, flow_id, json={"next_step_id": "migrate"}
)
flow_id = data["flow_id"]
assert data["description_placeholders"] == {
"ip": "test.local",
"mac": "11:22:33:44:55:ab",
"model": "esp32-iso-poe",
"name": "test",
"stored_mac": "11:22:33:44:55:aa",
}
assert data["type"] == FlowResultType.FORM
assert data["step_id"] == "migrate"
caplog.clear()
data = await process_repair_fix_flow(client, flow_id)
assert data["type"] == FlowResultType.CREATE_ENTRY
await hass.async_block_till_done()
assert "Unexpected device found" not in caplog.text
assert issue_registry.async_get_issue(DOMAIN, issue_id) is None
assert mock_config_entry.unique_id == "11:22:33:44:55:ab"
ent_reg_entry = entity_registry.async_get("binary_sensor.test_mybinary_sensor")
assert ent_reg_entry
assert ent_reg_entry.unique_id == "11:22:33:44:55:AB-binary_sensor-mybinary_sensor"
entries = er.async_entries_for_config_entry(
entity_registry, mock_config_entry.entry_id
)
assert entries is not None
for entry in entries:
assert entry.unique_id.startswith("11:22:33:44:55:AB-")
dev_entry = device_registry.async_get_device(
identifiers={}, connections={(dr.CONNECTION_NETWORK_MAC, "11:22:33:44:55:ab")}
)
assert dev_entry is not None
old_dev_entry = device_registry.async_get_device(
identifiers={}, connections={(dr.CONNECTION_NETWORK_MAC, "11:22:33:44:55:aa")}
)
assert old_dev_entry is None