Address open review issues in sharkiq integration (#39504)

homeassistant/components/sharkiq/__init__.py

@@ -10,7 +10,6 @@ from sharkiqpy import (
     SharkIqNotAuthedError,
     get_ayla_api,
 )
-import voluptuous as vol
 
 from homeassistant import exceptions
 from homeassistant.const import CONF_PASSWORD, CONF_USERNAME
@@ -18,8 +17,6 @@ from homeassistant.const import CONF_PASSWORD, CONF_USERNAME
 from .const import API_TIMEOUT, COMPONENTS, DOMAIN, LOGGER
 from .update_coordinator import SharkIqUpdateCoordinator
 
-CONFIG_SCHEMA = vol.Schema({DOMAIN: vol.Schema({})}, extra=vol.ALLOW_EXTRA)
-
 
 class CannotConnect(exceptions.HomeAssistantError):
     """Error to indicate we cannot connect."""
@@ -28,7 +25,6 @@ class CannotConnect(exceptions.HomeAssistantError):
 async def async_setup(hass, config):
     """Set up the sharkiq environment."""
     hass.data.setdefault(DOMAIN, {})
-    if DOMAIN not in config:
     return True
 
 
@@ -38,11 +34,11 @@ async def async_connect_or_timeout(ayla_api: AylaApi) -> bool:
         with async_timeout.timeout(API_TIMEOUT):
             LOGGER.debug("Initialize connection to Ayla networks API")
             await ayla_api.async_sign_in()
-    except SharkIqAuthError as exc:
+    except SharkIqAuthError:
-        LOGGER.error("Authentication error connecting to Shark IQ api", exc_info=exc)
+        LOGGER.error("Authentication error connecting to Shark IQ api")
         return False
     except asyncio.TimeoutError as exc:
-        LOGGER.error("Timeout expired", exc_info=exc)
+        LOGGER.error("Timeout expired")
         raise CannotConnect from exc
 
     return True
@@ -90,7 +86,6 @@ async def async_disconnect_or_timeout(coordinator: SharkIqUpdateCoordinator):
             await coordinator.ayla_api.async_sign_out()
         except (SharkIqAuthError, SharkIqAuthExpiringError, SharkIqNotAuthedError):
             pass
-    return True
 
 
 async def async_update_options(hass, config_entry):

homeassistant/components/sharkiq/config_flow.py

@@ -50,7 +50,6 @@ class SharkIqConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
         errors = {}
         info = None
 
-        if user_input is not None:
         # noinspection PyBroadException
         try:
             info = await validate_input(self.hass, user_input)
@@ -69,6 +68,8 @@ class SharkIqConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
         if user_input is not None:
             info, errors = await self._async_validate_input(user_input)
             if info:
+                await self.async_set_unique_id(user_input[CONF_USERNAME])
+                self._abort_if_unique_id_configured()
                 return self.async_create_entry(title=info["title"], data=user_input)
 
         return self.async_show_form(

homeassistant/components/sharkiq/manifest.json

@@ -4,6 +4,5 @@
   "config_flow": true,
   "documentation": "https://www.home-assistant.io/integrations/sharkiq",
   "requirements": ["sharkiqpy==0.1.8"],
-  "dependencies": [],
   "codeowners": ["@ajmarks"]
 }

homeassistant/components/sharkiq/strings.json

@@ -6,6 +6,12 @@
           "username": "[%key:common::config_flow::data::username%]",
           "password": "[%key:common::config_flow::data::password%]"
         }
+      },
+      "reauth": {
+        "data": {
+          "username": "[%key:common::config_flow::data::username%]",
+          "password": "[%key:common::config_flow::data::password%]"
+        }
       }
     },
     "error": {
@@ -14,7 +20,10 @@
       "unknown": "[%key:common::config_flow::error::unknown%]"
     },
     "abort": {
-      "already_configured_account": "[%key:common::config_flow::abort::already_configured_account%]"
+      "already_configured_account": "[%key:common::config_flow::abort::already_configured_account%]",
+      "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]",
+      "reauth_successful": "[%key:common::config_flow::data::access_token%] updated successfully",
+      "unknown": "[%key:common::config_flow::error::unknown%]"
     }
   }
 }

homeassistant/components/sharkiq/translations/en.json

@@ -1,7 +1,8 @@
 {
     "config": {
         "abort": {
-            "already_configured_account": "Account is already configured"
+            "already_configured_account": "Account is already configured",
+            "reauth_successful": "Reauthentication successful"
         },
         "error": {
             "cannot_connect": "Failed to connect",
@@ -9,6 +10,12 @@
             "unknown": "Unexpected error"
         },
         "step": {
+            "reauth": {
+                "data": {
+                    "password": "Password",
+                    "username": "Username"
+                }
+            },
             "user": {
                 "data": {
                     "password": "Password",

homeassistant/components/sharkiq/update_coordinator.py

@@ -1,5 +1,6 @@
 """Data update coordinator for shark iq vacuums."""
 
+import asyncio
 from typing import Dict, List, Set
 
 from async_timeout import timeout
@@ -30,7 +31,7 @@ class SharkIqUpdateCoordinator(DataUpdateCoordinator):
     ) -> None:
         """Set up the SharkIqUpdateCoordinator class."""
         self.ayla_api = ayla_api
-        self.shark_vacs: Dict[SharkIqVacuum] = {
+        self.shark_vacs: Dict[str, SharkIqVacuum] = {
             sharkiq.serial_number: sharkiq for sharkiq in shark_vacs
         }
         self._config_entry = config_entry
@@ -51,7 +52,7 @@ class SharkIqUpdateCoordinator(DataUpdateCoordinator):
     async def _async_update_vacuum(sharkiq: SharkIqVacuum) -> None:
         """Asynchronously update the data for a single vacuum."""
         dsn = sharkiq.serial_number
-        LOGGER.info("Updating sharkiq data for device DSN %s", dsn)
+        LOGGER.debug("Updating sharkiq data for device DSN %s", dsn)
         with timeout(API_TIMEOUT):
             await sharkiq.async_update()
 
@@ -65,15 +66,15 @@ class SharkIqUpdateCoordinator(DataUpdateCoordinator):
                 if v["connection_status"] == "Online" and v["dsn"] in self.shark_vacs
             }
 
-            LOGGER.info("Updating sharkiq data")
+            LOGGER.debug("Updating sharkiq data")
-            for dsn in self._online_dsns:
+            online_vacs = (self.shark_vacs[dsn] for dsn in self.online_dsns)
-                await self._async_update_vacuum(self.shark_vacs[dsn])
+            await asyncio.gather(*[self._async_update_vacuum(v) for v in online_vacs])
         except (
             SharkIqAuthError,
             SharkIqNotAuthedError,
             SharkIqAuthExpiringError,
         ) as err:
-            LOGGER.exception("Bad auth state", exc_info=err)
+            LOGGER.exception("Bad auth state")
             flow_context = {
                 "source": "reauth",
                 "unique_id": self._config_entry.unique_id,
@@ -96,7 +97,7 @@ class SharkIqUpdateCoordinator(DataUpdateCoordinator):
 
             raise UpdateFailed(err) from err
         except Exception as err:  # pylint: disable=broad-except
-            LOGGER.exception("Unexpected error updating SharkIQ", exc_info=err)
+            LOGGER.exception("Unexpected error updating SharkIQ")
             raise UpdateFailed(err) from err
 
         return True

homeassistant/components/sharkiq/vacuum.py

@@ -66,16 +66,25 @@ ATTR_RECHARGE_RESUME = "recharge_and_resume"
 ATTR_RSSI = "rssi"
 
 
+async def async_setup_entry(hass, config_entry, async_add_entities):
+    """Set up the Shark IQ vacuum cleaner."""
+    coordinator: SharkIqUpdateCoordinator = hass.data[DOMAIN][config_entry.entry_id]
+    devices: Iterable["SharkIqVacuum"] = coordinator.shark_vacs.values()
+    device_names = [d.name for d in devices]
+    LOGGER.debug(
+        "Found %d Shark IQ device(s): %s",
+        len(device_names),
+        ", ".join([d.name for d in devices]),
+    )
+    async_add_entities([SharkVacuumEntity(d, coordinator) for d in devices])
+
+
 class SharkVacuumEntity(CoordinatorEntity, StateVacuumEntity):
     """Shark IQ vacuum entity."""
 
     def __init__(self, sharkiq: SharkIqVacuum, coordinator: SharkIqUpdateCoordinator):
         """Create a new SharkVacuumEntity."""
         super().__init__(coordinator)
-        if sharkiq.serial_number not in coordinator.shark_vacs:
-            raise RuntimeError(
-                f"Shark IQ robot {sharkiq.serial_number} is not known to the coordinator"
-            )
         self.sharkiq = sharkiq
 
     def clean_spot(self, **kwargs):
@@ -163,8 +172,6 @@ class SharkVacuumEntity(CoordinatorEntity, StateVacuumEntity):
         In the app, these are (usually) handled by showing the robot as stopped and sending the
         user a notification.
         """
-        if self.recharging_to_resume:
-            return STATE_RECHARGING_TO_RESUME
         if self.is_docked:
             return STATE_DOCKED
         return self.operating_mode
@@ -229,7 +236,7 @@ class SharkVacuumEntity(CoordinatorEntity, StateVacuumEntity):
     @property
     def fan_speed_list(self):
         """Get the list of available fan speed steps of the vacuum cleaner."""
-        return list(FAN_SPEEDS_MAP.keys())
+        return list(FAN_SPEEDS_MAP)
 
     # Various attributes we want to expose
     @property
@@ -257,16 +264,3 @@ class SharkVacuumEntity(CoordinatorEntity, StateVacuumEntity):
             ATTR_RECHARGE_RESUME: self.recharge_resume,
         }
         return data
-
-
-async def async_setup_entry(hass, config_entry, async_add_entities):
-    """Set up the Shark IQ vacuum cleaner."""
-    coordinator: SharkIqUpdateCoordinator = hass.data[DOMAIN][config_entry.entry_id]
-    devices: Iterable["SharkIqVacuum"] = coordinator.shark_vacs.values()
-    device_names = [d.name for d in devices]
-    LOGGER.debug(
-        "Found %d Shark IQ device(s): %s",
-        len(device_names),
-        ", ".join([d.name for d in devices]),
-    )
-    async_add_entities([SharkVacuumEntity(d, coordinator) for d in devices])

tests/components/sharkiq/test_config_flow.py

@@ -38,7 +38,6 @@ async def test_form(hass):
             result["flow_id"],
             CONFIG,
         )
-        await hass.async_block_till_done()
 
     assert result2["type"] == "create_entry"
     assert result2["title"] == f"{TEST_USERNAME:s}"
@@ -118,7 +117,6 @@ async def test_reauth(hass):
     ), patch("sharkiqpy.AylaApi.async_sign_in", return_value=True):
         mock_config = MockConfigEntry(domain=DOMAIN, unique_id=UNIQUE_ID, data=CONFIG)
         mock_config.add_to_hass(hass)
-        hass.config_entries.async_update_entry(mock_config, data=CONFIG)
 
         result = await hass.config_entries.flow.async_init(
             DOMAIN, context={"source": "reauth", "unique_id": UNIQUE_ID}, data=CONFIG

tests/components/sharkiq/test_shark_iq.py

@@ -1,6 +1,7 @@
 """Test the Shark IQ vacuum entity."""
 from copy import deepcopy
 import enum
+import json
 from typing import Dict, List
 
 from sharkiqpy import AylaApi, Properties, SharkIqAuthError, SharkIqVacuum, get_ayla_api
@@ -11,7 +12,6 @@ from homeassistant.components.sharkiq.vacuum import (
     ATTR_ERROR_MSG,
     ATTR_LOW_LIGHT,
     ATTR_RECHARGE_RESUME,
-    STATE_RECHARGING_TO_RESUME,
     SharkVacuumEntity,
 )
 from homeassistant.components.vacuum import (
@@ -44,12 +44,6 @@ from .const import (
 
 from tests.async_mock import MagicMock, patch
 
-try:
-    import ujson as json
-except ImportError:
-    import json
-
-
 MockAyla = MagicMock(spec=AylaApi)  # pylint: disable=invalid-name
 
 
@@ -109,7 +103,7 @@ async def test_shark_operation_modes(hass: HomeAssistant) -> None:
     shark.sharkiq.set_property_value(Properties.DOCKED_STATUS, 1)
     assert isinstance(shark.is_docked, bool) and shark.is_docked
     assert isinstance(shark.recharging_to_resume, bool) and shark.recharging_to_resume
-    assert shark.state == STATE_RECHARGING_TO_RESUME
+    assert shark.state == STATE_DOCKED
 
     shark.sharkiq.set_property_value(Properties.RECHARGING_TO_RESUME, 0)
     assert shark.state == STATE_DOCKED
@@ -175,9 +169,8 @@ async def test_shark_metadata(hass: HomeAssistant) -> None:
         "model": "RV1001AE",
         "sw_version": "Dummy Firmware 1.0",
     }
-    state_json = json.dumps(shark.device_info, sort_keys=True)
+
-    target_json = json.dumps(target_device_info, sort_keys=True)
+    assert shark.device_info == target_device_info
-    assert state_json == target_json
 
 
 def _get_async_update(err=None):
@@ -225,29 +218,20 @@ async def test_coordinator_match(hass: HomeAssistant):
 
     coordinator = SharkIqUpdateCoordinator(hass, None, ayla_api, [shark_vac1])
 
-    # The first should succeed, the second should fail
+    api = SharkVacuumEntity(shark_vac1, coordinator)
-    api1 = SharkVacuumEntity(shark_vac1, coordinator)
-    try:
-        _ = SharkVacuumEntity(shark_vac2, coordinator)
-    except RuntimeError:
-        api2_failed = True
-    else:
-        api2_failed = False
-    assert api2_failed
-
     coordinator.last_update_success = True
     coordinator._online_dsns = set()  # pylint: disable=protected-access
-    assert not api1.is_online
+    assert not api.is_online
-    assert not api1.available
+    assert not api.available
 
     coordinator._online_dsns = {  # pylint: disable=protected-access
         shark_vac1.serial_number
     }
-    assert api1.is_online
+    assert api.is_online
-    assert api1.available
+    assert api.available
 
     coordinator.last_update_success = False
-    assert not api1.available
+    assert not api.available
 
 
 async def test_simple_properties(hass: HomeAssistant):