Replace apcaccess dependency with aioapcaccess in apcupsd (#104571)

* Replace apcaccess dependency with async version aioapcaccess

* Upgrade the dependency to the latest version (v0.4.2)

* Handle asyncio.IncompleteReadError

homeassistant/components/apcupsd/coordinator.py

@@ -7,7 +7,7 @@ from datetime import timedelta
 import logging
 from typing import Final
 
-from apcaccess import status
+import aioapcaccess
 
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.core import HomeAssistant
@@ -90,13 +90,8 @@ class APCUPSdCoordinator(DataUpdateCoordinator[OrderedDict[str, str]]):
         Note that the result dict uses upper case for each resource, where our
         integration uses lower cases as keys internally.
         """
-
         async with asyncio.timeout(10):
             try:
-                raw = await self.hass.async_add_executor_job(
+                return await aioapcaccess.request_status(self._host, self._port)
-                    status.get, self._host, self._port
+            except (OSError, asyncio.IncompleteReadError) as error:
-                )
-                result: OrderedDict[str, str] = status.parse(raw)
-                return result
-            except OSError as error:
                 raise UpdateFailed(error) from error

homeassistant/components/apcupsd/manifest.json

@@ -7,5 +7,5 @@
   "iot_class": "local_polling",
   "loggers": ["apcaccess"],
   "quality_scale": "silver",
-  "requirements": ["apcaccess==0.0.13"]
+  "requirements": ["aioapcaccess==0.4.2"]
 }

requirements_all.txt

@@ -196,6 +196,9 @@ aioairzone==0.6.9
 # homeassistant.components.ambient_station
 aioambient==2023.04.0
 
+# homeassistant.components.apcupsd
+aioapcaccess==0.4.2
+
 # homeassistant.components.aseko_pool_live
 aioaseko==0.0.2
 
@@ -433,9 +436,6 @@ anova-wifi==0.10.0
 # homeassistant.components.anthemav
 anthemav==1.4.1
 
-# homeassistant.components.apcupsd
-apcaccess==0.0.13
-
 # homeassistant.components.weatherkit
 apple_weatherkit==1.1.2
 

requirements_test_all.txt

@@ -175,6 +175,9 @@ aioairzone==0.6.9
 # homeassistant.components.ambient_station
 aioambient==2023.04.0
 
+# homeassistant.components.apcupsd
+aioapcaccess==0.4.2
+
 # homeassistant.components.aseko_pool_live
 aioaseko==0.0.2
 
@@ -397,9 +400,6 @@ anova-wifi==0.10.0
 # homeassistant.components.anthemav
 anthemav==1.4.1
 
-# homeassistant.components.apcupsd
-apcaccess==0.0.13
-
 # homeassistant.components.weatherkit
 apple_weatherkit==1.1.2
 

tests/components/apcupsd/__init__.py

@@ -95,10 +95,7 @@ async def async_init_integration(
 
     entry.add_to_hass(hass)
 
-    with (
+    with patch("aioapcaccess.request_status", return_value=status):
-        patch("apcaccess.status.parse", return_value=status),
-        patch("apcaccess.status.get", return_value=b""),
-    ):
         assert await hass.config_entries.async_setup(entry.entry_id)
         await hass.async_block_till_done()
 

tests/components/apcupsd/test_config_flow.py

@@ -24,7 +24,7 @@ def _patch_setup():
 
 async def test_config_flow_cannot_connect(hass: HomeAssistant) -> None:
     """Test config flow setup with connection error."""
-    with patch("apcaccess.status.get") as mock_get:
+    with patch("aioapcaccess.request_status") as mock_get:
         mock_get.side_effect = OSError()
 
         result = await hass.config_entries.flow.async_init(
@@ -38,10 +38,7 @@ async def test_config_flow_cannot_connect(hass: HomeAssistant) -> None:
 
 async def test_config_flow_no_status(hass: HomeAssistant) -> None:
     """Test config flow setup with successful connection but no status is reported."""
-    with (
+    with patch("aioapcaccess.request_status", return_value={}):  # Returns no status.
-        patch("apcaccess.status.parse", return_value={}),  # Returns no status.
-        patch("apcaccess.status.get", return_value=b""),
-    ):
         result = await hass.config_entries.flow.async_init(
             DOMAIN, context={"source": SOURCE_USER}
         )
@@ -64,11 +61,10 @@ async def test_config_flow_duplicate(hass: HomeAssistant) -> None:
     mock_entry.add_to_hass(hass)
 
     with (
-        patch("apcaccess.status.parse") as mock_parse,
+        patch("aioapcaccess.request_status") as mock_request_status,
-        patch("apcaccess.status.get", return_value=b""),
         _patch_setup(),
     ):
-        mock_parse.return_value = MOCK_STATUS
+        mock_request_status.return_value = MOCK_STATUS
 
         # Now, create the integration again using the same config data, we should reject
         # the creation due same host / port.
@@ -98,7 +94,7 @@ async def test_config_flow_duplicate(hass: HomeAssistant) -> None:
         # Now we change the serial number and add it again. This should be successful.
         another_device_status = copy(MOCK_STATUS)
         another_device_status["SERIALNO"] = MOCK_STATUS["SERIALNO"] + "ZZZ"
-        mock_parse.return_value = another_device_status
+        mock_request_status.return_value = another_device_status
 
         result = await hass.config_entries.flow.async_init(
             DOMAIN,
@@ -112,8 +108,7 @@ async def test_config_flow_duplicate(hass: HomeAssistant) -> None:
 async def test_flow_works(hass: HomeAssistant) -> None:
     """Test successful creation of config entries via user configuration."""
     with (
-        patch("apcaccess.status.parse", return_value=MOCK_STATUS),
+        patch("aioapcaccess.request_status", return_value=MOCK_STATUS),
-        patch("apcaccess.status.get", return_value=b""),
         _patch_setup() as mock_setup,
     ):
         result = await hass.config_entries.flow.async_init(
@@ -152,12 +147,11 @@ async def test_flow_minimal_status(
     integration will vary.
     """
     with (
-        patch("apcaccess.status.parse") as mock_parse,
+        patch("aioapcaccess.request_status") as mock_request_status,
-        patch("apcaccess.status.get", return_value=b""),
         _patch_setup() as mock_setup,
     ):
         status = MOCK_MINIMAL_STATUS | extra_status
-        mock_parse.return_value = status
+        mock_request_status.return_value = status
 
         result = await hass.config_entries.flow.async_init(
             DOMAIN, context={CONF_SOURCE: SOURCE_USER}, data=CONF_DATA

tests/components/apcupsd/test_init.py

@@ -1,4 +1,5 @@
 """Test init of APCUPSd integration."""
+import asyncio
 from collections import OrderedDict
 from unittest.mock import patch
 
@@ -97,7 +98,11 @@ async def test_multiple_integrations(hass: HomeAssistant) -> None:
     assert state1.state != state2.state
 
 
-async def test_connection_error(hass: HomeAssistant) -> None:
+@pytest.mark.parametrize(
+    "error",
+    (OSError(), asyncio.IncompleteReadError(partial=b"", expected=0)),
+)
+async def test_connection_error(hass: HomeAssistant, error: Exception) -> None:
     """Test connection error during integration setup."""
     entry = MockConfigEntry(
         version=1,
@@ -109,10 +114,7 @@ async def test_connection_error(hass: HomeAssistant) -> None:
 
     entry.add_to_hass(hass)
 
-    with (
+    with patch("aioapcaccess.request_status", side_effect=error):
-        patch("apcaccess.status.parse", side_effect=OSError()),
-        patch("apcaccess.status.get"),
-    ):
         await hass.config_entries.async_setup(entry.entry_id)
         assert entry.state is ConfigEntryState.SETUP_RETRY
 
@@ -156,12 +158,9 @@ async def test_availability(hass: HomeAssistant) -> None:
     assert state.state != STATE_UNAVAILABLE
     assert pytest.approx(float(state.state)) == 14.0
 
-    with (
+    with patch("aioapcaccess.request_status") as mock_request_status:
-        patch("apcaccess.status.parse") as mock_parse,
-        patch("apcaccess.status.get", return_value=b""),
-    ):
         # Mock a network error and then trigger an auto-polling event.
-        mock_parse.side_effect = OSError()
+        mock_request_status.side_effect = OSError()
         future = utcnow() + UPDATE_INTERVAL
         async_fire_time_changed(hass, future)
         await hass.async_block_till_done()
@@ -172,8 +171,8 @@ async def test_availability(hass: HomeAssistant) -> None:
         assert state.state == STATE_UNAVAILABLE
 
         # Reset the API to return a new status and update.
-        mock_parse.side_effect = None
+        mock_request_status.side_effect = None
-        mock_parse.return_value = MOCK_STATUS | {"LOADPCT": "15.0 Percent"}
+        mock_request_status.return_value = MOCK_STATUS | {"LOADPCT": "15.0 Percent"}
         future = future + UPDATE_INTERVAL
         async_fire_time_changed(hass, future)
         await hass.async_block_till_done()

tests/components/apcupsd/test_sensor.py

@@ -127,10 +127,7 @@ async def test_state_update(hass: HomeAssistant) -> None:
     assert state.state == "14.0"
 
     new_status = MOCK_STATUS | {"LOADPCT": "15.0 Percent"}
-    with (
+    with patch("aioapcaccess.request_status", return_value=new_status):
-        patch("apcaccess.status.parse", return_value=new_status),
-        patch("apcaccess.status.get", return_value=b""),
-    ):
         future = utcnow() + timedelta(minutes=2)
         async_fire_time_changed(hass, future)
         await hass.async_block_till_done()
@@ -154,11 +151,8 @@ async def test_manual_update_entity(hass: HomeAssistant) -> None:
     # Setup HASS for calling the update_entity service.
     await async_setup_component(hass, "homeassistant", {})
 
-    with (
+    with patch("aioapcaccess.request_status") as mock_request_status:
-        patch("apcaccess.status.parse") as mock_parse,
+        mock_request_status.return_value = MOCK_STATUS | {
-        patch("apcaccess.status.get", return_value=b"") as mock_get,
-    ):
-        mock_parse.return_value = MOCK_STATUS | {
             "LOADPCT": "15.0 Percent",
             "BCHARGE": "99.0 Percent",
         }
@@ -174,8 +168,7 @@ async def test_manual_update_entity(hass: HomeAssistant) -> None:
         )
         # Even if we requested updates for two entities, our integration should smartly
         # group the API calls to just one.
-        assert mock_parse.call_count == 1
+        assert mock_request_status.call_count == 1
-        assert mock_get.call_count == 1
 
         # The new state should be effective.
         state = hass.states.get("sensor.ups_load")
@@ -194,10 +187,9 @@ async def test_multiple_manual_update_entity(hass: HomeAssistant) -> None:
     # Setup HASS for calling the update_entity service.
     await async_setup_component(hass, "homeassistant", {})
 
-    with (
+    with patch(
-        patch("apcaccess.status.parse", return_value=MOCK_STATUS) as mock_parse,
+        "aioapcaccess.request_status", return_value=MOCK_STATUS
-        patch("apcaccess.status.get", return_value=b"") as mock_get,
+    ) as mock_request_status:
-    ):
         # Fast-forward time to just pass the initial debouncer cooldown.
         future = utcnow() + timedelta(seconds=REQUEST_REFRESH_COOLDOWN)
         async_fire_time_changed(hass, future)
@@ -207,5 +199,4 @@ async def test_multiple_manual_update_entity(hass: HomeAssistant) -> None:
             {ATTR_ENTITY_ID: ["sensor.ups_load", "sensor.ups_input_voltage"]},
             blocking=True,
         )
-        assert mock_parse.call_count == 1
+        assert mock_request_status.call_count == 1
-        assert mock_get.call_count == 1