From cc677ec0f5de04c21c77d7ff6408c9832d1bf0e2 Mon Sep 17 00:00:00 2001
From: andrewsayre <6730289+andrewsayre@users.noreply.github.com>
Date: Thu, 14 Feb 2019 20:22:25 -0600
Subject: [PATCH 1/4] Improve component setup error logging/notification

---
 .../smartthings/.translations/en.json         |  3 +-
 .../components/smartthings/__init__.py        |  2 +-
 .../components/smartthings/config_flow.py     |  9 ++++--
 .../components/smartthings/strings.json       |  3 +-
 requirements_all.txt                          |  2 +-
 requirements_test_all.txt                     |  2 +-
 .../smartthings/test_config_flow.py           | 32 ++++++++++++++++---
 7 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/homeassistant/components/smartthings/.translations/en.json b/homeassistant/components/smartthings/.translations/en.json
index f2775b30ae23b1..2091ddb00a265b 100644
--- a/homeassistant/components/smartthings/.translations/en.json
+++ b/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": {
diff --git a/homeassistant/components/smartthings/__init__.py b/homeassistant/components/smartthings/__init__.py
index 04da29aa55e69c..af9730b219676f 100644
--- a/homeassistant/components/smartthings/__init__.py
+++ b/homeassistant/components/smartthings/__init__.py
@@ -22,7 +22,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__)
diff --git a/homeassistant/components/smartthings/config_flow.py b/homeassistant/components/smartthings/config_flow.py
index b280036a6152ba..a5b343f6e97c3c 100644
--- a/homeassistant/components/smartthings/config_flow.py
+++ b/homeassistant/components/smartthings/config_flow.py
@@ -1,7 +1,6 @@
 """Config flow to configure SmartThings."""
 import logging
 
-from aiohttp.client_exceptions import ClientResponseError
 import voluptuous as vol
 
 from homeassistant import config_entries
@@ -50,7 +49,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 +86,17 @@ 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 ClientResponseError as ex:
+        except APIResponseError as ex:
             if ex.status == 401:
                 errors[CONF_ACCESS_TOKEN] = "token_unauthorized"
             elif ex.status == 403:
                 errors[CONF_ACCESS_TOKEN] = "token_forbidden"
+            elif ex.is_target_error():
+                errors['base'] = 'webhook_error'
             else:
                 errors['base'] = "app_setup_error"
+            _LOGGER.exception("Unexpected error setting up the SmartApp: %s",
+                              ex.raw_error_response)
             return self._show_step_user(errors)
         except Exception:  # pylint:disable=broad-except
             errors['base'] = "app_setup_error"
diff --git a/homeassistant/components/smartthings/strings.json b/homeassistant/components/smartthings/strings.json
index 1fb4e878cb463a..bcbe02f6011b4e 100644
--- a/homeassistant/components/smartthings/strings.json
+++ b/homeassistant/components/smartthings/strings.json
@@ -21,7 +21,8 @@
         "token_already_setup": "The token has already been setup.",
         "app_setup_error": "Unable to setup the SmartApp.  Please try again.",
         "app_not_installed": "Please ensure you have installed and authorized the Home Assistant SmartApp and try again.",
-        "base_url_not_https": "The `base_url` for the `http` component must be configured and start with `https://`."
+        "base_url_not_https": "The `base_url` for the `http` component must be configured and start with `https://`.",
+        "webhook_error": "SmartThings could not validate the endpoint configured in `base_url`. Please review the [component requirements]({component_url})."
     }
   }
 }
\ No newline at end of file
diff --git a/requirements_all.txt b/requirements_all.txt
index 01a2b576e5acd1..bcc42caa52670c 100644
--- a/requirements_all.txt
+++ b/requirements_all.txt
@@ -1241,7 +1241,7 @@ pysma==0.3.1
 pysmartapp==0.3.0
 
 # homeassistant.components.smartthings
-pysmartthings==0.6.1
+pysmartthings==0.6.2
 
 # homeassistant.components.device_tracker.snmp
 # homeassistant.components.sensor.snmp
diff --git a/requirements_test_all.txt b/requirements_test_all.txt
index a4ebc2301e16b6..50e83ba952be72 100644
--- a/requirements_test_all.txt
+++ b/requirements_test_all.txt
@@ -217,7 +217,7 @@ pyqwikswitch==0.8
 pysmartapp==0.3.0
 
 # homeassistant.components.smartthings
-pysmartthings==0.6.1
+pysmartthings==0.6.2
 
 # homeassistant.components.sonos
 pysonos==0.0.6
diff --git a/tests/components/smartthings/test_config_flow.py b/tests/components/smartthings/test_config_flow.py
index 4d2a43a52c7508..b5282ec89572e9 100644
--- a/tests/components/smartthings/test_config_flow.py
+++ b/tests/components/smartthings/test_config_flow.py
@@ -1,8 +1,8 @@
 """Tests for the SmartThings config flow module."""
-from unittest.mock import patch
+from unittest.mock import Mock, patch
 from uuid import uuid4
 
-from aiohttp.client_exceptions import ClientResponseError
+from pysmartthings import APIResponseError
 
 from homeassistant import data_entry_flow
 from homeassistant.components.smartthings.config_flow import (
@@ -78,8 +78,9 @@ async def test_token_unauthorized(hass, smartthings_mock):
     flow = SmartThingsFlowHandler()
     flow.hass = hass
 
+    data = {'error': {}}
     smartthings_mock.return_value.apps.return_value = mock_coro(
-        exception=ClientResponseError(None, None, status=401))
+        exception=APIResponseError(None, None, data=data, status=401))
 
     result = await flow.async_step_user({'access_token': str(uuid4())})
 
@@ -93,8 +94,9 @@ async def test_token_forbidden(hass, smartthings_mock):
     flow = SmartThingsFlowHandler()
     flow.hass = hass
 
+    data = {'error': {}}
     smartthings_mock.return_value.apps.return_value = mock_coro(
-        exception=ClientResponseError(None, None, status=403))
+        exception=APIResponseError(None, None, data=data, status=403))
 
     result = await flow.async_step_user({'access_token': str(uuid4())})
 
@@ -103,13 +105,33 @@ async def test_token_forbidden(hass, smartthings_mock):
     assert result['errors'] == {'access_token': 'token_forbidden'}
 
 
+async def test_webhook_error(hass, smartthings_mock):
+    """Test an error is shown when the token is forbidden."""
+    flow = SmartThingsFlowHandler()
+    flow.hass = hass
+
+    data = {'error': {}}
+    error = APIResponseError(None, None, data=data, status=422)
+    error.is_target_error = Mock(return_value=True)
+
+    smartthings_mock.return_value.apps.return_value = mock_coro(
+        exception=error)
+
+    result = await flow.async_step_user({'access_token': str(uuid4())})
+
+    assert result['type'] == data_entry_flow.RESULT_TYPE_FORM
+    assert result['step_id'] == 'user'
+    assert result['errors'] == {'base': 'webhook_error'}
+
+
 async def test_unknown_api_error(hass, smartthings_mock):
     """Test an error is shown when there is an unknown API error."""
     flow = SmartThingsFlowHandler()
     flow.hass = hass
 
+    data = {'error': {}}
     smartthings_mock.return_value.apps.return_value = mock_coro(
-        exception=ClientResponseError(None, None, status=500))
+        exception=APIResponseError(None, None, data=data, status=500))
 
     result = await flow.async_step_user({'access_token': str(uuid4())})
 

From 873f5e47a4a8ceb7e182fab71134829d6d20669f Mon Sep 17 00:00:00 2001
From: andrewsayre <6730289+andrewsayre@users.noreply.github.com>
Date: Thu, 14 Feb 2019 22:05:53 -0600
Subject: [PATCH 2/4] Prevent capabilities from being represented my multiple
 platforms

---
 .../components/smartthings/__init__.py        | 33 +++++++++++++++++
 .../components/smartthings/binary_sensor.py   | 15 ++++++--
 .../components/smartthings/climate.py         | 36 +++++++++++--------
 homeassistant/components/smartthings/const.py |  8 +++--
 homeassistant/components/smartthings/fan.py   | 14 +++++---
 homeassistant/components/smartthings/light.py | 26 ++++++++------
 homeassistant/components/smartthings/lock.py  | 13 ++++---
 .../components/smartthings/sensor.py          | 21 +++++++----
 .../components/smartthings/switch.py          | 25 +++++--------
 tests/components/smartthings/test_climate.py  | 13 -------
 tests/components/smartthings/test_fan.py      | 20 -----------
 tests/components/smartthings/test_light.py    | 19 ----------
 tests/components/smartthings/test_lock.py     |  6 ----
 tests/components/smartthings/test_switch.py   | 17 ---------
 14 files changed, 127 insertions(+), 139 deletions(-)

diff --git a/homeassistant/components/smartthings/__init__.py b/homeassistant/components/smartthings/__init__.py
index af9730b219676f..64d2523606ce09 100644
--- a/homeassistant/components/smartthings/__init__.py
+++ b/homeassistant/components/smartthings/__init__.py
@@ -1,5 +1,6 @@
 """Support for SmartThings Cloud."""
 import asyncio
+import importlib
 import logging
 from typing import Iterable
 
@@ -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
diff --git a/homeassistant/components/smartthings/binary_sensor.py b/homeassistant/components/smartthings/binary_sensor.py
index 2fbb6f719da763..2af42e73e33e7c 100644
--- a/homeassistant/components/smartthings/binary_sensor.py
+++ b/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]]:
+    """Get the supported capabilities from the supplied list."""
+    return [capability for capability in CAPABILITY_TO_ATTRIB
+            if capability in capabilities]
+
+
 class SmartThingsBinarySensor(SmartThingsEntity, BinarySensorDevice):
     """Define a SmartThings Binary Sensor."""
 
diff --git a/homeassistant/components/smartthings/climate.py b/homeassistant/components/smartthings/climate.py
index d70d865202df4d..ee2faa23983de7 100644
--- a/homeassistant/components/smartthings/climate.py
+++ b/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]]:
+    """Get the supported capabilities from the supplied list."""
     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):
diff --git a/homeassistant/components/smartthings/const.py b/homeassistant/components/smartthings/const.py
index 25cd9e8305f207..27260b155d138f 100644
--- a/homeassistant/components/smartthings/const.py
+++ b/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}))$"
diff --git a/homeassistant/components/smartthings/fan.py b/homeassistant/components/smartthings/fan.py
index 4de1744c9b8bb2..d2c4db319a10f1 100644
--- a/homeassistant/components/smartthings/fan.py
+++ b/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]]:
+    """Get the supported capabilities from the supplied list."""
     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):
diff --git a/homeassistant/components/smartthings/light.py b/homeassistant/components/smartthings/light.py
index ce4b00ca1fe8c4..0366785ca419f1 100644
--- a/homeassistant/components/smartthings/light.py
+++ b/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]]:
+    """Get the supported capabilities from the supplied list."""
     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):
diff --git a/homeassistant/components/smartthings/lock.py b/homeassistant/components/smartthings/lock.py
index e756cbfa918338..f681e21576a794 100644
--- a/homeassistant/components/smartthings/lock.py
+++ b/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]]:
+    """Get the supported capabilities from the supplied list."""
     from pysmartthings import Capability
-    return Capability.lock in device.capabilities
+
+    if Capability.lock in capabilities:
+        return [Capability.lock]
+    return None
 
 
 class SmartThingsLock(SmartThingsEntity, LockDevice):
diff --git a/homeassistant/components/smartthings/sensor.py b/homeassistant/components/smartthings/sensor.py
index eb83334c6b3d15..d12cd482d77fe4 100644
--- a/homeassistant/components/smartthings/sensor.py
+++ b/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]]:
+    """Get the supported capabilities from the supplied list."""
+    return [capability for capability in CAPABILITY_TO_SENSORS
+            if capability in capabilities]
+
+
 class SmartThingsSensor(SmartThingsEntity):
     """Define a SmartThings Binary Sensor."""
 
diff --git a/homeassistant/components/smartthings/switch.py b/homeassistant/components/smartthings/switch.py
index 08cdb74ed774e3..a26a769beaecbb 100644
--- a/homeassistant/components/smartthings/switch.py
+++ b/homeassistant/components/smartthings/switch.py
@@ -1,4 +1,6 @@
 """Support for switches through the SmartThings cloud API."""
+from typing import Optional, Sequence
+
 from homeassistant.components.switch import SwitchDevice
 
 from . import SmartThingsEntity
@@ -18,28 +20,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(
         [SmartThingsSwitch(device) for device in broker.devices.values()
-         if is_switch(device)])
+         if broker.any_assigned(device.device_id, 'switch')])
 
 
-def is_switch(device):
-    """Determine if the device should be represented as a switch."""
+def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
+    """Get the supported capabilities from the supplied list."""
     from pysmartthings import Capability
 
     # Must be able to be turned on/off.
-    if Capability.switch not in device.capabilities:
-        return False
-    # Must not have a capability represented by other types.
-    non_switch_capabilities = [
-        Capability.color_control,
-        Capability.color_temperature,
-        Capability.fan_speed,
-        Capability.switch_level
-    ]
-    if any(capability in device.capabilities
-           for capability in non_switch_capabilities):
-        return False
-
-    return True
+    if Capability.switch in capabilities:
+        return [Capability.switch]
+    return None
 
 
 class SmartThingsSwitch(SmartThingsEntity, SwitchDevice):
diff --git a/tests/components/smartthings/test_climate.py b/tests/components/smartthings/test_climate.py
index c615ca66e34c48..d8b1346d225bfb 100644
--- a/tests/components/smartthings/test_climate.py
+++ b/tests/components/smartthings/test_climate.py
@@ -100,19 +100,6 @@ async def test_async_setup_platform():
     await climate.async_setup_platform(None, None, None)
 
 
-def test_is_climate(device_factory, legacy_thermostat,
-                    basic_thermostat, thermostat):
-    """Test climate devices are correctly identified."""
-    other_devices = [
-        device_factory('Unknown', ['Unknown']),
-        device_factory("Switch 1", [Capability.switch])
-    ]
-    for device in [legacy_thermostat, basic_thermostat, thermostat]:
-        assert climate.is_climate(device), device.name
-    for device in other_devices:
-        assert not climate.is_climate(device), device.name
-
-
 async def test_legacy_thermostat_entity_state(hass, legacy_thermostat):
     """Tests the state attributes properly match the thermostat type."""
     await setup_platform(hass, CLIMATE_DOMAIN, legacy_thermostat)
diff --git a/tests/components/smartthings/test_fan.py b/tests/components/smartthings/test_fan.py
index 99627e866d9ff2..db8d9b512de397 100644
--- a/tests/components/smartthings/test_fan.py
+++ b/tests/components/smartthings/test_fan.py
@@ -39,26 +39,6 @@ async def test_async_setup_platform():
     await fan.async_setup_platform(None, None, None)
 
 
-def test_is_fan(device_factory):
-    """Test fans are correctly identified."""
-    non_fans = [
-        device_factory('Unknown', ['Unknown']),
-        device_factory("Switch 1", [Capability.switch]),
-        device_factory("Non-Switchable Fan", [Capability.fan_speed]),
-        device_factory("Color Light",
-                       [Capability.switch, Capability.switch_level,
-                        Capability.color_control,
-                        Capability.color_temperature])
-    ]
-    fan_device = device_factory(
-        "Fan 1", [Capability.switch, Capability.switch_level,
-                  Capability.fan_speed])
-
-    assert fan.is_fan(fan_device), fan_device.name
-    for device in non_fans:
-        assert not fan.is_fan(device), device.name
-
-
 async def test_entity_state(hass, device_factory):
     """Tests the state attributes properly match the fan types."""
     device = device_factory(
diff --git a/tests/components/smartthings/test_light.py b/tests/components/smartthings/test_light.py
index a4f1103f270f58..72bc5da906319a 100644
--- a/tests/components/smartthings/test_light.py
+++ b/tests/components/smartthings/test_light.py
@@ -65,25 +65,6 @@ async def test_async_setup_platform():
     await light.async_setup_platform(None, None, None)
 
 
-def test_is_light(device_factory, light_devices):
-    """Test lights are correctly identified."""
-    non_lights = [
-        device_factory('Unknown', ['Unknown']),
-        device_factory("Fan 1",
-                       [Capability.switch, Capability.switch_level,
-                        Capability.fan_speed]),
-        device_factory("Switch 1", [Capability.switch]),
-        device_factory("Can't be turned off",
-                       [Capability.switch_level, Capability.color_control,
-                        Capability.color_temperature])
-    ]
-
-    for device in light_devices:
-        assert light.is_light(device), device.name
-    for device in non_lights:
-        assert not light.is_light(device), device.name
-
-
 async def test_entity_state(hass, light_devices):
     """Tests the state attributes properly match the light types."""
     await _setup_platform(hass, *light_devices)
diff --git a/tests/components/smartthings/test_lock.py b/tests/components/smartthings/test_lock.py
index c73f4ff549ee59..3739a2dc9b5179 100644
--- a/tests/components/smartthings/test_lock.py
+++ b/tests/components/smartthings/test_lock.py
@@ -20,12 +20,6 @@ async def test_async_setup_platform():
     await lock.async_setup_platform(None, None, None)
 
 
-def test_is_lock(device_factory):
-    """Test locks are correctly identified."""
-    lock_device = device_factory('Lock', [Capability.lock])
-    assert lock.is_lock(lock_device)
-
-
 async def test_entity_and_device_attributes(hass, device_factory):
     """Test the attributes of the entity are correct."""
     # Arrange
diff --git a/tests/components/smartthings/test_switch.py b/tests/components/smartthings/test_switch.py
index 15ff3adce86c79..3f2bedd4f1315c 100644
--- a/tests/components/smartthings/test_switch.py
+++ b/tests/components/smartthings/test_switch.py
@@ -35,23 +35,6 @@ async def test_async_setup_platform():
     await switch.async_setup_platform(None, None, None)
 
 
-def test_is_switch(device_factory):
-    """Test switches are correctly identified."""
-    switch_device = device_factory('Switch', [Capability.switch])
-    non_switch_devices = [
-        device_factory('Light', [Capability.switch, Capability.switch_level]),
-        device_factory('Fan', [Capability.switch, Capability.fan_speed]),
-        device_factory('Color Light', [Capability.switch,
-                                       Capability.color_control]),
-        device_factory('Temp Light', [Capability.switch,
-                                      Capability.color_temperature]),
-        device_factory('Unknown', ['Unknown']),
-    ]
-    assert switch.is_switch(switch_device)
-    for non_switch_device in non_switch_devices:
-        assert not switch.is_switch(non_switch_device)
-
-
 async def test_entity_and_device_attributes(hass, device_factory):
     """Test the attributes of the entity are correct."""
     # Arrange

From cf2492a0c2a99854dd97aece2bd018250db31db7 Mon Sep 17 00:00:00 2001
From: andrewsayre <6730289+andrewsayre@users.noreply.github.com>
Date: Thu, 14 Feb 2019 22:11:26 -0600
Subject: [PATCH 3/4] Improved logging of received updates

---
 homeassistant/components/smartthings/__init__.py | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/homeassistant/components/smartthings/__init__.py b/homeassistant/components/smartthings/__init__.py
index 64d2523606ce09..3cf38c358bc76f 100644
--- a/homeassistant/components/smartthings/__init__.py
+++ b/homeassistant/components/smartthings/__init__.py
@@ -200,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)

From 518ceeabc24e2844518907d9d813287e9ef3a943 Mon Sep 17 00:00:00 2001
From: andrewsayre <6730289+andrewsayre@users.noreply.github.com>
Date: Fri, 15 Feb 2019 07:01:13 -0600
Subject: [PATCH 4/4] Updates based on review feedback

---
 .../components/smartthings/binary_sensor.py   |  2 +-
 .../components/smartthings/climate.py         |  2 +-
 .../components/smartthings/config_flow.py     | 14 ++++++---
 homeassistant/components/smartthings/fan.py   |  2 +-
 homeassistant/components/smartthings/light.py |  2 +-
 homeassistant/components/smartthings/lock.py  |  2 +-
 .../components/smartthings/sensor.py          |  2 +-
 .../components/smartthings/switch.py          |  2 +-
 .../smartthings/test_config_flow.py           | 30 ++++++++++++++-----
 9 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/homeassistant/components/smartthings/binary_sensor.py b/homeassistant/components/smartthings/binary_sensor.py
index 2af42e73e33e7c..45101601d5ffb2 100644
--- a/homeassistant/components/smartthings/binary_sensor.py
+++ b/homeassistant/components/smartthings/binary_sensor.py
@@ -51,7 +51,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
 
 
 def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
-    """Get the supported capabilities from the supplied list."""
+    """Return all capabilities supported if minimum required are present."""
     return [capability for capability in CAPABILITY_TO_ATTRIB
             if capability in capabilities]
 
diff --git a/homeassistant/components/smartthings/climate.py b/homeassistant/components/smartthings/climate.py
index ee2faa23983de7..d1f58cf91f17b6 100644
--- a/homeassistant/components/smartthings/climate.py
+++ b/homeassistant/components/smartthings/climate.py
@@ -54,7 +54,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
 
 
 def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
-    """Get the supported capabilities from the supplied list."""
+    """Return all capabilities supported if minimum required are present."""
     from pysmartthings import Capability
 
     supported = [
diff --git a/homeassistant/components/smartthings/config_flow.py b/homeassistant/components/smartthings/config_flow.py
index a5b343f6e97c3c..4663222c3b4403 100644
--- a/homeassistant/components/smartthings/config_flow.py
+++ b/homeassistant/components/smartthings/config_flow.py
@@ -1,6 +1,7 @@
 """Config flow to configure SmartThings."""
 import logging
 
+from aiohttp import ClientResponseError
 import voluptuous as vol
 
 from homeassistant import config_entries
@@ -87,16 +88,21 @@ async def async_step_user(self, user_input=None):
             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"
-            elif ex.is_target_error():
-                errors['base'] = 'webhook_error'
             else:
                 errors['base'] = "app_setup_error"
-            _LOGGER.exception("Unexpected error setting up the SmartApp: %s",
-                              ex.raw_error_response)
+                _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"
diff --git a/homeassistant/components/smartthings/fan.py b/homeassistant/components/smartthings/fan.py
index d2c4db319a10f1..e722cd21d65a61 100644
--- a/homeassistant/components/smartthings/fan.py
+++ b/homeassistant/components/smartthings/fan.py
@@ -35,7 +35,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
 
 
 def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
-    """Get the supported capabilities from the supplied list."""
+    """Return all capabilities supported if minimum required are present."""
     from pysmartthings import Capability
 
     supported = [Capability.switch, Capability.fan_speed]
diff --git a/homeassistant/components/smartthings/light.py b/homeassistant/components/smartthings/light.py
index 0366785ca419f1..79a5eabc20a3cd 100644
--- a/homeassistant/components/smartthings/light.py
+++ b/homeassistant/components/smartthings/light.py
@@ -29,7 +29,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
 
 
 def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
-    """Get the supported capabilities from the supplied list."""
+    """Return all capabilities supported if minimum required are present."""
     from pysmartthings import Capability
 
     supported = [
diff --git a/homeassistant/components/smartthings/lock.py b/homeassistant/components/smartthings/lock.py
index f681e21576a794..bc5ab7a8ccdd1c 100644
--- a/homeassistant/components/smartthings/lock.py
+++ b/homeassistant/components/smartthings/lock.py
@@ -31,7 +31,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
 
 
 def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
-    """Get the supported capabilities from the supplied list."""
+    """Return all capabilities supported if minimum required are present."""
     from pysmartthings import Capability
 
     if Capability.lock in capabilities:
diff --git a/homeassistant/components/smartthings/sensor.py b/homeassistant/components/smartthings/sensor.py
index d12cd482d77fe4..32047c179b4113 100644
--- a/homeassistant/components/smartthings/sensor.py
+++ b/homeassistant/components/smartthings/sensor.py
@@ -176,7 +176,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
 
 
 def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
-    """Get the supported capabilities from the supplied list."""
+    """Return all capabilities supported if minimum required are present."""
     return [capability for capability in CAPABILITY_TO_SENSORS
             if capability in capabilities]
 
diff --git a/homeassistant/components/smartthings/switch.py b/homeassistant/components/smartthings/switch.py
index a26a769beaecbb..5a1224f4fc2d93 100644
--- a/homeassistant/components/smartthings/switch.py
+++ b/homeassistant/components/smartthings/switch.py
@@ -24,7 +24,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities):
 
 
 def get_capabilities(capabilities: Sequence[str]) -> Optional[Sequence[str]]:
-    """Get the supported capabilities from the supplied list."""
+    """Return all capabilities supported if minimum required are present."""
     from pysmartthings import Capability
 
     # Must be able to be turned on/off.
diff --git a/tests/components/smartthings/test_config_flow.py b/tests/components/smartthings/test_config_flow.py
index b5282ec89572e9..7d3357031319ce 100644
--- a/tests/components/smartthings/test_config_flow.py
+++ b/tests/components/smartthings/test_config_flow.py
@@ -2,6 +2,7 @@
 from unittest.mock import Mock, patch
 from uuid import uuid4
 
+from aiohttp import ClientResponseError
 from pysmartthings import APIResponseError
 
 from homeassistant import data_entry_flow
@@ -78,9 +79,8 @@ async def test_token_unauthorized(hass, smartthings_mock):
     flow = SmartThingsFlowHandler()
     flow.hass = hass
 
-    data = {'error': {}}
     smartthings_mock.return_value.apps.return_value = mock_coro(
-        exception=APIResponseError(None, None, data=data, status=401))
+        exception=ClientResponseError(None, None, status=401))
 
     result = await flow.async_step_user({'access_token': str(uuid4())})
 
@@ -94,9 +94,8 @@ async def test_token_forbidden(hass, smartthings_mock):
     flow = SmartThingsFlowHandler()
     flow.hass = hass
 
-    data = {'error': {}}
     smartthings_mock.return_value.apps.return_value = mock_coro(
-        exception=APIResponseError(None, None, data=data, status=403))
+        exception=ClientResponseError(None, None, status=403))
 
     result = await flow.async_step_user({'access_token': str(uuid4())})
 
@@ -106,7 +105,7 @@ async def test_token_forbidden(hass, smartthings_mock):
 
 
 async def test_webhook_error(hass, smartthings_mock):
-    """Test an error is shown when the token is forbidden."""
+    """Test an error is when there's an error with the webhook endpoint."""
     flow = SmartThingsFlowHandler()
     flow.hass = hass
 
@@ -124,14 +123,31 @@ async def test_webhook_error(hass, smartthings_mock):
     assert result['errors'] == {'base': 'webhook_error'}
 
 
+async def test_api_error(hass, smartthings_mock):
+    """Test an error is shown when other API errors occur."""
+    flow = SmartThingsFlowHandler()
+    flow.hass = hass
+
+    data = {'error': {}}
+    error = APIResponseError(None, None, data=data, status=400)
+
+    smartthings_mock.return_value.apps.return_value = mock_coro(
+        exception=error)
+
+    result = await flow.async_step_user({'access_token': str(uuid4())})
+
+    assert result['type'] == data_entry_flow.RESULT_TYPE_FORM
+    assert result['step_id'] == 'user'
+    assert result['errors'] == {'base': 'app_setup_error'}
+
+
 async def test_unknown_api_error(hass, smartthings_mock):
     """Test an error is shown when there is an unknown API error."""
     flow = SmartThingsFlowHandler()
     flow.hass = hass
 
-    data = {'error': {}}
     smartthings_mock.return_value.apps.return_value = mock_coro(
-        exception=APIResponseError(None, None, data=data, status=500))
+        exception=ClientResponseError(None, None, status=404))
 
     result = await flow.async_step_user({'access_token': str(uuid4())})