Improve evohome exception handling and fix bugs (#22140)

* Use latest client library, evohomeclient v0.3.1

* Fix issue #22097: Failed to call service climate/turn_on...

* BUGFIX: handle case where a Zone doesn't have a temperature

* BUGFIX: missing exception handler, and inappropriate delint hints

* Improve exception handling, and also better messages

* improve code (REDACT secrets); remove TODOs

* minor refactor - improve error message

* more refactoring - improve error message

* remove TODOs

* update to latest evohomeclient library

* Use latest client library, evohomeclient v0.3.1

* Fix issue #22097: Failed to call service climate/turn_on...

* BUGFIX: handle case where a Zone doesn't have a temperature

* BUGFIX: missing exception handler, and inappropriate delint hints

* Improve exception handling, and also better messages

* improve code (REDACT secrets); remove TODOs

* minor refactor - improve error message

* more refactoring - improve error message

* remove TODOs

* update to latest evohomeclient library

* fix requests for houndci-bot

* Tidy up requests exception handling

* Correct lint error

* update to latest client library

* minor de-lint

* more cleanup of exceptions, messages

* refactored for new exception

* fix error in requirements*_all.txt

* de-lint

* delint unused import

* import 3rd-party library only inside methods

* change honeywell tests

* delint, fix typo

* we dont log usernames, passwords, etc.

* de-lint

homeassistant/components/evohome/__init__.py

@@ -8,20 +8,18 @@
 from datetime import timedelta
 import logging
 
-from requests.exceptions import HTTPError
+import requests.exceptions
 import voluptuous as vol
 
 from homeassistant.const import (
     CONF_SCAN_INTERVAL, CONF_USERNAME, CONF_PASSWORD,
-    EVENT_HOMEASSISTANT_START,
+    EVENT_HOMEASSISTANT_START)
-    HTTP_BAD_REQUEST, HTTP_SERVICE_UNAVAILABLE, HTTP_TOO_MANY_REQUESTS
-)
 from homeassistant.core import callback
 import homeassistant.helpers.config_validation as cv
 from homeassistant.helpers.discovery import load_platform
 from homeassistant.helpers.dispatcher import async_dispatcher_send
 
-REQUIREMENTS = ['evohomeclient==0.2.8']
+REQUIREMENTS = ['evohomeclient==0.3.2']
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -43,6 +41,10 @@ CONFIG_SCHEMA = vol.Schema({
     }),
 }, extra=vol.ALLOW_EXTRA)
 
+CONF_SECRETS = [
+    CONF_USERNAME, CONF_PASSWORD,
+]
+
 # These are used to help prevent E501 (line too long) violations.
 GWS = 'gateways'
 TCS = 'temperatureControlSystems'
@@ -66,51 +68,40 @@ def setup(hass, hass_config):
     scan_interval = timedelta(
         minutes=(scan_interval.total_seconds() + 59) // 60)
 
-    from evohomeclient2 import EvohomeClient
+    import evohomeclient2
 
     try:
-        client = EvohomeClient(
+        client = evo_data['client'] = evohomeclient2.EvohomeClient(
             evo_data['params'][CONF_USERNAME],
             evo_data['params'][CONF_PASSWORD],
             debug=False
         )
 
-    except HTTPError as err:
+    except evohomeclient2.AuthenticationError as err:
-        if err.response.status_code == HTTP_BAD_REQUEST:
+        _LOGGER.error(
-            _LOGGER.error(
+            "setup(): Failed to authenticate with the vendor's server. "
-                "setup(): Failed to connect with the vendor's web servers. "
+            "Check your username and password are correct. "
-                "Check your username (%s), and password are correct."
+            "Resolve any errors and restart HA. Message is: %s",
-                "Unable to continue. Resolve any errors and restart HA.",
+            err
-                evo_data['params'][CONF_USERNAME]
+        )
-            )
-
-        elif err.response.status_code == HTTP_SERVICE_UNAVAILABLE:
-            _LOGGER.error(
-                "setup(): Failed to connect with the vendor's web servers. "
-                "The server is not contactable. Unable to continue. "
-                "Resolve any errors and restart HA."
-            )
-
-        elif err.response.status_code == HTTP_TOO_MANY_REQUESTS:
-            _LOGGER.error(
-                "setup(): Failed to connect with the vendor's web servers. "
-                "You have exceeded the api rate limit. Unable to continue. "
-                "Wait a while (say 10 minutes) and restart HA."
-            )
-
-        else:
-            raise  # We don't expect/handle any other HTTPErrors
-
         return False
 
-    finally:  # Redact username, password as no longer needed
+    except requests.exceptions.ConnectionError:
-        evo_data['params'][CONF_USERNAME] = 'REDACTED'
+        _LOGGER.error(
-        evo_data['params'][CONF_PASSWORD] = 'REDACTED'
+            "setup(): Unable to connect with the vendor's server. "
+            "Check your network and the vendor's status page. "
+            "Resolve any errors and restart HA."
+        )
+        return False
+
+    finally:  # Redact any config data that's no longer needed
+        for parameter in CONF_SECRETS:
+            evo_data['params'][parameter] = 'REDACTED' \
+                if evo_data['params'][parameter] else None
 
-    evo_data['client'] = client
     evo_data['status'] = {}
 
-    # Redact any installation data we'll never need
+    # Redact any installation data that's no longer needed
     for loc in client.installation_info:
         loc['locationInfo']['locationId'] = 'REDACTED'
         loc['locationInfo']['locationOwner'] = 'REDACTED'
@@ -120,18 +111,21 @@ def setup(hass, hass_config):
 
     # Pull down the installation configuration
     loc_idx = evo_data['params'][CONF_LOCATION_IDX]
-
     try:
         evo_data['config'] = client.installation_info[loc_idx]
+
     except IndexError:
-        _LOGGER.warning(
+        _LOGGER.error(
-            "setup(): Parameter '%s'=%s, is outside its range (0-%s)",
+            "setup(): config error, '%s' = %s, but its valid range is 0-%s. "
-            CONF_LOCATION_IDX, loc_idx, len(client.installation_info) - 1)
+            "Unable to continue. Fix any configuration errors and restart HA.",
+            CONF_LOCATION_IDX, loc_idx, len(client.installation_info) - 1
+        )
         return False
 
     if _LOGGER.isEnabledFor(logging.DEBUG):
         tmp_loc = dict(evo_data['config'])
         tmp_loc['locationInfo']['postcode'] = 'REDACTED'
+
         if 'dhw' in tmp_loc[GWS][0][TCS][0]:  # if this location has DHW...
             tmp_loc[GWS][0][TCS][0]['dhw'] = '...'
 
@@ -139,6 +133,11 @@ def setup(hass, hass_config):
 
     load_platform(hass, 'climate', DOMAIN, {}, hass_config)
 
+    if 'dhw' in evo_data['config'][GWS][0][TCS][0]:
+        _LOGGER.warning(
+            "setup(): DHW found, but this component doesn't support DHW."
+        )
+
     @callback
     def _first_update(event):
         """When HA has started, the hub knows to retrieve it's first update."""

homeassistant/components/evohome/climate.py

@@ -2,22 +2,22 @@
 from datetime import datetime, timedelta
 import logging
 
-from requests.exceptions import HTTPError
+import requests.exceptions
 
 from homeassistant.components.climate import ClimateDevice
 from homeassistant.components.climate.const import (
     STATE_AUTO, STATE_ECO, STATE_MANUAL, SUPPORT_AWAY_MODE, SUPPORT_ON_OFF,
     SUPPORT_OPERATION_MODE, SUPPORT_TARGET_TEMPERATURE)
 from homeassistant.const import (
-    CONF_SCAN_INTERVAL, HTTP_TOO_MANY_REQUESTS, PRECISION_HALVES, STATE_OFF,
+    CONF_SCAN_INTERVAL, HTTP_SERVICE_UNAVAILABLE, HTTP_TOO_MANY_REQUESTS,
-    TEMP_CELSIUS)
+    PRECISION_HALVES, STATE_OFF, TEMP_CELSIUS)
 from homeassistant.core import callback
 from homeassistant.helpers.dispatcher import (
     async_dispatcher_connect, dispatcher_send)
 
 from . import (
     CONF_LOCATION_IDX, DATA_EVOHOME, DISPATCHER_EVOHOME, EVO_CHILD, EVO_PARENT,
-    GWS, SCAN_INTERVAL_DEFAULT, TCS)
+    GWS, TCS)
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -81,7 +81,7 @@ async def async_setup_platform(hass, hass_config, async_add_entities,
 
     # evohomeclient has exposed no means of accessing non-default location
     # (i.e. loc_idx > 0) other than using a protected member, such as below
-    tcs_obj_ref = client.locations[loc_idx]._gateways[0]._control_systems[0]  # noqa E501; pylint: disable=protected-access
+    tcs_obj_ref = client.locations[loc_idx]._gateways[0]._control_systems[0]  # noqa: E501; pylint: disable=protected-access
 
     _LOGGER.debug(
         "Found Controller, id=%s [%s], name=%s (location_idx=%s)",
@@ -128,23 +128,43 @@ class EvoClimateDevice(ClimateDevice):
         if packet['to'] & self._type and packet['signal'] == 'refresh':
             self.async_schedule_update_ha_state(force_refresh=True)
 
-    def _handle_requests_exceptions(self, err):
+    def _handle_exception(self, err):
-        if err.response.status_code == HTTP_TOO_MANY_REQUESTS:
+        try:
-            # execute a backoff: pause, and also reduce rate
+            import evohomeclient2
-            old_interval = self._params[CONF_SCAN_INTERVAL]
+            raise err
-            new_interval = min(old_interval, SCAN_INTERVAL_DEFAULT) * 2
-            self._params[CONF_SCAN_INTERVAL] = new_interval
 
+        except evohomeclient2.AuthenticationError:
+            _LOGGER.error(
+                "Failed to (re)authenticate with the vendor's server. "
+                "This may be a temporary error. Message is: %s",
+                err
+            )
+
+        except requests.exceptions.ConnectionError:
+            # this appears to be common with Honeywell's servers
             _LOGGER.warning(
-                "API rate limit has been exceeded. Suspending polling for %s "
+                "Unable to connect with the vendor's server. "
-                "seconds, and increasing '%s' from %s to %s seconds",
+                "Check your network and the vendor's status page."
-                new_interval * 3, CONF_SCAN_INTERVAL, old_interval,
+            )
-                new_interval)
 
-            self._timers['statusUpdated'] = datetime.now() + new_interval * 3
+        except requests.exceptions.HTTPError:
+            if err.response.status_code == HTTP_SERVICE_UNAVAILABLE:
+                _LOGGER.warning(
+                    "Vendor says their server is currently unavailable. "
+                    "This may be temporary; check the vendor's status page."
+                )
 
-        else:
+            elif err.response.status_code == HTTP_TOO_MANY_REQUESTS:
-            raise err  # we dont handle any other HTTPErrors
+                _LOGGER.warning(
+                    "The vendor's API rate limit has been exceeded. "
+                    "So will cease polling, and will resume after %s seconds.",
+                    (self._params[CONF_SCAN_INTERVAL] * 3).total_seconds()
+                )
+                self._timers['statusUpdated'] = datetime.now() + \
+                    self._params[CONF_SCAN_INTERVAL] * 3
+
+            else:
+                raise  # we don't expect/handle any other HTTPErrors
 
     @property
     def name(self) -> str:
@@ -239,7 +259,8 @@ class EvoZone(EvoClimateDevice):
     @property
     def current_temperature(self):
         """Return the current temperature of the evohome Zone."""
-        return self._status['temperatureStatus']['temperature']
+        return (self._status['temperatureStatus']['temperature']
+                if self._status['temperatureStatus']['isAvailable'] else None)
 
     @property
     def current_operation(self):
@@ -284,9 +305,11 @@ class EvoZone(EvoClimateDevice):
           - None for PermanentOverride (i.e. indefinitely)
         """
         try:
+            import evohomeclient2
             self._obj.set_temperature(temperature, until)
-        except HTTPError as err:
+        except (requests.exceptions.RequestException,
-            self._handle_exception("HTTPError", str(err))  # noqa: E501; pylint: disable=no-member
+                evohomeclient2.AuthenticationError) as err:
+            self._handle_exception(err)
 
     def set_temperature(self, **kwargs):
         """Set new target temperature, indefinitely."""
@@ -334,9 +357,11 @@ class EvoZone(EvoClimateDevice):
     def _set_operation_mode(self, operation_mode):
         if operation_mode == EVO_FOLLOW:
             try:
-                self._obj.cancel_temp_override(self._obj)
+                import evohomeclient2
-            except HTTPError as err:
+                self._obj.cancel_temp_override()
-                self._handle_exception("HTTPError", str(err))  # noqa: E501; pylint: disable=no-member
+            except (requests.exceptions.RequestException,
+                    evohomeclient2.AuthenticationError) as err:
+                self._handle_exception(err)
 
         elif operation_mode == EVO_TEMPOVER:
             _LOGGER.error(
@@ -496,9 +521,11 @@ class EvoController(EvoClimateDevice):
 
     def _set_operation_mode(self, operation_mode):
         try:
+            import evohomeclient2
             self._obj._set_status(operation_mode)  # noqa: E501; pylint: disable=protected-access
-        except HTTPError as err:
+        except (requests.exceptions.RequestException,
-            self._handle_requests_exceptions(err)
+                evohomeclient2.AuthenticationError) as err:
+            self._handle_exception(err)
 
     def set_operation_mode(self, operation_mode):
         """Set new target operation mode for the TCS.
@@ -532,10 +559,12 @@ class EvoController(EvoClimateDevice):
         loc_idx = self._params[CONF_LOCATION_IDX]
 
         try:
+            import evohomeclient2
             self._status.update(
                 self._client.locations[loc_idx].status()[GWS][0][TCS][0])
-        except HTTPError as err:  # check if we've exceeded the api rate limit
+        except (requests.exceptions.RequestException,
-            self._handle_requests_exceptions(err)
+                evohomeclient2.AuthenticationError) as err:
+            self._handle_exception(err)
         else:
             self._timers['statusUpdated'] = datetime.now()
             self._available = True

homeassistant/components/honeywell/climate.py

@@ -5,7 +5,6 @@ For more details about this platform, please refer to the documentation at
 https://home-assistant.io/components/climate.honeywell/
 """
 import logging
-import socket
 import datetime
 
 import requests
@@ -21,7 +20,7 @@ from homeassistant.const import (
     CONF_PASSWORD, CONF_USERNAME, TEMP_CELSIUS, TEMP_FAHRENHEIT,
     ATTR_TEMPERATURE, CONF_REGION)
 
-REQUIREMENTS = ['evohomeclient==0.2.8', 'somecomfort==0.5.2']
+REQUIREMENTS = ['evohomeclient==0.3.2', 'somecomfort==0.5.2']
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -78,9 +77,10 @@ def _setup_round(username, password, config, add_entities):
                 [RoundThermostat(evo_api, zone['id'], i == 0, away_temp)],
                 True
             )
-    except socket.error:
+    except requests.exceptions.RequestException as err:
         _LOGGER.error(
-            "Connection error logging into the honeywell evohome web service")
+            "Connection error logging into the honeywell evohome web service, "
+            "hint: %s", err)
         return False
     return True
 

requirements_all.txt

@@ -414,7 +414,7 @@ eternalegypt==0.0.6
 
 # homeassistant.components.evohome
 # homeassistant.components.honeywell.climate
-evohomeclient==0.2.8
+evohomeclient==0.3.2
 
 # homeassistant.components.dlib_face_detect.image_processing
 # homeassistant.components.dlib_face_identify.image_processing

requirements_test_all.txt

@@ -92,7 +92,7 @@ ephem==3.7.6.0
 
 # homeassistant.components.evohome
 # homeassistant.components.honeywell.climate
-evohomeclient==0.2.8
+evohomeclient==0.3.2
 
 # homeassistant.components.feedreader
 feedparser-homeassistant==5.2.2.dev1

tests/components/honeywell/test_climate.py

@@ -1,9 +1,9 @@
 """The test the Honeywell thermostat module."""
-import socket
 import unittest
 from unittest import mock
 
 import voluptuous as vol
+import requests.exceptions
 import somecomfort
 
 from homeassistant.const import (
@@ -247,7 +247,8 @@ class TestHoneywell(unittest.TestCase):
             honeywell.CONF_AWAY_TEMPERATURE: 20,
             honeywell.CONF_REGION: 'eu',
         }
-        mock_evo.return_value.temperatures.side_effect = socket.error
+        mock_evo.return_value.temperatures.side_effect = \
+            requests.exceptions.RequestException
         add_entities = mock.MagicMock()
         hass = mock.MagicMock()
         assert not honeywell.setup_platform(hass, config, add_entities)