Clean up OpenUV config flow (#17349)

* Cleaned up OpenUV config flow

* Added proper listener removal

* Added proper exception

* Unnecessary exception message

* Moved API key error to correct place

* Member-requested changes (part 1)

* Hound

* Member-requested changes (part 2)

* Cleanup

* Fixed tests

homeassistant/components/binary_sensor/openuv.py

@@ -50,12 +50,12 @@ class OpenUvBinarySensor(OpenUvEntity, BinarySensorDevice):
         """Initialize the sensor."""
         super().__init__(openuv)
 
+        self._async_unsub_dispatcher_connect = None
         self._entry_id = entry_id
         self._icon = icon
         self._latitude = openuv.client.latitude
         self._longitude = openuv.client.longitude
         self._name = name
-        self._dispatch_remove = None
         self._sensor_type = sensor_type
         self._state = None
 
@@ -80,16 +80,20 @@ class OpenUvBinarySensor(OpenUvEntity, BinarySensorDevice):
         return '{0}_{1}_{2}'.format(
             self._latitude, self._longitude, self._sensor_type)
 
-    @callback
-    def _update_data(self):
-        """Update the state."""
-        self.async_schedule_update_ha_state(True)
-
     async def async_added_to_hass(self):
         """Register callbacks."""
-        self._dispatch_remove = async_dispatcher_connect(
+        @callback
-            self.hass, TOPIC_UPDATE, self._update_data)
+        def update():
-        self.async_on_remove(self._dispatch_remove)
+            """Update the state."""
+            self.async_schedule_update_ha_state(True)
+
+        self._async_unsub_dispatcher_connect = async_dispatcher_connect(
+            self.hass, TOPIC_UPDATE, update)
+
+    async def async_will_remove_from_hass(self):
+        """Disconnect dispatcher listener when removed."""
+        if self._async_unsub_dispatcher_connect:
+            self._async_unsub_dispatcher_connect()
 
     async def async_update(self):
         """Update the state."""

homeassistant/components/openuv/__init__.py

@@ -14,13 +14,14 @@ from homeassistant.const import (
     ATTR_ATTRIBUTION, CONF_API_KEY, CONF_BINARY_SENSORS, CONF_ELEVATION,
     CONF_LATITUDE, CONF_LONGITUDE, CONF_MONITORED_CONDITIONS,
     CONF_SCAN_INTERVAL, CONF_SENSORS)
+from homeassistant.exceptions import ConfigEntryNotReady
 from homeassistant.helpers import aiohttp_client, config_validation as cv
 from homeassistant.helpers.dispatcher import async_dispatcher_send
 from homeassistant.helpers.entity import Entity
 from homeassistant.helpers.event import async_track_time_interval
 
 from .config_flow import configured_instances
-from .const import DOMAIN
+from .const import DEFAULT_SCAN_INTERVAL, DOMAIN
 
 REQUIREMENTS = ['pyopenuv==1.0.4']
 _LOGGER = logging.getLogger(__name__)
@@ -31,7 +32,6 @@ DATA_PROTECTION_WINDOW = 'protection_window'
 DATA_UV = 'uv'
 
 DEFAULT_ATTRIBUTION = 'Data provided by OpenUV'
-DEFAULT_SCAN_INTERVAL = timedelta(minutes=30)
 
 NOTIFICATION_ID = 'openuv_notification'
 NOTIFICATION_TITLE = 'OpenUV Component Setup'
@@ -85,18 +85,17 @@ SENSOR_SCHEMA = vol.Schema({
 })
 
 CONFIG_SCHEMA = vol.Schema({
-    DOMAIN:
+    DOMAIN: vol.Schema({
-        vol.Schema({
+        vol.Required(CONF_API_KEY): cv.string,
-            vol.Required(CONF_API_KEY): cv.string,
+        vol.Optional(CONF_ELEVATION): float,
-            vol.Optional(CONF_ELEVATION): float,
+        vol.Optional(CONF_LATITUDE): cv.latitude,
-            vol.Optional(CONF_LATITUDE): cv.latitude,
+        vol.Optional(CONF_LONGITUDE): cv.longitude,
-            vol.Optional(CONF_LONGITUDE): cv.longitude,
+        vol.Optional(CONF_BINARY_SENSORS, default={}):
-            vol.Optional(CONF_BINARY_SENSORS, default={}):
+            BINARY_SENSOR_SCHEMA,
-                BINARY_SENSOR_SCHEMA,
+        vol.Optional(CONF_SENSORS, default={}): SENSOR_SCHEMA,
-            vol.Optional(CONF_SENSORS, default={}): SENSOR_SCHEMA,
+        vol.Optional(CONF_SCAN_INTERVAL, default=DEFAULT_SCAN_INTERVAL):
-            vol.Optional(CONF_SCAN_INTERVAL, default=DEFAULT_SCAN_INTERVAL):
+            cv.time_period,
-                cv.time_period,
+    })
-        })
 }, extra=vol.ALLOW_EXTRA)
 
 
@@ -110,27 +109,26 @@ async def async_setup(hass, config):
         return True
 
     conf = config[DOMAIN]
-    latitude = conf.get(CONF_LATITUDE, hass.config.latitude)
+    latitude = conf.get(CONF_LATITUDE)
-    longitude = conf.get(CONF_LONGITUDE, hass.config.longitude)
+    longitude = conf.get(CONF_LONGITUDE)
-    elevation = conf.get(CONF_ELEVATION, hass.config.elevation)
 
     identifier = '{0}, {1}'.format(latitude, longitude)
+    if identifier in configured_instances(hass):
+        return True
 
-    if identifier not in configured_instances(hass):
+    hass.async_create_task(
-        hass.async_add_job(
+        hass.config_entries.flow.async_init(
-            hass.config_entries.flow.async_init(
+            DOMAIN,
-                DOMAIN,
+            context={'source': SOURCE_IMPORT},
-                context={'source': SOURCE_IMPORT},
+            data={
-                data={
+                CONF_API_KEY: conf[CONF_API_KEY],
-                    CONF_API_KEY: conf[CONF_API_KEY],
+                CONF_LATITUDE: latitude,
-                    CONF_LATITUDE: latitude,
+                CONF_LONGITUDE: longitude,
-                    CONF_LONGITUDE: longitude,
+                CONF_ELEVATION: conf.get(CONF_ELEVATION),
-                    CONF_ELEVATION: elevation,
+                CONF_BINARY_SENSORS: conf[CONF_BINARY_SENSORS],
-                    CONF_BINARY_SENSORS: conf[CONF_BINARY_SENSORS],
+                CONF_SENSORS: conf[CONF_SENSORS],
-                    CONF_SENSORS: conf[CONF_SENSORS],
+                CONF_SCAN_INTERVAL: conf[CONF_SCAN_INTERVAL],
-                }))
+            }))
-
-    hass.data[DOMAIN][CONF_SCAN_INTERVAL] = conf[CONF_SCAN_INTERVAL]
 
     return True
 
@@ -145,11 +143,10 @@ async def async_setup_entry(hass, config_entry):
         openuv = OpenUV(
             Client(
                 config_entry.data[CONF_API_KEY],
-                config_entry.data.get(CONF_LATITUDE, hass.config.latitude),
+                config_entry.data[CONF_LATITUDE],
-                config_entry.data.get(CONF_LONGITUDE, hass.config.longitude),
+                config_entry.data[CONF_LONGITUDE],
                 websession,
-                altitude=config_entry.data.get(
+                altitude=config_entry.data[CONF_ELEVATION]),
-                    CONF_ELEVATION, hass.config.elevation)),
             config_entry.data.get(CONF_BINARY_SENSORS, {}).get(
                 CONF_MONITORED_CONDITIONS, list(BINARY_SENSORS)),
             config_entry.data.get(CONF_SENSORS, {}).get(
@@ -157,20 +154,14 @@ async def async_setup_entry(hass, config_entry):
         await openuv.async_update()
         hass.data[DOMAIN][DATA_OPENUV_CLIENT][config_entry.entry_id] = openuv
     except OpenUvError as err:
-        _LOGGER.error('An error occurred: %s', str(err))
+        _LOGGER.error('Config entry failed: %s', err)
-        hass.components.persistent_notification.create(
+        raise ConfigEntryNotReady
-            'Error: {0}<br />'
-            'You will need to restart hass after fixing.'
-            ''.format(err),
-            title=NOTIFICATION_TITLE,
-            notification_id=NOTIFICATION_ID)
-        return False
 
     for component in ('binary_sensor', 'sensor'):
         hass.async_create_task(hass.config_entries.async_forward_entry_setup(
             config_entry, component))
 
-    async def refresh_sensors(event_time):
+    async def refresh(event_time):
         """Refresh OpenUV data."""
         _LOGGER.debug('Refreshing OpenUV data')
         await openuv.async_update()
@@ -178,24 +169,25 @@ async def async_setup_entry(hass, config_entry):
 
     hass.data[DOMAIN][DATA_OPENUV_LISTENER][
         config_entry.entry_id] = async_track_time_interval(
-            hass, refresh_sensors,
+            hass,
-            hass.data[DOMAIN].get(CONF_SCAN_INTERVAL, DEFAULT_SCAN_INTERVAL))
+            refresh,
+            timedelta(seconds=config_entry.data[CONF_SCAN_INTERVAL]))
 
     return True
 
 
 async def async_unload_entry(hass, config_entry):
     """Unload an OpenUV config entry."""
-    for component in ('binary_sensor', 'sensor'):
-        await hass.config_entries.async_forward_entry_unload(
-            config_entry, component)
-
     hass.data[DOMAIN][DATA_OPENUV_CLIENT].pop(config_entry.entry_id)
 
     remove_listener = hass.data[DOMAIN][DATA_OPENUV_LISTENER].pop(
         config_entry.entry_id)
     remove_listener()
 
+    for component in ('binary_sensor', 'sensor'):
+        await hass.config_entries.async_forward_entry_unload(
+            config_entry, component)
+
     return True
 
 

homeassistant/components/openuv/config_flow.py

@@ -1,16 +1,15 @@
 """Config flow to configure the OpenUV component."""
 
-from collections import OrderedDict
-
 import voluptuous as vol
 
 from homeassistant import config_entries
 from homeassistant.core import callback
 from homeassistant.const import (
-    CONF_API_KEY, CONF_ELEVATION, CONF_LATITUDE, CONF_LONGITUDE)
+    CONF_API_KEY, CONF_ELEVATION, CONF_LATITUDE, CONF_LONGITUDE,
+    CONF_SCAN_INTERVAL)
 from homeassistant.helpers import aiohttp_client, config_validation as cv
 
-from .const import DOMAIN
+from .const import DEFAULT_SCAN_INTERVAL, DOMAIN
 
 
 @callback
@@ -33,6 +32,24 @@ class OpenUvFlowHandler(config_entries.ConfigFlow):
         """Initialize the config flow."""
         pass
 
+    async def _show_form(self, errors=None):
+        """Show the form to the user."""
+        data_schema = vol.Schema({
+            vol.Required(CONF_API_KEY): str,
+            vol.Optional(CONF_LATITUDE, default=self.hass.config.latitude):
+                cv.latitude,
+            vol.Optional(CONF_LONGITUDE, default=self.hass.config.longitude):
+                cv.longitude,
+            vol.Optional(CONF_ELEVATION, default=self.hass.config.elevation):
+                vol.Coerce(float),
+        })
+
+        return self.async_show_form(
+            step_id='user',
+            data_schema=data_schema,
+            errors=errors if errors else {},
+        )
+
     async def async_step_import(self, import_config):
         """Import a config entry from configuration.yaml."""
         return await self.async_step_user(import_config)
@@ -41,34 +58,31 @@ class OpenUvFlowHandler(config_entries.ConfigFlow):
         """Handle the start of the config flow."""
         from pyopenuv.util import validate_api_key
 
-        errors = {}
+        if not user_input:
+            return await self._show_form()
 
-        if user_input is not None:
+        latitude = user_input[CONF_LATITUDE]
-            identifier = '{0}, {1}'.format(
+        longitude = user_input[CONF_LONGITUDE]
-                user_input.get(CONF_LATITUDE, self.hass.config.latitude),
+        elevation = user_input[CONF_ELEVATION]
-                user_input.get(CONF_LONGITUDE, self.hass.config.longitude))
 
-            if identifier in configured_instances(self.hass):
+        identifier = '{0}, {1}'.format(latitude, longitude)
-                errors['base'] = 'identifier_exists'
+        if identifier in configured_instances(self.hass):
-            else:
+            return await self._show_form({CONF_LATITUDE: 'identifier_exists'})
-                websession = aiohttp_client.async_get_clientsession(self.hass)
-                api_key_validation = await validate_api_key(
-                    user_input[CONF_API_KEY], websession)
-                if api_key_validation:
-                    return self.async_create_entry(
-                        title=identifier,
-                        data=user_input,
-                    )
-                errors['base'] = 'invalid_api_key'
 
-        data_schema = OrderedDict()
+        websession = aiohttp_client.async_get_clientsession(self.hass)
-        data_schema[vol.Required(CONF_API_KEY)] = str
+        api_key_validation = await validate_api_key(
-        data_schema[vol.Optional(CONF_LATITUDE)] = cv.latitude
+            user_input[CONF_API_KEY], websession)
-        data_schema[vol.Optional(CONF_LONGITUDE)] = cv.longitude
-        data_schema[vol.Optional(CONF_ELEVATION)] = vol.Coerce(float)
 
-        return self.async_show_form(
+        if not api_key_validation:
-            step_id='user',
+            return await self._show_form({CONF_API_KEY: 'invalid_api_key'})
-            data_schema=vol.Schema(data_schema),
+
-            errors=errors,
+        scan_interval = user_input.get(
-        )
+            CONF_SCAN_INTERVAL, DEFAULT_SCAN_INTERVAL)
+        user_input.update({
+            CONF_LATITUDE: latitude,
+            CONF_LONGITUDE: longitude,
+            CONF_ELEVATION: elevation,
+            CONF_SCAN_INTERVAL: scan_interval.seconds,
+        })
+
+        return self.async_create_entry(title=identifier, data=user_input)

homeassistant/components/openuv/const.py

@@ -1,3 +1,6 @@
 """Define constants for the OpenUV component."""
+from datetime import timedelta
 
 DOMAIN = 'openuv'
+
+DEFAULT_SCAN_INTERVAL = timedelta(minutes=30)

homeassistant/components/sensor/openuv.py

@@ -64,7 +64,7 @@ class OpenUvSensor(OpenUvEntity):
         """Initialize the sensor."""
         super().__init__(openuv)
 
-        self._dispatch_remove = None
+        self._async_unsub_dispatcher_connect = None
         self._entry_id = entry_id
         self._icon = icon
         self._latitude = openuv.client.latitude
@@ -100,16 +100,20 @@ class OpenUvSensor(OpenUvEntity):
         """Return the unit the value is expressed in."""
         return self._unit
 
-    @callback
-    def _update_data(self):
-        """Update the state."""
-        self.async_schedule_update_ha_state(True)
-
     async def async_added_to_hass(self):
         """Register callbacks."""
-        self._dispatch_remove = async_dispatcher_connect(
+        @callback
-            self.hass, TOPIC_UPDATE, self._update_data)
+        def update():
-        self.async_on_remove(self._dispatch_remove)
+            """Update the state."""
+            self.async_schedule_update_ha_state(True)
+
+        self._async_unsub_dispatcher_connect = async_dispatcher_connect(
+            self.hass, TOPIC_UPDATE, update)
+
+    async def async_will_remove_from_hass(self):
+        """Disconnect dispatcher listener when removed."""
+        if self._async_unsub_dispatcher_connect:
+            self._async_unsub_dispatcher_connect()
 
     async def async_update(self):
         """Update the state."""

tests/components/openuv/test_config_flow.py

@@ -1,10 +1,12 @@
 """Define tests for the OpenUV config flow."""
+from datetime import timedelta
 from unittest.mock import patch
 
 from homeassistant import data_entry_flow
 from homeassistant.components.openuv import DOMAIN, config_flow
 from homeassistant.const import (
-    CONF_API_KEY, CONF_ELEVATION, CONF_LATITUDE, CONF_LONGITUDE)
+    CONF_API_KEY, CONF_ELEVATION, CONF_LATITUDE, CONF_LONGITUDE,
+    CONF_SCAN_INTERVAL)
 
 from tests.common import MockConfigEntry, mock_coro
 
@@ -23,7 +25,7 @@ async def test_duplicate_error(hass):
     flow.hass = hass
 
     result = await flow.async_step_user(user_input=conf)
-    assert result['errors'] == {'base': 'identifier_exists'}
+    assert result['errors'] == {CONF_LATITUDE: 'identifier_exists'}
 
 
 async def test_invalid_api_key(hass):
@@ -41,7 +43,7 @@ async def test_invalid_api_key(hass):
     with patch('pyopenuv.util.validate_api_key',
                return_value=mock_coro(False)):
         result = await flow.async_step_user(user_input=conf)
-        assert result['errors'] == {'base': 'invalid_api_key'}
+        assert result['errors'] == {CONF_API_KEY: 'invalid_api_key'}
 
 
 async def test_show_form(hass):
@@ -57,25 +59,6 @@ async def test_show_form(hass):
 
 async def test_step_import(hass):
     """Test that the import step works."""
-    conf = {
-        CONF_API_KEY: '12345abcde',
-    }
-
-    flow = config_flow.OpenUvFlowHandler()
-    flow.hass = hass
-
-    with patch('pyopenuv.util.validate_api_key',
-               return_value=mock_coro(True)):
-        result = await flow.async_step_import(import_config=conf)
-
-        assert result['type'] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
-        assert result['title'] == '{0}, {1}'.format(
-            hass.config.latitude, hass.config.longitude)
-        assert result['data'] == conf
-
-
-async def test_step_user(hass):
-    """Test that the user step works."""
     conf = {
         CONF_API_KEY: '12345abcde',
         CONF_ELEVATION: 59.1234,
@@ -86,11 +69,44 @@ async def test_step_user(hass):
     flow = config_flow.OpenUvFlowHandler()
     flow.hass = hass
 
+    with patch('pyopenuv.util.validate_api_key',
+               return_value=mock_coro(True)):
+        result = await flow.async_step_import(import_config=conf)
+
+        assert result['type'] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
+        assert result['title'] == '39.128712, -104.9812612'
+        assert result['data'] == {
+            CONF_API_KEY: '12345abcde',
+            CONF_ELEVATION: 59.1234,
+            CONF_LATITUDE: 39.128712,
+            CONF_LONGITUDE: -104.9812612,
+            CONF_SCAN_INTERVAL: 1800,
+        }
+
+
+async def test_step_user(hass):
+    """Test that the user step works."""
+    conf = {
+        CONF_API_KEY: '12345abcde',
+        CONF_ELEVATION: 59.1234,
+        CONF_LATITUDE: 39.128712,
+        CONF_LONGITUDE: -104.9812612,
+        CONF_SCAN_INTERVAL: timedelta(minutes=5)
+    }
+
+    flow = config_flow.OpenUvFlowHandler()
+    flow.hass = hass
+
     with patch('pyopenuv.util.validate_api_key',
                return_value=mock_coro(True)):
         result = await flow.async_step_user(user_input=conf)
 
         assert result['type'] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY
-        assert result['title'] == '{0}, {1}'.format(
+        assert result['title'] == '39.128712, -104.9812612'
-            conf[CONF_LATITUDE], conf[CONF_LONGITUDE])
+        assert result['data'] == {
-        assert result['data'] == conf
+            CONF_API_KEY: '12345abcde',
+            CONF_ELEVATION: 59.1234,
+            CONF_LATITUDE: 39.128712,
+            CONF_LONGITUDE: -104.9812612,
+            CONF_SCAN_INTERVAL: 300,
+        }