SmartThings Component Enhancements/Fixes (home-assistant#21085)

* Improve component setup error logging/notification

* Prevent capabilities from being represented my multiple platforms

* Improved logging of received updates

* Updates based on review feedback

homeassistant/components/smartthings/.translations/en.json

@@ -7,7 +7,8 @@
             "token_already_setup": "The token has already been setup.",
             "token_forbidden": "The token does not have the required OAuth scopes.",
             "token_invalid_format": "The token must be in the UID/GUID format",
-            "token_unauthorized": "The token is invalid or no longer authorized."
+            "token_unauthorized": "The token is invalid or no longer authorized.",
+            "webhook_error": "SmartThings could not validate the endpoint configured in `base_url`. Please review the [component requirements]({component_url})."
         },
         "step": {
             "user": {

homeassistant/components/smartthings/__init__.py

@@ -1,5 +1,6 @@
 """Support for SmartThings Cloud."""
 import asyncio
+import importlib
 import logging
 from typing import Iterable
 
@@ -22,7 +23,7 @@
 from .smartapp import (
     setup_smartapp, setup_smartapp_endpoint, validate_installed_app)
 
-REQUIREMENTS = ['pysmartapp==0.3.0', 'pysmartthings==0.6.1']
+REQUIREMENTS = ['pysmartapp==0.3.0', 'pysmartthings==0.6.2']
 DEPENDENCIES = ['webhook']
 
 _LOGGER = logging.getLogger(__name__)
@@ -132,9 +133,41 @@ def __init__(self, hass: HomeAssistantType, devices: Iterable,
         """Create a new instance of the DeviceBroker."""
         self._hass = hass
         self._installed_app_id = installed_app_id
+        self.assignments = self._assign_capabilities(devices)
         self.devices = {device.device_id: device for device in devices}
         self.event_handler_disconnect = None
 
+    def _assign_capabilities(self, devices: Iterable):
+        """Assign platforms to capabilities."""
+        assignments = {}
+        for device in devices:
+            capabilities = device.capabilities.copy()
+            slots = {}
+            for platform_name in SUPPORTED_PLATFORMS:
+                platform = importlib.import_module(
+                    '.' + platform_name, self.__module__)
+                assigned = platform.get_capabilities(capabilities)
+                if not assigned:
+                    continue
+                # Draw-down capabilities and set slot assignment
+                for capability in assigned:
+                    if capability not in capabilities:
+                        continue
+                    capabilities.remove(capability)
+                    slots[capability] = platform_name
+            assignments[device.device_id] = slots
+        return assignments
+
+    def get_assigned(self, device_id: str, platform: str):
+        """Get the capabilities assigned to the platform."""
+        slots = self.assignments.get(device_id, {})
+        return [key for key, value in slots.items() if value == platform]
+
+    def any_assigned(self, device_id: str, platform: str):
+        """Return True if the platform has any assigned capabilities."""
+        slots = self.assignments.get(device_id, {})
+        return any(value for value in slots.values() if value == platform)
+
     async def event_handler(self, req, resp, app):
         """Broker for incoming events."""
         from pysmartapp.event import EVENT_TYPE_DEVICE
@@ -167,10 +200,18 @@ async def event_handler(self, req, resp, app):
                 }
                 self._hass.bus.async_fire(EVENT_BUTTON, data)
                 _LOGGER.debug("Fired button event: %s", data)
+            else:
+                data = {
+                    'location_id': evt.location_id,
+                    'device_id': evt.device_id,
+                    'component_id': evt.component_id,
+                    'capability': evt.capability,
+                    'attribute': evt.attribute,
+                    'value': evt.value,
+                }
+                _LOGGER.debug("Push update received: %s", data)
 
             updated_devices.add(device.device_id)
-        _LOGGER.debug("Update received with %s events and updated %s devices",
-                      len(req.events), len(updated_devices))
 
         async_dispatcher_send(self._hass, SIGNAL_SMARTTHINGS_UPDATE,
                               updated_devices)

homeassistant/components/smartthings/binary_sensor.py

@@ -1,4 +1,6 @@
 """Support for binary sensors through the SmartThings cloud API."""
+from typing import Optional, Sequence
+
 from homeassistant.components.binary_sensor import BinarySensorDevice
 
 from . import SmartThingsEntity
@@ -41,12 +43,19 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
     broker = hass.data[DOMAIN][DATA_BROKERS][config_entry.entry_id]
     sensors = []
     for device in broker.devices.values():
-        for capability, attrib in CAPABILITY_TO_ATTRIB.items():
-            if capability in device.capabilities:
-                sensors.append(SmartThingsBinarySensor(device, attrib))
+        for capability in broker.get_assigned(
+                device.device_id, 'binary_sensor'):
+            attrib = CAPABILITY_TO_ATTRIB[capability]
+            sensors.append(SmartThingsBinarySensor(device, attrib))
     async_add_entities(sensors)
 
 
+def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
+    """Return all capabilities supported if minimum required are present."""
+    return [capability for capability in CAPABILITY_TO_ATTRIB
+            if capability in capabilities]
+
+
 class SmartThingsBinarySensor(SmartThingsEntity, BinarySensorDevice):
     """Define a SmartThings Binary Sensor."""
 

homeassistant/components/smartthings/climate.py

@@ -1,11 +1,12 @@
 """Support for climate devices through the SmartThings cloud API."""
 import asyncio
+from typing import Optional, Sequence
 
 from homeassistant.components.climate import 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, SUPPORT_OPERATION_MODE, SUPPORT_TARGET_TEMPERATURE,
+    STATE_AUTO, STATE_COOL, STATE_ECO, STATE_HEAT, SUPPORT_FAN_MODE,
+    SUPPORT_OPERATION_MODE, SUPPORT_TARGET_TEMPERATURE,
     SUPPORT_TARGET_TEMPERATURE_HIGH, SUPPORT_TARGET_TEMPERATURE_LOW)
 from homeassistant.const import (
     ATTR_TEMPERATURE, STATE_OFF, TEMP_CELSIUS, TEMP_FAHRENHEIT)
@@ -49,30 +50,37 @@ 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 is_climate(device)])
+         if broker.any_assigned(device.device_id, 'climate')])
 
 
-def is_climate(device):
-    """Determine if the device should be represented as a climate entity."""
+def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
+    """Return all capabilities supported if minimum required are present."""
     from pysmartthings import Capability
 
+    supported = [
+        Capability.thermostat,
+        Capability.temperature_measurement,
+        Capability.thermostat_cooling_setpoint,
+        Capability.thermostat_heating_setpoint,
+        Capability.thermostat_mode,
+        Capability.relative_humidity_measurement,
+        Capability.thermostat_operating_state,
+        Capability.thermostat_fan_mode
+    ]
     # Can have this legacy/deprecated capability
-    if Capability.thermostat in device.capabilities:
-        return True
+    if Capability.thermostat in capabilities:
+        return supported
     # Or must have all of these
     climate_capabilities = [
         Capability.temperature_measurement,
         Capability.thermostat_cooling_setpoint,
         Capability.thermostat_heating_setpoint,
         Capability.thermostat_mode]
-    if all(capability in device.capabilities
+    if all(capability in capabilities
            for capability in climate_capabilities):
-        return True
-    # Optional capabilities:
-    # relative_humidity_measurement -> state attribs
-    # thermostat_operating_state -> state attribs
-    # thermostat_fan_mode -> SUPPORT_FAN_MODE
-    return False
+        return supported
+
+    return None
 
 
 class SmartThingsThermostat(SmartThingsEntity, ClimateDevice):

homeassistant/components/smartthings/config_flow.py

@@ -1,7 +1,7 @@
 """Config flow to configure SmartThings."""
 import logging
 
-from aiohttp.client_exceptions import ClientResponseError
+from aiohttp import ClientResponseError
 import voluptuous as vol
 
 from homeassistant import config_entries
@@ -50,7 +50,7 @@ async def async_step_import(self, user_input=None):
 
     async def async_step_user(self, user_input=None):
         """Get access token and validate it."""
-        from pysmartthings import SmartThings
+        from pysmartthings import APIResponseError, SmartThings
 
         errors = {}
         if not self.hass.config.api.base_url.lower().startswith('https://'):
@@ -87,13 +87,22 @@ async def async_step_user(self, user_input=None):
                 app = await create_app(self.hass, self.api)
             setup_smartapp(self.hass, app)
             self.app_id = app.app_id
+        except APIResponseError as ex:
+            if ex.is_target_error():
+                errors['base'] = 'webhook_error'
+            else:
+                errors['base'] = "app_setup_error"
+            _LOGGER.exception("API error setting up the SmartApp: %s",
+                              ex.raw_error_response)
+            return self._show_step_user(errors)
         except ClientResponseError as ex:
             if ex.status == 401:
                 errors[CONF_ACCESS_TOKEN] = "token_unauthorized"
             elif ex.status == 403:
                 errors[CONF_ACCESS_TOKEN] = "token_forbidden"
             else:
                 errors['base'] = "app_setup_error"
+                _LOGGER.exception("Unexpected error setting up the SmartApp")
             return self._show_step_user(errors)
         except Exception:  # pylint:disable=broad-except
             errors['base'] = "app_setup_error"

homeassistant/components/smartthings/const.py

@@ -18,14 +18,16 @@
 SETTINGS_INSTANCE_ID = "hassInstanceId"
 STORAGE_KEY = DOMAIN
 STORAGE_VERSION = 1
+# Ordered 'specific to least-specific platform' in order for capabilities
+# to be drawn-down and represented by the appropriate platform.
 SUPPORTED_PLATFORMS = [
-    'binary_sensor',
     'climate',
     'fan',
     'light',
     'lock',
-    'sensor',
-    'switch'
+    'switch',
+    'binary_sensor',
+    'sensor'
 ]
 VAL_UID = "^(?:([0-9a-fA-F]{32})|([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]" \
           "{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}))$"

homeassistant/components/smartthings/fan.py

@@ -1,4 +1,6 @@
 """Support for fans through the SmartThings cloud API."""
+from typing import Optional, Sequence
+
 from homeassistant.components.fan import (
     SPEED_HIGH, SPEED_LOW, SPEED_MEDIUM, SPEED_OFF, SUPPORT_SET_SPEED,
     FanEntity)
@@ -29,15 +31,17 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
     broker = hass.data[DOMAIN][DATA_BROKERS][config_entry.entry_id]
     async_add_entities(
         [SmartThingsFan(device) for device in broker.devices.values()
-         if is_fan(device)])
+         if broker.any_assigned(device.device_id, 'fan')])
 
 
-def is_fan(device):
-    """Determine if the device should be represented as a fan."""
+def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
+    """Return all capabilities supported if minimum required are present."""
     from pysmartthings import Capability
+
+    supported = [Capability.switch, Capability.fan_speed]
     # Must have switch and fan_speed
-    return all(capability in device.capabilities
-               for capability in [Capability.switch, Capability.fan_speed])
+    if all(capability in capabilities for capability in supported):
+        return supported
 
 
 class SmartThingsFan(SmartThingsEntity, FanEntity):

homeassistant/components/smartthings/light.py

@@ -1,5 +1,6 @@
 """Support for lights through the SmartThings cloud API."""
 import asyncio
+from typing import Optional, Sequence
 
 from homeassistant.components.light import (
     ATTR_BRIGHTNESS, ATTR_COLOR_TEMP, ATTR_HS_COLOR, ATTR_TRANSITION,
@@ -24,29 +25,32 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
     broker = hass.data[DOMAIN][DATA_BROKERS][config_entry.entry_id]
     async_add_entities(
         [SmartThingsLight(device) for device in broker.devices.values()
-         if is_light(device)], True)
+         if broker.any_assigned(device.device_id, 'light')], True)
 
 
-def is_light(device):
-    """Determine if the device should be represented as a light."""
+def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
+    """Return all capabilities supported if minimum required are present."""
     from pysmartthings import Capability
 
+    supported = [
+        Capability.switch,
+        Capability.switch_level,
+        Capability.color_control,
+        Capability.color_temperature,
+    ]
     # Must be able to be turned on/off.
-    if Capability.switch not in device.capabilities:
-        return False
-    # Not a fan (which might also have switch_level)
-    if Capability.fan_speed in device.capabilities:
-        return False
+    if Capability.switch not in capabilities:
+        return None
     # Must have one of these
     light_capabilities = [
         Capability.color_control,
         Capability.color_temperature,
         Capability.switch_level
     ]
-    if any(capability in device.capabilities
+    if any(capability in capabilities
            for capability in light_capabilities):
-        return True
-    return False
+        return supported
+    return None
 
 
 def convert_scale(value, value_scale, target_scale, round_digits=4):

homeassistant/components/smartthings/lock.py

@@ -1,4 +1,6 @@
 """Support for locks through the SmartThings cloud API."""
+from typing import Optional, Sequence
+
 from homeassistant.components.lock import LockDevice
 
 from . import SmartThingsEntity
@@ -25,13 +27,16 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
     broker = hass.data[DOMAIN][DATA_BROKERS][config_entry.entry_id]
     async_add_entities(
         [SmartThingsLock(device) for device in broker.devices.values()
-         if is_lock(device)])
+         if broker.any_assigned(device.device_id, 'lock')])
 
 
-def is_lock(device):
-    """Determine if the device supports the lock capability."""
+def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
+    """Return all capabilities supported if minimum required are present."""
     from pysmartthings import Capability
-    return Capability.lock in device.capabilities
+
+    if Capability.lock in capabilities:
+        return [Capability.lock]
+    return None
 
 
 class SmartThingsLock(SmartThingsEntity, LockDevice):

homeassistant/components/smartthings/sensor.py

@@ -1,5 +1,6 @@
 """Support for sensors through the SmartThings cloud API."""
 from collections import namedtuple
+from typing import Optional, Sequence
 
 from homeassistant.const import (
     DEVICE_CLASS_BATTERY, DEVICE_CLASS_HUMIDITY, DEVICE_CLASS_ILLUMINANCE,
@@ -164,16 +165,22 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
     broker = hass.data[DOMAIN][DATA_BROKERS][config_entry.entry_id]
     sensors = []
     for device in broker.devices.values():
-        for capability, maps in CAPABILITY_TO_SENSORS.items():
-            if capability in device.capabilities:
-                sensors.extend([
-                    SmartThingsSensor(
-                        device, m.attribute, m.name, m.default_unit,
-                        m.device_class)
-                    for m in maps])
+        for capability in broker.get_assigned(device.device_id, 'sensor'):
+            maps = CAPABILITY_TO_SENSORS[capability]
+            sensors.extend([
+                SmartThingsSensor(
+                    device, m.attribute, m.name, m.default_unit,
+                    m.device_class)
+                for m in maps])
     async_add_entities(sensors)
 
 
+def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
+    """Return all capabilities supported if minimum required are present."""
+    return [capability for capability in CAPABILITY_TO_SENSORS
+            if capability in capabilities]
+
+
 class SmartThingsSensor(SmartThingsEntity):
     """Define a SmartThings Binary Sensor."""