Improve evohome exception handling and fix bugs

homeassistant/components/climate/honeywell.py

@@ -21,7 +21,7 @@
     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.1', 'somecomfort==0.5.2']
 
 _LOGGER = logging.getLogger(__name__)
 

homeassistant/components/evohome/__init__.py

@@ -8,7 +8,7 @@
 from datetime import timedelta
 import logging
 
-from requests.exceptions import HTTPError
+import requests.exceptions
 import voluptuous as vol
 
 from homeassistant.const import (
@@ -21,7 +21,7 @@
 from homeassistant.helpers.discovery import load_platform
 from homeassistant.helpers.dispatcher import async_dispatcher_send
 
-REQUIREMENTS = ['evohomeclient==0.2.8']
+REQUIREMENTS = ['evohomeclient==0.3.1']
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -43,6 +43,10 @@
     }),
 }, 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'
@@ -69,48 +73,56 @@ def setup(hass, hass_config):
     from evohomeclient2 import EvohomeClient
 
     try:
-        client = EvohomeClient(
+        client = evo_data['client'] = EvohomeClient(
             evo_data['params'][CONF_USERNAME],
             evo_data['params'][CONF_PASSWORD],
             debug=False
         )
 
-    except HTTPError as err:
+    except requests.exceptions.ConnectionError:
+        _LOGGER.error(
+            "setup(): Failed to connect with the vendor's web servers. "
+            "The server is not contactable (maybe the vendor's fault?). "
+            "Unable to continue. Resolve any errors and restart HA."
+        )
+        raise  # Expose the actual exception to the user
+
+    except requests.exceptions.HTTPError as err:
         if err.response.status_code == HTTP_BAD_REQUEST:
             _LOGGER.error(
                 "setup(): Failed to connect with the vendor's web servers. "
-                "Check your username (%s), and password are correct."
+                "Check your username (%s), and password are correct. "
                 "Unable to continue. Resolve any errors and restart HA.",
                 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."
+                "The vendor says the server is currently unavailable. "
+                "Unable to continue. Wait a while 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."
+                "The vendor's API rate limit has been exceeded. "
+                "Unable to continue. Wait a while and restart HA."
             )
 
         else:
-            raise  # We don't expect/handle any other HTTPErrors
+            raise  # we don't expect/handle any other HTTPErrors
 
         return False
 
-    finally:  # Redact username, password as no longer needed
-        evo_data['params'][CONF_USERNAME] = 'REDACTED'
-        evo_data['params'][CONF_PASSWORD] = 'REDACTED'
+    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,25 +132,34 @@ 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(
-            "setup(): Parameter '%s'=%s, is outside its range (0-%s)",
-            CONF_LOCATION_IDX, loc_idx, len(client.installation_info) - 1)
+        _LOGGER.error(
+            "setup(): Invalid configuration (unknown location). "
+            "Parameter '%s'=%s, is outside its valid range (0-%s). "
+            "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'] = '...'
 
         _LOGGER.debug("setup(): evo_data['config']=%s", tmp_loc)
 
     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

@@ -14,16 +14,16 @@
 )
 from homeassistant.components.evohome import (
     DATA_EVOHOME, DISPATCHER_EVOHOME,
-    CONF_LOCATION_IDX, SCAN_INTERVAL_DEFAULT,
+    CONF_LOCATION_IDX,
     EVO_PARENT, EVO_CHILD,
     GWS, TCS,
 )
 from homeassistant.const import (
     CONF_SCAN_INTERVAL,
-    HTTP_TOO_MANY_REQUESTS,
+    HTTP_SERVICE_UNAVAILABLE, HTTP_TOO_MANY_REQUESTS,
     PRECISION_HALVES,
     STATE_OFF,
-    TEMP_CELSIUS
+    TEMP_CELSIUS,
 )
 from homeassistant.core import callback
 from homeassistant.helpers.dispatcher import (
@@ -140,23 +140,41 @@ def _connect(self, packet):
         if packet['to'] & self._type and packet['signal'] == 'refresh':
             self.async_schedule_update_ha_state(force_refresh=True)
 
-    def _handle_requests_exceptions(self, err):
-        if err.response.status_code == HTTP_TOO_MANY_REQUESTS:
-            # execute a backoff: pause, and also reduce rate
-            old_interval = self._params[CONF_SCAN_INTERVAL]
-            new_interval = min(old_interval, SCAN_INTERVAL_DEFAULT) * 2
-            self._params[CONF_SCAN_INTERVAL] = new_interval
+    def _handle_exception(self, err):
+        try:
+            raise 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 "
-                "seconds, and increasing '%s' from %s to %s seconds",
-                new_interval * 3, CONF_SCAN_INTERVAL, old_interval,
-                new_interval)
-
-            self._timers['statusUpdated'] = datetime.now() + new_interval * 3
-
-        else:
-            raise err  # we dont handle any other HTTPErrors
+                "The vendor's web servers appear to be uncontactable, so "
+                "unable to get the latest state data during this cycle. "
+                "Hint: This is often a problem with the vendor's network."
+            )
+            return True
+
+        except requests.exceptions.HTTPError:
+            if err.response.status_code == HTTP_TOO_MANY_REQUESTS:
+                _LOGGER.warning(
+                    "The vendor's API rate limit has been exceeded, so unable"
+                    "to get the latest state data during this polling cycle. "
+                    "Suspending 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
+
+            if err.response.status_code == HTTP_SERVICE_UNAVAILABLE:
+                # this appears to be common with Honeywell servers
+                _LOGGER.warning(
+                    "The vendor's web servers appear unavailable, so unable "
+                    "to get the latest state data during this polling cycle. "
+                    "Note: This is a problem with the vendor's network."
+                )
+                return True
+
+            raise  # we don't expect/handle any other HTTPErrors
 
     @property
     def name(self) -> str:
@@ -251,7 +269,8 @@ def target_temperature(self):
     @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):
@@ -298,7 +317,7 @@ def _set_temperature(self, temperature, until=None):
         try:
             self._obj.set_temperature(temperature, until)
         except HTTPError as err:
-            self._handle_exception("HTTPError", str(err))  # noqa: E501; pylint: disable=no-member
+            self._handle_exception(err)
 
     def set_temperature(self, **kwargs):
         """Set new target temperature, indefinitely."""
@@ -346,9 +365,9 @@ def set_operation_mode(self, operation_mode):
     def _set_operation_mode(self, operation_mode):
         if operation_mode == EVO_FOLLOW:
             try:
-                self._obj.cancel_temp_override(self._obj)
+                self._obj.cancel_temp_override()
             except HTTPError as err:
-                self._handle_exception("HTTPError", str(err))  # noqa: E501; pylint: disable=no-member
+                self._handle_exception(err)
 
         elif operation_mode == EVO_TEMPOVER:
             _LOGGER.error(
@@ -508,9 +527,9 @@ def turn_away_mode_off(self):
 
     def _set_operation_mode(self, operation_mode):
         try:
-            self._obj._set_status(operation_mode)  # noqa: E501; pylint: disable=protected-access
+            self._obj._set_status(operation_mode)                                # noqa: E501; pylint: disable=protected-access
         except HTTPError as err:
-            self._handle_requests_exceptions(err)
+            self._handle_exception(err)
 
     def set_operation_mode(self, operation_mode):
         """Set new target operation mode for the TCS.
@@ -547,7 +566,7 @@ def update(self):
             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
-            self._handle_requests_exceptions(err)
+            self._handle_exception(err)
         else:
             self._timers['statusUpdated'] = datetime.now()
             self._available = True

requirements_all.txt

@@ -417,7 +417,7 @@ eternalegypt==0.0.5
 
 # homeassistant.components.evohome
 # homeassistant.components.climate.honeywell
-evohomeclient==0.2.8
+evohomeclient==0.3.1
 
 # homeassistant.components.dlib_face_detect.image_processing
 # homeassistant.components.dlib_face_identify.image_processing

requirements_test_all.txt

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