Fallback to config entry ID as unique ID when serialno is not available for APCUPSD (#130852)

homeassistant/components/apcupsd/binary_sensor.py

@@ -53,10 +53,8 @@ class OnlineStatus(CoordinatorEntity[APCUPSdCoordinator], BinarySensorEntity):
         """Initialize the APCUPSd binary device."""
         super().__init__(coordinator, context=description.key.upper())
 
-        # Set up unique id and device info if serial number is available.
-        if (serial_no := coordinator.data.serial_no) is not None:
-            self._attr_unique_id = f"{serial_no}_{description.key}"
         self.entity_description = description
+        self._attr_unique_id = f"{coordinator.unique_device_id}_{description.key}"
         self._attr_device_info = coordinator.device_info
 
     @property

homeassistant/components/apcupsd/coordinator.py

@@ -85,11 +85,16 @@ class APCUPSdCoordinator(DataUpdateCoordinator[APCUPSdData]):
         self._host = host
         self._port = port
 
+    @property
+    def unique_device_id(self) -> str:
+        """Return a unique ID of the device, which is the serial number (if available) or the config entry ID."""
+        return self.data.serial_no or self.config_entry.entry_id
+
     @property
     def device_info(self) -> DeviceInfo:
         """Return the DeviceInfo of this APC UPS, if serial number is available."""
         return DeviceInfo(
-            identifiers={(DOMAIN, self.data.serial_no or self.config_entry.entry_id)},
+            identifiers={(DOMAIN, self.unique_device_id)},
             model=self.data.model,
             manufacturer="APC",
             name=self.data.name or "APC UPS",

homeassistant/components/apcupsd/sensor.py

@@ -458,11 +458,8 @@ class APCUPSdSensor(CoordinatorEntity[APCUPSdCoordinator], SensorEntity):
         """Initialize the sensor."""
         super().__init__(coordinator=coordinator, context=description.key.upper())
 
-        # Set up unique id and device info if serial number is available.
-        if (serial_no := coordinator.data.serial_no) is not None:
-            self._attr_unique_id = f"{serial_no}_{description.key}"
-
         self.entity_description = description
+        self._attr_unique_id = f"{coordinator.unique_device_id}_{description.key}"
         self._attr_device_info = coordinator.device_info
 
         # Initial update of attributes.

tests/components/apcupsd/__init__.py

@@ -1,10 +1,13 @@
 """Tests for the APCUPSd component."""
 
+from __future__ import annotations
+
 from collections import OrderedDict
 from typing import Final
 from unittest.mock import patch
 
 from homeassistant.components.apcupsd.const import DOMAIN
+from homeassistant.components.apcupsd.coordinator import APCUPSdData
 from homeassistant.config_entries import SOURCE_USER
 from homeassistant.const import CONF_HOST, CONF_PORT
 from homeassistant.core import HomeAssistant
@@ -79,7 +82,7 @@ MOCK_MINIMAL_STATUS: Final = OrderedDict(
 
 
 async def async_init_integration(
-    hass: HomeAssistant, host: str = "test", status=None
+    hass: HomeAssistant, host: str = "test", status: dict[str, str] | None = None
 ) -> MockConfigEntry:
     """Set up the APC UPS Daemon integration in HomeAssistant."""
     if status is None:
@@ -90,7 +93,7 @@ async def async_init_integration(
         domain=DOMAIN,
         title="APCUPSd",
         data=CONF_DATA | {CONF_HOST: host},
-        unique_id=status.get("SERIALNO", None),
+        unique_id=APCUPSdData(status).serial_no,
         source=SOURCE_USER,
     )
 

tests/components/apcupsd/test_init.py

@@ -28,8 +28,8 @@ from tests.common import MockConfigEntry, async_fire_time_changed
         # Contains "SERIALNO" but no "UPSNAME" field.
         # We should create devices for the entities and prefix their IDs with default "APC UPS".
         MOCK_MINIMAL_STATUS | {"SERIALNO": "XXXX"},
-        # Does not contain either "SERIALNO" field.
+        # Does not contain either "SERIALNO" field or "UPSNAME" field. Our integration should work
-        # We should _not_ create devices for the entities and their IDs will not have prefixes.
+        # fine without it by falling back to config entry ID as unique ID and "APC UPS" as default name.
         MOCK_MINIMAL_STATUS,
         # Some models report "Blank" as SERIALNO, but we should treat it as not reported.
         MOCK_MINIMAL_STATUS | {"SERIALNO": "Blank"},
@@ -37,13 +37,8 @@ from tests.common import MockConfigEntry, async_fire_time_changed
 )
 async def test_async_setup_entry(hass: HomeAssistant, status: OrderedDict) -> None:
     """Test a successful setup entry."""
-    # Minimal status does not contain "SERIALNO" field, which is used to determine the
-    # unique ID of this integration. But, the integration should work fine without it.
-    # In such a case, the device will not be added either
     await async_init_integration(hass, status=status)
 
-    prefix = ""
-    if "SERIALNO" in status and status["SERIALNO"] != "Blank":
     prefix = slugify(status.get("UPSNAME", "APC UPS")) + "_"
 
     # Verify successful setup by querying the status sensor.
@@ -72,15 +67,13 @@ async def test_device_entry(
     hass: HomeAssistant, status: OrderedDict, device_registry: dr.DeviceRegistry
 ) -> None:
     """Test successful setup of device entries."""
-    await async_init_integration(hass, status=status)
+    config_entry = await async_init_integration(hass, status=status)
 
     # Verify device info is properly set up.
-    if "SERIALNO" not in status or status["SERIALNO"] == "Blank":
-        assert len(device_registry.devices) == 0
-        return
-
     assert len(device_registry.devices) == 1
-    entry = device_registry.async_get_device({(DOMAIN, status["SERIALNO"])})
+    entry = device_registry.async_get_device(
+        {(DOMAIN, config_entry.unique_id or config_entry.entry_id)}
+    )
     assert entry is not None
     # Specify the mapping between field name and the expected fields in device entry.
     fields = {

tests/components/apcupsd/test_sensor.py

@@ -244,11 +244,14 @@ async def test_sensor_unknown(hass: HomeAssistant) -> None:
     """Test if our integration can properly certain sensors as unknown when it becomes so."""
     await async_init_integration(hass, status=MOCK_MINIMAL_STATUS)
 
-    assert hass.states.get("sensor.mode").state == MOCK_MINIMAL_STATUS["UPSMODE"]
+    ups_mode_id = "sensor.apc_ups_mode"
+    last_self_test_id = "sensor.apc_ups_last_self_test"
+
+    assert hass.states.get(ups_mode_id).state == MOCK_MINIMAL_STATUS["UPSMODE"]
     # Last self test sensor should be added even if our status does not report it initially (it is
     # a sensor that appears only after a periodical or manual self test is performed).
-    assert hass.states.get("sensor.last_self_test") is not None
+    assert hass.states.get(last_self_test_id) is not None
-    assert hass.states.get("sensor.last_self_test").state == STATE_UNKNOWN
+    assert hass.states.get(last_self_test_id).state == STATE_UNKNOWN
 
     # Simulate an event (a self test) such that "LASTSTEST" field is being reported, the state of
     # the sensor should be properly updated with the corresponding value.
@@ -259,7 +262,7 @@ async def test_sensor_unknown(hass: HomeAssistant) -> None:
         future = utcnow() + timedelta(minutes=2)
         async_fire_time_changed(hass, future)
         await hass.async_block_till_done()
-    assert hass.states.get("sensor.last_self_test").state == "1970-01-01 00:00:00 0000"
+    assert hass.states.get(last_self_test_id).state == "1970-01-01 00:00:00 0000"
 
     # Simulate another event (e.g., daemon restart) such that "LASTSTEST" is no longer reported.
     with patch("aioapcaccess.request_status") as mock_request_status:
@@ -268,4 +271,4 @@ async def test_sensor_unknown(hass: HomeAssistant) -> None:
         async_fire_time_changed(hass, future)
         await hass.async_block_till_done()
     # The state should become unknown again.
-    assert hass.states.get("sensor.last_self_test").state == STATE_UNKNOWN
+    assert hass.states.get(last_self_test_id).state == STATE_UNKNOWN