Fix rainbird unique id (#99704)

* Don't set a unique id for devices with no serial

* Add additional check for the same config entry host/port when there is no serial

* Update homeassistant/components/rainbird/config_flow.py

Co-authored-by: Robert Resch <robert@resch.dev>

* Update tests/components/rainbird/test_config_flow.py

Co-authored-by: Robert Resch <robert@resch.dev>

* Update tests/components/rainbird/test_config_flow.py

Co-authored-by: Robert Resch <robert@resch.dev>

---------

Co-authored-by: Robert Resch <robert@resch.dev>

homeassistant/components/rainbird/config_flow.py

@@ -125,8 +125,13 @@ class RainbirdConfigFlowHandler(config_entries.ConfigFlow, domain=DOMAIN):
         options: dict[str, Any],
     ) -> FlowResult:
         """Create the config entry."""
-        await self.async_set_unique_id(serial_number)
+        # Prevent devices with the same serial number. If the device does not have a serial number
-        self._abort_if_unique_id_configured()
+        # 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)
         return self.async_create_entry(
             title=data[CONF_HOST],
             data=data,

tests/components/rainbird/conftest.py

@@ -35,6 +35,7 @@ SERIAL_NUMBER = 0x12635436566
 
 # Get serial number Command 0x85. Serial is 0x12635436566
 SERIAL_RESPONSE = "850000012635436566"
+ZERO_SERIAL_RESPONSE = "850000000000000000"
 # Model and version command 0x82
 MODEL_AND_VERSION_RESPONSE = "820006090C"
 # Get available stations command 0x83
@@ -84,6 +85,12 @@ def yaml_config() -> dict[str, Any]:
     return {}
 
 
+@pytest.fixture
+async def unique_id() -> str:
+    """Fixture for serial number used in the config entry."""
+    return SERIAL_NUMBER
+
+
 @pytest.fixture
 async def config_entry_data() -> dict[str, Any]:
     """Fixture for MockConfigEntry data."""
@@ -92,13 +99,14 @@ async def config_entry_data() -> dict[str, Any]:
 
 @pytest.fixture
 async def config_entry(
-    config_entry_data: dict[str, Any] | None
+    config_entry_data: dict[str, Any] | None,
+    unique_id: str,
 ) -> MockConfigEntry | None:
     """Fixture for MockConfigEntry."""
     if config_entry_data is None:
         return None
     return MockConfigEntry(
-        unique_id=SERIAL_NUMBER,
+        unique_id=unique_id,
         domain=DOMAIN,
         data=config_entry_data,
         options={ATTR_DURATION: DEFAULT_TRIGGER_TIME_MINUTES},

tests/components/rainbird/test_config_flow.py

@@ -3,6 +3,7 @@
 import asyncio
 from collections.abc import Generator
 from http import HTTPStatus
+from typing import Any
 from unittest.mock import Mock, patch
 
 import pytest
@@ -19,8 +20,11 @@ from .conftest import (
     CONFIG_ENTRY_DATA,
     HOST,
     PASSWORD,
+    SERIAL_NUMBER,
     SERIAL_RESPONSE,
     URL,
+    ZERO_SERIAL_RESPONSE,
+    ComponentSetup,
     mock_response,
 )
 
@@ -66,19 +70,132 @@ async def complete_flow(hass: HomeAssistant) -> FlowResult:
     )
 
 
-async def test_controller_flow(hass: HomeAssistant, mock_setup: Mock) -> None:
+@pytest.mark.parametrize(
+    ("responses", "expected_config_entry", "expected_unique_id"),
+    [
+        (
+            [mock_response(SERIAL_RESPONSE)],
+            CONFIG_ENTRY_DATA,
+            SERIAL_NUMBER,
+        ),
+        (
+            [mock_response(ZERO_SERIAL_RESPONSE)],
+            {**CONFIG_ENTRY_DATA, "serial_number": 0},
+            None,
+        ),
+    ],
+)
+async def test_controller_flow(
+    hass: HomeAssistant,
+    mock_setup: Mock,
+    expected_config_entry: dict[str, str],
+    expected_unique_id: int | None,
+) -> None:
     """Test the controller is setup correctly."""
 
     result = await complete_flow(hass)
     assert result.get("type") == "create_entry"
     assert result.get("title") == HOST
     assert "result" in result
-    assert result["result"].data == CONFIG_ENTRY_DATA
+    assert dict(result["result"].data) == expected_config_entry
     assert result["result"].options == {ATTR_DURATION: 6}
+    assert result["result"].unique_id == expected_unique_id
 
     assert len(mock_setup.mock_calls) == 1
 
 
+@pytest.mark.parametrize(
+    (
+        "unique_id",
+        "config_entry_data",
+        "config_flow_responses",
+        "expected_config_entry",
+    ),
+    [
+        (
+            "other-serial-number",
+            {**CONFIG_ENTRY_DATA, "host": "other-host"},
+            [mock_response(SERIAL_RESPONSE)],
+            CONFIG_ENTRY_DATA,
+        ),
+        (
+            None,
+            {**CONFIG_ENTRY_DATA, "serial_number": 0, "host": "other-host"},
+            [mock_response(ZERO_SERIAL_RESPONSE)],
+            {**CONFIG_ENTRY_DATA, "serial_number": 0},
+        ),
+    ],
+    ids=["with-serial", "zero-serial"],
+)
+async def test_multiple_config_entries(
+    hass: HomeAssistant,
+    setup_integration: ComponentSetup,
+    responses: list[AiohttpClientMockResponse],
+    config_flow_responses: list[AiohttpClientMockResponse],
+    expected_config_entry: dict[str, Any] | None,
+) -> None:
+    """Test setting up multiple config entries that refer to different devices."""
+    assert await setup_integration()
+
+    entries = hass.config_entries.async_entries(DOMAIN)
+    assert len(entries) == 1
+    assert entries[0].state == ConfigEntryState.LOADED
+
+    responses.clear()
+    responses.extend(config_flow_responses)
+
+    result = await complete_flow(hass)
+    assert result.get("type") == FlowResultType.CREATE_ENTRY
+    assert dict(result.get("result").data) == expected_config_entry
+
+    entries = hass.config_entries.async_entries(DOMAIN)
+    assert len(entries) == 2
+
+
+@pytest.mark.parametrize(
+    (
+        "unique_id",
+        "config_entry_data",
+        "config_flow_responses",
+    ),
+    [
+        (
+            SERIAL_NUMBER,
+            CONFIG_ENTRY_DATA,
+            [mock_response(SERIAL_RESPONSE)],
+        ),
+        (
+            None,
+            {**CONFIG_ENTRY_DATA, "serial_number": 0},
+            [mock_response(ZERO_SERIAL_RESPONSE)],
+        ),
+    ],
+    ids=[
+        "duplicate-serial-number",
+        "duplicate-host-port-no-serial",
+    ],
+)
+async def test_duplicate_config_entries(
+    hass: HomeAssistant,
+    setup_integration: ComponentSetup,
+    responses: list[AiohttpClientMockResponse],
+    config_flow_responses: list[AiohttpClientMockResponse],
+) -> None:
+    """Test that a device can not be registered twice."""
+    assert await setup_integration()
+
+    entries = hass.config_entries.async_entries(DOMAIN)
+    assert len(entries) == 1
+    assert entries[0].state == ConfigEntryState.LOADED
+
+    responses.clear()
+    responses.extend(config_flow_responses)
+
+    result = await complete_flow(hass)
+    assert result.get("type") == FlowResultType.ABORT
+    assert result.get("reason") == "already_configured"
+
+
 async def test_controller_cannot_connect(
     hass: HomeAssistant,
     mock_setup: Mock,