From 43d8c0bb6e07185ea46dd6c18de9bbfc2fc1a661 Mon Sep 17 00:00:00 2001 From: Yuxin Wang Date: Wed, 22 Jan 2025 23:10:52 -0500 Subject: [PATCH] Fallback to None for literal "Blank" serial number for APCUPSD integration (#136297) * Fallback to None for Blank serial number * Fix comments --- homeassistant/components/apcupsd/coordinator.py | 5 ++++- tests/components/apcupsd/test_config_flow.py | 2 ++ tests/components/apcupsd/test_init.py | 8 ++++++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/apcupsd/coordinator.py b/homeassistant/components/apcupsd/coordinator.py index 768e9605967..1ae12d8c4b0 100644 --- a/homeassistant/components/apcupsd/coordinator.py +++ b/homeassistant/components/apcupsd/coordinator.py @@ -44,7 +44,10 @@ class APCUPSdData(dict[str, str]): @property def serial_no(self) -> str | None: """Return the unique serial number of the UPS, if available.""" - return self.get("SERIALNO") + sn = self.get("SERIALNO") + # We had user reports that some UPS models simply return "Blank" as serial number, in + # which case we fall back to `None` to indicate that it is actually not available. + return None if sn == "Blank" else sn class APCUPSdCoordinator(DataUpdateCoordinator[APCUPSdData]): diff --git a/tests/components/apcupsd/test_config_flow.py b/tests/components/apcupsd/test_config_flow.py index 88594260579..0b8386dbb5a 100644 --- a/tests/components/apcupsd/test_config_flow.py +++ b/tests/components/apcupsd/test_config_flow.py @@ -125,6 +125,8 @@ async def test_flow_works(hass: HomeAssistant) -> None: ({"UPSNAME": "Friendly Name"}, "Friendly Name"), ({"MODEL": "MODEL X"}, "MODEL X"), ({"SERIALNO": "ZZZZ"}, "ZZZZ"), + # Some models report "Blank" as serial number, which we should treat it as not reported. + ({"SERIALNO": "Blank"}, "APC UPS"), ({}, "APC UPS"), ], ) diff --git a/tests/components/apcupsd/test_init.py b/tests/components/apcupsd/test_init.py index 723ec164eae..6bb94ca2948 100644 --- a/tests/components/apcupsd/test_init.py +++ b/tests/components/apcupsd/test_init.py @@ -31,6 +31,8 @@ from tests.common import MockConfigEntry, async_fire_time_changed # Does not contain either "SERIALNO" field. # We should _not_ create devices for the entities and their IDs will not have prefixes. MOCK_MINIMAL_STATUS, + # Some models report "Blank" as SERIALNO, but we should treat it as not reported. + MOCK_MINIMAL_STATUS | {"SERIALNO": "Blank"}, ], ) async def test_async_setup_entry(hass: HomeAssistant, status: OrderedDict) -> None: @@ -41,7 +43,7 @@ async def test_async_setup_entry(hass: HomeAssistant, status: OrderedDict) -> No await async_init_integration(hass, status=status) prefix = "" - if "SERIALNO" in status: + if "SERIALNO" in status and status["SERIALNO"] != "Blank": prefix = slugify(status.get("UPSNAME", "APC UPS")) + "_" # Verify successful setup by querying the status sensor. @@ -56,6 +58,8 @@ async def test_async_setup_entry(hass: HomeAssistant, status: OrderedDict) -> No [ # We should not create device entries if SERIALNO is not reported. MOCK_MINIMAL_STATUS, + # Some models report "Blank" as SERIALNO, but we should treat it as not reported. + MOCK_MINIMAL_STATUS | {"SERIALNO": "Blank"}, # We should set the device name to be the friendly UPSNAME field if available. MOCK_MINIMAL_STATUS | {"SERIALNO": "XXXX", "UPSNAME": "MyUPS"}, # Otherwise, we should fall back to default device name --- "APC UPS". @@ -71,7 +75,7 @@ async def test_device_entry( await async_init_integration(hass, status=status) # Verify device info is properly set up. - if "SERIALNO" not in status: + if "SERIALNO" not in status or status["SERIALNO"] == "Blank": assert len(device_registry.devices) == 0 return