From 682943511a41a5f7d3ca56bdc01bcaad36e70883 Mon Sep 17 00:00:00 2001 From: Martin Hjelmare Date: Thu, 4 Mar 2021 23:14:24 +0100 Subject: [PATCH] Make zwave_js add-on manager more flexible (#47356) --- homeassistant/components/zwave_js/__init__.py | 10 ++- homeassistant/components/zwave_js/addon.py | 66 ++++++++++++---- .../components/zwave_js/config_flow.py | 17 ++-- tests/components/zwave_js/test_config_flow.py | 77 +++++++------------ 4 files changed, 95 insertions(+), 75 deletions(-) diff --git a/homeassistant/components/zwave_js/__init__.py b/homeassistant/components/zwave_js/__init__.py index 0c5a55c25fb..637ea0fe08a 100644 --- a/homeassistant/components/zwave_js/__init__.py +++ b/homeassistant/components/zwave_js/__init__.py @@ -483,11 +483,15 @@ async def async_ensure_addon_running(hass: HomeAssistant, entry: ConfigEntry) -> network_key: str = entry.data[CONF_NETWORK_KEY] if not addon_is_installed: - addon_manager.async_schedule_install_addon(usb_path, network_key) + addon_manager.async_schedule_install_setup_addon( + usb_path, network_key, catch_error=True + ) raise ConfigEntryNotReady if not addon_is_running: - addon_manager.async_schedule_setup_addon(usb_path, network_key) + addon_manager.async_schedule_setup_addon( + usb_path, network_key, catch_error=True + ) raise ConfigEntryNotReady @@ -497,4 +501,4 @@ def async_ensure_addon_updated(hass: HomeAssistant) -> None: addon_manager: AddonManager = get_addon_manager(hass) if addon_manager.task_in_progress(): raise ConfigEntryNotReady - addon_manager.async_schedule_update_addon() + addon_manager.async_schedule_update_addon(catch_error=True) diff --git a/homeassistant/components/zwave_js/addon.py b/homeassistant/components/zwave_js/addon.py index 54169dcaf94..b8d020cfb00 100644 --- a/homeassistant/components/zwave_js/addon.py +++ b/homeassistant/components/zwave_js/addon.py @@ -67,8 +67,8 @@ class AddonManager: """Set up the add-on manager.""" self._hass = hass self._install_task: Optional[asyncio.Task] = None + self._start_task: Optional[asyncio.Task] = None self._update_task: Optional[asyncio.Task] = None - self._setup_task: Optional[asyncio.Task] = None def task_in_progress(self) -> bool: """Return True if any of the add-on tasks are in progress.""" @@ -76,7 +76,7 @@ class AddonManager: task and not task.done() for task in ( self._install_task, - self._setup_task, + self._start_task, self._update_task, ) ) @@ -125,8 +125,21 @@ class AddonManager: await async_install_addon(self._hass, ADDON_SLUG) @callback - def async_schedule_install_addon( - self, usb_path: str, network_key: str + def async_schedule_install_addon(self, catch_error: bool = False) -> asyncio.Task: + """Schedule a task that installs the Z-Wave JS add-on. + + Only schedule a new install task if the there's no running task. + """ + if not self._install_task or self._install_task.done(): + LOGGER.info("Z-Wave JS add-on is not installed. Installing add-on") + self._install_task = self._async_schedule_addon_operation( + self.async_install_addon, catch_error=catch_error + ) + return self._install_task + + @callback + def async_schedule_install_setup_addon( + self, usb_path: str, network_key: str, catch_error: bool = False ) -> asyncio.Task: """Schedule a task that installs and sets up the Z-Wave JS add-on. @@ -136,7 +149,9 @@ class AddonManager: LOGGER.info("Z-Wave JS add-on is not installed. Installing add-on") self._install_task = self._async_schedule_addon_operation( self.async_install_addon, - partial(self.async_setup_addon, usb_path, network_key), + partial(self.async_configure_addon, usb_path, network_key), + self.async_start_addon, + catch_error=catch_error, ) return self._install_task @@ -161,7 +176,7 @@ class AddonManager: await async_update_addon(self._hass, ADDON_SLUG) @callback - def async_schedule_update_addon(self) -> asyncio.Task: + def async_schedule_update_addon(self, catch_error: bool = False) -> asyncio.Task: """Schedule a task that updates and sets up the Z-Wave JS add-on. Only schedule a new update task if the there's no running task. @@ -169,7 +184,9 @@ class AddonManager: if not self._update_task or self._update_task.done(): LOGGER.info("Trying to update the Z-Wave JS add-on") self._update_task = self._async_schedule_addon_operation( - self.async_create_snapshot, self.async_update_addon + self.async_create_snapshot, + self.async_update_addon, + catch_error=catch_error, ) return self._update_task @@ -178,12 +195,25 @@ class AddonManager: """Start the Z-Wave JS add-on.""" await async_start_addon(self._hass, ADDON_SLUG) + @callback + def async_schedule_start_addon(self, catch_error: bool = False) -> asyncio.Task: + """Schedule a task that starts the Z-Wave JS add-on. + + Only schedule a new start task if the there's no running task. + """ + if not self._start_task or self._start_task.done(): + LOGGER.info("Z-Wave JS add-on is not running. Starting add-on") + self._start_task = self._async_schedule_addon_operation( + self.async_start_addon, catch_error=catch_error + ) + return self._start_task + @api_error("Failed to stop the Z-Wave JS add-on") async def async_stop_addon(self) -> None: """Stop the Z-Wave JS add-on.""" await async_stop_addon(self._hass, ADDON_SLUG) - async def async_setup_addon(self, usb_path: str, network_key: str) -> None: + async def async_configure_addon(self, usb_path: str, network_key: str) -> None: """Configure and start Z-Wave JS add-on.""" addon_options = await self.async_get_addon_options() @@ -195,22 +225,22 @@ class AddonManager: if new_addon_options != addon_options: await self.async_set_addon_options(new_addon_options) - await self.async_start_addon() - @callback def async_schedule_setup_addon( - self, usb_path: str, network_key: str + self, usb_path: str, network_key: str, catch_error: bool = False ) -> asyncio.Task: """Schedule a task that configures and starts the Z-Wave JS add-on. Only schedule a new setup task if the there's no running task. """ - if not self._setup_task or self._setup_task.done(): + if not self._start_task or self._start_task.done(): LOGGER.info("Z-Wave JS add-on is not running. Starting add-on") - self._setup_task = self._async_schedule_addon_operation( - partial(self.async_setup_addon, usb_path, network_key) + self._start_task = self._async_schedule_addon_operation( + partial(self.async_configure_addon, usb_path, network_key), + self.async_start_addon, + catch_error=catch_error, ) - return self._setup_task + return self._start_task @api_error("Failed to create a snapshot of the Z-Wave JS add-on.") async def async_create_snapshot(self) -> None: @@ -227,7 +257,9 @@ class AddonManager: ) @callback - def _async_schedule_addon_operation(self, *funcs: Callable) -> asyncio.Task: + def _async_schedule_addon_operation( + self, *funcs: Callable, catch_error: bool = False + ) -> asyncio.Task: """Schedule an add-on task.""" async def addon_operation() -> None: @@ -236,6 +268,8 @@ class AddonManager: try: await func() except AddonError as err: + if not catch_error: + raise LOGGER.error(err) break diff --git a/homeassistant/components/zwave_js/config_flow.py b/homeassistant/components/zwave_js/config_flow.py index ac466223fb6..4929f7e7869 100644 --- a/homeassistant/components/zwave_js/config_flow.py +++ b/homeassistant/components/zwave_js/config_flow.py @@ -180,11 +180,6 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): self, user_input: Optional[Dict[str, Any]] = None ) -> Dict[str, Any]: """Handle logic when on Supervisor host.""" - # Only one entry with Supervisor add-on support is allowed. - for entry in self.hass.config_entries.async_entries(DOMAIN): - if entry.data.get(CONF_USE_ADDON): - return await self.async_step_manual() - if user_input is None: return self.async_show_form( step_id="on_supervisor", data_schema=ON_SUPERVISOR_SCHEMA @@ -297,7 +292,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): assert self.hass addon_manager: AddonManager = get_addon_manager(self.hass) try: - await addon_manager.async_start_addon() + await addon_manager.async_schedule_start_addon() # Sleep some seconds to let the add-on start properly before connecting. for _ in range(ADDON_SETUP_TIMEOUT_ROUNDS): await asyncio.sleep(ADDON_SETUP_TIMEOUT) @@ -346,7 +341,13 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): version_info.home_id, raise_on_progress=False ) - self._abort_if_unique_id_configured() + self._abort_if_unique_id_configured( + updates={ + CONF_URL: self.ws_address, + CONF_USB_PATH: self.usb_path, + CONF_NETWORK_KEY: self.network_key, + } + ) return self._async_create_entry_from_vars() async def _async_get_addon_info(self) -> dict: @@ -389,7 +390,7 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Install the Z-Wave JS add-on.""" addon_manager: AddonManager = get_addon_manager(self.hass) try: - await addon_manager.async_install_addon() + await addon_manager.async_schedule_install_addon() finally: # Continue the flow after show progress when the task is done. self.hass.async_create_task( diff --git a/tests/components/zwave_js/test_config_flow.py b/tests/components/zwave_js/test_config_flow.py index c6b59e07412..7eea126e52e 100644 --- a/tests/components/zwave_js/test_config_flow.py +++ b/tests/components/zwave_js/test_config_flow.py @@ -519,49 +519,6 @@ async def test_not_addon(hass, supervisor): assert len(mock_setup_entry.mock_calls) == 1 -async def test_addon_already_configured(hass, supervisor): - """Test add-on already configured leads to manual step.""" - entry = MockConfigEntry( - domain=DOMAIN, data={"use_addon": True}, title=TITLE, unique_id=5678 - ) - entry.add_to_hass(hass) - - await setup.async_setup_component(hass, "persistent_notification", {}) - - result = await hass.config_entries.flow.async_init( - DOMAIN, context={"source": config_entries.SOURCE_USER} - ) - - assert result["type"] == "form" - assert result["step_id"] == "manual" - - with patch( - "homeassistant.components.zwave_js.async_setup", return_value=True - ) as mock_setup, patch( - "homeassistant.components.zwave_js.async_setup_entry", - return_value=True, - ) as mock_setup_entry: - result = await hass.config_entries.flow.async_configure( - result["flow_id"], - { - "url": "ws://localhost:3000", - }, - ) - await hass.async_block_till_done() - - assert result["type"] == "create_entry" - assert result["title"] == TITLE - assert result["data"] == { - "url": "ws://localhost:3000", - "usb_path": None, - "network_key": None, - "use_addon": False, - "integration_created_addon": False, - } - assert len(mock_setup.mock_calls) == 1 - assert len(mock_setup_entry.mock_calls) == 2 - - @pytest.mark.parametrize("discovery_info", [{"config": ADDON_DISCOVERY_INFO}]) async def test_addon_running( hass, @@ -673,9 +630,18 @@ async def test_addon_running_already_configured( hass, supervisor, addon_running, addon_options, get_addon_discovery_info ): """Test that only one unique instance is allowed when add-on is running.""" - addon_options["device"] = "/test" - addon_options["network_key"] = "abc123" - entry = MockConfigEntry(domain=DOMAIN, data={}, title=TITLE, unique_id=1234) + addon_options["device"] = "/test_new" + addon_options["network_key"] = "def456" + entry = MockConfigEntry( + domain=DOMAIN, + data={ + "url": "ws://localhost:3000", + "usb_path": "/test", + "network_key": "abc123", + }, + title=TITLE, + unique_id=1234, + ) entry.add_to_hass(hass) await setup.async_setup_component(hass, "persistent_notification", {}) @@ -692,6 +658,9 @@ async def test_addon_running_already_configured( assert result["type"] == "abort" assert result["reason"] == "already_configured" + assert entry.data["url"] == "ws://host1:3001" + assert entry.data["usb_path"] == "/test_new" + assert entry.data["network_key"] == "def456" @pytest.mark.parametrize("discovery_info", [{"config": ADDON_DISCOVERY_INFO}]) @@ -897,7 +866,16 @@ async def test_addon_installed_already_configured( get_addon_discovery_info, ): """Test that only one unique instance is allowed when add-on is installed.""" - entry = MockConfigEntry(domain=DOMAIN, data={}, title=TITLE, unique_id=1234) + entry = MockConfigEntry( + domain=DOMAIN, + data={ + "url": "ws://localhost:3000", + "usb_path": "/test", + "network_key": "abc123", + }, + title=TITLE, + unique_id=1234, + ) entry.add_to_hass(hass) await setup.async_setup_component(hass, "persistent_notification", {}) @@ -916,7 +894,7 @@ async def test_addon_installed_already_configured( assert result["step_id"] == "configure_addon" result = await hass.config_entries.flow.async_configure( - result["flow_id"], {"usb_path": "/test", "network_key": "abc123"} + result["flow_id"], {"usb_path": "/test_new", "network_key": "def456"} ) assert result["type"] == "progress" @@ -927,6 +905,9 @@ async def test_addon_installed_already_configured( assert result["type"] == "abort" assert result["reason"] == "already_configured" + assert entry.data["url"] == "ws://host1:3001" + assert entry.data["usb_path"] == "/test_new" + assert entry.data["network_key"] == "def456" @pytest.mark.parametrize("discovery_info", [{"config": ADDON_DISCOVERY_INFO}])