Address late review of ondilo_ico (#44837)

* Updates following comments in PR 44728

* Make all api calls in same thread context

* Set API as parameter to get_all_pools_data

* extract pools data retrieval function to api class

homeassistant/components/ondilo_ico/api.py

@@ -1,11 +1,14 @@
 """API for Ondilo ICO bound to Home Assistant OAuth."""
 from asyncio import run_coroutine_threadsafe
+import logging
 
 from ondilo import Ondilo
 
 from homeassistant import config_entries, core
 from homeassistant.helpers import config_entry_oauth2_flow
 
+_LOGGER = logging.getLogger(__name__)
+
 
 class OndiloClient(Ondilo):
     """Provide Ondilo ICO authentication tied to an OAuth2 based config entry."""
@@ -31,3 +34,17 @@ class OndiloClient(Ondilo):
         ).result()
 
         return self.session.token
+
+    def get_all_pools_data(self) -> dict:
+        """Fetch pools and add pool details and last measures to pool data."""
+
+        pools = self.get_pools()
+        for pool in pools:
+            _LOGGER.debug(
+                "Retrieving data for pool/spa: %s, id: %d", pool["name"], pool["id"]
+            )
+            pool["ICO"] = self.get_ICO_details(pool["id"])
+            pool["sensors"] = self.get_last_pool_measures(pool["id"])
+            _LOGGER.debug("Retrieved the following sensors data: %s", pool["sensors"])
+
+        return pools

homeassistant/components/ondilo_ico/manifest.json

@@ -6,9 +6,6 @@
   "requirements": [
     "ondilo==0.2.0"
   ],
-  "ssdp": [],
-  "zeroconf": [],
-  "homekit": {},
   "dependencies": [
     "http"
   ],

homeassistant/components/ondilo_ico/sensor.py

@@ -1,5 +1,4 @@
 """Platform for sensor integration."""
-import asyncio
 from datetime import timedelta
 import logging
 
@@ -25,24 +24,23 @@ SENSOR_TYPES = {
     "temperature": [
         "Temperature",
         TEMP_CELSIUS,
-        "mdi:thermometer",
+        None,
         DEVICE_CLASS_TEMPERATURE,
     ],
     "orp": ["Oxydo Reduction Potential", "mV", "mdi:pool", None],
     "ph": ["pH", "", "mdi:pool", None],
     "tds": ["TDS", CONCENTRATION_PARTS_PER_MILLION, "mdi:pool", None],
-    "battery": ["Battery", PERCENTAGE, "mdi:battery", DEVICE_CLASS_BATTERY],
+    "battery": ["Battery", PERCENTAGE, None, DEVICE_CLASS_BATTERY],
     "rssi": [
         "RSSI",
         PERCENTAGE,
-        "mdi:wifi-strength-2",
+        None,
         DEVICE_CLASS_SIGNAL_STRENGTH,
     ],
     "salt": ["Salt", "mg/L", "mdi:pool", None],
 }
 
 SCAN_INTERVAL = timedelta(hours=1)
-
 _LOGGER = logging.getLogger(__name__)
 
 
@@ -51,13 +49,6 @@ async def async_setup_entry(hass, entry, async_add_entities):
 
     api = hass.data[DOMAIN][entry.entry_id]
 
-    def get_all_pool_data(pool):
-        """Add pool details and last measures to pool data."""
-        pool["ICO"] = api.get_ICO_details(pool["id"])
-        pool["sensors"] = api.get_last_pool_measures(pool["id"])
-
-        return pool
-
     async def async_update_data():
         """Fetch data from API endpoint.
 
@@ -65,14 +56,7 @@ async def async_setup_entry(hass, entry, async_add_entities):
         so entities can quickly look up their data.
         """
         try:
-            pools = await hass.async_add_executor_job(api.get_pools)
+            return await hass.async_add_executor_job(api.get_all_pools_data)
-
-            return await asyncio.gather(
-                *[
-                    hass.async_add_executor_job(get_all_pool_data, pool)
-                    for pool in pools
-                ]
-            )
 
         except OndiloError as err:
             raise UpdateFailed(f"Error communicating with API: {err}") from err
@@ -145,11 +129,6 @@ class OndiloICO(CoordinatorEntity):
     @property
     def state(self):
         """Last value of the sensor."""
-        _LOGGER.debug(
-            "Retrieving Ondilo sensor %s state value: %s",
-            self._name,
-            self._devdata()["value"],
-        )
         return self._devdata()["value"]
 
     @property

tests/components/ondilo_ico/test_config_flow.py

@@ -2,7 +2,6 @@
 from unittest.mock import patch
 
 from homeassistant import config_entries, data_entry_flow, setup
-from homeassistant.components.ondilo_ico import config_flow
 from homeassistant.components.ondilo_ico.const import (
     DOMAIN,
     OAUTH2_AUTHORIZE,
@@ -23,9 +22,6 @@ async def test_abort_if_existing_entry(hass):
     """Check flow abort when an entry already exist."""
     MockConfigEntry(domain=DOMAIN).add_to_hass(hass)
 
-    flow = config_flow.OAuth2FlowHandler()
-    flow.hass = hass
-
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": config_entries.SOURCE_USER}
     )