From 81a2fb9615421b1ac81b4328011897b8e55cba65 Mon Sep 17 00:00:00 2001 From: Maciej Bieniek Date: Wed, 29 Apr 2020 16:27:45 +0200 Subject: [PATCH 01/14] Reload braviatv entry after options update (#34576) * Reload entry after options update * Undo update listener when unloading --- homeassistant/components/braviatv/__init__.py | 17 +++++++++++++++-- .../components/braviatv/config_flow.py | 3 ++- homeassistant/components/braviatv/const.py | 2 ++ .../components/braviatv/media_player.py | 3 ++- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/braviatv/__init__.py b/homeassistant/components/braviatv/__init__.py index 9c55ef01cee..46fd8675358 100644 --- a/homeassistant/components/braviatv/__init__.py +++ b/homeassistant/components/braviatv/__init__.py @@ -5,7 +5,7 @@ from bravia_tv import BraviaRC from homeassistant.const import CONF_HOST, CONF_MAC -from .const import DOMAIN +from .const import BRAVIARC, DOMAIN, UNDO_UPDATE_LISTENER PLATFORMS = ["media_player"] @@ -20,8 +20,13 @@ async def async_setup_entry(hass, config_entry): host = config_entry.data[CONF_HOST] mac = config_entry.data[CONF_MAC] + undo_listener = config_entry.add_update_listener(update_listener) + hass.data.setdefault(DOMAIN, {}) - hass.data[DOMAIN][config_entry.entry_id] = BraviaRC(host, mac) + hass.data[DOMAIN][config_entry.entry_id] = { + BRAVIARC: BraviaRC(host, mac), + UNDO_UPDATE_LISTENER: undo_listener, + } for component in PLATFORMS: hass.async_create_task( @@ -41,7 +46,15 @@ async def async_unload_entry(hass, config_entry): ] ) ) + + hass.data[DOMAIN][config_entry.entry_id][UNDO_UPDATE_LISTENER]() + if unload_ok: hass.data[DOMAIN].pop(config_entry.entry_id) return unload_ok + + +async def update_listener(hass, config_entry): + """Handle options update.""" + await hass.config_entries.async_reload(config_entry.entry_id) diff --git a/homeassistant/components/braviatv/config_flow.py b/homeassistant/components/braviatv/config_flow.py index be2a91c8429..660e2e83ea1 100644 --- a/homeassistant/components/braviatv/config_flow.py +++ b/homeassistant/components/braviatv/config_flow.py @@ -15,6 +15,7 @@ from .const import ( # pylint:disable=unused-import ATTR_CID, ATTR_MAC, ATTR_MODEL, + BRAVIARC, CLIENTID_PREFIX, CONF_IGNORED_SOURCES, DOMAIN, @@ -152,7 +153,7 @@ class BraviaTVOptionsFlowHandler(config_entries.OptionsFlow): async def async_step_init(self, user_input=None): """Manage the options.""" - self.braviarc = self.hass.data[DOMAIN][self.config_entry.entry_id] + self.braviarc = self.hass.data[DOMAIN][self.config_entry.entry_id][BRAVIARC] if not self.braviarc.is_connected(): await self.hass.async_add_executor_job( self.braviarc.connect, self.pin, CLIENTID_PREFIX, NICKNAME, diff --git a/homeassistant/components/braviatv/const.py b/homeassistant/components/braviatv/const.py index 1fa96e6a98d..a5d7a88d4c3 100644 --- a/homeassistant/components/braviatv/const.py +++ b/homeassistant/components/braviatv/const.py @@ -6,8 +6,10 @@ ATTR_MODEL = "model" CONF_IGNORED_SOURCES = "ignored_sources" +BRAVIARC = "braviarc" BRAVIA_CONFIG_FILE = "bravia.conf" CLIENTID_PREFIX = "HomeAssistant" DEFAULT_NAME = f"{ATTR_MANUFACTURER} Bravia TV" DOMAIN = "braviatv" NICKNAME = "Home Assistant" +UNDO_UPDATE_LISTENER = "undo_update_listener" diff --git a/homeassistant/components/braviatv/media_player.py b/homeassistant/components/braviatv/media_player.py index 718f99d8357..e4b28c0c2ab 100644 --- a/homeassistant/components/braviatv/media_player.py +++ b/homeassistant/components/braviatv/media_player.py @@ -30,6 +30,7 @@ from homeassistant.util.json import load_json from .const import ( ATTR_MANUFACTURER, BRAVIA_CONFIG_FILE, + BRAVIARC, CLIENTID_PREFIX, CONF_IGNORED_SOURCES, DEFAULT_NAME, @@ -103,7 +104,7 @@ async def async_setup_entry(hass, config_entry, async_add_entities): "model": config_entry.title, } - braviarc = hass.data[DOMAIN][config_entry.entry_id] + braviarc = hass.data[DOMAIN][config_entry.entry_id][BRAVIARC] ignored_sources = config_entry.options.get(CONF_IGNORED_SOURCES, []) From 6d9aafd3b07ee93b7f88673b8eca73e25efc0a7b Mon Sep 17 00:00:00 2001 From: Pascal Vizeli Date: Wed, 29 Apr 2020 18:06:25 +0200 Subject: [PATCH 02/14] Fix CVE-2020-1967 (#34853) --- build.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/build.json b/build.json index 331999b5470..b2c3cedc378 100644 --- a/build.json +++ b/build.json @@ -1,11 +1,11 @@ { "image": "homeassistant/{arch}-homeassistant", "build_from": { - "aarch64": "homeassistant/aarch64-homeassistant-base:7.1.0", - "armhf": "homeassistant/armhf-homeassistant-base:7.1.0", - "armv7": "homeassistant/armv7-homeassistant-base:7.1.0", - "amd64": "homeassistant/amd64-homeassistant-base:7.1.0", - "i386": "homeassistant/i386-homeassistant-base:7.1.0" + "aarch64": "homeassistant/aarch64-homeassistant-base:7.2.0", + "armhf": "homeassistant/armhf-homeassistant-base:7.2.0", + "armv7": "homeassistant/armv7-homeassistant-base:7.2.0", + "amd64": "homeassistant/amd64-homeassistant-base:7.2.0", + "i386": "homeassistant/i386-homeassistant-base:7.2.0" }, "labels": { "io.hass.type": "core" From 3bf1cf4f853ea22b63ab171c87cb5760a91fe5c9 Mon Sep 17 00:00:00 2001 From: Andrew Sayre <6730289+andrewsayre@users.noreply.github.com> Date: Wed, 29 Apr 2020 16:05:20 -0500 Subject: [PATCH 03/14] SmartThings continue correct config flow after external auth (#34862) --- .../components/smartthings/__init__.py | 10 + .../components/smartthings/config_flow.py | 6 +- .../components/smartthings/smartapp.py | 59 +- .../smartthings/test_config_flow.py | 1028 +++++++++++------ tests/components/smartthings/test_smartapp.py | 62 - 5 files changed, 698 insertions(+), 467 deletions(-) diff --git a/homeassistant/components/smartthings/__init__.py b/homeassistant/components/smartthings/__init__.py index 97a7d32a9c1..e4d720c94e5 100644 --- a/homeassistant/components/smartthings/__init__.py +++ b/homeassistant/components/smartthings/__init__.py @@ -37,6 +37,7 @@ from .const import ( TOKEN_REFRESH_INTERVAL, ) from .smartapp import ( + format_unique_id, setup_smartapp, setup_smartapp_endpoint, smartapp_sync_subscriptions, @@ -76,6 +77,15 @@ async def async_migrate_entry(hass: HomeAssistantType, entry: ConfigEntry): async def async_setup_entry(hass: HomeAssistantType, entry: ConfigEntry): """Initialize config entry which represents an installed SmartApp.""" + # For backwards compat + if entry.unique_id is None: + hass.config_entries.async_update_entry( + entry, + unique_id=format_unique_id( + entry.data[CONF_APP_ID], entry.data[CONF_LOCATION_ID] + ), + ) + if not validate_webhook_requirements(hass): _LOGGER.warning( "The 'base_url' of the 'http' integration must be configured and start with 'https://'" diff --git a/homeassistant/components/smartthings/config_flow.py b/homeassistant/components/smartthings/config_flow.py index cb4623cea1c..c03ade4d8b1 100644 --- a/homeassistant/components/smartthings/config_flow.py +++ b/homeassistant/components/smartthings/config_flow.py @@ -7,7 +7,7 @@ from pysmartthings.installedapp import format_install_url import voluptuous as vol from homeassistant import config_entries -from homeassistant.const import CONF_ACCESS_TOKEN, HTTP_FORBIDDEN +from homeassistant.const import CONF_ACCESS_TOKEN, HTTP_FORBIDDEN, HTTP_UNAUTHORIZED from homeassistant.helpers.aiohttp_client import async_get_clientsession # pylint: disable=unused-import @@ -26,6 +26,7 @@ from .const import ( from .smartapp import ( create_app, find_app, + format_unique_id, get_webhook_url, setup_smartapp, setup_smartapp_endpoint, @@ -138,7 +139,7 @@ class SmartThingsFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): ) return self._show_step_pat(errors) except ClientResponseError as ex: - if ex.status == 401: + if ex.status == HTTP_UNAUTHORIZED: errors[CONF_ACCESS_TOKEN] = "token_unauthorized" _LOGGER.debug( "Unauthorized error received setting up SmartApp", exc_info=True @@ -183,6 +184,7 @@ class SmartThingsFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): ) self.location_id = user_input[CONF_LOCATION_ID] + await self.async_set_unique_id(format_unique_id(self.app_id, self.location_id)) return await self.async_step_authorize() async def async_step_authorize(self, user_input=None): diff --git a/homeassistant/components/smartthings/smartapp.py b/homeassistant/components/smartthings/smartapp.py index 0b86a430d89..7d02a04d2ff 100644 --- a/homeassistant/components/smartthings/smartapp.py +++ b/homeassistant/components/smartthings/smartapp.py @@ -39,7 +39,6 @@ from .const import ( CONF_CLOUDHOOK_URL, CONF_INSTALLED_APP_ID, CONF_INSTANCE_ID, - CONF_LOCATION_ID, CONF_REFRESH_TOKEN, DATA_BROKERS, DATA_MANAGER, @@ -53,6 +52,11 @@ from .const import ( _LOGGER = logging.getLogger(__name__) +def format_unique_id(app_id: str, location_id: str) -> str: + """Format the unique id for a config entry.""" + return f"{app_id}_{location_id}" + + async def find_app(hass: HomeAssistantType, api): """Find an existing SmartApp for this installation of hass.""" apps = await api.apps() @@ -366,13 +370,20 @@ async def smartapp_sync_subscriptions( _LOGGER.debug("Subscriptions for app '%s' are up-to-date", installed_app_id) -async def smartapp_install(hass: HomeAssistantType, req, resp, app): - """Handle a SmartApp installation and continue the config flow.""" +async def _continue_flow( + hass: HomeAssistantType, + app_id: str, + location_id: str, + installed_app_id: str, + refresh_token: str, +): + """Continue a config flow if one is in progress for the specific installed app.""" + unique_id = format_unique_id(app_id, location_id) flow = next( ( flow for flow in hass.config_entries.flow.async_progress() - if flow["handler"] == DOMAIN + if flow["handler"] == DOMAIN and flow["context"]["unique_id"] == unique_id ), None, ) @@ -380,18 +391,23 @@ async def smartapp_install(hass: HomeAssistantType, req, resp, app): await hass.config_entries.flow.async_configure( flow["flow_id"], { - CONF_INSTALLED_APP_ID: req.installed_app_id, - CONF_LOCATION_ID: req.location_id, - CONF_REFRESH_TOKEN: req.refresh_token, + CONF_INSTALLED_APP_ID: installed_app_id, + CONF_REFRESH_TOKEN: refresh_token, }, ) _LOGGER.debug( "Continued config flow '%s' for SmartApp '%s' under parent app '%s'", flow["flow_id"], - req.installed_app_id, - app.app_id, + installed_app_id, + app_id, ) + +async def smartapp_install(hass: HomeAssistantType, req, resp, app): + """Handle a SmartApp installation and continue the config flow.""" + await _continue_flow( + hass, app.app_id, req.location_id, req.installed_app_id, req.refresh_token + ) _LOGGER.debug( "Installed SmartApp '%s' under parent app '%s'", req.installed_app_id, @@ -420,30 +436,9 @@ async def smartapp_update(hass: HomeAssistantType, req, resp, app): app.app_id, ) - flow = next( - ( - flow - for flow in hass.config_entries.flow.async_progress() - if flow["handler"] == DOMAIN - ), - None, + await _continue_flow( + hass, app.app_id, req.location_id, req.installed_app_id, req.refresh_token ) - if flow is not None: - await hass.config_entries.flow.async_configure( - flow["flow_id"], - { - CONF_INSTALLED_APP_ID: req.installed_app_id, - CONF_LOCATION_ID: req.location_id, - CONF_REFRESH_TOKEN: req.refresh_token, - }, - ) - _LOGGER.debug( - "Continued config flow '%s' for SmartApp '%s' under parent app '%s'", - flow["flow_id"], - req.installed_app_id, - app.app_id, - ) - _LOGGER.debug( "Updated SmartApp '%s' under parent app '%s'", req.installed_app_id, app.app_id ) diff --git a/tests/components/smartthings/test_config_flow.py b/tests/components/smartthings/test_config_flow.py index dc046f718a8..81dbab917a3 100644 --- a/tests/components/smartthings/test_config_flow.py +++ b/tests/components/smartthings/test_config_flow.py @@ -8,28 +8,30 @@ from pysmartthings.installedapp import format_install_url from homeassistant import data_entry_flow from homeassistant.components.smartthings import smartapp -from homeassistant.components.smartthings.config_flow import SmartThingsFlowHandler from homeassistant.components.smartthings.const import ( CONF_APP_ID, CONF_INSTALLED_APP_ID, CONF_LOCATION_ID, CONF_OAUTH_CLIENT_ID, CONF_OAUTH_CLIENT_SECRET, - CONF_REFRESH_TOKEN, DOMAIN, ) -from homeassistant.const import CONF_ACCESS_TOKEN, HTTP_FORBIDDEN, HTTP_NOT_FOUND +from homeassistant.const import ( + CONF_ACCESS_TOKEN, + HTTP_FORBIDDEN, + HTTP_NOT_FOUND, + HTTP_UNAUTHORIZED, +) from tests.common import MockConfigEntry, mock_coro -async def test_step_import(hass): - """Test import returns user.""" - flow = SmartThingsFlowHandler() - flow.hass = hass - - result = await flow.async_step_import() - +async def test_import_shows_user_step(hass): + """Test import source shows the user form.""" + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "import"} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["step_id"] == "user" assert result["description_placeholders"][ @@ -37,237 +39,316 @@ async def test_step_import(hass): ] == smartapp.get_webhook_url(hass) -async def test_step_user(hass): - """Test the webhook confirmation is shown.""" - flow = SmartThingsFlowHandler() - flow.hass = hass - - result = await flow.async_step_user() - - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "user" - assert result["description_placeholders"][ - "webhook_url" - ] == smartapp.get_webhook_url(hass) - - -async def test_step_user_aborts_invalid_webhook(hass): - """Test flow aborts if webhook is invalid.""" - hass.config.api.base_url = "http://0.0.0.0" - flow = SmartThingsFlowHandler() - flow.hass = hass - - result = await flow.async_step_user() - - assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT - assert result["reason"] == "invalid_webhook_url" - assert result["description_placeholders"][ - "webhook_url" - ] == smartapp.get_webhook_url(hass) - assert "component_url" in result["description_placeholders"] - - -async def test_step_user_advances_to_pat(hass): - """Test user step advances to the pat step.""" - flow = SmartThingsFlowHandler() - flow.hass = hass - - result = await flow.async_step_user({}) - - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "pat" - - -async def test_step_pat(hass): - """Test pat step shows the input form.""" - flow = SmartThingsFlowHandler() - flow.hass = hass - - result = await flow.async_step_pat() - - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "pat" - assert result["errors"] == {} - assert result["data_schema"]({CONF_ACCESS_TOKEN: ""}) == {CONF_ACCESS_TOKEN: ""} - assert "token_url" in result["description_placeholders"] - assert "component_url" in result["description_placeholders"] - - -async def test_step_pat_defaults_token(hass): - """Test pat form defaults the token from another entry.""" +async def test_entry_created(hass, app, app_oauth_client, location, smartthings_mock): + """Test local webhook, new app, install event creates entry.""" token = str(uuid4()) - entry = MockConfigEntry(domain=DOMAIN, data={CONF_ACCESS_TOKEN: token}) - entry.add_to_hass(hass) - flow = SmartThingsFlowHandler() - flow.hass = hass - - result = await flow.async_step_pat() - - assert flow.access_token == token - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "pat" - assert result["errors"] == {} - assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} - assert "token_url" in result["description_placeholders"] - assert "component_url" in result["description_placeholders"] - - -async def test_step_pat_invalid_token(hass): - """Test an error is shown for invalid token formats.""" - flow = SmartThingsFlowHandler() - flow.hass = hass - token = "123456789" - - result = await flow.async_step_pat({CONF_ACCESS_TOKEN: token}) - - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "pat" - assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} - assert result["errors"] == {"access_token": "token_invalid_format"} - assert "token_url" in result["description_placeholders"] - assert "component_url" in result["description_placeholders"] - - -async def test_step_pat_unauthorized(hass, smartthings_mock): - """Test an error is shown when the token is not authorized.""" - flow = SmartThingsFlowHandler() - flow.hass = hass - request_info = Mock(real_url="http://example.com") - smartthings_mock.apps.side_effect = ClientResponseError( - request_info=request_info, history=None, status=401 - ) - token = str(uuid4()) - - result = await flow.async_step_pat({CONF_ACCESS_TOKEN: token}) - - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "pat" - assert result["errors"] == {CONF_ACCESS_TOKEN: "token_unauthorized"} - assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} - - -async def test_step_pat_forbidden(hass, smartthings_mock): - """Test an error is shown when the token is forbidden.""" - flow = SmartThingsFlowHandler() - flow.hass = hass - request_info = Mock(real_url="http://example.com") - smartthings_mock.apps.side_effect = ClientResponseError( - request_info=request_info, history=None, status=HTTP_FORBIDDEN - ) - token = str(uuid4()) - - result = await flow.async_step_pat({CONF_ACCESS_TOKEN: token}) - - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "pat" - assert result["errors"] == {CONF_ACCESS_TOKEN: "token_forbidden"} - assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} - - -async def test_step_pat_webhook_error(hass, smartthings_mock): - """Test an error is shown when there's an problem with the webhook endpoint.""" - flow = SmartThingsFlowHandler() - flow.hass = hass - data = {"error": {}} - request_info = Mock(real_url="http://example.com") - error = APIResponseError( - request_info=request_info, history=None, data=data, status=422 - ) - error.is_target_error = Mock(return_value=True) - smartthings_mock.apps.side_effect = error - token = str(uuid4()) - - result = await flow.async_step_pat({CONF_ACCESS_TOKEN: token}) - - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "pat" - assert result["errors"] == {"base": "webhook_error"} - assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} - - -async def test_step_pat_api_error(hass, smartthings_mock): - """Test an error is shown when other API errors occur.""" - flow = SmartThingsFlowHandler() - flow.hass = hass - data = {"error": {}} - request_info = Mock(real_url="http://example.com") - error = APIResponseError( - request_info=request_info, history=None, data=data, status=400 - ) - smartthings_mock.apps.side_effect = error - token = str(uuid4()) - - result = await flow.async_step_pat({CONF_ACCESS_TOKEN: token}) - - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "pat" - assert result["errors"] == {"base": "app_setup_error"} - assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} - - -async def test_step_pat_unknown_api_error(hass, smartthings_mock): - """Test an error is shown when there is an unknown API error.""" - flow = SmartThingsFlowHandler() - flow.hass = hass - request_info = Mock(real_url="http://example.com") - smartthings_mock.apps.side_effect = ClientResponseError( - request_info=request_info, history=None, status=HTTP_NOT_FOUND - ) - token = str(uuid4()) - - result = await flow.async_step_pat({CONF_ACCESS_TOKEN: token}) - - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "pat" - assert result["errors"] == {"base": "app_setup_error"} - assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} - - -async def test_step_pat_unknown_error(hass, smartthings_mock): - """Test an error is shown when there is an unknown API error.""" - flow = SmartThingsFlowHandler() - flow.hass = hass - smartthings_mock.apps.side_effect = Exception("Unknown error") - token = str(uuid4()) - - result = await flow.async_step_pat({CONF_ACCESS_TOKEN: token}) - - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "pat" - assert result["errors"] == {"base": "app_setup_error"} - assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} - - -async def test_step_pat_app_created_webhook( - hass, app, app_oauth_client, location, smartthings_mock -): - """Test SmartApp is created when one does not exist and shows location form.""" - flow = SmartThingsFlowHandler() - flow.hass = hass - + installed_app_id = str(uuid4()) + refresh_token = str(uuid4()) smartthings_mock.apps.return_value = [] smartthings_mock.create_app.return_value = (app, app_oauth_client) smartthings_mock.locations.return_value = [location] - token = str(uuid4()) + request = Mock() + request.installed_app_id = installed_app_id + request.auth_token = token + request.location_id = location.location_id + request.refresh_token = refresh_token - result = await flow.async_step_pat({CONF_ACCESS_TOKEN: token}) + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + assert result["description_placeholders"][ + "webhook_url" + ] == smartapp.get_webhook_url(hass) - assert flow.access_token == token - assert flow.app_id == app.app_id - assert flow.oauth_client_secret == app_oauth_client.client_secret - assert flow.oauth_client_id == app_oauth_client.client_id + # Advance to PAT screen + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + # Enter token and advance to location screen + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_ACCESS_TOKEN: token} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM assert result["step_id"] == "select_location" + # Select location and advance to external auth + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_LOCATION_ID: location.location_id} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_EXTERNAL_STEP + assert result["step_id"] == "authorize" + assert result["url"] == format_install_url(app.app_id, location.location_id) -async def test_step_pat_app_created_cloudhook( + # Complete external auth and advance to install + await smartapp.smartapp_install(hass, request, None, app) + + # Finish + result = await hass.config_entries.flow.async_configure(result["flow_id"]) + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["data"]["app_id"] == app.app_id + assert result["data"]["installed_app_id"] == installed_app_id + assert result["data"]["location_id"] == location.location_id + assert result["data"]["access_token"] == token + assert result["data"]["refresh_token"] == request.refresh_token + assert result["data"]["client_secret"] == app_oauth_client.client_secret + assert result["data"]["client_id"] == app_oauth_client.client_id + assert result["title"] == location.name + entry = next((entry for entry in hass.config_entries.async_entries(DOMAIN)), None,) + assert entry.unique_id == smartapp.format_unique_id( + app.app_id, location.location_id + ) + + +async def test_entry_created_from_update_event( hass, app, app_oauth_client, location, smartthings_mock ): - """Test SmartApp is created with a cloudhook and shows location form.""" - hass.config.components.add("cloud") + """Test local webhook, new app, update event creates entry.""" + token = str(uuid4()) + installed_app_id = str(uuid4()) + refresh_token = str(uuid4()) + smartthings_mock.apps.return_value = [] + smartthings_mock.create_app.return_value = (app, app_oauth_client) + smartthings_mock.locations.return_value = [location] + request = Mock() + request.installed_app_id = installed_app_id + request.auth_token = token + request.location_id = location.location_id + request.refresh_token = refresh_token + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + assert result["description_placeholders"][ + "webhook_url" + ] == smartapp.get_webhook_url(hass) + + # Advance to PAT screen + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + # Enter token and advance to location screen + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_ACCESS_TOKEN: token} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "select_location" + + # Select location and advance to external auth + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_LOCATION_ID: location.location_id} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_EXTERNAL_STEP + assert result["step_id"] == "authorize" + assert result["url"] == format_install_url(app.app_id, location.location_id) + + # Complete external auth and advance to install + await smartapp.smartapp_update(hass, request, None, app) + + # Finish + result = await hass.config_entries.flow.async_configure(result["flow_id"]) + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["data"]["app_id"] == app.app_id + assert result["data"]["installed_app_id"] == installed_app_id + assert result["data"]["location_id"] == location.location_id + assert result["data"]["access_token"] == token + assert result["data"]["refresh_token"] == request.refresh_token + assert result["data"]["client_secret"] == app_oauth_client.client_secret + assert result["data"]["client_id"] == app_oauth_client.client_id + assert result["title"] == location.name + entry = next((entry for entry in hass.config_entries.async_entries(DOMAIN)), None,) + assert entry.unique_id == smartapp.format_unique_id( + app.app_id, location.location_id + ) + + +async def test_entry_created_existing_app_new_oauth_client( + hass, app, app_oauth_client, location, smartthings_mock +): + """Test entry is created with an existing app and generation of a new oauth client.""" + token = str(uuid4()) + installed_app_id = str(uuid4()) + refresh_token = str(uuid4()) + smartthings_mock.apps.return_value = [app] + smartthings_mock.generate_app_oauth.return_value = app_oauth_client + smartthings_mock.locations.return_value = [location] + request = Mock() + request.installed_app_id = installed_app_id + request.auth_token = token + request.location_id = location.location_id + request.refresh_token = refresh_token + + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + assert result["description_placeholders"][ + "webhook_url" + ] == smartapp.get_webhook_url(hass) + + # Advance to PAT screen + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + # Enter token and advance to location screen + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_ACCESS_TOKEN: token} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "select_location" + + # Select location and advance to external auth + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_LOCATION_ID: location.location_id} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_EXTERNAL_STEP + assert result["step_id"] == "authorize" + assert result["url"] == format_install_url(app.app_id, location.location_id) + + # Complete external auth and advance to install + await smartapp.smartapp_install(hass, request, None, app) + + # Finish + result = await hass.config_entries.flow.async_configure(result["flow_id"]) + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["data"]["app_id"] == app.app_id + assert result["data"]["installed_app_id"] == installed_app_id + assert result["data"]["location_id"] == location.location_id + assert result["data"]["access_token"] == token + assert result["data"]["refresh_token"] == request.refresh_token + assert result["data"]["client_secret"] == app_oauth_client.client_secret + assert result["data"]["client_id"] == app_oauth_client.client_id + assert result["title"] == location.name + entry = next((entry for entry in hass.config_entries.async_entries(DOMAIN)), None,) + assert entry.unique_id == smartapp.format_unique_id( + app.app_id, location.location_id + ) + + +async def test_entry_created_existing_app_copies_oauth_client( + hass, app, location, smartthings_mock +): + """Test entry is created with an existing app and copies the oauth client from another entry.""" + token = str(uuid4()) + installed_app_id = str(uuid4()) + refresh_token = str(uuid4()) + oauth_client_id = str(uuid4()) + oauth_client_secret = str(uuid4()) + smartthings_mock.apps.return_value = [app] + smartthings_mock.locations.return_value = [location] + request = Mock() + request.installed_app_id = installed_app_id + request.auth_token = token + request.location_id = location.location_id + request.refresh_token = refresh_token + entry = MockConfigEntry( + domain=DOMAIN, + data={ + CONF_APP_ID: app.app_id, + CONF_OAUTH_CLIENT_ID: oauth_client_id, + CONF_OAUTH_CLIENT_SECRET: oauth_client_secret, + CONF_LOCATION_ID: str(uuid4()), + CONF_INSTALLED_APP_ID: str(uuid4()), + CONF_ACCESS_TOKEN: token, + }, + ) + entry.add_to_hass(hass) + + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + assert result["description_placeholders"][ + "webhook_url" + ] == smartapp.get_webhook_url(hass) + + # Advance to PAT screen + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + # Assert access token is defaulted to an existing entry for convenience. + assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} + + # Enter token and advance to location screen + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_ACCESS_TOKEN: token} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "select_location" + + # Select location and advance to external auth + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_LOCATION_ID: location.location_id} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_EXTERNAL_STEP + assert result["step_id"] == "authorize" + assert result["url"] == format_install_url(app.app_id, location.location_id) + + # Complete external auth and advance to install + await smartapp.smartapp_install(hass, request, None, app) + + # Finish + result = await hass.config_entries.flow.async_configure(result["flow_id"]) + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["data"]["app_id"] == app.app_id + assert result["data"]["installed_app_id"] == installed_app_id + assert result["data"]["location_id"] == location.location_id + assert result["data"]["access_token"] == token + assert result["data"]["refresh_token"] == request.refresh_token + assert result["data"]["client_secret"] == oauth_client_secret + assert result["data"]["client_id"] == oauth_client_id + assert result["title"] == location.name + entry = next( + ( + entry + for entry in hass.config_entries.async_entries(DOMAIN) + if entry.data[CONF_INSTALLED_APP_ID] == installed_app_id + ), + None, + ) + assert entry.unique_id == smartapp.format_unique_id( + app.app_id, location.location_id + ) + + +async def test_entry_created_with_cloudhook( + hass, app, app_oauth_client, location, smartthings_mock +): + """Test cloud, new app, install event creates entry.""" + hass.config.components.add("cloud") # Unload the endpoint so we can reload it under the cloud. await smartapp.unload_smartapp_endpoint(hass) + token = str(uuid4()) + installed_app_id = str(uuid4()) + refresh_token = str(uuid4()) + smartthings_mock.apps.return_value = [] + smartthings_mock.create_app.return_value = (app, app_oauth_client) + smartthings_mock.locations.return_value = [location] + request = Mock() + request.installed_app_id = installed_app_id + request.auth_token = token + request.location_id = location.location_id + request.refresh_token = refresh_token with patch.object( hass.components.cloud, "async_active_subscription", return_value=True @@ -279,163 +360,368 @@ async def test_step_pat_app_created_cloudhook( await smartapp.setup_smartapp_endpoint(hass) - flow = SmartThingsFlowHandler() - flow.hass = hass - smartthings_mock.apps.return_value = [] - smartthings_mock.create_app.return_value = (app, app_oauth_client) - smartthings_mock.locations.return_value = [location] - token = str(uuid4()) - - result = await flow.async_step_pat({CONF_ACCESS_TOKEN: token}) - - assert flow.access_token == token - assert flow.app_id == app.app_id - assert flow.oauth_client_secret == app_oauth_client.client_secret - assert flow.oauth_client_id == app_oauth_client.client_id + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "select_location" + assert result["step_id"] == "user" + assert result["description_placeholders"][ + "webhook_url" + ] == smartapp.get_webhook_url(hass) assert mock_create_cloudhook.call_count == 1 + # Advance to PAT screen + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] -async def test_step_pat_app_updated_webhook( + # Enter token and advance to location screen + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_ACCESS_TOKEN: token} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "select_location" + + # Select location and advance to external auth + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_LOCATION_ID: location.location_id} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_EXTERNAL_STEP + assert result["step_id"] == "authorize" + assert result["url"] == format_install_url(app.app_id, location.location_id) + + # Complete external auth and advance to install + await smartapp.smartapp_install(hass, request, None, app) + + # Finish + result = await hass.config_entries.flow.async_configure(result["flow_id"]) + assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY + assert result["data"]["app_id"] == app.app_id + assert result["data"]["installed_app_id"] == installed_app_id + assert result["data"]["location_id"] == location.location_id + assert result["data"]["access_token"] == token + assert result["data"]["refresh_token"] == request.refresh_token + assert result["data"]["client_secret"] == app_oauth_client.client_secret + assert result["data"]["client_id"] == app_oauth_client.client_id + assert result["title"] == location.name + entry = next( + (entry for entry in hass.config_entries.async_entries(DOMAIN)), None, + ) + assert entry.unique_id == smartapp.format_unique_id( + app.app_id, location.location_id + ) + + +async def test_invalid_webhook_aborts(hass): + """Test flow aborts if webhook is invalid.""" + hass.config.api.base_url = "http://0.0.0.0" + + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT + assert result["reason"] == "invalid_webhook_url" + assert result["description_placeholders"][ + "webhook_url" + ] == smartapp.get_webhook_url(hass) + assert "component_url" in result["description_placeholders"] + + +async def test_invalid_token_shows_error(hass): + """Test an error is shown for invalid token formats.""" + token = "123456789" + + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + assert result["description_placeholders"][ + "webhook_url" + ] == smartapp.get_webhook_url(hass) + + # Advance to PAT screen + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + # Enter token + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_ACCESS_TOKEN: token} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} + assert result["errors"] == {CONF_ACCESS_TOKEN: "token_invalid_format"} + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + +async def test_unauthorized_token_shows_error(hass, smartthings_mock): + """Test an error is shown for unauthorized token formats.""" + token = str(uuid4()) + request_info = Mock(real_url="http://example.com") + smartthings_mock.apps.side_effect = ClientResponseError( + request_info=request_info, history=None, status=HTTP_UNAUTHORIZED + ) + + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + assert result["description_placeholders"][ + "webhook_url" + ] == smartapp.get_webhook_url(hass) + + # Advance to PAT screen + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + # Enter token + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_ACCESS_TOKEN: token} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} + assert result["errors"] == {CONF_ACCESS_TOKEN: "token_unauthorized"} + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + +async def test_forbidden_token_shows_error(hass, smartthings_mock): + """Test an error is shown for forbidden token formats.""" + token = str(uuid4()) + request_info = Mock(real_url="http://example.com") + smartthings_mock.apps.side_effect = ClientResponseError( + request_info=request_info, history=None, status=HTTP_FORBIDDEN + ) + + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + assert result["description_placeholders"][ + "webhook_url" + ] == smartapp.get_webhook_url(hass) + + # Advance to PAT screen + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + # Enter token + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_ACCESS_TOKEN: token} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} + assert result["errors"] == {CONF_ACCESS_TOKEN: "token_forbidden"} + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + +async def test_webhook_problem_shows_error(hass, smartthings_mock): + """Test an error is shown when there's an problem with the webhook endpoint.""" + token = str(uuid4()) + data = {"error": {}} + request_info = Mock(real_url="http://example.com") + error = APIResponseError( + request_info=request_info, history=None, data=data, status=422 + ) + error.is_target_error = Mock(return_value=True) + smartthings_mock.apps.side_effect = error + + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + assert result["description_placeholders"][ + "webhook_url" + ] == smartapp.get_webhook_url(hass) + + # Advance to PAT screen + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + # Enter token + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_ACCESS_TOKEN: token} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} + assert result["errors"] == {"base": "webhook_error"} + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + +async def test_api_error_shows_error(hass, smartthings_mock): + """Test an error is shown when other API errors occur.""" + token = str(uuid4()) + data = {"error": {}} + request_info = Mock(real_url="http://example.com") + error = APIResponseError( + request_info=request_info, history=None, data=data, status=400 + ) + smartthings_mock.apps.side_effect = error + + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + assert result["description_placeholders"][ + "webhook_url" + ] == smartapp.get_webhook_url(hass) + + # Advance to PAT screen + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + # Enter token + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_ACCESS_TOKEN: token} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} + assert result["errors"] == {"base": "app_setup_error"} + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + +async def test_unknown_response_error_shows_error(hass, smartthings_mock): + """Test an error is shown when there is an unknown API error.""" + token = str(uuid4()) + request_info = Mock(real_url="http://example.com") + error = ClientResponseError( + request_info=request_info, history=None, status=HTTP_NOT_FOUND + ) + smartthings_mock.apps.side_effect = error + + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + assert result["description_placeholders"][ + "webhook_url" + ] == smartapp.get_webhook_url(hass) + + # Advance to PAT screen + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + # Enter token + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_ACCESS_TOKEN: token} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} + assert result["errors"] == {"base": "app_setup_error"} + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + +async def test_unknown_error_shows_error(hass, smartthings_mock): + """Test an error is shown when there is an unknown API error.""" + token = str(uuid4()) + smartthings_mock.apps.side_effect = Exception("Unknown error") + + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + assert result["description_placeholders"][ + "webhook_url" + ] == smartapp.get_webhook_url(hass) + + # Advance to PAT screen + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + # Enter token + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_ACCESS_TOKEN: token} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert result["data_schema"]({}) == {CONF_ACCESS_TOKEN: token} + assert result["errors"] == {"base": "app_setup_error"} + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + +async def test_no_available_locations_aborts( hass, app, app_oauth_client, location, smartthings_mock ): - """Test SmartApp is updated then show location form.""" - flow = SmartThingsFlowHandler() - flow.hass = hass - - smartthings_mock.apps.return_value = [app] - smartthings_mock.generate_app_oauth.return_value = app_oauth_client - smartthings_mock.locations.return_value = [location] - token = str(uuid4()) - - result = await flow.async_step_pat({CONF_ACCESS_TOKEN: token}) - - assert flow.access_token == token - assert flow.app_id == app.app_id - assert flow.oauth_client_secret == app_oauth_client.client_secret - assert flow.oauth_client_id == app_oauth_client.client_id - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "select_location" - - -async def test_step_pat_app_updated_webhook_from_existing_oauth_client( - hass, app, location, smartthings_mock -): - """Test SmartApp is updated from existing then show location form.""" - oauth_client_id = str(uuid4()) - oauth_client_secret = str(uuid4()) - entry = MockConfigEntry( - domain=DOMAIN, - data={ - CONF_APP_ID: app.app_id, - CONF_OAUTH_CLIENT_ID: oauth_client_id, - CONF_OAUTH_CLIENT_SECRET: oauth_client_secret, - CONF_LOCATION_ID: str(uuid4()), - }, - ) - entry.add_to_hass(hass) - flow = SmartThingsFlowHandler() - flow.hass = hass - smartthings_mock.apps.return_value = [app] - smartthings_mock.locations.return_value = [location] - token = str(uuid4()) - - result = await flow.async_step_pat({CONF_ACCESS_TOKEN: token}) - - assert flow.access_token == token - assert flow.app_id == app.app_id - assert flow.oauth_client_secret == oauth_client_secret - assert flow.oauth_client_id == oauth_client_id - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "select_location" - - -async def test_step_select_location(hass, location, smartthings_mock): - """Test select location shows form with available locations.""" - smartthings_mock.locations.return_value = [location] - flow = SmartThingsFlowHandler() - flow.hass = hass - flow.api = smartthings_mock - - result = await flow.async_step_select_location() - assert result["type"] == data_entry_flow.RESULT_TYPE_FORM - assert result["step_id"] == "select_location" - assert result["data_schema"]({CONF_LOCATION_ID: location.location_id}) == { - CONF_LOCATION_ID: location.location_id - } - - -async def test_step_select_location_aborts(hass, location, smartthings_mock): """Test select location aborts if no available locations.""" + token = str(uuid4()) + smartthings_mock.apps.return_value = [] + smartthings_mock.create_app.return_value = (app, app_oauth_client) smartthings_mock.locations.return_value = [location] entry = MockConfigEntry( domain=DOMAIN, data={CONF_LOCATION_ID: location.location_id} ) entry.add_to_hass(hass) - flow = SmartThingsFlowHandler() - flow.hass = hass - flow.api = smartthings_mock - result = await flow.async_step_select_location() + # Webhook confirmation shown + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": "user"} + ) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "user" + assert result["description_placeholders"][ + "webhook_url" + ] == smartapp.get_webhook_url(hass) + + # Advance to PAT screen + result = await hass.config_entries.flow.async_configure(result["flow_id"], {}) + assert result["type"] == data_entry_flow.RESULT_TYPE_FORM + assert result["step_id"] == "pat" + assert "token_url" in result["description_placeholders"] + assert "component_url" in result["description_placeholders"] + + # Enter token and advance to location screen + result = await hass.config_entries.flow.async_configure( + result["flow_id"], {CONF_ACCESS_TOKEN: token} + ) assert result["type"] == data_entry_flow.RESULT_TYPE_ABORT assert result["reason"] == "no_available_locations" - - -async def test_step_select_location_advances(hass): - """Test select location aborts if no available locations.""" - location_id = str(uuid4()) - app_id = str(uuid4()) - flow = SmartThingsFlowHandler() - flow.hass = hass - flow.app_id = app_id - - result = await flow.async_step_select_location({CONF_LOCATION_ID: location_id}) - - assert flow.location_id == location_id - assert result["type"] == data_entry_flow.RESULT_TYPE_EXTERNAL_STEP - assert result["step_id"] == "authorize" - assert result["url"] == format_install_url(app_id, location_id) - - -async def test_step_authorize_advances(hass): - """Test authorize step advances when completed.""" - installed_app_id = str(uuid4()) - refresh_token = str(uuid4()) - flow = SmartThingsFlowHandler() - flow.hass = hass - - result = await flow.async_step_authorize( - {CONF_INSTALLED_APP_ID: installed_app_id, CONF_REFRESH_TOKEN: refresh_token} - ) - - assert flow.installed_app_id == installed_app_id - assert flow.refresh_token == refresh_token - assert result["type"] == data_entry_flow.RESULT_TYPE_EXTERNAL_STEP_DONE - assert result["step_id"] == "install" - - -async def test_step_install_creates_entry(hass, location, smartthings_mock): - """Test a config entry is created once the app is installed.""" - flow = SmartThingsFlowHandler() - flow.hass = hass - flow.api = smartthings_mock - flow.access_token = str(uuid4()) - flow.app_id = str(uuid4()) - flow.installed_app_id = str(uuid4()) - flow.location_id = location.location_id - flow.oauth_client_id = str(uuid4()) - flow.oauth_client_secret = str(uuid4()) - flow.refresh_token = str(uuid4()) - - result = await flow.async_step_install() - - assert result["type"] == data_entry_flow.RESULT_TYPE_CREATE_ENTRY - assert result["data"]["app_id"] == flow.app_id - assert result["data"]["installed_app_id"] == flow.installed_app_id - assert result["data"]["location_id"] == flow.location_id - assert result["data"]["access_token"] == flow.access_token - assert result["data"]["refresh_token"] == flow.refresh_token - assert result["data"]["client_secret"] == flow.oauth_client_secret - assert result["data"]["client_id"] == flow.oauth_client_id - assert result["title"] == location.name diff --git a/tests/components/smartthings/test_smartapp.py b/tests/components/smartthings/test_smartapp.py index 4d7280a6a9e..efc4844cef2 100644 --- a/tests/components/smartthings/test_smartapp.py +++ b/tests/components/smartthings/test_smartapp.py @@ -6,8 +6,6 @@ from pysmartthings import AppEntity, Capability from homeassistant.components.smartthings import smartapp from homeassistant.components.smartthings.const import ( - CONF_INSTALLED_APP_ID, - CONF_LOCATION_ID, CONF_REFRESH_TOKEN, DATA_MANAGER, DOMAIN, @@ -39,36 +37,6 @@ async def test_update_app_updated_needed(hass, app): assert mock_app.classifications == app.classifications -async def test_smartapp_install_configures_flow(hass): - """Test install event continues an existing flow.""" - # Arrange - flow_id = str(uuid4()) - flows = [{"flow_id": flow_id, "handler": DOMAIN}] - app = Mock() - app.app_id = uuid4() - request = Mock() - request.installed_app_id = str(uuid4()) - request.auth_token = str(uuid4()) - request.location_id = str(uuid4()) - request.refresh_token = str(uuid4()) - - # Act - with patch.object( - hass.config_entries.flow, "async_progress", return_value=flows - ), patch.object(hass.config_entries.flow, "async_configure") as configure_mock: - - await smartapp.smartapp_install(hass, request, None, app) - - configure_mock.assert_called_once_with( - flow_id, - { - CONF_INSTALLED_APP_ID: request.installed_app_id, - CONF_LOCATION_ID: request.location_id, - CONF_REFRESH_TOKEN: request.refresh_token, - }, - ) - - async def test_smartapp_update_saves_token( hass, smartthings_mock, location, device_factory ): @@ -92,36 +60,6 @@ async def test_smartapp_update_saves_token( assert entry.data[CONF_REFRESH_TOKEN] == request.refresh_token -async def test_smartapp_update_configures_flow(hass): - """Test update event continues an existing flow.""" - # Arrange - flow_id = str(uuid4()) - flows = [{"flow_id": flow_id, "handler": DOMAIN}] - app = Mock() - app.app_id = uuid4() - request = Mock() - request.installed_app_id = str(uuid4()) - request.auth_token = str(uuid4()) - request.location_id = str(uuid4()) - request.refresh_token = str(uuid4()) - - # Act - with patch.object( - hass.config_entries.flow, "async_progress", return_value=flows - ), patch.object(hass.config_entries.flow, "async_configure") as configure_mock: - - await smartapp.smartapp_update(hass, request, None, app) - - configure_mock.assert_called_once_with( - flow_id, - { - CONF_INSTALLED_APP_ID: request.installed_app_id, - CONF_LOCATION_ID: request.location_id, - CONF_REFRESH_TOKEN: request.refresh_token, - }, - ) - - async def test_smartapp_uninstall(hass, config_entry): """Test the config entry is unloaded when the app is uninstalled.""" config_entry.add_to_hass(hass) From 5f7711e7a6bbf65645635029ec87b2a77f3801e6 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 29 Apr 2020 16:02:59 -0500 Subject: [PATCH 04/14] Abort nexia import if the username is already configured (#34863) --- homeassistant/components/nexia/config_flow.py | 3 ++ homeassistant/components/nexia/manifest.json | 2 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/nexia/test_config_flow.py | 41 +++++++++++++++++++ 5 files changed, 47 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/nexia/config_flow.py b/homeassistant/components/nexia/config_flow.py index c26e42a22d2..d71a5470c98 100644 --- a/homeassistant/components/nexia/config_flow.py +++ b/homeassistant/components/nexia/config_flow.py @@ -88,6 +88,9 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): async def async_step_import(self, user_input): """Handle import.""" + for entry in self._async_current_entries(): + if entry.data[CONF_USERNAME] == user_input[CONF_USERNAME]: + return self.async_abort(reason="already_configured") return await self.async_step_user(user_input) diff --git a/homeassistant/components/nexia/manifest.json b/homeassistant/components/nexia/manifest.json index f09d4d1a4d1..e86d2072db8 100644 --- a/homeassistant/components/nexia/manifest.json +++ b/homeassistant/components/nexia/manifest.json @@ -1,7 +1,7 @@ { "domain": "nexia", "name": "Nexia", - "requirements": ["nexia==0.9.2"], + "requirements": ["nexia==0.9.3"], "codeowners": ["@ryannazaretian", "@bdraco"], "documentation": "https://www.home-assistant.io/integrations/nexia", "config_flow": true diff --git a/requirements_all.txt b/requirements_all.txt index b252313922d..066f6de6fd0 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -931,7 +931,7 @@ netdisco==2.6.0 neurio==0.3.1 # homeassistant.components.nexia -nexia==0.9.2 +nexia==0.9.3 # homeassistant.components.nextcloud nextcloudmonitor==1.1.0 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 033c6580820..fb622f5640b 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -366,7 +366,7 @@ nessclient==0.9.15 netdisco==2.6.0 # homeassistant.components.nexia -nexia==0.9.2 +nexia==0.9.3 # homeassistant.components.nsw_fuel_station nsw-fuel-api-client==1.0.10 diff --git a/tests/components/nexia/test_config_flow.py b/tests/components/nexia/test_config_flow.py index 3cb57d77f12..ff6f8590287 100644 --- a/tests/components/nexia/test_config_flow.py +++ b/tests/components/nexia/test_config_flow.py @@ -74,3 +74,44 @@ async def test_form_cannot_connect(hass): assert result2["type"] == "form" assert result2["errors"] == {"base": "cannot_connect"} + + +async def test_form_import(hass): + """Test we get the form with import source.""" + await setup.async_setup_component(hass, "persistent_notification", {}) + + with patch( + "homeassistant.components.nexia.config_flow.NexiaHome.get_name", + return_value="myhouse", + ), patch( + "homeassistant.components.nexia.config_flow.NexiaHome.login", + side_effect=MagicMock(), + ), patch( + "homeassistant.components.nexia.async_setup", return_value=True + ) as mock_setup, patch( + "homeassistant.components.nexia.async_setup_entry", return_value=True, + ) as mock_setup_entry: + result = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_IMPORT}, + data={CONF_USERNAME: "username", CONF_PASSWORD: "password"}, + ) + + assert result["type"] == "create_entry" + assert result["title"] == "myhouse" + assert result["data"] == { + CONF_USERNAME: "username", + CONF_PASSWORD: "password", + } + await hass.async_block_till_done() + assert len(mock_setup.mock_calls) == 1 + assert len(mock_setup_entry.mock_calls) == 1 + + result2 = await hass.config_entries.flow.async_init( + DOMAIN, + context={"source": config_entries.SOURCE_IMPORT}, + data={CONF_USERNAME: "username", CONF_PASSWORD: "password"}, + ) + + assert result2["type"] == "abort" + assert result2["reason"] == "already_configured" From d391b87227b82239d770a3ba4a38450d82b69529 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 29 Apr 2020 16:00:31 -0500 Subject: [PATCH 05/14] Prevent homekit fans from going to 100% than speed when turning on (#34875) --- homeassistant/components/homekit/type_fans.py | 14 +++++++------- tests/components/homekit/test_type_fans.py | 15 +++++++-------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/homekit/type_fans.py b/homeassistant/components/homekit/type_fans.py index 9167c8fcf5d..b7208b1746c 100644 --- a/homeassistant/components/homekit/type_fans.py +++ b/homeassistant/components/homekit/type_fans.py @@ -94,13 +94,13 @@ class Fan(HomeAccessory): _LOGGER.debug("Fan _set_chars: %s", char_values) if CHAR_ACTIVE in char_values: if char_values[CHAR_ACTIVE]: - is_on = False - state = self.hass.states.get(self.entity_id) - if state and state.state == STATE_ON: - is_on = True - # Only set the state to active if we - # did not get a rotation speed or its off - if not is_on or CHAR_ROTATION_SPEED not in char_values: + # If the device supports set speed we + # do not want to turn on as it will take + # the fan to 100% than to the desired speed. + # + # Setting the speed will take care of turning + # on the fan if SUPPORT_SET_SPEED is set. + if not self.char_speed or CHAR_ROTATION_SPEED not in char_values: self.set_state(1) else: # Its off, nothing more to do as setting the diff --git a/tests/components/homekit/test_type_fans.py b/tests/components/homekit/test_type_fans.py index 915e7c59d7c..ca6e03217f3 100644 --- a/tests/components/homekit/test_type_fans.py +++ b/tests/components/homekit/test_type_fans.py @@ -419,8 +419,7 @@ async def test_fan_set_all_one_shot(hass, hk_driver, cls, events): ) await hass.async_block_till_done() acc.speed_mapping.speed_to_states.assert_called_with(42) - assert call_turn_on - assert call_turn_on[0].data[ATTR_ENTITY_ID] == entity_id + assert not call_turn_on assert call_set_speed[0] assert call_set_speed[0].data[ATTR_ENTITY_ID] == entity_id assert call_set_speed[0].data[ATTR_SPEED] == "ludicrous" @@ -430,11 +429,11 @@ async def test_fan_set_all_one_shot(hass, hk_driver, cls, events): assert call_set_direction[0] assert call_set_direction[0].data[ATTR_ENTITY_ID] == entity_id assert call_set_direction[0].data[ATTR_DIRECTION] == DIRECTION_REVERSE - assert len(events) == 4 + assert len(events) == 3 - assert events[1].data[ATTR_VALUE] is True - assert events[2].data[ATTR_VALUE] == DIRECTION_REVERSE - assert events[3].data[ATTR_VALUE] == "ludicrous" + assert events[0].data[ATTR_VALUE] is True + assert events[1].data[ATTR_VALUE] == DIRECTION_REVERSE + assert events[2].data[ATTR_VALUE] == "ludicrous" hass.states.async_set( entity_id, @@ -482,7 +481,7 @@ async def test_fan_set_all_one_shot(hass, hk_driver, cls, events): # and we set a fan speed await hass.async_block_till_done() acc.speed_mapping.speed_to_states.assert_called_with(42) - assert len(events) == 7 + assert len(events) == 6 assert call_set_speed[1] assert call_set_speed[1].data[ATTR_ENTITY_ID] == entity_id assert call_set_speed[1].data[ATTR_SPEED] == "ludicrous" @@ -526,7 +525,7 @@ async def test_fan_set_all_one_shot(hass, hk_driver, cls, events): ) await hass.async_block_till_done() - assert len(events) == 8 + assert len(events) == 7 assert call_turn_off assert call_turn_off[0].data[ATTR_ENTITY_ID] == entity_id assert len(call_set_speed) == 2 From 98bff965f54ac14db4174e5ddb68c43cf3bbe8f7 Mon Sep 17 00:00:00 2001 From: Aaron Bach Date: Wed, 29 Apr 2020 13:29:44 -0600 Subject: [PATCH 06/14] Fix Flu Near You exception re: stale coroutines (#34880) --- .../components/flunearyou/__init__.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/homeassistant/components/flunearyou/__init__.py b/homeassistant/components/flunearyou/__init__.py index 1e8e2e20b2d..6e1c8ddb3d2 100644 --- a/homeassistant/components/flunearyou/__init__.py +++ b/homeassistant/components/flunearyou/__init__.py @@ -131,15 +131,6 @@ class FluNearYouData: self.latitude = latitude self.longitude = longitude - self._api_coros = { - CATEGORY_CDC_REPORT: self._client.cdc_reports.status_by_coordinates( - latitude, longitude - ), - CATEGORY_USER_REPORT: self._client.user_reports.status_by_coordinates( - latitude, longitude - ), - } - self._api_category_count = { CATEGORY_CDC_REPORT: 0, CATEGORY_USER_REPORT: 0, @@ -155,8 +146,17 @@ class FluNearYouData: if self._api_category_count[api_category] == 0: return + if api_category == CATEGORY_CDC_REPORT: + api_coro = self._client.cdc_reports.status_by_coordinates( + self.latitude, self.longitude + ) + else: + api_coro = self._client.user_reports.status_by_coordinates( + self.latitude, self.longitude + ) + try: - self.data[api_category] = await self._api_coros[api_category] + self.data[api_category] = await api_coro except FluNearYouError as err: LOGGER.error("Unable to get %s data: %s", api_category, err) self.data[api_category] = None @@ -200,7 +200,7 @@ class FluNearYouData: """Update Flu Near You data.""" tasks = [ self._async_get_data_from_api(api_category) - for api_category in self._api_coros + for api_category in self._api_category_count ] await asyncio.gather(*tasks) From d891810e95b32334e478be049f7307a8bdac7351 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Wed, 29 Apr 2020 15:34:24 -0700 Subject: [PATCH 07/14] Fix Garmin Connect doing I/O in event loop (#34895) --- homeassistant/components/garmin_connect/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/garmin_connect/__init__.py b/homeassistant/components/garmin_connect/__init__.py index 8abdbbbbae9..85e8132bf02 100644 --- a/homeassistant/components/garmin_connect/__init__.py +++ b/homeassistant/components/garmin_connect/__init__.py @@ -86,6 +86,7 @@ class GarminConnectData: def __init__(self, hass, client): """Initialize.""" + self.hass = hass self.client = client self.data = None @@ -95,7 +96,9 @@ class GarminConnectData: today = date.today() try: - self.data = self.client.get_stats_and_body(today.isoformat()) + self.data = await self.hass.async_add_executor_job( + self.client.get_stats_and_body, today.isoformat() + ) except ( GarminConnectAuthenticationError, GarminConnectTooManyRequestsError, From e2475e67c60fc9abcffab223fad9b505cda645cf Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Wed, 29 Apr 2020 15:34:55 -0700 Subject: [PATCH 08/14] Fix Toon doing I/O in event loop (#34896) --- homeassistant/components/toon/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/toon/__init__.py b/homeassistant/components/toon/__init__.py index b078dab898d..595d3cc1ede 100644 --- a/homeassistant/components/toon/__init__.py +++ b/homeassistant/components/toon/__init__.py @@ -80,7 +80,7 @@ async def async_setup_entry(hass: HomeAssistantType, entry: ConfigType) -> bool: ) hass.data.setdefault(DATA_TOON_CLIENT, {})[entry.entry_id] = toon - toon_data = ToonData(hass, entry, toon) + toon_data = await hass.async_add_executor_job(ToonData, hass, entry, toon) hass.data.setdefault(DATA_TOON, {})[entry.entry_id] = toon_data async_track_time_interval(hass, toon_data.update, conf[CONF_SCAN_INTERVAL]) From 88c755518fa291d429f0568214459814406d6dcf Mon Sep 17 00:00:00 2001 From: jjlawren Date: Thu, 30 Apr 2020 02:10:02 -0500 Subject: [PATCH 09/14] Reduce log level for WebOS connection error (#34904) --- homeassistant/components/webostv/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/webostv/__init__.py b/homeassistant/components/webostv/__init__.py index f0a059fc5b8..a441a70888b 100644 --- a/homeassistant/components/webostv/__init__.py +++ b/homeassistant/components/webostv/__init__.py @@ -145,7 +145,7 @@ async def async_setup_tv_finalize(hass, config, conf, client): if client.connection is None: async_call_later(hass, 60, async_load_platforms) - _LOGGER.warning( + _LOGGER.debug( "No connection could be made with host %s, retrying in 60 seconds", conf.get(CONF_HOST), ) From 6b2a006feab25e5daa5749a87cb2c416c4066eb2 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 30 Apr 2020 02:09:33 -0500 Subject: [PATCH 10/14] Fix handling homekit thermostat states (#34905) --- .../components/homekit/type_thermostats.py | 146 +++++++---- .../homekit/test_type_thermostats.py | 231 +++++++++++++++++- 2 files changed, 319 insertions(+), 58 deletions(-) diff --git a/homeassistant/components/homekit/type_thermostats.py b/homeassistant/components/homekit/type_thermostats.py index 0a488917381..0da92ef3dba 100644 --- a/homeassistant/components/homekit/type_thermostats.py +++ b/homeassistant/components/homekit/type_thermostats.py @@ -117,12 +117,15 @@ class Thermostat(HomeAccessory): """Initialize a Thermostat accessory object.""" super().__init__(*args, category=CATEGORY_THERMOSTAT) self._unit = self.hass.config.units.temperature_unit + self._state_updates = 0 + self.hc_homekit_to_hass = None + self.hc_hass_to_homekit = None min_temp, max_temp = self.get_temperature_range() # Homekit only supports 10-38, overwriting - # the max to appears to work, but less than 10 causes + # the max to appears to work, but less than 0 causes # a crash on the home app - hc_min_temp = max(min_temp, HC_MIN_TEMP) + hc_min_temp = max(min_temp, 0) hc_max_temp = max_temp min_humidity = self.hass.states.get(self.entity_id).attributes.get( @@ -149,48 +152,17 @@ class Thermostat(HomeAccessory): CHAR_CURRENT_HEATING_COOLING, value=0 ) - # Target mode characteristics - hc_modes = state.attributes.get(ATTR_HVAC_MODES) - if hc_modes is None: - _LOGGER.error( - "%s: HVAC modes not yet available. Please disable auto start for homekit.", - self.entity_id, - ) - hc_modes = ( - HVAC_MODE_HEAT, - HVAC_MODE_COOL, - HVAC_MODE_HEAT_COOL, - HVAC_MODE_OFF, - ) - - # Determine available modes for this entity, - # Prefer HEAT_COOL over AUTO and COOL over FAN_ONLY, DRY - # - # HEAT_COOL is preferred over auto because HomeKit Accessory Protocol describes - # heating or cooling comes on to maintain a target temp which is closest to - # the Home Assistant spec - # - # HVAC_MODE_HEAT_COOL: The device supports heating/cooling to a range - self.hc_homekit_to_hass = { - c: s - for s, c in HC_HASS_TO_HOMEKIT.items() - if ( - s in hc_modes - and not ( - (s == HVAC_MODE_AUTO and HVAC_MODE_HEAT_COOL in hc_modes) - or ( - s in (HVAC_MODE_DRY, HVAC_MODE_FAN_ONLY) - and HVAC_MODE_COOL in hc_modes - ) - ) - ) - } - hc_valid_values = {k: v for v, k in self.hc_homekit_to_hass.items()} - + self._configure_hvac_modes(state) + # Must set the value first as setting + # valid_values happens before setting + # the value and if 0 is not a valid + # value this will throw self.char_target_heat_cool = serv_thermostat.configure_char( - CHAR_TARGET_HEATING_COOLING, valid_values=hc_valid_values, + CHAR_TARGET_HEATING_COOLING, value=list(self.hc_homekit_to_hass)[0] + ) + self.char_target_heat_cool.override_properties( + valid_values=self.hc_hass_to_homekit ) - # Current and target temperature characteristics self.char_current_temp = serv_thermostat.configure_char( @@ -249,7 +221,7 @@ class Thermostat(HomeAccessory): CHAR_CURRENT_HUMIDITY, value=50 ) - self.update_state(state) + self._update_state(state) serv_thermostat.setter_callback = self._set_chars @@ -356,6 +328,46 @@ class Thermostat(HomeAccessory): if CHAR_TARGET_HUMIDITY in char_values: self.set_target_humidity(char_values[CHAR_TARGET_HUMIDITY]) + def _configure_hvac_modes(self, state): + """Configure target mode characteristics.""" + hc_modes = state.attributes.get(ATTR_HVAC_MODES) + if not hc_modes: + # This cannot be none OR an empty list + _LOGGER.error( + "%s: HVAC modes not yet available. Please disable auto start for homekit.", + self.entity_id, + ) + hc_modes = ( + HVAC_MODE_HEAT, + HVAC_MODE_COOL, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_OFF, + ) + + # Determine available modes for this entity, + # Prefer HEAT_COOL over AUTO and COOL over FAN_ONLY, DRY + # + # HEAT_COOL is preferred over auto because HomeKit Accessory Protocol describes + # heating or cooling comes on to maintain a target temp which is closest to + # the Home Assistant spec + # + # HVAC_MODE_HEAT_COOL: The device supports heating/cooling to a range + self.hc_homekit_to_hass = { + c: s + for s, c in HC_HASS_TO_HOMEKIT.items() + if ( + s in hc_modes + and not ( + (s == HVAC_MODE_AUTO and HVAC_MODE_HEAT_COOL in hc_modes) + or ( + s in (HVAC_MODE_DRY, HVAC_MODE_FAN_ONLY) + and HVAC_MODE_COOL in hc_modes + ) + ) + ) + } + self.hc_hass_to_homekit = {k: v for v, k in self.hc_homekit_to_hass.items()} + def get_temperature_range(self): """Return min and max temperature range.""" max_temp = self.hass.states.get(self.entity_id).attributes.get(ATTR_MAX_TEMP) @@ -382,14 +394,46 @@ class Thermostat(HomeAccessory): def update_state(self, new_state): """Update thermostat state after state changed.""" + if self._state_updates < 3: + # When we get the first state updates + # we recheck valid hvac modes as the entity + # may not have been fully setup when we saw it the + # first time + original_hc_hass_to_homekit = self.hc_hass_to_homekit + self._configure_hvac_modes(new_state) + if self.hc_hass_to_homekit != original_hc_hass_to_homekit: + if self.char_target_heat_cool.value not in self.hc_homekit_to_hass: + # We must make sure the char value is + # in the new valid values before + # setting the new valid values or + # changing them with throw + self.char_target_heat_cool.set_value( + list(self.hc_homekit_to_hass)[0], should_notify=False + ) + self.char_target_heat_cool.override_properties( + valid_values=self.hc_hass_to_homekit + ) + self._state_updates += 1 + + self._update_state(new_state) + + def _update_state(self, new_state): + """Update state without rechecking the device features.""" features = new_state.attributes.get(ATTR_SUPPORTED_FEATURES, 0) # Update target operation mode FIRST hvac_mode = new_state.state if hvac_mode and hvac_mode in HC_HASS_TO_HOMEKIT: homekit_hvac_mode = HC_HASS_TO_HOMEKIT[hvac_mode] - if self.char_target_heat_cool.value != homekit_hvac_mode: - self.char_target_heat_cool.set_value(homekit_hvac_mode) + if homekit_hvac_mode in self.hc_homekit_to_hass: + if self.char_target_heat_cool.value != homekit_hvac_mode: + self.char_target_heat_cool.set_value(homekit_hvac_mode) + else: + _LOGGER.error( + "Cannot map hvac target mode: %s to homekit as only %s modes are supported", + hvac_mode, + self.hc_homekit_to_hass, + ) # Set current operation mode for supported thermostats hvac_action = new_state.attributes.get(ATTR_HVAC_ACTION) @@ -444,13 +488,13 @@ class Thermostat(HomeAccessory): # even if the device does not support it hc_hvac_mode = self.char_target_heat_cool.value if hc_hvac_mode == HC_HEAT_COOL_HEAT: - target_temp = self._temperature_to_homekit( - new_state.attributes.get(ATTR_TARGET_TEMP_LOW) - ) + temp_low = new_state.attributes.get(ATTR_TARGET_TEMP_LOW) + if isinstance(temp_low, (int, float)): + target_temp = self._temperature_to_homekit(temp_low) elif hc_hvac_mode == HC_HEAT_COOL_COOL: - target_temp = self._temperature_to_homekit( - new_state.attributes.get(ATTR_TARGET_TEMP_HIGH) - ) + temp_high = new_state.attributes.get(ATTR_TARGET_TEMP_HIGH) + if isinstance(temp_high, (int, float)): + target_temp = self._temperature_to_homekit(temp_high) if target_temp and self.char_target_temp.value != target_temp: self.char_target_temp.set_value(target_temp) diff --git a/tests/components/homekit/test_type_thermostats.py b/tests/components/homekit/test_type_thermostats.py index 8ee533521e8..82abed32c0e 100644 --- a/tests/components/homekit/test_type_thermostats.py +++ b/tests/components/homekit/test_type_thermostats.py @@ -43,7 +43,6 @@ from homeassistant.components.homekit.const import ( PROP_MIN_STEP, PROP_MIN_VALUE, ) -from homeassistant.components.homekit.type_thermostats import HC_MIN_TEMP from homeassistant.components.water_heater import DOMAIN as DOMAIN_WATER_HEATER from homeassistant.const import ( ATTR_ENTITY_ID, @@ -116,7 +115,7 @@ async def test_thermostat(hass, hk_driver, cls, events): assert acc.char_current_humidity is None assert acc.char_target_temp.properties[PROP_MAX_VALUE] == DEFAULT_MAX_TEMP - assert acc.char_target_temp.properties[PROP_MIN_VALUE] == HC_MIN_TEMP + assert acc.char_target_temp.properties[PROP_MIN_VALUE] == 7.0 assert acc.char_target_temp.properties[PROP_MIN_STEP] == 0.1 hass.states.async_set( @@ -126,6 +125,14 @@ async def test_thermostat(hass, hk_driver, cls, events): ATTR_TEMPERATURE: 22.2, ATTR_CURRENT_TEMPERATURE: 17.8, ATTR_HVAC_ACTION: CURRENT_HVAC_HEAT, + ATTR_HVAC_MODES: [ + HVAC_MODE_HEAT, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_FAN_ONLY, + HVAC_MODE_COOL, + HVAC_MODE_OFF, + HVAC_MODE_AUTO, + ], }, ) await hass.async_block_till_done() @@ -142,6 +149,14 @@ async def test_thermostat(hass, hk_driver, cls, events): ATTR_TEMPERATURE: 22.0, ATTR_CURRENT_TEMPERATURE: 23.0, ATTR_HVAC_ACTION: CURRENT_HVAC_IDLE, + ATTR_HVAC_MODES: [ + HVAC_MODE_HEAT, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_FAN_ONLY, + HVAC_MODE_COOL, + HVAC_MODE_OFF, + HVAC_MODE_AUTO, + ], }, ) await hass.async_block_till_done() @@ -158,6 +173,14 @@ async def test_thermostat(hass, hk_driver, cls, events): ATTR_TEMPERATURE: 20.0, ATTR_CURRENT_TEMPERATURE: 25.0, ATTR_HVAC_ACTION: CURRENT_HVAC_COOL, + ATTR_HVAC_MODES: [ + HVAC_MODE_HEAT, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_FAN_ONLY, + HVAC_MODE_COOL, + HVAC_MODE_OFF, + HVAC_MODE_AUTO, + ], }, ) await hass.async_block_till_done() @@ -202,6 +225,14 @@ async def test_thermostat(hass, hk_driver, cls, events): ATTR_TEMPERATURE: 22.0, ATTR_CURRENT_TEMPERATURE: 18.0, ATTR_HVAC_ACTION: CURRENT_HVAC_HEAT, + ATTR_HVAC_MODES: [ + HVAC_MODE_HEAT, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_FAN_ONLY, + HVAC_MODE_COOL, + HVAC_MODE_OFF, + HVAC_MODE_AUTO, + ], }, ) await hass.async_block_till_done() @@ -218,6 +249,14 @@ async def test_thermostat(hass, hk_driver, cls, events): ATTR_TEMPERATURE: 22.0, ATTR_CURRENT_TEMPERATURE: 25.0, ATTR_HVAC_ACTION: CURRENT_HVAC_COOL, + ATTR_HVAC_MODES: [ + HVAC_MODE_HEAT, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_FAN_ONLY, + HVAC_MODE_COOL, + HVAC_MODE_OFF, + HVAC_MODE_AUTO, + ], }, ) await hass.async_block_till_done() @@ -234,6 +273,14 @@ async def test_thermostat(hass, hk_driver, cls, events): ATTR_TEMPERATURE: 22.0, ATTR_CURRENT_TEMPERATURE: 22.0, ATTR_HVAC_ACTION: CURRENT_HVAC_IDLE, + ATTR_HVAC_MODES: [ + HVAC_MODE_HEAT, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_FAN_ONLY, + HVAC_MODE_COOL, + HVAC_MODE_OFF, + HVAC_MODE_AUTO, + ], }, ) await hass.async_block_till_done() @@ -250,6 +297,14 @@ async def test_thermostat(hass, hk_driver, cls, events): ATTR_TEMPERATURE: 22.0, ATTR_CURRENT_TEMPERATURE: 22.0, ATTR_HVAC_ACTION: CURRENT_HVAC_FAN, + ATTR_HVAC_MODES: [ + HVAC_MODE_HEAT, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_FAN_ONLY, + HVAC_MODE_COOL, + HVAC_MODE_OFF, + HVAC_MODE_AUTO, + ], }, ) await hass.async_block_till_done() @@ -369,7 +424,15 @@ async def test_thermostat_auto(hass, hk_driver, cls, events): HVAC_MODE_OFF, { ATTR_SUPPORTED_FEATURES: SUPPORT_TARGET_TEMPERATURE - | SUPPORT_TARGET_TEMPERATURE_RANGE + | SUPPORT_TARGET_TEMPERATURE_RANGE, + ATTR_HVAC_MODES: [ + HVAC_MODE_HEAT, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_FAN_ONLY, + HVAC_MODE_COOL, + HVAC_MODE_OFF, + HVAC_MODE_AUTO, + ], }, ) await hass.async_block_till_done() @@ -383,10 +446,10 @@ async def test_thermostat_auto(hass, hk_driver, cls, events): assert acc.char_heating_thresh_temp.value == 19.0 assert acc.char_cooling_thresh_temp.properties[PROP_MAX_VALUE] == DEFAULT_MAX_TEMP - assert acc.char_cooling_thresh_temp.properties[PROP_MIN_VALUE] == HC_MIN_TEMP + assert acc.char_cooling_thresh_temp.properties[PROP_MIN_VALUE] == 7.0 assert acc.char_cooling_thresh_temp.properties[PROP_MIN_STEP] == 0.1 assert acc.char_heating_thresh_temp.properties[PROP_MAX_VALUE] == DEFAULT_MAX_TEMP - assert acc.char_heating_thresh_temp.properties[PROP_MIN_VALUE] == HC_MIN_TEMP + assert acc.char_heating_thresh_temp.properties[PROP_MIN_VALUE] == 7.0 assert acc.char_heating_thresh_temp.properties[PROP_MIN_STEP] == 0.1 hass.states.async_set( @@ -397,6 +460,14 @@ async def test_thermostat_auto(hass, hk_driver, cls, events): ATTR_TARGET_TEMP_LOW: 20.0, ATTR_CURRENT_TEMPERATURE: 18.0, ATTR_HVAC_ACTION: CURRENT_HVAC_HEAT, + ATTR_HVAC_MODES: [ + HVAC_MODE_HEAT, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_FAN_ONLY, + HVAC_MODE_COOL, + HVAC_MODE_OFF, + HVAC_MODE_AUTO, + ], }, ) await hass.async_block_till_done() @@ -415,6 +486,14 @@ async def test_thermostat_auto(hass, hk_driver, cls, events): ATTR_TARGET_TEMP_LOW: 19.0, ATTR_CURRENT_TEMPERATURE: 24.0, ATTR_HVAC_ACTION: CURRENT_HVAC_COOL, + ATTR_HVAC_MODES: [ + HVAC_MODE_HEAT, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_FAN_ONLY, + HVAC_MODE_COOL, + HVAC_MODE_OFF, + HVAC_MODE_AUTO, + ], }, ) await hass.async_block_till_done() @@ -433,6 +512,14 @@ async def test_thermostat_auto(hass, hk_driver, cls, events): ATTR_TARGET_TEMP_LOW: 19.0, ATTR_CURRENT_TEMPERATURE: 21.0, ATTR_HVAC_ACTION: CURRENT_HVAC_IDLE, + ATTR_HVAC_MODES: [ + HVAC_MODE_HEAT, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_FAN_ONLY, + HVAC_MODE_COOL, + HVAC_MODE_OFF, + HVAC_MODE_AUTO, + ], }, ) await hass.async_block_till_done() @@ -1094,10 +1181,10 @@ async def test_thermostat_without_target_temp_only_range(hass, hk_driver, cls, e assert acc.char_heating_thresh_temp.value == 19.0 assert acc.char_cooling_thresh_temp.properties[PROP_MAX_VALUE] == DEFAULT_MAX_TEMP - assert acc.char_cooling_thresh_temp.properties[PROP_MIN_VALUE] == HC_MIN_TEMP + assert acc.char_cooling_thresh_temp.properties[PROP_MIN_VALUE] == 7.0 assert acc.char_cooling_thresh_temp.properties[PROP_MIN_STEP] == 0.1 assert acc.char_heating_thresh_temp.properties[PROP_MAX_VALUE] == DEFAULT_MAX_TEMP - assert acc.char_heating_thresh_temp.properties[PROP_MIN_VALUE] == HC_MIN_TEMP + assert acc.char_heating_thresh_temp.properties[PROP_MIN_VALUE] == 7.0 assert acc.char_heating_thresh_temp.properties[PROP_MIN_STEP] == 0.1 hass.states.async_set( @@ -1109,6 +1196,14 @@ async def test_thermostat_without_target_temp_only_range(hass, hk_driver, cls, e ATTR_CURRENT_TEMPERATURE: 18.0, ATTR_HVAC_ACTION: CURRENT_HVAC_HEAT, ATTR_SUPPORTED_FEATURES: SUPPORT_TARGET_TEMPERATURE_RANGE, + ATTR_HVAC_MODES: [ + HVAC_MODE_HEAT, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_FAN_ONLY, + HVAC_MODE_COOL, + HVAC_MODE_OFF, + HVAC_MODE_AUTO, + ], }, ) await hass.async_block_till_done() @@ -1128,6 +1223,14 @@ async def test_thermostat_without_target_temp_only_range(hass, hk_driver, cls, e ATTR_CURRENT_TEMPERATURE: 24.0, ATTR_HVAC_ACTION: CURRENT_HVAC_COOL, ATTR_SUPPORTED_FEATURES: SUPPORT_TARGET_TEMPERATURE_RANGE, + ATTR_HVAC_MODES: [ + HVAC_MODE_HEAT, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_FAN_ONLY, + HVAC_MODE_COOL, + HVAC_MODE_OFF, + HVAC_MODE_AUTO, + ], }, ) await hass.async_block_till_done() @@ -1147,6 +1250,14 @@ async def test_thermostat_without_target_temp_only_range(hass, hk_driver, cls, e ATTR_CURRENT_TEMPERATURE: 21.0, ATTR_HVAC_ACTION: CURRENT_HVAC_IDLE, ATTR_SUPPORTED_FEATURES: SUPPORT_TARGET_TEMPERATURE_RANGE, + ATTR_HVAC_MODES: [ + HVAC_MODE_HEAT, + HVAC_MODE_HEAT_COOL, + HVAC_MODE_FAN_ONLY, + HVAC_MODE_COOL, + HVAC_MODE_OFF, + HVAC_MODE_AUTO, + ], }, ) await hass.async_block_till_done() @@ -1400,3 +1511,109 @@ async def test_water_heater_restore(hass, hk_driver, cls, events): "Heat", "Off", } + + +async def test_thermostat_with_no_modes_when_we_first_see(hass, hk_driver, cls, events): + """Test if a thermostat that is not ready when we first see it.""" + entity_id = "climate.test" + + # support_auto = True + hass.states.async_set( + entity_id, + HVAC_MODE_OFF, + { + ATTR_SUPPORTED_FEATURES: SUPPORT_TARGET_TEMPERATURE + | SUPPORT_TARGET_TEMPERATURE_RANGE, + ATTR_HVAC_MODES: [], + }, + ) + await hass.async_block_till_done() + acc = cls.thermostat(hass, hk_driver, "Climate", entity_id, 1, None) + hk_driver.add_accessory(acc) + + await acc.run_handler() + await hass.async_block_till_done() + + assert acc.char_cooling_thresh_temp.value == 23.0 + assert acc.char_heating_thresh_temp.value == 19.0 + + assert acc.char_cooling_thresh_temp.properties[PROP_MAX_VALUE] == DEFAULT_MAX_TEMP + assert acc.char_cooling_thresh_temp.properties[PROP_MIN_VALUE] == 7.0 + assert acc.char_cooling_thresh_temp.properties[PROP_MIN_STEP] == 0.1 + assert acc.char_heating_thresh_temp.properties[PROP_MAX_VALUE] == DEFAULT_MAX_TEMP + assert acc.char_heating_thresh_temp.properties[PROP_MIN_VALUE] == 7.0 + assert acc.char_heating_thresh_temp.properties[PROP_MIN_STEP] == 0.1 + + assert acc.char_target_heat_cool.value == 0 + + hass.states.async_set( + entity_id, + HVAC_MODE_HEAT_COOL, + { + ATTR_TARGET_TEMP_HIGH: 22.0, + ATTR_TARGET_TEMP_LOW: 20.0, + ATTR_CURRENT_TEMPERATURE: 18.0, + ATTR_HVAC_ACTION: CURRENT_HVAC_HEAT, + ATTR_HVAC_MODES: [HVAC_MODE_HEAT_COOL, HVAC_MODE_OFF, HVAC_MODE_AUTO], + }, + ) + await hass.async_block_till_done() + assert acc.char_heating_thresh_temp.value == 20.0 + assert acc.char_cooling_thresh_temp.value == 22.0 + assert acc.char_current_heat_cool.value == 1 + assert acc.char_target_heat_cool.value == 3 + assert acc.char_current_temp.value == 18.0 + assert acc.char_display_units.value == 0 + + +async def test_thermostat_with_no_off_after_recheck(hass, hk_driver, cls, events): + """Test if a thermostat that is not ready when we first see it that actually does not have off.""" + entity_id = "climate.test" + + # support_auto = True + hass.states.async_set( + entity_id, + HVAC_MODE_COOL, + { + ATTR_SUPPORTED_FEATURES: SUPPORT_TARGET_TEMPERATURE + | SUPPORT_TARGET_TEMPERATURE_RANGE, + ATTR_HVAC_MODES: [], + }, + ) + await hass.async_block_till_done() + acc = cls.thermostat(hass, hk_driver, "Climate", entity_id, 1, None) + hk_driver.add_accessory(acc) + + await acc.run_handler() + await hass.async_block_till_done() + + assert acc.char_cooling_thresh_temp.value == 23.0 + assert acc.char_heating_thresh_temp.value == 19.0 + + assert acc.char_cooling_thresh_temp.properties[PROP_MAX_VALUE] == DEFAULT_MAX_TEMP + assert acc.char_cooling_thresh_temp.properties[PROP_MIN_VALUE] == 7.0 + assert acc.char_cooling_thresh_temp.properties[PROP_MIN_STEP] == 0.1 + assert acc.char_heating_thresh_temp.properties[PROP_MAX_VALUE] == DEFAULT_MAX_TEMP + assert acc.char_heating_thresh_temp.properties[PROP_MIN_VALUE] == 7.0 + assert acc.char_heating_thresh_temp.properties[PROP_MIN_STEP] == 0.1 + + assert acc.char_target_heat_cool.value == 2 + + hass.states.async_set( + entity_id, + HVAC_MODE_HEAT_COOL, + { + ATTR_TARGET_TEMP_HIGH: 22.0, + ATTR_TARGET_TEMP_LOW: 20.0, + ATTR_CURRENT_TEMPERATURE: 18.0, + ATTR_HVAC_ACTION: CURRENT_HVAC_HEAT, + ATTR_HVAC_MODES: [HVAC_MODE_HEAT_COOL, HVAC_MODE_AUTO], + }, + ) + await hass.async_block_till_done() + assert acc.char_heating_thresh_temp.value == 20.0 + assert acc.char_cooling_thresh_temp.value == 22.0 + assert acc.char_current_heat_cool.value == 1 + assert acc.char_target_heat_cool.value == 3 + assert acc.char_current_temp.value == 18.0 + assert acc.char_display_units.value == 0 From 6f20a4a1811ae2dae723d5b38a5a0687629b5400 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 30 Apr 2020 02:08:56 -0500 Subject: [PATCH 11/14] Avoid error when battery appears after homekit has started (#34906) --- .../components/homekit/accessories.py | 10 ++++++- tests/components/homekit/test_accessories.py | 27 ++++++++++++++++--- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/homekit/accessories.py b/homeassistant/components/homekit/accessories.py index a18f42e76b6..4f0a840770c 100644 --- a/homeassistant/components/homekit/accessories.py +++ b/homeassistant/components/homekit/accessories.py @@ -99,6 +99,9 @@ class HomeAccessory(Accessory): self.entity_id = entity_id self.hass = hass self.debounce = {} + self._char_battery = None + self._char_charging = None + self._char_low_battery = None self.linked_battery_sensor = self.config.get(CONF_LINKED_BATTERY_SENSOR) self.linked_battery_charging_sensor = self.config.get( CONF_LINKED_BATTERY_CHARGING_SENSOR @@ -247,6 +250,10 @@ class HomeAccessory(Accessory): Only call this function if self._support_battery_level is True. """ + if not self._char_battery: + # Battery appeared after homekit was started + return + battery_level = convert_to_float(battery_level) if battery_level is not None: if self._char_battery.value != battery_level: @@ -258,7 +265,8 @@ class HomeAccessory(Accessory): "%s: Updated battery level to %d", self.entity_id, battery_level ) - if battery_charging is None: + # Charging state can appear after homekit was started + if battery_charging is None or not self._char_charging: return hk_charging = HK_CHARGING if battery_charging else HK_NOT_CHARGING diff --git a/tests/components/homekit/test_accessories.py b/tests/components/homekit/test_accessories.py index becbcb2d6d4..c4b61f68833 100644 --- a/tests/components/homekit/test_accessories.py +++ b/tests/components/homekit/test_accessories.py @@ -363,9 +363,30 @@ async def test_missing_linked_battery_sensor(hass, hk_driver, caplog): await hass.async_block_till_done() assert not acc.linked_battery_sensor - assert not hasattr(acc, "_char_battery") - assert not hasattr(acc, "_char_low_battery") - assert not hasattr(acc, "_char_charging") + assert acc._char_battery is None + assert acc._char_low_battery is None + assert acc._char_charging is None + + +async def test_battery_appears_after_startup(hass, hk_driver, caplog): + """Test battery level appears after homekit is started.""" + entity_id = "homekit.accessory" + hass.states.async_set(entity_id, None, {}) + await hass.async_block_till_done() + + acc = HomeAccessory( + hass, hk_driver, "Accessory without battery", entity_id, 2, None + ) + acc.update_state = lambda x: None + assert acc._char_battery is None + + await acc.run_handler() + await hass.async_block_till_done() + assert acc._char_battery is None + + hass.states.async_set(entity_id, None, {ATTR_BATTERY_LEVEL: 15}) + await hass.async_block_till_done() + assert acc._char_battery is None async def test_call_service(hass, hk_driver, events): From d974e64a8b1eceb23cdfaf372a4343c3ae00eb02 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 30 Apr 2020 02:08:40 -0500 Subject: [PATCH 12/14] Make sqlalchemy engine connect listener recorder specific (#34908) --- homeassistant/components/recorder/__init__.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/homeassistant/components/recorder/__init__.py b/homeassistant/components/recorder/__init__.py index cb5d1f4499f..fcccaa2fb9f 100644 --- a/homeassistant/components/recorder/__init__.py +++ b/homeassistant/components/recorder/__init__.py @@ -9,9 +9,7 @@ import threading import time from typing import Any, Dict, Optional -from sqlalchemy import create_engine, exc, select -from sqlalchemy.engine import Engine -from sqlalchemy.event import listens_for +from sqlalchemy import create_engine, event as sqlalchemy_event, exc, select from sqlalchemy.orm import scoped_session, sessionmaker from sqlalchemy.pool import StaticPool import voluptuous as vol @@ -488,15 +486,13 @@ class Recorder(threading.Thread): """Ensure database is ready to fly.""" kwargs = {} - # pylint: disable=unused-variable - @listens_for(Engine, "connect") - def setup_connection(dbapi_connection, connection_record): + def setup_recorder_connection(dbapi_connection, connection_record): """Dbapi specific connection settings.""" # We do not import sqlite3 here so mysql/other # users do not have to pay for it to be loaded in # memory - if self.db_url == "sqlite://" or ":memory:" in self.db_url: + if self.db_url.startswith("sqlite://"): old_isolation = dbapi_connection.isolation_level dbapi_connection.isolation_level = None cursor = dbapi_connection.cursor() @@ -519,6 +515,9 @@ class Recorder(threading.Thread): self.engine.dispose() self.engine = create_engine(self.db_url, **kwargs) + + sqlalchemy_event.listen(self.engine, "connect", setup_recorder_connection) + Base.metadata.create_all(self.engine) self.get_session = scoped_session(sessionmaker(bind=self.engine)) From 38f955934890da7938860b8aed9e0caa778848b5 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Thu, 30 Apr 2020 00:11:31 -0700 Subject: [PATCH 13/14] Bumped version to 0.109.1 --- homeassistant/const.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/const.py b/homeassistant/const.py index a30c0f0bdc5..2d0212f2a31 100644 --- a/homeassistant/const.py +++ b/homeassistant/const.py @@ -1,7 +1,7 @@ """Constants used by Home Assistant components.""" MAJOR_VERSION = 0 MINOR_VERSION = 109 -PATCH_VERSION = "0" +PATCH_VERSION = "1" __short_version__ = f"{MAJOR_VERSION}.{MINOR_VERSION}" __version__ = f"{__short_version__}.{PATCH_VERSION}" REQUIRED_PYTHON_VER = (3, 7, 0) From ade4e36da72be42d70ba505790085dcafdb06242 Mon Sep 17 00:00:00 2001 From: Chris Talkington Date: Thu, 30 Apr 2020 02:21:53 -0500 Subject: [PATCH 14/14] Use entry ID when IPP printer offers no identifier (#34316) --- homeassistant/components/ipp/__init__.py | 8 ++--- homeassistant/components/ipp/sensor.py | 39 ++++++++++++++++++------ tests/components/ipp/__init__.py | 10 ++++-- tests/components/ipp/test_sensor.py | 12 ++++++++ 4 files changed, 50 insertions(+), 19 deletions(-) diff --git a/homeassistant/components/ipp/__init__.py b/homeassistant/components/ipp/__init__.py index 5979caa37db..1258e1031b4 100644 --- a/homeassistant/components/ipp/__init__.py +++ b/homeassistant/components/ipp/__init__.py @@ -128,24 +128,20 @@ class IPPEntity(Entity): self, *, entry_id: str, + device_id: str, coordinator: IPPDataUpdateCoordinator, name: str, icon: str, enabled_default: bool = True, ) -> None: """Initialize the IPP entity.""" - self._device_id = None + self._device_id = device_id self._enabled_default = enabled_default self._entry_id = entry_id self._icon = icon self._name = name self.coordinator = coordinator - if coordinator.data.info.uuid is not None: - self._device_id = coordinator.data.info.uuid - elif coordinator.data.info.serial is not None: - self._device_id = coordinator.data.info.serial - @property def name(self) -> str: """Return the name of the entity.""" diff --git a/homeassistant/components/ipp/sensor.py b/homeassistant/components/ipp/sensor.py index 5c29be09d94..bbb051d3158 100644 --- a/homeassistant/components/ipp/sensor.py +++ b/homeassistant/components/ipp/sensor.py @@ -32,13 +32,21 @@ async def async_setup_entry( """Set up IPP sensor based on a config entry.""" coordinator: IPPDataUpdateCoordinator = hass.data[DOMAIN][entry.entry_id] + # config flow sets this to either UUID, serial number or None + unique_id = entry.unique_id + + if unique_id is None: + unique_id = entry.entry_id + sensors = [] - sensors.append(IPPPrinterSensor(entry.entry_id, coordinator)) - sensors.append(IPPUptimeSensor(entry.entry_id, coordinator)) + sensors.append(IPPPrinterSensor(entry.entry_id, unique_id, coordinator)) + sensors.append(IPPUptimeSensor(entry.entry_id, unique_id, coordinator)) for marker_index in range(len(coordinator.data.markers)): - sensors.append(IPPMarkerSensor(entry.entry_id, coordinator, marker_index)) + sensors.append( + IPPMarkerSensor(entry.entry_id, unique_id, coordinator, marker_index) + ) async_add_entities(sensors, True) @@ -52,6 +60,7 @@ class IPPSensor(IPPEntity): coordinator: IPPDataUpdateCoordinator, enabled_default: bool = True, entry_id: str, + unique_id: str, icon: str, key: str, name: str, @@ -62,13 +71,12 @@ class IPPSensor(IPPEntity): self._key = key self._unique_id = None - if coordinator.data.info.uuid is not None: - self._unique_id = f"{coordinator.data.info.uuid}_{key}" - elif coordinator.data.info.serial is not None: - self._unique_id = f"{coordinator.data.info.serial}_{key}" + if unique_id is not None: + self._unique_id = f"{unique_id}_{key}" super().__init__( entry_id=entry_id, + device_id=unique_id, coordinator=coordinator, name=name, icon=icon, @@ -90,7 +98,11 @@ class IPPMarkerSensor(IPPSensor): """Defines an IPP marker sensor.""" def __init__( - self, entry_id: str, coordinator: IPPDataUpdateCoordinator, marker_index: int + self, + entry_id: str, + unique_id: str, + coordinator: IPPDataUpdateCoordinator, + marker_index: int, ) -> None: """Initialize IPP marker sensor.""" self.marker_index = marker_index @@ -98,6 +110,7 @@ class IPPMarkerSensor(IPPSensor): super().__init__( coordinator=coordinator, entry_id=entry_id, + unique_id=unique_id, icon="mdi:water", key=f"marker_{marker_index}", name=f"{coordinator.data.info.name} {coordinator.data.markers[marker_index].name}", @@ -133,11 +146,14 @@ class IPPMarkerSensor(IPPSensor): class IPPPrinterSensor(IPPSensor): """Defines an IPP printer sensor.""" - def __init__(self, entry_id: str, coordinator: IPPDataUpdateCoordinator) -> None: + def __init__( + self, entry_id: str, unique_id: str, coordinator: IPPDataUpdateCoordinator + ) -> None: """Initialize IPP printer sensor.""" super().__init__( coordinator=coordinator, entry_id=entry_id, + unique_id=unique_id, icon="mdi:printer", key="printer", name=coordinator.data.info.name, @@ -166,12 +182,15 @@ class IPPPrinterSensor(IPPSensor): class IPPUptimeSensor(IPPSensor): """Defines a IPP uptime sensor.""" - def __init__(self, entry_id: str, coordinator: IPPDataUpdateCoordinator) -> None: + def __init__( + self, entry_id: str, unique_id: str, coordinator: IPPDataUpdateCoordinator + ) -> None: """Initialize IPP uptime sensor.""" super().__init__( coordinator=coordinator, enabled_default=False, entry_id=entry_id, + unique_id=unique_id, icon="mdi:clock-outline", key="uptime", name=f"{coordinator.data.info.name} Uptime", diff --git a/tests/components/ipp/__init__.py b/tests/components/ipp/__init__.py index a8c79324494..f0dc45417e1 100644 --- a/tests/components/ipp/__init__.py +++ b/tests/components/ipp/__init__.py @@ -62,7 +62,11 @@ def load_fixture_binary(filename): async def init_integration( - hass: HomeAssistant, aioclient_mock: AiohttpClientMocker, skip_setup: bool = False, + hass: HomeAssistant, + aioclient_mock: AiohttpClientMocker, + skip_setup: bool = False, + uuid: str = "cfe92100-67c4-11d4-a45f-f8d027761251", + unique_id: str = "cfe92100-67c4-11d4-a45f-f8d027761251", ) -> MockConfigEntry: """Set up the IPP integration in Home Assistant.""" fixture = "ipp/get-printer-attributes.bin" @@ -74,14 +78,14 @@ async def init_integration( entry = MockConfigEntry( domain=DOMAIN, - unique_id="cfe92100-67c4-11d4-a45f-f8d027761251", + unique_id=unique_id, data={ CONF_HOST: "192.168.1.31", CONF_PORT: 631, CONF_SSL: False, CONF_VERIFY_SSL: True, CONF_BASE_PATH: "/ipp/print", - CONF_UUID: "cfe92100-67c4-11d4-a45f-f8d027761251", + CONF_UUID: uuid, }, ) diff --git a/tests/components/ipp/test_sensor.py b/tests/components/ipp/test_sensor.py index b7db606d870..e6830f559c6 100644 --- a/tests/components/ipp/test_sensor.py +++ b/tests/components/ipp/test_sensor.py @@ -94,3 +94,15 @@ async def test_disabled_by_default_sensors( assert entry assert entry.disabled assert entry.disabled_by == "integration" + + +async def test_missing_entry_unique_id( + hass: HomeAssistant, aioclient_mock: AiohttpClientMocker +) -> None: + """Test the unique_id of IPP sensor when printer is missing identifiers.""" + entry = await init_integration(hass, aioclient_mock, uuid=None, unique_id=None) + registry = await hass.helpers.entity_registry.async_get_registry() + + entity = registry.async_get("sensor.epson_xp_6000_series") + assert entity + assert entity.unique_id == f"{entry.entry_id}_printer"