From 31bf5b8ff0f86e8a1c2200b387f9ec8b22cfc603 Mon Sep 17 00:00:00 2001 From: Thibault Cohen Date: Thu, 2 Mar 2017 02:49:49 -0500 Subject: [PATCH] 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 | 164 +++++++++++++++--- tests/components/climate/test_honeywell.py | 57 ++++-- 2 files changed, 191 insertions(+), 30 deletions(-) diff --git a/homeassistant/components/climate/honeywell.py b/homeassistant/components/climate/honeywell.py index 3387baf76d8..7b65ed4f077 100644 --- a/homeassistant/components/climate/honeywell.py +++ b/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.Coerce(float), + vol.Optional(CONF_AWAY_TEMPERATURE, + 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': - self._device.setpoint_cool = temperature - else: - self._device.setpoint_heat = temperature + # Get current mode + mode = self._device.system_mode + # Set hold if this is not the case + 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_SYSTEM_MODE: self._device.system_mode, + ATTR_FAN_MODE: self._device.fan_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.""" - pass + """Turn away on. + + 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.""" - self._device.refresh() + 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] diff --git a/tests/components/climate/test_honeywell.py b/tests/components/climate/test_honeywell.py index 13d7eb65257..a4cdda2adc4 100644 --- a/tests/components/climate/test_honeywell.py +++ b/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_2[0]), - mock.call(mock_sc.return_value, devices_2[1]), + mock.call(mock_sc.return_value, devices_1[0], 18, 28, + 'user', 'pass'), + 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', - honeywell.ATTR_SYSTEM_MODE: 'heat', + ATTR_FAN_MODE: 'auto', + 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, - honeywell.ATTR_SYSTEM_MODE: 'heat', + ATTR_FAN_MODE: None, + 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)