Fix removal of devices during Z-Wave migration (#145867)

homeassistant/components/zwave_js/__init__.py

@@ -94,6 +94,7 @@ from .const import (
     CONF_DATA_COLLECTION_OPTED_IN,
     CONF_INSTALLER_MODE,
     CONF_INTEGRATION_CREATED_ADDON,
+    CONF_KEEP_OLD_DEVICES,
     CONF_LR_S2_ACCESS_CONTROL_KEY,
     CONF_LR_S2_AUTHENTICATED_KEY,
     CONF_NETWORK_KEY,
@@ -405,6 +406,7 @@ class DriverEvents:
 
         # Devices that are in the device registry that are not known by the controller
         # can be removed
+        if not self.config_entry.data.get(CONF_KEEP_OLD_DEVICES):
             for device in stored_devices:
                 if device not in known_devices and device not in provisioned_devices:
                     self.dev_reg.async_remove_device(device.id)

homeassistant/components/zwave_js/config_flow.py

@@ -56,6 +56,7 @@ from .const import (
     CONF_ADDON_S2_AUTHENTICATED_KEY,
     CONF_ADDON_S2_UNAUTHENTICATED_KEY,
     CONF_INTEGRATION_CREATED_ADDON,
+    CONF_KEEP_OLD_DEVICES,
     CONF_LR_S2_ACCESS_CONTROL_KEY,
     CONF_LR_S2_AUTHENTICATED_KEY,
     CONF_S0_LEGACY_KEY,
@@ -1383,9 +1384,20 @@ class ZWaveJSConfigFlow(ConfigFlow, domain=DOMAIN):
         config_entry = self._reconfigure_config_entry
         assert config_entry is not None
 
+        # Make sure we keep the old devices
+        # so that user customizations are not lost,
+        # when loading the config entry.
+        self.hass.config_entries.async_update_entry(
+            config_entry, data=config_entry.data | {CONF_KEEP_OLD_DEVICES: True}
+        )
+
         # Reload the config entry to reconnect the client after the addon restart
         await self.hass.config_entries.async_reload(config_entry.entry_id)
 
+        data = config_entry.data.copy()
+        data.pop(CONF_KEEP_OLD_DEVICES, None)
+        self.hass.config_entries.async_update_entry(config_entry, data=data)
+
         @callback
         def forward_progress(event: dict) -> None:
             """Forward progress events to frontend."""
@@ -1436,6 +1448,15 @@ class ZWaveJSConfigFlow(ConfigFlow, domain=DOMAIN):
                     config_entry, unique_id=str(version_info.home_id)
                 )
             await self.hass.config_entries.async_reload(config_entry.entry_id)
+
+            # Reload the config entry two times to clean up
+            # the stale device entry.
+            # Since both the old and the new controller have the same node id,
+            # but different hardware identifiers, the integration
+            # will create a new device for the new controller, on the first reload,
+            # but not immediately remove the old device.
+            await self.hass.config_entries.async_reload(config_entry.entry_id)
+
         finally:
             for unsub in unsubs:
                 unsub()

homeassistant/components/zwave_js/const.py

@@ -27,6 +27,7 @@ CONF_ADDON_LR_S2_ACCESS_CONTROL_KEY = "lr_s2_access_control_key"
 CONF_ADDON_LR_S2_AUTHENTICATED_KEY = "lr_s2_authenticated_key"
 CONF_INSTALLER_MODE = "installer_mode"
 CONF_INTEGRATION_CREATED_ADDON = "integration_created_addon"
+CONF_KEEP_OLD_DEVICES = "keep_old_devices"
 CONF_NETWORK_KEY = "network_key"
 CONF_S0_LEGACY_KEY = "s0_legacy_key"
 CONF_S2_ACCESS_CONTROL_KEY = "s2_access_control_key"

tests/components/zwave_js/test_config_flow.py

@@ -15,6 +15,7 @@ import pytest
 from serial.tools.list_ports_common import ListPortInfo
 from voluptuous import InInvalid
 from zwave_js_server.exceptions import FailedCommand
+from zwave_js_server.model.node import Node
 from zwave_js_server.version import VersionInfo
 
 from homeassistant import config_entries, data_entry_flow
@@ -40,6 +41,7 @@ from homeassistant.components.zwave_js.const import (
 from homeassistant.components.zwave_js.helpers import SERVER_VERSION_TIMEOUT
 from homeassistant.core import HomeAssistant
 from homeassistant.data_entry_flow import FlowResultType
+from homeassistant.helpers import device_registry as dr
 from homeassistant.helpers.service_info.hassio import HassioServiceInfo
 from homeassistant.helpers.service_info.usb import UsbServiceInfo
 from homeassistant.helpers.service_info.zeroconf import ZeroconfServiceInfo
@@ -969,7 +971,7 @@ async def test_usb_discovery_migration(
     assert client.connect.call_count == 2
 
     await hass.async_block_till_done()
-    assert client.connect.call_count == 3
+    assert client.connect.call_count == 4
     assert entry.state is config_entries.ConfigEntryState.LOADED
     assert client.driver.controller.async_restore_nvm.call_count == 1
     assert len(events) == 2
@@ -983,6 +985,7 @@ async def test_usb_discovery_migration(
     assert entry.data["url"] == "ws://host1:3001"
     assert entry.data["usb_path"] == USB_DISCOVERY_INFO.device
     assert entry.data["use_addon"] is True
+    assert "keep_old_devices" not in entry.data
     assert entry.unique_id == "5678"
 
 
@@ -1097,7 +1100,7 @@ async def test_usb_discovery_migration_restore_driver_ready_timeout(
         assert client.connect.call_count == 2
 
         await hass.async_block_till_done()
-        assert client.connect.call_count == 3
+        assert client.connect.call_count == 4
         assert entry.state is config_entries.ConfigEntryState.LOADED
         assert client.driver.controller.async_restore_nvm.call_count == 1
         assert len(events) == 2
@@ -1108,9 +1111,10 @@ async def test_usb_discovery_migration_restore_driver_ready_timeout(
 
     assert result["type"] is FlowResultType.ABORT
     assert result["reason"] == "migration_successful"
-    assert integration.data["url"] == "ws://host1:3001"
+    assert entry.data["url"] == "ws://host1:3001"
-    assert integration.data["usb_path"] == USB_DISCOVERY_INFO.device
+    assert entry.data["usb_path"] == USB_DISCOVERY_INFO.device
-    assert integration.data["use_addon"] is True
+    assert entry.data["use_addon"] is True
+    assert "keep_old_devices" not in entry.data
 
 
 @pytest.mark.usefixtures("supervisor", "addon_installed")
@@ -3422,6 +3426,7 @@ async def test_reconfigure_migrate_no_addon(
 
     assert result["type"] is FlowResultType.ABORT
     assert result["reason"] == "addon_required"
+    assert "keep_old_devices" not in entry.data
 
 
 @pytest.mark.usefixtures("mock_sdk_version")
@@ -3446,6 +3451,7 @@ async def test_reconfigure_migrate_low_sdk_version(
 
     assert result["type"] is FlowResultType.ABORT
     assert result["reason"] == "migration_low_sdk_version"
+    assert "keep_old_devices" not in entry.data
 
 
 @pytest.mark.usefixtures("supervisor", "addon_running")
@@ -3457,15 +3463,22 @@ async def test_reconfigure_migrate_low_sdk_version(
         "final_unique_id",
     ),
     [
-        (None, "4321", None, "8765"),
+        (None, "4321", None, "3245146787"),
-        (aiohttp.ClientError("Boom"), "1234", None, "8765"),
+        (aiohttp.ClientError("Boom"), "3245146787", None, "3245146787"),
         (None, "4321", aiohttp.ClientError("Boom"), "5678"),
-        (aiohttp.ClientError("Boom"), "1234", aiohttp.ClientError("Boom"), "5678"),
+        (
+            aiohttp.ClientError("Boom"),
+            "3245146787",
+            aiohttp.ClientError("Boom"),
+            "5678",
+        ),
     ],
 )
 async def test_reconfigure_migrate_with_addon(
     hass: HomeAssistant,
     client: MagicMock,
+    device_registry: dr.DeviceRegistry,
+    multisensor_6: Node,
     integration: MockConfigEntry,
     restart_addon: AsyncMock,
     addon_options: dict[str, Any],
@@ -3482,9 +3495,9 @@ async def test_reconfigure_migrate_with_addon(
     version_info.home_id = 4321
     entry = integration
     assert client.connect.call_count == 1
+    assert client.driver.controller.home_id == 3245146787
     hass.config_entries.async_update_entry(
         entry,
-        unique_id="1234",
         data={
             "url": "ws://localhost:3000",
             "use_addon": True,
@@ -3493,6 +3506,39 @@ async def test_reconfigure_migrate_with_addon(
     )
     addon_options["device"] = "/dev/ttyUSB0"
 
+    controller_node = client.driver.controller.own_node
+    controller_device_id = (
+        f"{client.driver.controller.home_id}-{controller_node.node_id}"
+    )
+    controller_device_id_ext = (
+        f"{controller_device_id}-{controller_node.manufacturer_id}:"
+        f"{controller_node.product_type}:{controller_node.product_id}"
+    )
+
+    assert len(device_registry.devices) == 2
+    # Verify there's a device entry for the controller.
+    device = device_registry.async_get_device(
+        identifiers={(DOMAIN, controller_device_id)}
+    )
+    assert device
+    assert device == device_registry.async_get_device(
+        identifiers={(DOMAIN, controller_device_id_ext)}
+    )
+    assert device.manufacturer == "AEON Labs"
+    assert device.model == "ZW090"
+    assert device.name == "Z‐Stick Gen5 USB Controller"
+    # Verify there's a device entry for the multisensor.
+    sensor_device_id = f"{client.driver.controller.home_id}-{multisensor_6.node_id}"
+    device = device_registry.async_get_device(identifiers={(DOMAIN, sensor_device_id)})
+    assert device
+    assert device.manufacturer == "AEON Labs"
+    assert device.model == "ZW100"
+    assert device.name == "Multisensor 6"
+    # Customize the sensor device name.
+    device_registry.async_update_device(
+        device.id, name_by_user="Custom Sensor Device Name"
+    )
+
     async def mock_backup_nvm_raw():
         await asyncio.sleep(0)
         client.driver.controller.emit(
@@ -3521,6 +3567,7 @@ async def test_reconfigure_migrate_with_addon(
             "nvm restore progress",
             {"event": "nvm restore progress", "bytesWritten": 100, "total": 200},
         )
+        client.driver.controller.data["homeId"] = 3245146787
         client.driver.emit(
             "driver ready", {"event": "driver ready", "source": "driver"}
         )
@@ -3591,6 +3638,17 @@ async def test_reconfigure_migrate_with_addon(
         "core_zwave_js", AddonsOptions(config={"device": "/test"})
     )
 
+    # Simulate the new connected controller hardware labels.
+    # This will cause a new device entry to be created
+    # when the config entry is loaded before restoring NVM.
+    controller_node = client.driver.controller.own_node
+    controller_node.data["manufacturerId"] = 999
+    controller_node.data["productId"] = 999
+    controller_node.device_config.data["description"] = "New Device Name"
+    controller_node.device_config.data["label"] = "New Device Model"
+    controller_node.device_config.data["manufacturer"] = "New Device Manufacturer"
+    client.driver.controller.data["homeId"] = 5678
+
     await hass.async_block_till_done()
 
     assert restart_addon.call_args == call("core_zwave_js")
@@ -3599,14 +3657,14 @@ async def test_reconfigure_migrate_with_addon(
 
     assert entry.unique_id == "5678"
     get_server_version.side_effect = restore_server_version_side_effect
-    version_info.home_id = 8765
+    version_info.home_id = 3245146787
 
     assert result["type"] is FlowResultType.SHOW_PROGRESS
     assert result["step_id"] == "restore_nvm"
     assert client.connect.call_count == 2
 
     await hass.async_block_till_done()
-    assert client.connect.call_count == 3
+    assert client.connect.call_count == 4
     assert entry.state is config_entries.ConfigEntryState.LOADED
     assert client.driver.controller.async_restore_nvm.call_count == 1
     assert len(events) == 2
@@ -3620,8 +3678,29 @@ async def test_reconfigure_migrate_with_addon(
     assert entry.data["url"] == "ws://host1:3001"
     assert entry.data["usb_path"] == "/test"
     assert entry.data["use_addon"] is True
+    assert "keep_old_devices" not in entry.data
     assert entry.unique_id == final_unique_id
 
+    assert len(device_registry.devices) == 2
+    controller_device_id_ext = (
+        f"{controller_device_id}-{controller_node.manufacturer_id}:"
+        f"{controller_node.product_type}:{controller_node.product_id}"
+    )
+    device = device_registry.async_get_device(
+        identifiers={(DOMAIN, controller_device_id_ext)}
+    )
+    assert device
+    assert device.manufacturer == "New Device Manufacturer"
+    assert device.model == "New Device Model"
+    assert device.name == "New Device Name"
+    device = device_registry.async_get_device(identifiers={(DOMAIN, sensor_device_id)})
+    assert device
+    assert device.manufacturer == "AEON Labs"
+    assert device.model == "ZW100"
+    assert device.name == "Multisensor 6"
+    assert device.name_by_user == "Custom Sensor Device Name"
+    assert client.driver.controller.home_id == 3245146787
+
 
 @pytest.mark.usefixtures("supervisor", "addon_running")
 async def test_reconfigure_migrate_reset_driver_ready_timeout(
@@ -3755,7 +3834,7 @@ async def test_reconfigure_migrate_reset_driver_ready_timeout(
     assert client.connect.call_count == 2
 
     await hass.async_block_till_done()
-    assert client.connect.call_count == 3
+    assert client.connect.call_count == 4
     assert entry.state is config_entries.ConfigEntryState.LOADED
     assert client.driver.controller.async_restore_nvm.call_count == 1
     assert len(events) == 2
@@ -3770,6 +3849,7 @@ async def test_reconfigure_migrate_reset_driver_ready_timeout(
     assert entry.data["usb_path"] == "/test"
     assert entry.data["use_addon"] is True
     assert entry.unique_id == "5678"
+    assert "keep_old_devices" not in entry.data
 
 
 @pytest.mark.usefixtures("supervisor", "addon_running")
@@ -3895,7 +3975,7 @@ async def test_reconfigure_migrate_restore_driver_ready_timeout(
         assert client.connect.call_count == 2
 
         await hass.async_block_till_done()
-        assert client.connect.call_count == 3
+        assert client.connect.call_count == 4
         assert entry.state is config_entries.ConfigEntryState.LOADED
         assert client.driver.controller.async_restore_nvm.call_count == 1
         assert len(events) == 2
@@ -3906,9 +3986,10 @@ async def test_reconfigure_migrate_restore_driver_ready_timeout(
 
     assert result["type"] is FlowResultType.ABORT
     assert result["reason"] == "migration_successful"
-    assert integration.data["url"] == "ws://host1:3001"
+    assert entry.data["url"] == "ws://host1:3001"
-    assert integration.data["usb_path"] == "/test"
+    assert entry.data["usb_path"] == "/test"
-    assert integration.data["use_addon"] is True
+    assert entry.data["use_addon"] is True
+    assert "keep_old_devices" not in entry.data
 
 
 async def test_reconfigure_migrate_backup_failure(
@@ -3942,6 +4023,7 @@ async def test_reconfigure_migrate_backup_failure(
 
     assert result["type"] is FlowResultType.ABORT
     assert result["reason"] == "backup_failed"
+    assert "keep_old_devices" not in entry.data
 
 
 async def test_reconfigure_migrate_backup_file_failure(
@@ -3988,6 +4070,7 @@ async def test_reconfigure_migrate_backup_file_failure(
 
     assert result["type"] is FlowResultType.ABORT
     assert result["reason"] == "backup_failed"
+    assert "keep_old_devices" not in entry.data
 
 
 @pytest.mark.usefixtures("supervisor", "addon_running")
@@ -4073,6 +4156,7 @@ async def test_reconfigure_migrate_start_addon_failure(
 
     assert result["type"] is FlowResultType.ABORT
     assert result["reason"] == "addon_start_failed"
+    assert "keep_old_devices" not in entry.data
 
 
 @pytest.mark.usefixtures("supervisor", "addon_running", "restart_addon")
@@ -4187,6 +4271,7 @@ async def test_reconfigure_migrate_restore_failure(
     hass.config_entries.flow.async_abort(result["flow_id"])
 
     assert len(hass.config_entries.flow.async_progress()) == 0
+    assert "keep_old_devices" not in entry.data
 
 
 async def test_get_driver_failure_intent_migrate(
@@ -4196,13 +4281,13 @@ async def test_get_driver_failure_intent_migrate(
     """Test get driver failure in intent migrate step."""
     entry = integration
     hass.config_entries.async_update_entry(
-        integration, unique_id="1234", data={**integration.data, "use_addon": True}
+        entry, unique_id="1234", data={**entry.data, "use_addon": True}
     )
     result = await entry.start_reconfigure_flow(hass)
     assert result["type"] is FlowResultType.MENU
     assert result["step_id"] == "reconfigure"
 
-    await hass.config_entries.async_unload(integration.entry_id)
+    await hass.config_entries.async_unload(entry.entry_id)
 
     result = await hass.config_entries.flow.async_configure(
         result["flow_id"], {"next_step_id": "intent_migrate"}
@@ -4210,6 +4295,7 @@ async def test_get_driver_failure_intent_migrate(
 
     assert result["type"] is FlowResultType.ABORT
     assert result["reason"] == "config_entry_not_loaded"
+    assert "keep_old_devices" not in entry.data
 
 
 async def test_get_driver_failure_instruct_unplug(
@@ -4231,7 +4317,7 @@ async def test_get_driver_failure_instruct_unplug(
     )
     entry = integration
     hass.config_entries.async_update_entry(
-        integration, unique_id="1234", data={**integration.data, "use_addon": True}
+        entry, unique_id="1234", data={**entry.data, "use_addon": True}
     )
     result = await entry.start_reconfigure_flow(hass)
     assert result["type"] is FlowResultType.MENU
@@ -4254,7 +4340,7 @@ async def test_get_driver_failure_instruct_unplug(
         assert client.driver.controller.async_backup_nvm_raw.call_count == 1
         assert mock_file.call_count == 1
 
-    await hass.config_entries.async_unload(integration.entry_id)
+    await hass.config_entries.async_unload(entry.entry_id)
 
     result = await hass.config_entries.flow.async_configure(result["flow_id"])
 
@@ -4270,7 +4356,7 @@ async def test_hard_reset_failure(
     """Test hard reset failure."""
     entry = integration
     hass.config_entries.async_update_entry(
-        integration, unique_id="1234", data={**integration.data, "use_addon": True}
+        entry, unique_id="1234", data={**entry.data, "use_addon": True}
     )
 
     async def mock_backup_nvm_raw():
@@ -4320,7 +4406,7 @@ async def test_choose_serial_port_usb_ports_failure(
     """Test choose serial port usb ports failure."""
     entry = integration
     hass.config_entries.async_update_entry(
-        integration, unique_id="1234", data={**integration.data, "use_addon": True}
+        entry, unique_id="1234", data={**entry.data, "use_addon": True}
     )
 
     async def mock_backup_nvm_raw():