Improve Honeywell US climate component (#5313)

* Improve Honeywell US climate component

* Fix tests

* Fix tests

* Add cool_away_temp and heat_away_temp for honeywell US

* Fix honeywell tests

* Fix PR comments

homeassistant/components/climate/honeywell.py

@@ -6,10 +6,15 @@ https://home-assistant.io/components/climate.honeywell/
 """
 import logging
 import socket
+import datetime
 
 import voluptuous as vol
+import requests
 
-from homeassistant.components.climate import (ClimateDevice, PLATFORM_SCHEMA)
+from homeassistant.components.climate import (ClimateDevice, PLATFORM_SCHEMA,
+                                              ATTR_FAN_MODE, ATTR_FAN_LIST,
+                                              ATTR_OPERATION_MODE,
+                                              ATTR_OPERATION_LIST)
 from homeassistant.const import (
     CONF_PASSWORD, CONF_USERNAME, TEMP_CELSIUS, TEMP_FAHRENHEIT,
     ATTR_TEMPERATURE)
@@ -21,27 +26,35 @@ REQUIREMENTS = ['evohomeclient==0.2.5',
 _LOGGER = logging.getLogger(__name__)
 
 ATTR_FAN = 'fan'
-ATTR_FANMODE = 'fanmode'
 ATTR_SYSTEM_MODE = 'system_mode'
+ATTR_CURRENT_OPERATION = 'equipment_output_status'
 
 CONF_AWAY_TEMPERATURE = 'away_temperature'
+CONF_COOL_AWAY_TEMPERATURE = 'away_cool_temperature'
+CONF_HEAT_AWAY_TEMPERATURE = 'away_heat_temperature'
 CONF_REGION = 'region'
 
 DEFAULT_AWAY_TEMPERATURE = 16
+DEFAULT_COOL_AWAY_TEMPERATURE = 30
+DEFAULT_HEAT_AWAY_TEMPERATURE = 16
 DEFAULT_REGION = 'eu'
 REGIONS = ['eu', 'us']
 
 PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({
     vol.Required(CONF_USERNAME): cv.string,
     vol.Required(CONF_PASSWORD): cv.string,
-    vol.Optional(CONF_AWAY_TEMPERATURE, default=DEFAULT_AWAY_TEMPERATURE):
+    vol.Optional(CONF_AWAY_TEMPERATURE,
-        vol.Coerce(float),
+                 default=DEFAULT_AWAY_TEMPERATURE): vol.Coerce(float),
+    vol.Optional(CONF_COOL_AWAY_TEMPERATURE,
+                 default=DEFAULT_COOL_AWAY_TEMPERATURE): vol.Coerce(float),
+    vol.Optional(CONF_HEAT_AWAY_TEMPERATURE,
+                 default=DEFAULT_HEAT_AWAY_TEMPERATURE): vol.Coerce(float),
     vol.Optional(CONF_REGION, default=DEFAULT_REGION): vol.In(REGIONS),
 })
 
 
 def setup_platform(hass, config, add_devices, discovery_info=None):
-    """Setup the HoneywelL thermostat."""
+    """Setup the Honeywell thermostat."""
     username = config.get(CONF_USERNAME)
     password = config.get(CONF_PASSWORD)
     region = config.get(CONF_REGION)
@@ -88,8 +101,11 @@ def _setup_us(username, password, config, add_devices):
 
     dev_id = config.get('thermostat')
     loc_id = config.get('location')
+    cool_away_temp = config.get(CONF_COOL_AWAY_TEMPERATURE)
+    heat_away_temp = config.get(CONF_HEAT_AWAY_TEMPERATURE)
 
-    add_devices([HoneywellUSThermostat(client, device)
+    add_devices([HoneywellUSThermostat(client, device, cool_away_temp,
+                                       heat_away_temp, username, password)
                  for location in client.locations_by_id.values()
                  for device in location.devices_by_id.values()
                  if ((not loc_id or location.locationid == loc_id) and
@@ -160,7 +176,7 @@ class RoundThermostat(ClimateDevice):
     def turn_away_mode_on(self):
         """Turn away on.
 
-        Evohome does have a proprietary away mode, but it doesn't really work
+        Honeywell does have a proprietary away mode, but it doesn't really work
         the way it should. For example: If you set a temperature manually
         it doesn't get overwritten when away mode is switched on.
         """
@@ -199,10 +215,16 @@ class RoundThermostat(ClimateDevice):
 class HoneywellUSThermostat(ClimateDevice):
     """Representation of a Honeywell US Thermostat."""
 
-    def __init__(self, client, device):
+    def __init__(self, client, device, cool_away_temp,
+                 heat_away_temp, username, password):
         """Initialize the thermostat."""
         self._client = client
         self._device = device
+        self._cool_away_temp = cool_away_temp
+        self._heat_away_temp = heat_away_temp
+        self._away = False
+        self._username = username
+        self._password = password
 
     @property
     def is_fan_on(self):
@@ -236,7 +258,10 @@ class HoneywellUSThermostat(ClimateDevice):
     @property
     def current_operation(self: ClimateDevice) -> str:
         """Return current operation ie. heat, cool, idle."""
-        return getattr(self._device, ATTR_SYSTEM_MODE, None)
+        oper = getattr(self._device, ATTR_CURRENT_OPERATION, None)
+        if oper == "off":
+            oper = "idle"
+        return oper
 
     def set_temperature(self, **kwargs):
         """Set target temperature."""
@@ -245,29 +270,84 @@ class HoneywellUSThermostat(ClimateDevice):
             return
         import somecomfort
         try:
-            if self._device.system_mode == 'cool':
+            # Get current mode
-                self._device.setpoint_cool = temperature
+            mode = self._device.system_mode
-            else:
+            # Set hold if this is not the case
-                self._device.setpoint_heat = temperature
+            if getattr(self._device, "hold_{}".format(mode)) is False:
+                # Get next period key
+                next_period_key = '{}NextPeriod'.format(mode.capitalize())
+                # Get next period raw value
+                next_period = self._device.raw_ui_data.get(next_period_key)
+                # Get next period time
+                hour, minute = divmod(next_period * 15, 60)
+                # Set hold time
+                setattr(self._device,
+                        "hold_{}".format(mode),
+                        datetime.time(hour, minute))
+            # Set temperature
+            setattr(self._device,
+                    "setpoint_{}".format(mode),
+                    temperature)
         except somecomfort.SomeComfortError:
             _LOGGER.error('Temperature %.1f out of range', temperature)
 
     @property
     def device_state_attributes(self):
         """Return the device specific state attributes."""
-        return {
+        import somecomfort
+        data = {
             ATTR_FAN: (self.is_fan_on and 'running' or 'idle'),
-            ATTR_FANMODE: self._device.fan_mode,
+            ATTR_FAN_MODE: self._device.fan_mode,
-            ATTR_SYSTEM_MODE: self._device.system_mode,
+            ATTR_OPERATION_MODE: self._device.system_mode,
         }
+        data[ATTR_FAN_LIST] = somecomfort.FAN_MODES
+        data[ATTR_OPERATION_LIST] = somecomfort.SYSTEM_MODES
+        return data
+
+    @property
+    def is_away_mode_on(self):
+        """Return true if away mode is on."""
+        return self._away
 
     def turn_away_mode_on(self):
-        """Turn away on."""
+        """Turn away on.
-        pass
+
+        Somecomfort does have a proprietary away mode, but it doesn't really
+        work the way it should. For example: If you set a temperature manually
+        it doesn't get overwritten when away mode is switched on.
+        """
+        self._away = True
+        import somecomfort
+        try:
+            # Get current mode
+            mode = self._device.system_mode
+        except somecomfort.SomeComfortError:
+            _LOGGER.error('Can not get system mode')
+            return
+        try:
+
+            # Set permanent hold
+            setattr(self._device,
+                    "hold_{}".format(mode),
+                    True)
+            # Set temperature
+            setattr(self._device,
+                    "setpoint_{}".format(mode),
+                    getattr(self, "_{}_away_temp".format(mode)))
+        except somecomfort.SomeComfortError:
+            _LOGGER.error('Temperature %.1f out of range',
+                          getattr(self, "_{}_away_temp".format(mode)))
 
     def turn_away_mode_off(self):
         """Turn away off."""
-        pass
+        self._away = False
+        import somecomfort
+        try:
+            # Disabling all hold modes
+            self._device.hold_cool = False
+            self._device.hold_heat = False
+        except somecomfort.SomeComfortError:
+            _LOGGER.error('Can not stop hold mode')
 
     def set_operation_mode(self: ClimateDevice, operation_mode: str) -> None:
         """Set the system mode (Cool, Heat, etc)."""
@@ -276,4 +356,48 @@ class HoneywellUSThermostat(ClimateDevice):
 
     def update(self):
         """Update the state."""
+        import somecomfort
+        retries = 3
+        while retries > 0:
+            try:
                 self._device.refresh()
+                break
+            except (somecomfort.client.APIRateLimited, OSError,
+                    requests.exceptions.ReadTimeout) as exp:
+                retries -= 1
+                if retries == 0:
+                    raise exp
+                if not self._retry():
+                    raise exp
+                _LOGGER.error("SomeComfort update failed, Retrying "
+                              "- Error: %s", exp)
+
+    def _retry(self):
+        """Recreate a new somecomfort client.
+
+        When we got an error, the best way to be sure that the next query
+        will succeed, is to recreate a new somecomfort client.
+        """
+        import somecomfort
+        try:
+            self._client = somecomfort.SomeComfort(self._username,
+                                                   self._password)
+        except somecomfort.AuthError:
+            _LOGGER.error('Failed to login to honeywell account %s',
+                          self._username)
+            return False
+        except somecomfort.SomeComfortError as ex:
+            _LOGGER.error('Failed to initialize honeywell client: %s',
+                          str(ex))
+            return False
+
+        devices = [device
+                   for location in self._client.locations_by_id.values()
+                   for device in location.devices_by_id.values()
+                   if device.name == self._device.name]
+
+        if len(devices) != 1:
+            _LOGGER.error('Failed to find device %s', self._device.name)
+            return False
+
+        self._device = devices[0]

tests/components/climate/test_honeywell.py

@@ -8,6 +8,9 @@ import somecomfort
 
 from homeassistant.const import (
     CONF_USERNAME, CONF_PASSWORD, TEMP_CELSIUS, TEMP_FAHRENHEIT)
+from homeassistant.components.climate import (
+    ATTR_FAN_MODE, ATTR_OPERATION_MODE, ATTR_FAN_LIST, ATTR_OPERATION_LIST)
+
 import homeassistant.components.climate.honeywell as honeywell
 
 
@@ -22,15 +25,21 @@ class TestHoneywell(unittest.TestCase):
         config = {
             CONF_USERNAME: 'user',
             CONF_PASSWORD: 'pass',
+            honeywell.CONF_COOL_AWAY_TEMPERATURE: 18,
+            honeywell.CONF_HEAT_AWAY_TEMPERATURE: 28,
             honeywell.CONF_REGION: 'us',
         }
         bad_pass_config = {
             CONF_USERNAME: 'user',
+            honeywell.CONF_COOL_AWAY_TEMPERATURE: 18,
+            honeywell.CONF_HEAT_AWAY_TEMPERATURE: 28,
             honeywell.CONF_REGION: 'us',
         }
         bad_region_config = {
             CONF_USERNAME: 'user',
             CONF_PASSWORD: 'pass',
+            honeywell.CONF_COOL_AWAY_TEMPERATURE: 18,
+            honeywell.CONF_HEAT_AWAY_TEMPERATURE: 28,
             honeywell.CONF_REGION: 'un',
         }
 
@@ -65,9 +74,12 @@ class TestHoneywell(unittest.TestCase):
         self.assertEqual(mock_sc.call_count, 1)
         self.assertEqual(mock_sc.call_args, mock.call('user', 'pass'))
         mock_ht.assert_has_calls([
-            mock.call(mock_sc.return_value, devices_1[0]),
+            mock.call(mock_sc.return_value, devices_1[0], 18, 28,
-            mock.call(mock_sc.return_value, devices_2[0]),
+                      'user', 'pass'),
-            mock.call(mock_sc.return_value, devices_2[1]),
+            mock.call(mock_sc.return_value, devices_2[0], 18, 28,
+                      'user', 'pass'),
+            mock.call(mock_sc.return_value, devices_2[1], 18, 28,
+                      'user', 'pass'),
         ])
 
     @mock.patch('somecomfort.SomeComfort')
@@ -324,8 +336,12 @@ class TestHoneywellUS(unittest.TestCase):
         """Test the setup method."""
         self.client = mock.MagicMock()
         self.device = mock.MagicMock()
+        self.cool_away_temp = 18
+        self.heat_away_temp = 28
         self.honeywell = honeywell.HoneywellUSThermostat(
-            self.client, self.device)
+            self.client, self.device,
+            self.cool_away_temp, self.heat_away_temp,
+            'user', 'password')
 
         self.device.fan_running = True
         self.device.name = 'test'
@@ -369,11 +385,9 @@ class TestHoneywellUS(unittest.TestCase):
     def test_set_operation_mode(self: unittest.TestCase) -> None:
         """Test setting the operation mode."""
         self.honeywell.set_operation_mode('cool')
-        self.assertEqual('cool', self.honeywell.current_operation)
         self.assertEqual('cool', self.device.system_mode)
 
         self.honeywell.set_operation_mode('heat')
-        self.assertEqual('heat', self.honeywell.current_operation)
         self.assertEqual('heat', self.device.system_mode)
 
     def test_set_temp_fail(self):
@@ -386,8 +400,10 @@ class TestHoneywellUS(unittest.TestCase):
         """Test the attributes."""
         expected = {
             honeywell.ATTR_FAN: 'running',
-            honeywell.ATTR_FANMODE: 'auto',
+            ATTR_FAN_MODE: 'auto',
-            honeywell.ATTR_SYSTEM_MODE: 'heat',
+            ATTR_OPERATION_MODE: 'heat',
+            ATTR_FAN_LIST: somecomfort.FAN_MODES,
+            ATTR_OPERATION_LIST: somecomfort.SYSTEM_MODES,
         }
         self.assertEqual(expected, self.honeywell.device_state_attributes)
         expected['fan'] = 'idle'
@@ -400,7 +416,28 @@ class TestHoneywellUS(unittest.TestCase):
         self.device.fan_mode = None
         expected = {
             honeywell.ATTR_FAN: 'idle',
-            honeywell.ATTR_FANMODE: None,
+            ATTR_FAN_MODE: None,
-            honeywell.ATTR_SYSTEM_MODE: 'heat',
+            ATTR_OPERATION_MODE: 'heat',
+            ATTR_FAN_LIST: somecomfort.FAN_MODES,
+            ATTR_OPERATION_LIST: somecomfort.SYSTEM_MODES,
         }
         self.assertEqual(expected, self.honeywell.device_state_attributes)
+
+    def test_heat_away_mode(self):
+        """Test setting the heat away mode."""
+        self.honeywell.set_operation_mode('heat')
+        self.assertFalse(self.honeywell.is_away_mode_on)
+        self.honeywell.turn_away_mode_on()
+        self.assertTrue(self.honeywell.is_away_mode_on)
+        self.assertEqual(self.device.setpoint_heat, self.heat_away_temp)
+        self.assertEqual(self.device.hold_heat, True)
+
+        self.honeywell.turn_away_mode_off()
+        self.assertFalse(self.honeywell.is_away_mode_on)
+        self.assertEqual(self.device.hold_heat, False)
+
+    def test_retry(self):
+        """Test retry connection."""
+        old_device = self.honeywell._device
+        self.honeywell._retry()
+        self.assertEqual(self.honeywell._device, old_device)