diff --git a/homeassistant/components/rainbird/__init__.py b/homeassistant/components/rainbird/__init__.py index a97af14f449..e7a7c1200b9 100644 --- a/homeassistant/components/rainbird/__init__.py +++ b/homeassistant/components/rainbird/__init__.py @@ -1,17 +1,25 @@ """Support for Rain Bird Irrigation system LNK WiFi Module.""" from __future__ import annotations +import logging + from pyrainbird.async_client import AsyncRainbirdClient, AsyncRainbirdController from pyrainbird.exceptions import RainbirdApiException from homeassistant.config_entries import ConfigEntry -from homeassistant.const import CONF_HOST, CONF_PASSWORD, Platform +from homeassistant.const import CONF_HOST, CONF_MAC, CONF_PASSWORD, Platform from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady +from homeassistant.helpers import entity_registry as er from homeassistant.helpers.aiohttp_client import async_get_clientsession +from homeassistant.helpers.device_registry import format_mac +from homeassistant.helpers.entity_registry import async_entries_for_config_entry +from .const import CONF_SERIAL_NUMBER from .coordinator import RainbirdData +_LOGGER = logging.getLogger(__name__) + PLATFORMS = [ Platform.SWITCH, Platform.SENSOR, @@ -36,6 +44,18 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: entry.data[CONF_PASSWORD], ) ) + + if not (await _async_fix_unique_id(hass, controller, entry)): + return False + if mac_address := entry.data.get(CONF_MAC): + _async_fix_entity_unique_id( + hass, + er.async_get(hass), + entry.entry_id, + format_mac(mac_address), + str(entry.data[CONF_SERIAL_NUMBER]), + ) + try: model_info = await controller.get_model_and_version() except RainbirdApiException as err: @@ -51,6 +71,72 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: return True +async def _async_fix_unique_id( + hass: HomeAssistant, controller: AsyncRainbirdController, entry: ConfigEntry +) -> bool: + """Update the config entry with a unique id based on the mac address.""" + _LOGGER.debug("Checking for migration of config entry (%s)", entry.unique_id) + if not (mac_address := entry.data.get(CONF_MAC)): + try: + wifi_params = await controller.get_wifi_params() + except RainbirdApiException as err: + _LOGGER.warning("Unable to fix missing unique id: %s", err) + return True + + if (mac_address := wifi_params.mac_address) is None: + _LOGGER.warning("Unable to fix missing unique id (mac address was None)") + return True + + new_unique_id = format_mac(mac_address) + if entry.unique_id == new_unique_id and CONF_MAC in entry.data: + _LOGGER.debug("Config entry already in correct state") + return True + + entries = hass.config_entries.async_entries(DOMAIN) + for existing_entry in entries: + if existing_entry.unique_id == new_unique_id: + _LOGGER.warning( + "Unable to fix missing unique id (already exists); Removing duplicate entry" + ) + hass.async_create_background_task( + hass.config_entries.async_remove(entry.entry_id), + "Remove rainbird config entry", + ) + return False + + _LOGGER.debug("Updating unique id to %s", new_unique_id) + hass.config_entries.async_update_entry( + entry, + unique_id=new_unique_id, + data={ + **entry.data, + CONF_MAC: mac_address, + }, + ) + return True + + +def _async_fix_entity_unique_id( + hass: HomeAssistant, + entity_registry: er.EntityRegistry, + config_entry_id: str, + mac_address: str, + serial_number: str, +) -> None: + """Migrate existing entity if current one can't be found and an old one exists.""" + entity_entries = async_entries_for_config_entry(entity_registry, config_entry_id) + for entity_entry in entity_entries: + unique_id = str(entity_entry.unique_id) + if unique_id.startswith(mac_address): + continue + if (suffix := unique_id.removeprefix(str(serial_number))) != unique_id: + new_unique_id = f"{mac_address}{suffix}" + _LOGGER.debug("Updating unique id from %s to %s", unique_id, new_unique_id) + entity_registry.async_update_entity( + entity_entry.entity_id, new_unique_id=new_unique_id + ) + + async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Unload a config entry.""" diff --git a/homeassistant/components/rainbird/config_flow.py b/homeassistant/components/rainbird/config_flow.py index bf6682e7a6f..f90e13d37f3 100644 --- a/homeassistant/components/rainbird/config_flow.py +++ b/homeassistant/components/rainbird/config_flow.py @@ -11,15 +11,17 @@ from pyrainbird.async_client import ( AsyncRainbirdController, RainbirdApiException, ) +from pyrainbird.data import WifiParams import voluptuous as vol from homeassistant import config_entries from homeassistant.config_entries import ConfigEntry -from homeassistant.const import CONF_HOST, CONF_PASSWORD +from homeassistant.const import CONF_HOST, CONF_MAC, CONF_PASSWORD from homeassistant.core import callback from homeassistant.data_entry_flow import FlowResult from homeassistant.helpers import config_validation as cv, selector from homeassistant.helpers.aiohttp_client import async_get_clientsession +from homeassistant.helpers.device_registry import format_mac from .const import ( ATTR_DURATION, @@ -69,7 +71,7 @@ class RainbirdConfigFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): error_code: str | None = None if user_input: try: - serial_number = await self._test_connection( + serial_number, wifi_params = await self._test_connection( user_input[CONF_HOST], user_input[CONF_PASSWORD] ) except ConfigFlowError as err: @@ -77,11 +79,11 @@ class RainbirdConfigFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): error_code = err.error_code else: return await self.async_finish( - serial_number, data={ CONF_HOST: user_input[CONF_HOST], CONF_PASSWORD: user_input[CONF_PASSWORD], CONF_SERIAL_NUMBER: serial_number, + CONF_MAC: wifi_params.mac_address, }, options={ATTR_DURATION: DEFAULT_TRIGGER_TIME_MINUTES}, ) @@ -92,8 +94,10 @@ class RainbirdConfigFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): errors={"base": error_code} if error_code else None, ) - async def _test_connection(self, host: str, password: str) -> str: - """Test the connection and return the device serial number. + async def _test_connection( + self, host: str, password: str + ) -> tuple[str, WifiParams]: + """Test the connection and return the device identifiers. Raises a ConfigFlowError on failure. """ @@ -106,7 +110,10 @@ class RainbirdConfigFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): ) try: async with asyncio.timeout(TIMEOUT_SECONDS): - return await controller.get_serial_number() + return await asyncio.gather( + controller.get_serial_number(), + controller.get_wifi_params(), + ) except asyncio.TimeoutError as err: raise ConfigFlowError( f"Timeout connecting to Rain Bird controller: {str(err)}", @@ -120,18 +127,28 @@ class RainbirdConfigFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): async def async_finish( self, - serial_number: str, data: dict[str, Any], options: dict[str, Any], ) -> FlowResult: """Create the config entry.""" - # Prevent devices with the same serial number. If the device does not have a serial number - # then we can at least prevent configuring the same host twice. - if serial_number: - await self.async_set_unique_id(serial_number) - self._abort_if_unique_id_configured() - else: - self._async_abort_entries_match(data) + # The integration has historically used a serial number, but not all devices + # historically had a valid one. Now the mac address is used as a unique id + # and serial is still persisted in config entry data in case it is needed + # in the future. + # Either way, also prevent configuring the same host twice. + await self.async_set_unique_id(format_mac(data[CONF_MAC])) + self._abort_if_unique_id_configured( + updates={ + CONF_HOST: data[CONF_HOST], + CONF_PASSWORD: data[CONF_PASSWORD], + } + ) + self._async_abort_entries_match( + { + CONF_HOST: data[CONF_HOST], + CONF_PASSWORD: data[CONF_PASSWORD], + } + ) return self.async_create_entry( title=data[CONF_HOST], data=data, diff --git a/tests/components/rainbird/conftest.py b/tests/components/rainbird/conftest.py index 6e8d58219c1..52b98e5c6b6 100644 --- a/tests/components/rainbird/conftest.py +++ b/tests/components/rainbird/conftest.py @@ -3,6 +3,7 @@ from __future__ import annotations from http import HTTPStatus +import json from typing import Any from unittest.mock import patch @@ -24,6 +25,8 @@ HOST = "example.com" URL = "http://example.com/stick" PASSWORD = "password" SERIAL_NUMBER = 0x12635436566 +MAC_ADDRESS = "4C:A1:61:00:11:22" +MAC_ADDRESS_UNIQUE_ID = "4c:a1:61:00:11:22" # # Response payloads below come from pyrainbird test cases. @@ -50,6 +53,20 @@ RAIN_DELAY = "B60010" # 0x10 is 16 RAIN_DELAY_OFF = "B60000" # ACK command 0x10, Echo 0x06 ACK_ECHO = "0106" +WIFI_PARAMS_RESPONSE = { + "macAddress": MAC_ADDRESS, + "localIpAddress": "1.1.1.38", + "localNetmask": "255.255.255.0", + "localGateway": "1.1.1.1", + "rssi": -61, + "wifiSsid": "wifi-ssid-name", + "wifiPassword": "wifi-password-name", + "wifiSecurity": "wpa2-aes", + "apTimeoutNoLan": 20, + "apTimeoutIdle": 20, + "apSecurity": "unknown", + "stickVersion": "Rain Bird Stick Rev C/1.63", +} CONFIG = { @@ -62,10 +79,16 @@ CONFIG = { } } +CONFIG_ENTRY_DATA_OLD_FORMAT = { + "host": HOST, + "password": PASSWORD, + "serial_number": SERIAL_NUMBER, +} CONFIG_ENTRY_DATA = { "host": HOST, "password": PASSWORD, "serial_number": SERIAL_NUMBER, + "mac": MAC_ADDRESS, } @@ -77,14 +100,23 @@ def platforms() -> list[Platform]: @pytest.fixture async def config_entry_unique_id() -> str: - """Fixture for serial number used in the config entry.""" + """Fixture for config entry unique id.""" + return MAC_ADDRESS_UNIQUE_ID + + +@pytest.fixture +async def serial_number() -> int: + """Fixture for serial number used in the config entry data.""" return SERIAL_NUMBER @pytest.fixture -async def config_entry_data() -> dict[str, Any]: +async def config_entry_data(serial_number: int) -> dict[str, Any]: """Fixture for MockConfigEntry data.""" - return CONFIG_ENTRY_DATA + return { + **CONFIG_ENTRY_DATA, + "serial_number": serial_number, + } @pytest.fixture @@ -123,17 +155,24 @@ def setup_platforms( yield -def rainbird_response(data: str) -> bytes: +def rainbird_json_response(result: dict[str, str]) -> bytes: """Create a fake API response.""" return encryption.encrypt( - '{"jsonrpc": "2.0", "result": {"data":"%s"}, "id": 1} ' % data, + '{"jsonrpc": "2.0", "result": %s, "id": 1} ' % json.dumps(result), PASSWORD, ) +def mock_json_response(result: dict[str, str]) -> AiohttpClientMockResponse: + """Create a fake AiohttpClientMockResponse.""" + return AiohttpClientMockResponse( + "POST", URL, response=rainbird_json_response(result) + ) + + def mock_response(data: str) -> AiohttpClientMockResponse: """Create a fake AiohttpClientMockResponse.""" - return AiohttpClientMockResponse("POST", URL, response=rainbird_response(data)) + return mock_json_response({"data": data}) def mock_response_error( diff --git a/tests/components/rainbird/test_binary_sensor.py b/tests/components/rainbird/test_binary_sensor.py index 7b9fb41ed1f..afe18337377 100644 --- a/tests/components/rainbird/test_binary_sensor.py +++ b/tests/components/rainbird/test_binary_sensor.py @@ -1,6 +1,8 @@ """Tests for rainbird sensor platform.""" +from http import HTTPStatus + import pytest from homeassistant.config_entries import ConfigEntryState @@ -8,7 +10,12 @@ from homeassistant.const import Platform from homeassistant.core import HomeAssistant from homeassistant.helpers import entity_registry as er -from .conftest import RAIN_SENSOR_OFF, RAIN_SENSOR_ON, SERIAL_NUMBER +from .conftest import ( + CONFIG_ENTRY_DATA_OLD_FORMAT, + RAIN_SENSOR_OFF, + RAIN_SENSOR_ON, + mock_response_error, +) from tests.common import MockConfigEntry from tests.test_util.aiohttp import AiohttpClientMockResponse @@ -51,47 +58,25 @@ async def test_rainsensor( @pytest.mark.parametrize( - ("config_entry_unique_id", "entity_unique_id"), + ("config_entry_data", "config_entry_unique_id", "setup_config_entry"), [ - (SERIAL_NUMBER, "1263613994342-rainsensor"), - # Some existing config entries may have a "0" serial number but preserve - # their unique id - (0, "0-rainsensor"), - ], -) -async def test_unique_id( - hass: HomeAssistant, - entity_registry: er.EntityRegistry, - entity_unique_id: str, -) -> None: - """Test rainsensor binary sensor.""" - rainsensor = hass.states.get("binary_sensor.rain_bird_controller_rainsensor") - assert rainsensor is not None - assert rainsensor.attributes == { - "friendly_name": "Rain Bird Controller Rainsensor", - "icon": "mdi:water", - } - - entity_entry = entity_registry.async_get( - "binary_sensor.rain_bird_controller_rainsensor" - ) - assert entity_entry - assert entity_entry.unique_id == entity_unique_id - - -@pytest.mark.parametrize( - ("config_entry_unique_id"), - [ - (None), + (CONFIG_ENTRY_DATA_OLD_FORMAT, None, None), ], ) async def test_no_unique_id( hass: HomeAssistant, responses: list[AiohttpClientMockResponse], entity_registry: er.EntityRegistry, + config_entry: MockConfigEntry, ) -> None: """Test rainsensor binary sensor with no unique id.""" + # Failure to migrate config entry to a unique id + responses.insert(0, mock_response_error(HTTPStatus.SERVICE_UNAVAILABLE)) + + await config_entry.async_setup(hass) + assert config_entry.state == ConfigEntryState.LOADED + rainsensor = hass.states.get("binary_sensor.rain_bird_controller_rainsensor") assert rainsensor is not None assert ( diff --git a/tests/components/rainbird/test_calendar.py b/tests/components/rainbird/test_calendar.py index d6c14834342..04e423a399c 100644 --- a/tests/components/rainbird/test_calendar.py +++ b/tests/components/rainbird/test_calendar.py @@ -17,7 +17,7 @@ from homeassistant.const import Platform from homeassistant.core import HomeAssistant from homeassistant.helpers import entity_registry as er -from .conftest import mock_response, mock_response_error +from .conftest import CONFIG_ENTRY_DATA_OLD_FORMAT, mock_response, mock_response_error from tests.common import MockConfigEntry from tests.test_util.aiohttp import AiohttpClientMockResponse @@ -210,7 +210,7 @@ async def test_event_state( entity = entity_registry.async_get(TEST_ENTITY) assert entity - assert entity.unique_id == 1263613994342 + assert entity.unique_id == "4c:a1:61:00:11:22" @pytest.mark.parametrize( @@ -280,18 +280,26 @@ async def test_program_schedule_disabled( @pytest.mark.parametrize( - ("config_entry_unique_id"), + ("config_entry_data", "config_entry_unique_id", "setup_config_entry"), [ - (None), + (CONFIG_ENTRY_DATA_OLD_FORMAT, None, None), ], ) async def test_no_unique_id( hass: HomeAssistant, get_events: GetEventsFn, + responses: list[AiohttpClientMockResponse], entity_registry: er.EntityRegistry, + config_entry: MockConfigEntry, ) -> None: """Test calendar entity with no unique id.""" + # Failure to migrate config entry to a unique id + responses.insert(0, mock_response_error(HTTPStatus.SERVICE_UNAVAILABLE)) + + await config_entry.async_setup(hass) + assert config_entry.state == ConfigEntryState.LOADED + state = hass.states.get(TEST_ENTITY) assert state is not None assert state.attributes.get("friendly_name") == "Rain Bird Controller" diff --git a/tests/components/rainbird/test_config_flow.py b/tests/components/rainbird/test_config_flow.py index f93da8d9839..6c0e13fef39 100644 --- a/tests/components/rainbird/test_config_flow.py +++ b/tests/components/rainbird/test_config_flow.py @@ -19,11 +19,14 @@ from homeassistant.data_entry_flow import FlowResult, FlowResultType from .conftest import ( CONFIG_ENTRY_DATA, HOST, + MAC_ADDRESS_UNIQUE_ID, PASSWORD, SERIAL_NUMBER, SERIAL_RESPONSE, URL, + WIFI_PARAMS_RESPONSE, ZERO_SERIAL_RESPONSE, + mock_json_response, mock_response, ) @@ -34,7 +37,7 @@ from tests.test_util.aiohttp import AiohttpClientMocker, AiohttpClientMockRespon @pytest.fixture(name="responses") def mock_responses() -> list[AiohttpClientMockResponse]: """Set up fake serial number response when testing the connection.""" - return [mock_response(SERIAL_RESPONSE)] + return [mock_response(SERIAL_RESPONSE), mock_json_response(WIFI_PARAMS_RESPONSE)] @pytest.fixture(autouse=True) @@ -74,14 +77,20 @@ async def complete_flow(hass: HomeAssistant) -> FlowResult: ("responses", "expected_config_entry", "expected_unique_id"), [ ( - [mock_response(SERIAL_RESPONSE)], + [ + mock_response(SERIAL_RESPONSE), + mock_json_response(WIFI_PARAMS_RESPONSE), + ], CONFIG_ENTRY_DATA, - SERIAL_NUMBER, + MAC_ADDRESS_UNIQUE_ID, ), ( - [mock_response(ZERO_SERIAL_RESPONSE)], + [ + mock_response(ZERO_SERIAL_RESPONSE), + mock_json_response(WIFI_PARAMS_RESPONSE), + ], {**CONFIG_ENTRY_DATA, "serial_number": 0}, - None, + MAC_ADDRESS_UNIQUE_ID, ), ], ) @@ -115,17 +124,32 @@ async def test_controller_flow( ( "other-serial-number", {**CONFIG_ENTRY_DATA, "host": "other-host"}, - [mock_response(SERIAL_RESPONSE)], + [mock_response(SERIAL_RESPONSE), mock_json_response(WIFI_PARAMS_RESPONSE)], + CONFIG_ENTRY_DATA, + ), + ( + "11:22:33:44:55:66", + { + **CONFIG_ENTRY_DATA, + "host": "other-host", + }, + [ + mock_response(SERIAL_RESPONSE), + mock_json_response(WIFI_PARAMS_RESPONSE), + ], CONFIG_ENTRY_DATA, ), ( None, {**CONFIG_ENTRY_DATA, "serial_number": 0, "host": "other-host"}, - [mock_response(ZERO_SERIAL_RESPONSE)], + [ + mock_response(ZERO_SERIAL_RESPONSE), + mock_json_response(WIFI_PARAMS_RESPONSE), + ], {**CONFIG_ENTRY_DATA, "serial_number": 0}, ), ], - ids=["with-serial", "zero-serial"], + ids=["with-serial", "with-mac-address", "zero-serial"], ) async def test_multiple_config_entries( hass: HomeAssistant, @@ -154,22 +178,52 @@ async def test_multiple_config_entries( "config_entry_unique_id", "config_entry_data", "config_flow_responses", + "expected_config_entry_data", ), [ + # Config entry is a pure duplicate with the same mac address unique id + ( + MAC_ADDRESS_UNIQUE_ID, + CONFIG_ENTRY_DATA, + [ + mock_response(SERIAL_RESPONSE), + mock_json_response(WIFI_PARAMS_RESPONSE), + ], + CONFIG_ENTRY_DATA, + ), + # Old unique id with serial, but same host ( SERIAL_NUMBER, CONFIG_ENTRY_DATA, - [mock_response(SERIAL_RESPONSE)], + [mock_response(SERIAL_RESPONSE), mock_json_response(WIFI_PARAMS_RESPONSE)], + CONFIG_ENTRY_DATA, ), + # Old unique id with no serial, but same host ( None, {**CONFIG_ENTRY_DATA, "serial_number": 0}, - [mock_response(ZERO_SERIAL_RESPONSE)], + [ + mock_response(ZERO_SERIAL_RESPONSE), + mock_json_response(WIFI_PARAMS_RESPONSE), + ], + {**CONFIG_ENTRY_DATA, "serial_number": 0}, + ), + # Enters a different hostname that points to the same mac address + ( + MAC_ADDRESS_UNIQUE_ID, + { + **CONFIG_ENTRY_DATA, + "host": f"other-{HOST}", + }, + [mock_response(SERIAL_RESPONSE), mock_json_response(WIFI_PARAMS_RESPONSE)], + CONFIG_ENTRY_DATA, # Updated the host ), ], ids=[ - "duplicate-serial-number", + "duplicate-mac-unique-id", + "duplicate-host-legacy-serial-number", "duplicate-host-port-no-serial", + "duplicate-duplicate-hostname", ], ) async def test_duplicate_config_entries( @@ -177,6 +231,7 @@ async def test_duplicate_config_entries( config_entry: MockConfigEntry, responses: list[AiohttpClientMockResponse], config_flow_responses: list[AiohttpClientMockResponse], + expected_config_entry_data: dict[str, Any], ) -> None: """Test that a device can not be registered twice.""" await config_entry.async_setup(hass) @@ -186,8 +241,10 @@ async def test_duplicate_config_entries( responses.extend(config_flow_responses) result = await complete_flow(hass) + assert len(hass.config_entries.async_entries(DOMAIN)) == 1 assert result.get("type") == FlowResultType.ABORT assert result.get("reason") == "already_configured" + assert dict(config_entry.data) == expected_config_entry_data async def test_controller_cannot_connect( diff --git a/tests/components/rainbird/test_init.py b/tests/components/rainbird/test_init.py index 7ec22b88867..db9c4c8739e 100644 --- a/tests/components/rainbird/test_init.py +++ b/tests/components/rainbird/test_init.py @@ -6,12 +6,21 @@ from http import HTTPStatus import pytest +from homeassistant.components.rainbird.const import DOMAIN from homeassistant.config_entries import ConfigEntryState +from homeassistant.const import CONF_MAC from homeassistant.core import HomeAssistant +from homeassistant.helpers import entity_registry as er from .conftest import ( CONFIG_ENTRY_DATA, + CONFIG_ENTRY_DATA_OLD_FORMAT, + MAC_ADDRESS, + MAC_ADDRESS_UNIQUE_ID, MODEL_AND_VERSION_RESPONSE, + SERIAL_NUMBER, + WIFI_PARAMS_RESPONSE, + mock_json_response, mock_response, mock_response_error, ) @@ -20,22 +29,11 @@ from tests.common import MockConfigEntry from tests.test_util.aiohttp import AiohttpClientMockResponse -@pytest.mark.parametrize( - ("config_entry_data", "initial_response"), - [ - (CONFIG_ENTRY_DATA, None), - ], - ids=["config_entry"], -) async def test_init_success( hass: HomeAssistant, config_entry: MockConfigEntry, - responses: list[AiohttpClientMockResponse], - initial_response: AiohttpClientMockResponse | None, ) -> None: """Test successful setup and unload.""" - if initial_response: - responses.insert(0, initial_response) await config_entry.async_setup(hass) assert config_entry.state == ConfigEntryState.LOADED @@ -88,6 +86,196 @@ async def test_communication_failure( config_entry_state: list[ConfigEntryState], ) -> None: """Test unable to talk to device on startup, which fails setup.""" - await config_entry.async_setup(hass) assert config_entry.state == config_entry_state + + +@pytest.mark.parametrize( + ("config_entry_unique_id", "config_entry_data"), + [ + ( + None, + {**CONFIG_ENTRY_DATA, "mac": None}, + ), + ], + ids=["config_entry"], +) +async def test_fix_unique_id( + hass: HomeAssistant, + responses: list[AiohttpClientMockResponse], + config_entry: MockConfigEntry, +) -> None: + """Test fix of a config entry with no unique id.""" + + responses.insert(0, mock_json_response(WIFI_PARAMS_RESPONSE)) + + entries = hass.config_entries.async_entries(DOMAIN) + assert len(entries) == 1 + assert entries[0].state == ConfigEntryState.NOT_LOADED + assert entries[0].unique_id is None + assert entries[0].data.get(CONF_MAC) is None + + await config_entry.async_setup(hass) + assert config_entry.state == ConfigEntryState.LOADED + + # Verify config entry now has a unique id + entries = hass.config_entries.async_entries(DOMAIN) + assert len(entries) == 1 + assert entries[0].state == ConfigEntryState.LOADED + assert entries[0].unique_id == MAC_ADDRESS_UNIQUE_ID + assert entries[0].data.get(CONF_MAC) == MAC_ADDRESS + + +@pytest.mark.parametrize( + ( + "config_entry_unique_id", + "config_entry_data", + "initial_response", + "expected_warning", + ), + [ + ( + None, + CONFIG_ENTRY_DATA_OLD_FORMAT, + mock_response_error(HTTPStatus.SERVICE_UNAVAILABLE), + "Unable to fix missing unique id:", + ), + ( + None, + CONFIG_ENTRY_DATA_OLD_FORMAT, + mock_response_error(HTTPStatus.NOT_FOUND), + "Unable to fix missing unique id:", + ), + ( + None, + CONFIG_ENTRY_DATA_OLD_FORMAT, + mock_response("bogus"), + "Unable to fix missing unique id (mac address was None)", + ), + ], + ids=["service_unavailable", "not_found", "unexpected_response_format"], +) +async def test_fix_unique_id_failure( + hass: HomeAssistant, + initial_response: AiohttpClientMockResponse, + responses: list[AiohttpClientMockResponse], + expected_warning: str, + caplog: pytest.LogCaptureFixture, + config_entry: MockConfigEntry, +) -> None: + """Test a failure during fix of a config entry with no unique id.""" + + responses.insert(0, initial_response) + + await config_entry.async_setup(hass) + # Config entry is loaded, but not updated + assert config_entry.state == ConfigEntryState.LOADED + assert config_entry.unique_id is None + + assert expected_warning in caplog.text + + +@pytest.mark.parametrize( + ("config_entry_unique_id"), + [(MAC_ADDRESS_UNIQUE_ID)], +) +async def test_fix_unique_id_duplicate( + hass: HomeAssistant, + config_entry: MockConfigEntry, + responses: list[AiohttpClientMockResponse], + caplog: pytest.LogCaptureFixture, +) -> None: + """Test that a config entry unique id already exists during fix.""" + # Add a second config entry that has no unique id, but has the same + # mac address. When fixing the unique id, it can't use the mac address + # since it already exists. + other_entry = MockConfigEntry( + unique_id=None, + domain=DOMAIN, + data=CONFIG_ENTRY_DATA_OLD_FORMAT, + ) + other_entry.add_to_hass(hass) + + # Responses for the second config entry. This first fetches wifi params + # to repair the unique id. + responses_copy = [*responses] + responses.append(mock_json_response(WIFI_PARAMS_RESPONSE)) + responses.extend(responses_copy) + + await config_entry.async_setup(hass) + assert config_entry.state == ConfigEntryState.LOADED + assert config_entry.unique_id == MAC_ADDRESS_UNIQUE_ID + + await other_entry.async_setup(hass) + # Config entry unique id could not be updated since it already exists + assert other_entry.state == ConfigEntryState.SETUP_ERROR + + assert "Unable to fix missing unique id (already exists)" in caplog.text + + await hass.async_block_till_done() + assert len(hass.config_entries.async_entries(DOMAIN)) == 1 + + +@pytest.mark.parametrize( + ( + "config_entry_unique_id", + "serial_number", + "entity_unique_id", + "expected_unique_id", + ), + [ + (SERIAL_NUMBER, SERIAL_NUMBER, SERIAL_NUMBER, MAC_ADDRESS_UNIQUE_ID), + ( + SERIAL_NUMBER, + SERIAL_NUMBER, + f"{SERIAL_NUMBER}-rain-delay", + f"{MAC_ADDRESS_UNIQUE_ID}-rain-delay", + ), + ("0", 0, "0", MAC_ADDRESS_UNIQUE_ID), + ( + "0", + 0, + "0-rain-delay", + f"{MAC_ADDRESS_UNIQUE_ID}-rain-delay", + ), + ( + MAC_ADDRESS_UNIQUE_ID, + SERIAL_NUMBER, + MAC_ADDRESS_UNIQUE_ID, + MAC_ADDRESS_UNIQUE_ID, + ), + ( + MAC_ADDRESS_UNIQUE_ID, + SERIAL_NUMBER, + f"{MAC_ADDRESS_UNIQUE_ID}-rain-delay", + f"{MAC_ADDRESS_UNIQUE_ID}-rain-delay", + ), + ], + ids=( + "serial-number", + "serial-number-with-suffix", + "zero-serial", + "zero-serial-suffix", + "new-format", + "new-format-suffx", + ), +) +async def test_fix_entity_unique_ids( + hass: HomeAssistant, + config_entry: MockConfigEntry, + entity_unique_id: str, + expected_unique_id: str, +) -> None: + """Test fixing entity unique ids from old unique id formats.""" + + entity_registry = er.async_get(hass) + entity_entry = entity_registry.async_get_or_create( + DOMAIN, "number", unique_id=entity_unique_id, config_entry=config_entry + ) + + await config_entry.async_setup(hass) + assert config_entry.state == ConfigEntryState.LOADED + + entity_entry = entity_registry.async_get(entity_entry.id) + assert entity_entry + assert entity_entry.unique_id == expected_unique_id diff --git a/tests/components/rainbird/test_number.py b/tests/components/rainbird/test_number.py index 0beae1f5a95..79b8fd5ec37 100644 --- a/tests/components/rainbird/test_number.py +++ b/tests/components/rainbird/test_number.py @@ -6,7 +6,7 @@ import pytest from homeassistant.components import number from homeassistant.components.rainbird import DOMAIN -from homeassistant.config_entries import ConfigEntry, ConfigEntryState +from homeassistant.config_entries import ConfigEntryState from homeassistant.const import ATTR_ENTITY_ID, Platform from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError @@ -14,15 +14,16 @@ from homeassistant.helpers import device_registry as dr, entity_registry as er from .conftest import ( ACK_ECHO, + CONFIG_ENTRY_DATA_OLD_FORMAT, + MAC_ADDRESS, RAIN_DELAY, RAIN_DELAY_OFF, - SERIAL_NUMBER, mock_response, mock_response_error, ) from tests.common import MockConfigEntry -from tests.test_util.aiohttp import AiohttpClientMocker +from tests.test_util.aiohttp import AiohttpClientMocker, AiohttpClientMockResponse @pytest.fixture @@ -66,46 +67,23 @@ async def test_number_values( entity_entry = entity_registry.async_get("number.rain_bird_controller_rain_delay") assert entity_entry - assert entity_entry.unique_id == "1263613994342-rain-delay" - - -@pytest.mark.parametrize( - ("config_entry_unique_id", "entity_unique_id"), - [ - (SERIAL_NUMBER, "1263613994342-rain-delay"), - # Some existing config entries may have a "0" serial number but preserve - # their unique id - (0, "0-rain-delay"), - ], -) -async def test_unique_id( - hass: HomeAssistant, - entity_registry: er.EntityRegistry, - entity_unique_id: str, -) -> None: - """Test number platform.""" - - raindelay = hass.states.get("number.rain_bird_controller_rain_delay") - assert raindelay is not None - assert ( - raindelay.attributes.get("friendly_name") == "Rain Bird Controller Rain delay" - ) - - entity_entry = entity_registry.async_get("number.rain_bird_controller_rain_delay") - assert entity_entry - assert entity_entry.unique_id == entity_unique_id + assert entity_entry.unique_id == "4c:a1:61:00:11:22-rain-delay" async def test_set_value( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker, responses: list[str], - config_entry: ConfigEntry, ) -> None: """Test setting the rain delay number.""" + raindelay = hass.states.get("number.rain_bird_controller_rain_delay") + assert raindelay is not None + device_registry = dr.async_get(hass) - device = device_registry.async_get_device(identifiers={(DOMAIN, SERIAL_NUMBER)}) + device = device_registry.async_get_device( + identifiers={(DOMAIN, MAC_ADDRESS.lower())} + ) assert device assert device.name == "Rain Bird Controller" assert device.model == "ESP-TM2" @@ -138,7 +116,6 @@ async def test_set_value_error( hass: HomeAssistant, aioclient_mock: AiohttpClientMocker, responses: list[str], - config_entry: ConfigEntry, status: HTTPStatus, expected_msg: str, ) -> None: @@ -162,17 +139,25 @@ async def test_set_value_error( @pytest.mark.parametrize( - ("config_entry_unique_id"), + ("config_entry_data", "config_entry_unique_id", "setup_config_entry"), [ - (None), + (CONFIG_ENTRY_DATA_OLD_FORMAT, None, None), ], ) async def test_no_unique_id( hass: HomeAssistant, + responses: list[AiohttpClientMockResponse], entity_registry: er.EntityRegistry, + config_entry: MockConfigEntry, ) -> None: """Test number platform with no unique id.""" + # Failure to migrate config entry to a unique id + responses.insert(0, mock_response_error(HTTPStatus.SERVICE_UNAVAILABLE)) + + await config_entry.async_setup(hass) + assert config_entry.state == ConfigEntryState.LOADED + raindelay = hass.states.get("number.rain_bird_controller_rain_delay") assert raindelay is not None assert ( diff --git a/tests/components/rainbird/test_sensor.py b/tests/components/rainbird/test_sensor.py index 00d778335c5..2a0195f8d97 100644 --- a/tests/components/rainbird/test_sensor.py +++ b/tests/components/rainbird/test_sensor.py @@ -1,5 +1,6 @@ """Tests for rainbird sensor platform.""" +from http import HTTPStatus import pytest @@ -8,9 +9,15 @@ from homeassistant.const import Platform from homeassistant.core import HomeAssistant from homeassistant.helpers import entity_registry as er -from .conftest import CONFIG_ENTRY_DATA, RAIN_DELAY, RAIN_DELAY_OFF +from .conftest import ( + CONFIG_ENTRY_DATA_OLD_FORMAT, + RAIN_DELAY, + RAIN_DELAY_OFF, + mock_response_error, +) from tests.common import MockConfigEntry +from tests.test_util.aiohttp import AiohttpClientMockResponse @pytest.fixture @@ -49,37 +56,38 @@ async def test_sensors( entity_entry = entity_registry.async_get("sensor.rain_bird_controller_raindelay") assert entity_entry - assert entity_entry.unique_id == "1263613994342-raindelay" + assert entity_entry.unique_id == "4c:a1:61:00:11:22-raindelay" @pytest.mark.parametrize( - ("config_entry_unique_id", "config_entry_data"), + ("config_entry_unique_id", "config_entry_data", "setup_config_entry"), [ # Config entry setup without a unique id since it had no serial number ( None, { - **CONFIG_ENTRY_DATA, - "serial_number": 0, - }, - ), - # Legacy case for old config entries with serial number 0 preserves old behavior - ( - "0", - { - **CONFIG_ENTRY_DATA, + **CONFIG_ENTRY_DATA_OLD_FORMAT, "serial_number": 0, }, + None, ), ], ) async def test_sensor_no_unique_id( hass: HomeAssistant, entity_registry: er.EntityRegistry, + responses: list[AiohttpClientMockResponse], config_entry_unique_id: str | None, + config_entry: MockConfigEntry, ) -> None: """Test sensor platform with no unique id.""" + # Failure to migrate config entry to a unique id + responses.insert(0, mock_response_error(HTTPStatus.SERVICE_UNAVAILABLE)) + + await config_entry.async_setup(hass) + assert config_entry.state == ConfigEntryState.LOADED + raindelay = hass.states.get("sensor.rain_bird_controller_raindelay") assert raindelay is not None assert raindelay.attributes.get("friendly_name") == "Rain Bird Controller Raindelay" diff --git a/tests/components/rainbird/test_switch.py b/tests/components/rainbird/test_switch.py index e2b6a99d01a..f9c03f63dd3 100644 --- a/tests/components/rainbird/test_switch.py +++ b/tests/components/rainbird/test_switch.py @@ -13,12 +13,13 @@ from homeassistant.helpers import entity_registry as er from .conftest import ( ACK_ECHO, + CONFIG_ENTRY_DATA_OLD_FORMAT, EMPTY_STATIONS_RESPONSE, HOST, + MAC_ADDRESS, PASSWORD, RAIN_DELAY_OFF, RAIN_SENSOR_OFF, - SERIAL_NUMBER, ZONE_3_ON_RESPONSE, ZONE_5_ON_RESPONSE, ZONE_OFF_RESPONSE, @@ -109,7 +110,7 @@ async def test_zones( # Verify unique id for one of the switches entity_entry = entity_registry.async_get("switch.rain_bird_sprinkler_3") - assert entity_entry.unique_id == "1263613994342-3" + assert entity_entry.unique_id == "4c:a1:61:00:11:22-3" async def test_switch_on( @@ -226,6 +227,7 @@ async def test_irrigation_service( "1": "Garden Sprinkler", "2": "Back Yard", }, + "mac": MAC_ADDRESS, } ) ], @@ -274,9 +276,9 @@ async def test_switch_error( @pytest.mark.parametrize( - ("config_entry_unique_id"), + ("config_entry_data", "config_entry_unique_id", "setup_config_entry"), [ - (None), + (CONFIG_ENTRY_DATA_OLD_FORMAT, None, None), ], ) async def test_no_unique_id( @@ -284,8 +286,15 @@ async def test_no_unique_id( aioclient_mock: AiohttpClientMocker, responses: list[AiohttpClientMockResponse], entity_registry: er.EntityRegistry, + config_entry: MockConfigEntry, ) -> None: - """Test an irrigation switch with no unique id.""" + """Test an irrigation switch with no unique id due to migration failure.""" + + # Failure to migrate config entry to a unique id + responses.insert(0, mock_response_error(HTTPStatus.SERVICE_UNAVAILABLE)) + + await config_entry.async_setup(hass) + assert config_entry.state == ConfigEntryState.LOADED zone = hass.states.get("switch.rain_bird_sprinkler_3") assert zone is not None @@ -294,31 +303,3 @@ async def test_no_unique_id( entity_entry = entity_registry.async_get("switch.rain_bird_sprinkler_3") assert entity_entry is None - - -@pytest.mark.parametrize( - ("config_entry_unique_id", "entity_unique_id"), - [ - (SERIAL_NUMBER, "1263613994342-3"), - # Some existing config entries may have a "0" serial number but preserve - # their unique id - (0, "0-3"), - ], -) -async def test_has_unique_id( - hass: HomeAssistant, - aioclient_mock: AiohttpClientMocker, - responses: list[AiohttpClientMockResponse], - entity_registry: er.EntityRegistry, - entity_unique_id: str, -) -> None: - """Test an irrigation switch with no unique id.""" - - zone = hass.states.get("switch.rain_bird_sprinkler_3") - assert zone is not None - assert zone.attributes.get("friendly_name") == "Rain Bird Sprinkler 3" - assert zone.state == "off" - - entity_entry = entity_registry.async_get("switch.rain_bird_sprinkler_3") - assert entity_entry - assert entity_entry.unique_id == entity_unique_id