From dae8cd880183d6e375cf229f6c6375d077507d65 Mon Sep 17 00:00:00 2001 From: Santobert Date: Sun, 6 Oct 2019 20:10:11 +0200 Subject: [PATCH] Bump pybotvac and use new exceptions (#27249) * Bump pybotvac * Fix tests * Remove super calls * Surround some more statements * Correct logger message for vacuum --- .../components/neato/.translations/en.json | 3 +- homeassistant/components/neato/__init__.py | 30 ++++--- homeassistant/components/neato/camera.py | 49 ++++++++--- homeassistant/components/neato/config_flow.py | 6 +- homeassistant/components/neato/const.py | 2 + homeassistant/components/neato/manifest.json | 2 +- homeassistant/components/neato/strings.json | 3 +- homeassistant/components/neato/switch.py | 41 +++++---- homeassistant/components/neato/vacuum.py | 83 ++++++++++++------- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/neato/test_config_flow.py | 32 ++++++- 12 files changed, 178 insertions(+), 77 deletions(-) diff --git a/homeassistant/components/neato/.translations/en.json b/homeassistant/components/neato/.translations/en.json index dc13242cc1d..69cdb48a560 100644 --- a/homeassistant/components/neato/.translations/en.json +++ b/homeassistant/components/neato/.translations/en.json @@ -13,7 +13,8 @@ } }, "error": { - "invalid_credentials": "Invalid credentials" + "invalid_credentials": "Invalid credentials", + "unexpected_error": "Unexpected error" }, "create_entry": { "default": "See [Neato documentation]({docs_url})." diff --git a/homeassistant/components/neato/__init__.py b/homeassistant/components/neato/__init__.py index 8fd545c59bb..feaffeaeb6d 100644 --- a/homeassistant/components/neato/__init__.py +++ b/homeassistant/components/neato/__init__.py @@ -3,7 +3,7 @@ import asyncio import logging from datetime import timedelta -from requests.exceptions import HTTPError, ConnectionError as ConnError +from pybotvac.exceptions import NeatoException, NeatoLoginException, NeatoRobotException import voluptuous as vol from homeassistant.config_entries import SOURCE_IMPORT @@ -20,6 +20,7 @@ from .const import ( NEATO_ROBOTS, NEATO_PERSISTENT_MAPS, NEATO_MAP_DATA, + SCAN_INTERVAL_MINUTES, VALID_VENDORS, ) @@ -103,7 +104,12 @@ async def async_setup_entry(hass, entry): _LOGGER.debug("Failed to login to Neato API") return False - await hass.async_add_executor_job(hub.update_robots) + try: + await hass.async_add_executor_job(hub.update_robots) + except NeatoRobotException: + _LOGGER.debug("Failed to connect to Neato API") + return False + for component in ("camera", "vacuum", "switch"): hass.async_create_task( hass.config_entries.async_forward_entry_setup(entry, component) @@ -144,17 +150,19 @@ class NeatoHub: self.config[CONF_USERNAME], self.config[CONF_PASSWORD], self._vendor ) self.logged_in = True - except (HTTPError, ConnError): - _LOGGER.error("Unable to connect to Neato API") + + _LOGGER.debug("Successfully connected to Neato API") + self._hass.data[NEATO_ROBOTS] = self.my_neato.robots + self._hass.data[NEATO_PERSISTENT_MAPS] = self.my_neato.persistent_maps + self._hass.data[NEATO_MAP_DATA] = self.my_neato.maps + except NeatoException as ex: + if isinstance(ex, NeatoLoginException): + _LOGGER.error("Invalid credentials") + else: + _LOGGER.error("Unable to connect to Neato API") self.logged_in = False - return - _LOGGER.debug("Successfully connected to Neato API") - self._hass.data[NEATO_ROBOTS] = self.my_neato.robots - self._hass.data[NEATO_PERSISTENT_MAPS] = self.my_neato.persistent_maps - self._hass.data[NEATO_MAP_DATA] = self.my_neato.maps - - @Throttle(timedelta(seconds=300)) + @Throttle(timedelta(minutes=SCAN_INTERVAL_MINUTES)) def update_robots(self): """Update the robot states.""" _LOGGER.debug("Running HUB.update_robots %s", self._hass.data[NEATO_ROBOTS]) diff --git a/homeassistant/components/neato/camera.py b/homeassistant/components/neato/camera.py index c565fa3d9ac..2604c6276a5 100644 --- a/homeassistant/components/neato/camera.py +++ b/homeassistant/components/neato/camera.py @@ -2,13 +2,21 @@ from datetime import timedelta import logging +from pybotvac.exceptions import NeatoRobotException + from homeassistant.components.camera import Camera -from .const import NEATO_DOMAIN, NEATO_MAP_DATA, NEATO_ROBOTS, NEATO_LOGIN +from .const import ( + NEATO_DOMAIN, + NEATO_MAP_DATA, + NEATO_ROBOTS, + NEATO_LOGIN, + SCAN_INTERVAL_MINUTES, +) _LOGGER = logging.getLogger(__name__) -SCAN_INTERVAL = timedelta(minutes=10) +SCAN_INTERVAL = timedelta(minutes=SCAN_INTERVAL_MINUTES) async def async_setup_platform(hass, config, async_add_entities, discovery_info=None): @@ -37,9 +45,9 @@ class NeatoCleaningMap(Camera): """Initialize Neato cleaning map.""" super().__init__() self.robot = robot - self._robot_name = "{} {}".format(self.robot.name, "Cleaning Map") + self.neato = hass.data[NEATO_LOGIN] if NEATO_LOGIN in hass.data else None + self._robot_name = f"{self.robot.name} Cleaning Map" self._robot_serial = self.robot.serial - self.neato = hass.data[NEATO_LOGIN] self._image_url = None self._image = None @@ -50,16 +58,31 @@ class NeatoCleaningMap(Camera): def update(self): """Check the contents of the map list.""" - self.neato.update_robots() - image_url = None - map_data = self.hass.data[NEATO_MAP_DATA] - image_url = map_data[self._robot_serial]["maps"][0]["url"] - if image_url == self._image_url: - _LOGGER.debug("The map image_url is the same as old") + if self.neato is None: + _LOGGER.error("Error while updating camera") + self._image = None + self._image_url = None return - image = self.neato.download_map(image_url) - self._image = image.read() - self._image_url = image_url + + _LOGGER.debug("Running camera update") + try: + self.neato.update_robots() + + image_url = None + map_data = self.hass.data[NEATO_MAP_DATA][self._robot_serial]["maps"][0] + image_url = map_data["url"] + if image_url == self._image_url: + _LOGGER.debug("The map image_url is the same as old") + return + + image = self.neato.download_map(image_url) + self._image = image.read() + self._image_url = image_url + + except NeatoRobotException as ex: + _LOGGER.error("Neato camera connection error: %s", ex) + self._image = None + self._image_url = None @property def name(self): diff --git a/homeassistant/components/neato/config_flow.py b/homeassistant/components/neato/config_flow.py index 0c71cdbd069..7ece3b8d300 100644 --- a/homeassistant/components/neato/config_flow.py +++ b/homeassistant/components/neato/config_flow.py @@ -3,7 +3,7 @@ import logging import voluptuous as vol -from requests.exceptions import HTTPError, ConnectionError as ConnError +from pybotvac.exceptions import NeatoLoginException, NeatoRobotException from homeassistant import config_entries from homeassistant.const import CONF_PASSWORD, CONF_USERNAME @@ -106,7 +106,9 @@ class NeatoConfigFlow(config_entries.ConfigFlow, domain=NEATO_DOMAIN): try: Account(username, password, this_vendor) - except (HTTPError, ConnError): + except NeatoLoginException: return "invalid_credentials" + except NeatoRobotException: + return "unexpected_error" return None diff --git a/homeassistant/components/neato/const.py b/homeassistant/components/neato/const.py index 6fb41bda710..4d4178a6875 100644 --- a/homeassistant/components/neato/const.py +++ b/homeassistant/components/neato/const.py @@ -9,6 +9,8 @@ NEATO_CONFIG = "neato_config" NEATO_MAP_DATA = "neato_map_data" NEATO_PERSISTENT_MAPS = "neato_persistent_maps" +SCAN_INTERVAL_MINUTES = 5 + VALID_VENDORS = ["neato", "vorwerk"] MODE = {1: "Eco", 2: "Turbo"} diff --git a/homeassistant/components/neato/manifest.json b/homeassistant/components/neato/manifest.json index 160f194cd63..a4d05e8849a 100644 --- a/homeassistant/components/neato/manifest.json +++ b/homeassistant/components/neato/manifest.json @@ -4,7 +4,7 @@ "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/neato", "requirements": [ - "pybotvac==0.0.15" + "pybotvac==0.0.16" ], "dependencies": [], "codeowners": [ diff --git a/homeassistant/components/neato/strings.json b/homeassistant/components/neato/strings.json index dc13242cc1d..69cdb48a560 100644 --- a/homeassistant/components/neato/strings.json +++ b/homeassistant/components/neato/strings.json @@ -13,7 +13,8 @@ } }, "error": { - "invalid_credentials": "Invalid credentials" + "invalid_credentials": "Invalid credentials", + "unexpected_error": "Unexpected error" }, "create_entry": { "default": "See [Neato documentation]({docs_url})." diff --git a/homeassistant/components/neato/switch.py b/homeassistant/components/neato/switch.py index 3efee11853d..8e85bef23b2 100644 --- a/homeassistant/components/neato/switch.py +++ b/homeassistant/components/neato/switch.py @@ -2,16 +2,16 @@ from datetime import timedelta import logging -import requests +from pybotvac.exceptions import NeatoRobotException from homeassistant.const import STATE_OFF, STATE_ON from homeassistant.helpers.entity import ToggleEntity -from .const import NEATO_DOMAIN, NEATO_LOGIN, NEATO_ROBOTS +from .const import NEATO_DOMAIN, NEATO_LOGIN, NEATO_ROBOTS, SCAN_INTERVAL_MINUTES _LOGGER = logging.getLogger(__name__) -SCAN_INTERVAL = timedelta(minutes=10) +SCAN_INTERVAL = timedelta(minutes=SCAN_INTERVAL_MINUTES) SWITCH_TYPE_SCHEDULE = "schedule" @@ -44,8 +44,8 @@ class NeatoConnectedSwitch(ToggleEntity): """Initialize the Neato Connected switches.""" self.type = switch_type self.robot = robot - self.neato = hass.data[NEATO_LOGIN] - self._robot_name = "{} {}".format(self.robot.name, SWITCH_TYPES[self.type][0]) + self.neato = hass.data[NEATO_LOGIN] if NEATO_LOGIN in hass.data else None + self._robot_name = f"{self.robot.name} {SWITCH_TYPES[self.type][0]}" self._state = None self._schedule_state = None self._clean_state = None @@ -53,17 +53,20 @@ class NeatoConnectedSwitch(ToggleEntity): def update(self): """Update the states of Neato switches.""" - _LOGGER.debug("Running switch update") - self.neato.update_robots() - try: - self._state = self.robot.state - except ( - requests.exceptions.ConnectionError, - requests.exceptions.HTTPError, - ) as ex: - _LOGGER.warning("Neato connection error: %s", ex) + if self.neato is None: + _LOGGER.error("Error while updating switches") self._state = None return + + _LOGGER.debug("Running switch update") + try: + self.neato.update_robots() + self._state = self.robot.state + except NeatoRobotException as ex: + _LOGGER.error("Neato switch connection error: %s", ex) + self._state = None + return + _LOGGER.debug("self._state=%s", self._state) if self.type == SWITCH_TYPE_SCHEDULE: _LOGGER.debug("State: %s", self._state) @@ -104,9 +107,15 @@ class NeatoConnectedSwitch(ToggleEntity): def turn_on(self, **kwargs): """Turn the switch on.""" if self.type == SWITCH_TYPE_SCHEDULE: - self.robot.enable_schedule() + try: + self.robot.enable_schedule() + except NeatoRobotException as ex: + _LOGGER.error("Neato switch connection error: %s", ex) def turn_off(self, **kwargs): """Turn the switch off.""" if self.type == SWITCH_TYPE_SCHEDULE: - self.robot.disable_schedule() + try: + self.robot.disable_schedule() + except NeatoRobotException as ex: + _LOGGER.error("Neato switch connection error: %s", ex) diff --git a/homeassistant/components/neato/vacuum.py b/homeassistant/components/neato/vacuum.py index 96c4e8f3c5f..bdb8cd0875e 100644 --- a/homeassistant/components/neato/vacuum.py +++ b/homeassistant/components/neato/vacuum.py @@ -2,7 +2,8 @@ from datetime import timedelta import logging -import requests +from pybotvac.exceptions import NeatoRobotException + import voluptuous as vol from homeassistant.components.vacuum import ( @@ -41,11 +42,12 @@ from .const import ( NEATO_MAP_DATA, NEATO_PERSISTENT_MAPS, NEATO_ROBOTS, + SCAN_INTERVAL_MINUTES, ) _LOGGER = logging.getLogger(__name__) -SCAN_INTERVAL = timedelta(minutes=5) +SCAN_INTERVAL = timedelta(minutes=SCAN_INTERVAL_MINUTES) SUPPORT_NEATO = ( SUPPORT_BATTERY @@ -154,22 +156,26 @@ class NeatoConnectedVacuum(StateVacuumDevice): def update(self): """Update the states of Neato Vacuums.""" - _LOGGER.debug("Running Neato Vacuums update") - if self._robot_stats is None: - self._robot_stats = self.robot.get_robot_info().json() - - self.neato.update_robots() - try: - self._state = self.robot.state - self._available = True - except ( - requests.exceptions.ConnectionError, - requests.exceptions.HTTPError, - ) as ex: - _LOGGER.warning("Neato connection error: %s", ex) + if self.neato is None: + _LOGGER.error("Error while updating vacuum") self._state = None self._available = False return + + try: + _LOGGER.debug("Running Neato Vacuums update") + if self._robot_stats is None: + self._robot_stats = self.robot.get_robot_info().json() + self.neato.update_robots() + self._state = self.robot.state + self._available = True + except NeatoRobotException as ex: + if self._available: # print only once when available + _LOGGER.error("Neato vacuum connection error: %s", ex) + self._state = None + self._available = False + return + _LOGGER.debug("self._state=%s", self._state) if "alert" in self._state: robot_alert = ALERTS.get(self._state["alert"]) @@ -313,33 +319,51 @@ class NeatoConnectedVacuum(StateVacuumDevice): def start(self): """Start cleaning or resume cleaning.""" - if self._state["state"] == 1: - self.robot.start_cleaning() - elif self._state["state"] == 3: - self.robot.resume_cleaning() + try: + if self._state["state"] == 1: + self.robot.start_cleaning() + elif self._state["state"] == 3: + self.robot.resume_cleaning() + except NeatoRobotException as ex: + _LOGGER.error("Neato vacuum connection error: %s", ex) def pause(self): """Pause the vacuum.""" - self.robot.pause_cleaning() + try: + self.robot.pause_cleaning() + except NeatoRobotException as ex: + _LOGGER.error("Neato vacuum connection error: %s", ex) def return_to_base(self, **kwargs): """Set the vacuum cleaner to return to the dock.""" - if self._clean_state == STATE_CLEANING: - self.robot.pause_cleaning() - self._clean_state = STATE_RETURNING - self.robot.send_to_base() + try: + if self._clean_state == STATE_CLEANING: + self.robot.pause_cleaning() + self._clean_state = STATE_RETURNING + self.robot.send_to_base() + except NeatoRobotException as ex: + _LOGGER.error("Neato vacuum connection error: %s", ex) def stop(self, **kwargs): """Stop the vacuum cleaner.""" - self.robot.stop_cleaning() + try: + self.robot.stop_cleaning() + except NeatoRobotException as ex: + _LOGGER.error("Neato vacuum connection error: %s", ex) def locate(self, **kwargs): """Locate the robot by making it emit a sound.""" - self.robot.locate() + try: + self.robot.locate() + except NeatoRobotException as ex: + _LOGGER.error("Neato vacuum connection error: %s", ex) def clean_spot(self, **kwargs): """Run a spot cleaning starting from the base.""" - self.robot.start_spot_cleaning() + try: + self.robot.start_spot_cleaning() + except NeatoRobotException as ex: + _LOGGER.error("Neato vacuum connection error: %s", ex) def neato_custom_cleaning(self, mode, navigation, category, zone=None, **kwargs): """Zone cleaning service call.""" @@ -355,4 +379,7 @@ class NeatoConnectedVacuum(StateVacuumDevice): return self._clean_state = STATE_CLEANING - self.robot.start_cleaning(mode, navigation, category, boundary_id) + try: + self.robot.start_cleaning(mode, navigation, category, boundary_id) + except NeatoRobotException as ex: + _LOGGER.error("Neato vacuum connection error: %s", ex) diff --git a/requirements_all.txt b/requirements_all.txt index 39d4ddcda62..075884183ff 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1102,7 +1102,7 @@ pyblackbird==0.5 # pybluez==0.22 # homeassistant.components.neato -pybotvac==0.0.15 +pybotvac==0.0.16 # homeassistant.components.nissan_leaf pycarwings2==2.9 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 187e50c4691..62be2f035a5 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -297,7 +297,7 @@ pyMetno==0.4.6 pyblackbird==0.5 # homeassistant.components.neato -pybotvac==0.0.15 +pybotvac==0.0.16 # homeassistant.components.cast pychromecast==4.0.1 diff --git a/tests/components/neato/test_config_flow.py b/tests/components/neato/test_config_flow.py index 99691c101a6..8eb67e5d3e1 100644 --- a/tests/components/neato/test_config_flow.py +++ b/tests/components/neato/test_config_flow.py @@ -103,11 +103,11 @@ async def test_abort_if_already_setup(hass, account): async def test_abort_on_invalid_credentials(hass): """Test when we have invalid credentials.""" - from requests.exceptions import HTTPError + from pybotvac.exceptions import NeatoLoginException flow = init_config_flow(hass) - with patch("pybotvac.Account", side_effect=HTTPError()): + with patch("pybotvac.Account", side_effect=NeatoLoginException()): result = await flow.async_step_user( { CONF_USERNAME: USERNAME, @@ -127,3 +127,31 @@ async def test_abort_on_invalid_credentials(hass): ) assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT assert result["reason"] == "invalid_credentials" + + +async def test_abort_on_unexpected_error(hass): + """Test when we have an unexpected error.""" + from pybotvac.exceptions import NeatoRobotException + + flow = init_config_flow(hass) + + with patch("pybotvac.Account", side_effect=NeatoRobotException()): + result = await flow.async_step_user( + { + CONF_USERNAME: USERNAME, + CONF_PASSWORD: PASSWORD, + CONF_VENDOR: VENDOR_NEATO, + } + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["errors"] == {"base": "unexpected_error"} + + result = await flow.async_step_import( + { + CONF_USERNAME: USERNAME, + CONF_PASSWORD: PASSWORD, + CONF_VENDOR: VENDOR_NEATO, + } + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "unexpected_error"