Improve tolerance of SmartThings Climate platform (#21383)

* Improve resilience of platform with buggy handlers

* Eliminate possibility of None in operation list

* Refactor variable name

homeassistant/components/smartthings/climate.py

@@ -1,8 +1,10 @@
 """Support for climate devices through the SmartThings cloud API."""
 import asyncio
-from typing import Optional, Sequence
+import logging
+from typing import Iterable, Optional, Sequence
 
-from homeassistant.components.climate import ClimateDevice
+from homeassistant.components.climate import (
+    DOMAIN as CLIMATE_DOMAIN, ClimateDevice)
 from homeassistant.components.climate.const import (
     ATTR_OPERATION_MODE, ATTR_TARGET_TEMP_HIGH, ATTR_TARGET_TEMP_LOW,
     STATE_AUTO, STATE_COOL, STATE_ECO, STATE_HEAT, SUPPORT_FAN_MODE,
@@ -38,6 +40,8 @@ UNIT_MAP = {
     'F': TEMP_FAHRENHEIT
 }
 
+_LOGGER = logging.getLogger(__name__)
+
 
 async def async_setup_platform(
         hass, config, async_add_entities, discovery_info=None):
@@ -50,7 +54,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
     broker = hass.data[DOMAIN][DATA_BROKERS][config_entry.entry_id]
     async_add_entities(
         [SmartThingsThermostat(device) for device in broker.devices.values()
-         if broker.any_assigned(device.device_id, 'climate')])
+         if broker.any_assigned(device.device_id, CLIMATE_DOMAIN)], True)
 
 
 def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
@@ -90,6 +94,8 @@ class SmartThingsThermostat(SmartThingsEntity, ClimateDevice):
         """Init the class."""
         super().__init__(device)
         self._supported_features = self._determine_features()
+        self._current_operation = None
+        self._operations = None
 
     def _determine_features(self):
         from pysmartthings import Capability
@@ -127,6 +133,7 @@ class SmartThingsThermostat(SmartThingsEntity, ClimateDevice):
         if operation_state:
             mode = STATE_TO_MODE[operation_state]
             await self._device.set_thermostat_mode(mode, set_status=True)
+            await self.async_update()
 
         # Heat/cool setpoint
         heating_setpoint = None
@@ -151,6 +158,33 @@ class SmartThingsThermostat(SmartThingsEntity, ClimateDevice):
         # the entity state ahead of receiving the confirming push updates
         self.async_schedule_update_ha_state(True)
 
+    async def async_update(self):
+        """Update the attributes of the climate device."""
+        thermostat_mode = self._device.status.thermostat_mode
+        self._current_operation = MODE_TO_STATE.get(thermostat_mode)
+        if self._current_operation is None:
+            _LOGGER.debug('Device %s (%s) returned an invalid'
+                          'thermostat mode: %s', self._device.label,
+                          self._device.device_id, thermostat_mode)
+
+        supported_modes = self._device.status.supported_thermostat_modes
+        if isinstance(supported_modes, Iterable):
+            operations = set()
+            for mode in supported_modes:
+                state = MODE_TO_STATE.get(mode)
+                if state is not None:
+                    operations.add(state)
+                else:
+                    _LOGGER.debug('Device %s (%s) returned an invalid '
+                                  'supported thermostat mode: %s',
+                                  self._device.label, self._device.device_id,
+                                  mode)
+            self._operations = operations
+        else:
+            _LOGGER.debug('Device %s (%s) returned invalid supported '
+                          'thermostat modes: %s', self._device.label,
+                          self._device.device_id, supported_modes)
+
     @property
     def current_fan_mode(self):
         """Return the fan setting."""
@@ -164,7 +198,7 @@ class SmartThingsThermostat(SmartThingsEntity, ClimateDevice):
     @property
     def current_operation(self):
         """Return current operation ie. heat, cool, idle."""
-        return MODE_TO_STATE[self._device.status.thermostat_mode]
+        return self._current_operation
 
     @property
     def current_temperature(self):
@@ -187,8 +221,7 @@ class SmartThingsThermostat(SmartThingsEntity, ClimateDevice):
     @property
     def operation_list(self):
         """Return the list of available operation modes."""
-        return {MODE_TO_STATE[mode] for mode in
+        return self._operations
-                self._device.status.supported_thermostat_modes}
 
     @property
     def supported_features(self):

tests/components/smartthings/test_climate.py

@@ -19,7 +19,8 @@ from homeassistant.components.climate.const import (
 from homeassistant.components.smartthings import climate
 from homeassistant.components.smartthings.const import DOMAIN
 from homeassistant.const import (
-    ATTR_ENTITY_ID, ATTR_SUPPORTED_FEATURES, ATTR_TEMPERATURE, STATE_OFF)
+    ATTR_ENTITY_ID, ATTR_SUPPORTED_FEATURES, ATTR_TEMPERATURE, STATE_OFF,
+    STATE_UNKNOWN)
 
 from .conftest import setup_platform
 
@@ -95,6 +96,25 @@ def thermostat_fixture(device_factory):
     return device
 
 
+@pytest.fixture(name="buggy_thermostat")
+def buggy_thermostat_fixture(device_factory):
+    """Fixture returns a buggy thermostat."""
+    device = device_factory(
+        "Buggy Thermostat",
+        capabilities=[
+            Capability.temperature_measurement,
+            Capability.thermostat_cooling_setpoint,
+            Capability.thermostat_heating_setpoint,
+            Capability.thermostat_mode],
+        status={
+            Attribute.thermostat_mode: 'heating',
+            Attribute.cooling_setpoint: 74,
+            Attribute.heating_setpoint: 68}
+    )
+    device.status.attributes[Attribute.temperature] = Status(70, 'F', None)
+    return device
+
+
 async def test_async_setup_platform():
     """Test setup platform does nothing (it uses config entries)."""
     await climate.async_setup_platform(None, None, None)
@@ -152,6 +172,29 @@ async def test_thermostat_entity_state(hass, thermostat):
     assert state.attributes[ATTR_CURRENT_HUMIDITY] == 34
 
 
+async def test_buggy_thermostat_entity_state(hass, buggy_thermostat):
+    """Tests the state attributes properly match the thermostat type."""
+    await setup_platform(hass, CLIMATE_DOMAIN, buggy_thermostat)
+    state = hass.states.get('climate.buggy_thermostat')
+    assert state.state == STATE_UNKNOWN
+    assert state.attributes[ATTR_SUPPORTED_FEATURES] == \
+        SUPPORT_OPERATION_MODE | SUPPORT_TARGET_TEMPERATURE_HIGH | \
+        SUPPORT_TARGET_TEMPERATURE_LOW | SUPPORT_TARGET_TEMPERATURE
+    assert ATTR_OPERATION_LIST not in state.attributes
+    assert state.attributes[ATTR_TEMPERATURE] is None
+    assert state.attributes[ATTR_CURRENT_TEMPERATURE] == 21.1  # celsius
+
+
+async def test_buggy_thermostat_invalid_mode(hass, buggy_thermostat):
+    """Tests when an invalid operation mode is included."""
+    buggy_thermostat.status.update_attribute_value(
+        Attribute.supported_thermostat_modes,
+        ['heat', 'emergency heat', 'other'])
+    await setup_platform(hass, CLIMATE_DOMAIN, buggy_thermostat)
+    state = hass.states.get('climate.buggy_thermostat')
+    assert state.attributes[ATTR_OPERATION_LIST] == {'heat'}
+
+
 async def test_set_fan_mode(hass, thermostat):
     """Test the fan mode is set successfully."""
     await setup_platform(hass, CLIMATE_DOMAIN, thermostat)