From 36ad24ce01de8c4ecff000b4e914f5be46142a50 Mon Sep 17 00:00:00 2001 From: Yuxin Wang Date: Mon, 24 Jul 2023 12:42:08 -0400 Subject: [PATCH] Add name and default name to device info of APCUPSD sensors (#94415) --- homeassistant/components/apcupsd/__init__.py | 26 ++++++---- .../components/apcupsd/binary_sensor.py | 10 +--- homeassistant/components/apcupsd/sensor.py | 9 +--- tests/components/apcupsd/__init__.py | 1 + tests/components/apcupsd/test_config_flow.py | 2 +- tests/components/apcupsd/test_init.py | 48 +++++++++++++++++++ 6 files changed, 69 insertions(+), 27 deletions(-) diff --git a/homeassistant/components/apcupsd/__init__.py b/homeassistant/components/apcupsd/__init__.py index 3fb8bf00b8a..bfe6fe6c80c 100644 --- a/homeassistant/components/apcupsd/__init__.py +++ b/homeassistant/components/apcupsd/__init__.py @@ -11,6 +11,7 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_HOST, CONF_PORT, Platform from homeassistant.core import HomeAssistant import homeassistant.helpers.config_validation as cv +from homeassistant.helpers.entity import DeviceInfo from homeassistant.util import Throttle _LOGGER = logging.getLogger(__name__) @@ -79,16 +80,6 @@ class APCUPSdData: return self.status[model_key] return None - @property - def sw_version(self) -> str | None: - """Return the software version of the APCUPSd, if available.""" - return self.status.get("VERSION") - - @property - def hw_version(self) -> str | None: - """Return the firmware version of the UPS, if available.""" - return self.status.get("FIRMWARE") - @property def serial_no(self) -> str | None: """Return the unique serial number of the UPS, if available.""" @@ -99,6 +90,21 @@ class APCUPSdData: """Return the STATFLAG indicating the status of the UPS, if available.""" return self.status.get("STATFLAG") + @property + def device_info(self) -> DeviceInfo | None: + """Return the DeviceInfo of this APC UPS for the sensors, if serial number is available.""" + if self.serial_no is None: + return None + + return DeviceInfo( + identifiers={(DOMAIN, self.serial_no)}, + model=self.model, + manufacturer="APC", + name=self.name if self.name is not None else "APC UPS", + hw_version=self.status.get("FIRMWARE"), + sw_version=self.status.get("VERSION"), + ) + @Throttle(MIN_TIME_BETWEEN_UPDATES) def update(self, **kwargs: Any) -> None: """Fetch the latest status from APCUPSd. diff --git a/homeassistant/components/apcupsd/binary_sensor.py b/homeassistant/components/apcupsd/binary_sensor.py index d45ad561d8d..bac8d18d58b 100644 --- a/homeassistant/components/apcupsd/binary_sensor.py +++ b/homeassistant/components/apcupsd/binary_sensor.py @@ -9,7 +9,6 @@ from homeassistant.components.binary_sensor import ( ) from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant -from homeassistant.helpers.entity import DeviceInfo from homeassistant.helpers.entity_platform import AddEntitiesCallback from . import DOMAIN, VALUE_ONLINE, APCUPSdData @@ -53,13 +52,8 @@ class OnlineStatus(BinarySensorEntity): # Set up unique id and device info if serial number is available. if (serial_no := data_service.serial_no) is not None: self._attr_unique_id = f"{serial_no}_{description.key}" - self._attr_device_info = DeviceInfo( - identifiers={(DOMAIN, serial_no)}, - model=data_service.model, - manufacturer="APC", - hw_version=data_service.hw_version, - sw_version=data_service.sw_version, - ) + self._attr_device_info = data_service.device_info + self.entity_description = description self._data_service = data_service diff --git a/homeassistant/components/apcupsd/sensor.py b/homeassistant/components/apcupsd/sensor.py index 8b7034357df..745be7e2d63 100644 --- a/homeassistant/components/apcupsd/sensor.py +++ b/homeassistant/components/apcupsd/sensor.py @@ -21,7 +21,6 @@ from homeassistant.const import ( UnitOfTime, ) from homeassistant.core import HomeAssistant -from homeassistant.helpers.entity import DeviceInfo from homeassistant.helpers.entity_platform import AddEntitiesCallback from . import DOMAIN, APCUPSdData @@ -496,13 +495,7 @@ class APCUPSdSensor(SensorEntity): # Set up unique id and device info if serial number is available. if (serial_no := data_service.serial_no) is not None: self._attr_unique_id = f"{serial_no}_{description.key}" - self._attr_device_info = DeviceInfo( - identifiers={(DOMAIN, serial_no)}, - model=data_service.model, - manufacturer="APC", - hw_version=data_service.hw_version, - sw_version=data_service.sw_version, - ) + self._attr_device_info = data_service.device_info self.entity_description = description self._data_service = data_service diff --git a/tests/components/apcupsd/__init__.py b/tests/components/apcupsd/__init__.py index f5c3f573030..b8a83f950d0 100644 --- a/tests/components/apcupsd/__init__.py +++ b/tests/components/apcupsd/__init__.py @@ -20,6 +20,7 @@ MOCK_STATUS: Final = OrderedDict( ("CABLE", "USB Cable"), ("DRIVER", "USB UPS Driver"), ("UPSMODE", "Stand Alone"), + ("UPSNAME", "MyUPS"), ("MODEL", "Back-UPS ES 600"), ("STATUS", "ONLINE"), ("LINEV", "124.0 Volts"), diff --git a/tests/components/apcupsd/test_config_flow.py b/tests/components/apcupsd/test_config_flow.py index a9ef4328e86..6ac7992f404 100644 --- a/tests/components/apcupsd/test_config_flow.py +++ b/tests/components/apcupsd/test_config_flow.py @@ -124,7 +124,7 @@ async def test_flow_works(hass: HomeAssistant) -> None: ) await hass.async_block_till_done() assert result["type"] == FlowResultType.CREATE_ENTRY - assert result["title"] == MOCK_STATUS["MODEL"] + assert result["title"] == MOCK_STATUS["UPSNAME"] assert result["data"] == CONF_DATA mock_setup.assert_called_once() diff --git a/tests/components/apcupsd/test_init.py b/tests/components/apcupsd/test_init.py index 6e00a382e79..8c29edabbc1 100644 --- a/tests/components/apcupsd/test_init.py +++ b/tests/components/apcupsd/test_init.py @@ -8,6 +8,7 @@ from homeassistant.components.apcupsd import DOMAIN from homeassistant.config_entries import SOURCE_USER, ConfigEntryState from homeassistant.const import STATE_UNAVAILABLE from homeassistant.core import HomeAssistant +from homeassistant.helpers import device_registry as dr from . import CONF_DATA, MOCK_MINIMAL_STATUS, MOCK_STATUS, async_init_integration @@ -28,6 +29,53 @@ async def test_async_setup_entry(hass: HomeAssistant, status: OrderedDict) -> No assert state.state == "on" +@pytest.mark.parametrize( + "status", + ( + # We should not create device entries if SERIALNO is not reported. + MOCK_MINIMAL_STATUS, + # 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". + MOCK_MINIMAL_STATUS | {"SERIALNO": "XXXX"}, + # We should create all fields of the device entry if they are available. + MOCK_STATUS, + ), +) +async def test_device_entry(hass: HomeAssistant, status: OrderedDict) -> None: + """Test successful setup of device entries.""" + await async_init_integration(hass, status=status) + + # Verify device info is properly set up. + device_entries = dr.async_get(hass) + + if "SERIALNO" not in status: + assert len(device_entries.devices) == 0 + return + + assert len(device_entries.devices) == 1 + entry = device_entries.async_get_device({(DOMAIN, status["SERIALNO"])}) + assert entry is not None + # Specify the mapping between field name and the expected fields in device entry. + fields = { + "UPSNAME": entry.name, + "MODEL": entry.model, + "VERSION": entry.sw_version, + "FIRMWARE": entry.hw_version, + } + + for field, entry_value in fields.items(): + if field in status: + assert entry_value == status[field] + elif field == "UPSNAME": + # Even if UPSNAME is not available, we must fall back to default "APC UPS". + assert entry_value == "APC UPS" + else: + assert entry_value is None + + assert entry.manufacturer == "APC" + + async def test_multiple_integrations(hass: HomeAssistant) -> None: """Test successful setup for multiple entries.""" # Load two integrations from two mock hosts.