From 8337d4613e61f4698016036e38f7ebe3bad6ae91 Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Mon, 30 Jan 2023 16:21:34 -0500 Subject: [PATCH] ZHA config flow cleanup (#86742) fixes undefined --- homeassistant/components/zha/config_flow.py | 25 ++++- homeassistant/components/zha/radio_manager.py | 4 +- homeassistant/components/zha/strings.json | 8 +- .../components/zha/translations/en.json | 10 +- tests/components/zha/test_config_flow.py | 103 +++++++++++++++--- 5 files changed, 125 insertions(+), 25 deletions(-) diff --git a/homeassistant/components/zha/config_flow.py b/homeassistant/components/zha/config_flow.py index 993526ec863..554a94b8450 100644 --- a/homeassistant/components/zha/config_flow.py +++ b/homeassistant/components/zha/config_flow.py @@ -38,6 +38,7 @@ DECONZ_DOMAIN = "deconz" FORMATION_STRATEGY = "formation_strategy" FORMATION_FORM_NEW_NETWORK = "form_new_network" +FORMATION_FORM_INITIAL_NETWORK = "form_initial_network" FORMATION_REUSE_SETTINGS = "reuse_settings" FORMATION_CHOOSE_AUTOMATIC_BACKUP = "choose_automatic_backup" FORMATION_UPLOAD_MANUAL_BACKUP = "upload_manual_backup" @@ -271,8 +272,21 @@ class BaseZhaFlow(FlowHandler): strategies.append(FORMATION_REUSE_SETTINGS) strategies.append(FORMATION_UPLOAD_MANUAL_BACKUP) - strategies.append(FORMATION_FORM_NEW_NETWORK) + # Do not show "erase network settings" if there are none to erase + if self._radio_mgr.current_settings is None: + strategies.append(FORMATION_FORM_INITIAL_NETWORK) + else: + strategies.append(FORMATION_FORM_NEW_NETWORK) + + # Automatically form a new network if we're onboarding with a brand new radio + if not onboarding.async_is_onboarded(self.hass) and set(strategies) == { + FORMATION_UPLOAD_MANUAL_BACKUP, + FORMATION_FORM_INITIAL_NETWORK, + }: + return await self.async_step_form_initial_network() + + # Otherwise, let the user choose return self.async_show_menu( step_id="choose_formation_strategy", menu_options=strategies, @@ -284,6 +298,13 @@ class BaseZhaFlow(FlowHandler): """Reuse the existing network settings on the stick.""" return await self._async_create_radio_entry() + async def async_step_form_initial_network( + self, user_input: dict[str, Any] | None = None + ) -> FlowResult: + """Form an initial network.""" + # This step exists only for translations, it does nothing new + return await self.async_step_form_new_network(user_input) + async def async_step_form_new_network( self, user_input: dict[str, Any] | None = None ) -> FlowResult: @@ -440,7 +461,7 @@ class ZhaConfigFlowHandler(BaseZhaFlow, config_entries.ConfigFlow, domain=DOMAIN return self.async_abort(reason="single_instance_allowed") # Without confirmation, discovery can automatically progress into parts of the - # config flow logic that interacts with hardware! + # config flow logic that interacts with hardware. if user_input is not None or not onboarding.async_is_onboarded(self.hass): # Probe the radio type if we don't have one yet if ( diff --git a/homeassistant/components/zha/radio_manager.py b/homeassistant/components/zha/radio_manager.py index bb5b7328789..1f02c94e61f 100644 --- a/homeassistant/components/zha/radio_manager.py +++ b/homeassistant/components/zha/radio_manager.py @@ -12,7 +12,7 @@ from typing import Any import voluptuous as vol from zigpy.application import ControllerApplication import zigpy.backups -from zigpy.config import CONF_DEVICE, CONF_DEVICE_PATH +from zigpy.config import CONF_DEVICE, CONF_DEVICE_PATH, CONF_NWK_BACKUP_ENABLED from zigpy.exceptions import NetworkNotFormed from homeassistant import config_entries @@ -127,6 +127,7 @@ class ZhaRadioManager: app_config[CONF_DATABASE] = database_path app_config[CONF_DEVICE] = self.device_settings + app_config[CONF_NWK_BACKUP_ENABLED] = False app_config = self.radio_type.controller.SCHEMA(app_config) app = await self.radio_type.controller.new( @@ -207,6 +208,7 @@ class ZhaRadioManager: # The list of backups will always exist self.backups = app.backups.backups.copy() + self.backups.sort(reverse=True, key=lambda b: b.backup_time) return backup diff --git a/homeassistant/components/zha/strings.json b/homeassistant/components/zha/strings.json index 36bd5a38ecc..132f6ed9d95 100644 --- a/homeassistant/components/zha/strings.json +++ b/homeassistant/components/zha/strings.json @@ -31,7 +31,8 @@ "title": "Network Formation", "description": "Choose the network settings for your radio.", "menu_options": { - "form_new_network": "Erase network settings and form a new network", + "form_new_network": "Erase network settings and create a new network", + "form_initial_network": "Create a network", "reuse_settings": "Keep radio network settings", "choose_automatic_backup": "Restore an automatic backup", "upload_manual_backup": "Upload a manual backup" @@ -86,11 +87,11 @@ }, "intent_migrate": { "title": "Migrate to a new radio", - "description": "Your old radio will be factory reset. If you are using a combined Z-Wave and Zigbee adapter like the HUSBZB-1, this will only reset the Zigbee portion.\n\nDo you wish to continue?" + "description": "Before plugging in your new radio, your old radio needs to be reset. An automatic backup will be performed. If you are using a combined Z-Wave and Zigbee adapter like the HUSBZB-1, this will only reset the Zigbee portion.\n\n*Note: if you are migrating from a **ConBee/RaspBee**, make sure it is running firmware `0x26720700` or newer! Otherwise, some devices may not be controllable after migrating until they are power cycled.*\n\nDo you wish to continue?" }, "instruct_unplug": { "title": "Unplug your old radio", - "description": "Your old radio has been reset. If the hardware is no longer needed, you can now unplug it." + "description": "Your old radio has been reset. If the hardware is no longer needed, you can now unplug it.\n\nYou can now plug in your new radio." }, "choose_serial_port": { "title": "[%key:component::zha::config::step::choose_serial_port::title%]", @@ -120,6 +121,7 @@ "description": "[%key:component::zha::config::step::choose_formation_strategy::description%]", "menu_options": { "form_new_network": "[%key:component::zha::config::step::choose_formation_strategy::menu_options::form_new_network%]", + "form_initial_network": "[%key:component::zha::config::step::choose_formation_strategy::menu_options::form_initial_network%]", "reuse_settings": "[%key:component::zha::config::step::choose_formation_strategy::menu_options::reuse_settings%]", "choose_automatic_backup": "[%key:component::zha::config::step::choose_formation_strategy::menu_options::choose_automatic_backup%]", "upload_manual_backup": "[%key:component::zha::config::step::choose_formation_strategy::menu_options::upload_manual_backup%]" diff --git a/homeassistant/components/zha/translations/en.json b/homeassistant/components/zha/translations/en.json index a9d13a7a3a0..59e8004ad39 100644 --- a/homeassistant/components/zha/translations/en.json +++ b/homeassistant/components/zha/translations/en.json @@ -22,7 +22,8 @@ "description": "Choose the network settings for your radio.", "menu_options": { "choose_automatic_backup": "Restore an automatic backup", - "form_new_network": "Erase network settings and form a new network", + "form_initial_network": "Create a network", + "form_new_network": "Erase network settings and create a new network", "reuse_settings": "Keep radio network settings", "upload_manual_backup": "Upload a manual backup" }, @@ -174,7 +175,8 @@ "description": "Choose the network settings for your radio.", "menu_options": { "choose_automatic_backup": "Restore an automatic backup", - "form_new_network": "Erase network settings and form a new network", + "form_initial_network": "Create a network", + "form_new_network": "Erase network settings and create a new network", "reuse_settings": "Keep radio network settings", "upload_manual_backup": "Upload a manual backup" }, @@ -192,11 +194,11 @@ "title": "Reconfigure ZHA" }, "instruct_unplug": { - "description": "Your old radio has been reset. If the hardware is no longer needed, you can now unplug it.", + "description": "Your old radio has been reset. If the hardware is no longer needed, you can now unplug it.\n\nYou can now plug in your new radio.", "title": "Unplug your old radio" }, "intent_migrate": { - "description": "Your old radio will be factory reset. If you are using a combined Z-Wave and Zigbee adapter like the HUSBZB-1, this will only reset the Zigbee portion.\n\nDo you wish to continue?", + "description": "Before plugging in your new radio, your old radio needs to be reset. An automatic backup will be performed. If you are using a combined Z-Wave and Zigbee adapter like the HUSBZB-1, this will only reset the Zigbee portion.\n\n*Note: if you are migrating from a **ConBee/RaspBee**, make sure it is running firmware `0x26720700` or newer! Otherwise, some devices may not be controllable after migrating until they are power cycled.*\n\nDo you wish to continue?", "title": "Migrate to a new radio" }, "manual_pick_radio_type": { diff --git a/tests/components/zha/test_config_flow.py b/tests/components/zha/test_config_flow.py index d457e0b6b8c..acff888dfde 100644 --- a/tests/components/zha/test_config_flow.py +++ b/tests/components/zha/test_config_flow.py @@ -1,6 +1,7 @@ """Tests for ZHA config flow.""" import copy +from datetime import timedelta import json from unittest.mock import AsyncMock, MagicMock, PropertyMock, create_autospec, patch import uuid @@ -67,12 +68,27 @@ def mock_app(): @pytest.fixture -def backup(): - """Zigpy network backup with non-default settings.""" - backup = zigpy.backups.NetworkBackup() - backup.node_info.ieee = zigpy.types.EUI64.convert("AA:BB:CC:DD:11:22:33:44") +def make_backup(): + """Zigpy network backup factory that creates unique backups with each call.""" + num_calls = 0 - return backup + def inner(*, backup_time_offset=0): + nonlocal num_calls + + backup = zigpy.backups.NetworkBackup() + backup.backup_time += timedelta(seconds=backup_time_offset) + backup.node_info.ieee = zigpy.types.EUI64.convert(f"AABBCCDDEE{num_calls:06X}") + num_calls += 1 + + return backup + + return inner + + +@pytest.fixture +def backup(make_backup): + """Zigpy network backup with non-default settings.""" + return make_backup() def mock_detect_radio_type(radio_type=RadioType.ezsp, ret=True): @@ -1101,6 +1117,56 @@ async def test_formation_strategy_form_new_network(pick_radio, mock_app, hass): assert result2["type"] == FlowResultType.CREATE_ENTRY +async def test_formation_strategy_form_initial_network(pick_radio, mock_app, hass): + """Test forming a new network, with no previous settings on the radio.""" + mock_app.load_network_info = AsyncMock(side_effect=NetworkNotFormed()) + + result, port = await pick_radio(RadioType.ezsp) + result2 = await hass.config_entries.flow.async_configure( + result["flow_id"], + user_input={"next_step_id": config_flow.FORMATION_FORM_INITIAL_NETWORK}, + ) + await hass.async_block_till_done() + + # A new network will be formed + mock_app.form_network.assert_called_once() + + assert result2["type"] == FlowResultType.CREATE_ENTRY + + +@patch(f"zigpy_znp.{PROBE_FUNCTION_PATH}", AsyncMock(return_value=True)) +async def test_onboarding_auto_formation_new_hardware(mock_app, hass): + """Test auto network formation with new hardware during onboarding.""" + mock_app.load_network_info = AsyncMock(side_effect=NetworkNotFormed()) + discovery_info = usb.UsbServiceInfo( + device="/dev/ttyZIGBEE", + pid="AAAA", + vid="AAAA", + serial_number="1234", + description="zigbee radio", + manufacturer="test", + ) + + with patch( + "homeassistant.components.onboarding.async_is_onboarded", return_value=False + ): + result = await hass.config_entries.flow.async_init( + DOMAIN, context={"source": SOURCE_USB}, data=discovery_info + ) + await hass.async_block_till_done() + + assert result["type"] == FlowResultType.CREATE_ENTRY + assert result["title"] == "zigbee radio" + assert result["data"] == { + "device": { + "baudrate": 115200, + "flow_control": None, + "path": "/dev/ttyZIGBEE", + }, + CONF_RADIO_TYPE: "znp", + } + + async def test_formation_strategy_reuse_settings(pick_radio, mock_app, hass): """Test reusing existing network settings.""" result, port = await pick_radio(RadioType.ezsp) @@ -1298,13 +1364,13 @@ def test_format_backup_choice(): ) @patch("homeassistant.components.zha.async_setup_entry", AsyncMock(return_value=True)) async def test_formation_strategy_restore_automatic_backup_ezsp( - pick_radio, mock_app, hass + pick_radio, mock_app, make_backup, hass ): """Test restoring an automatic backup (EZSP radio).""" mock_app.backups.backups = [ - MagicMock(), - MagicMock(), - MagicMock(), + make_backup(), + make_backup(), + make_backup(), ] backup = mock_app.backups.backups[1] # pick the second one backup.is_compatible_with = MagicMock(return_value=False) @@ -1347,13 +1413,13 @@ async def test_formation_strategy_restore_automatic_backup_ezsp( @patch("homeassistant.components.zha.async_setup_entry", AsyncMock(return_value=True)) @pytest.mark.parametrize("is_advanced", [True, False]) async def test_formation_strategy_restore_automatic_backup_non_ezsp( - is_advanced, pick_radio, mock_app, hass + is_advanced, pick_radio, mock_app, make_backup, hass ): """Test restoring an automatic backup (non-EZSP radio).""" mock_app.backups.backups = [ - MagicMock(), - MagicMock(), - MagicMock(), + make_backup(backup_time_offset=5), + make_backup(backup_time_offset=-3), + make_backup(backup_time_offset=2), ] backup = mock_app.backups.backups[1] # pick the second one backup.is_compatible_with = MagicMock(return_value=False) @@ -1375,13 +1441,20 @@ async def test_formation_strategy_restore_automatic_backup_non_ezsp( assert result2["type"] == FlowResultType.FORM assert result2["step_id"] == "choose_automatic_backup" - # We must prompt for overwriting the IEEE address + # We don't prompt for overwriting the IEEE address, since only EZSP needs this assert config_flow.OVERWRITE_COORDINATOR_IEEE not in result2["data_schema"].schema + # The backup choices are ordered by date + assert result2["data_schema"].schema["choose_automatic_backup"].container == [ + f"choice:{mock_app.backups.backups[0]!r}", + f"choice:{mock_app.backups.backups[2]!r}", + f"choice:{mock_app.backups.backups[1]!r}", + ] + result3 = await hass.config_entries.flow.async_configure( result2["flow_id"], user_input={ - config_flow.CHOOSE_AUTOMATIC_BACKUP: "choice:" + repr(backup), + config_flow.CHOOSE_AUTOMATIC_BACKUP: f"choice:{backup!r}", }, )