diff --git a/homeassistant/components/zwave_js/__init__.py b/homeassistant/components/zwave_js/__init__.py index ccadc452bc7..1321ef36f85 100644 --- a/homeassistant/components/zwave_js/__init__.py +++ b/homeassistant/components/zwave_js/__init__.py @@ -282,7 +282,8 @@ class DriverEvents: for node in controller.nodes.values() ] - # Devices that are in the device registry that are not known by the controller can be removed + # Devices that are in the device registry that are not known by the controller + # can be removed for device in stored_devices: if device not in known_devices: self.dev_reg.async_remove_device(device.id) @@ -509,25 +510,46 @@ class ControllerEvents: driver = self.driver_events.driver device_id = get_device_id(driver, node) device_id_ext = get_device_id_ext(driver, node) - device = self.dev_reg.async_get_device(identifiers={device_id}) + node_id_device = self.dev_reg.async_get_device(identifiers={device_id}) via_device_id = None controller = driver.controller # Get the controller node device ID if this node is not the controller if controller.own_node and controller.own_node != node: via_device_id = get_device_id(driver, controller.own_node) - # Replace the device if it can be determined that this node is not the - # same product as it was previously. - if ( - device_id_ext - and device - and len(device.identifiers) == 2 - and device_id_ext not in device.identifiers - ): - self.remove_device(device) - device = None - if device_id_ext: + # If there is a device with this node ID but with a different hardware + # signature, remove the node ID based identifier from it. The hardware + # signature can be different for one of two reasons: 1) in the ideal + # scenario, the node was replaced with a different node that's a different + # device entirely, or 2) the device erroneously advertised the wrong + # hardware identifiers (this is known to happen due to poor RF conditions). + # While we would like to remove the old device automatically for case 1, we + # have no way to distinguish between these reasons so we leave it up to the + # user to remove the old device manually. + if ( + node_id_device + and len(node_id_device.identifiers) == 2 + and device_id_ext not in node_id_device.identifiers + ): + new_identifiers = node_id_device.identifiers.copy() + new_identifiers.remove(device_id) + self.dev_reg.async_update_device( + node_id_device.id, new_identifiers=new_identifiers + ) + # If there is an orphaned device that already exists with this hardware + # based identifier, add the node ID based identifier to the orphaned + # device. + if ( + hardware_device := self.dev_reg.async_get_device( + identifiers={device_id_ext} + ) + ) and len(hardware_device.identifiers) == 1: + new_identifiers = hardware_device.identifiers.copy() + new_identifiers.add(device_id) + self.dev_reg.async_update_device( + hardware_device.id, new_identifiers=new_identifiers + ) ids = {device_id, device_id_ext} else: ids = {device_id} @@ -769,9 +791,12 @@ class NodeEvents: return driver = self.controller_events.driver_events.driver - notification: EntryControlNotification | NotificationNotification | PowerLevelNotification | MultilevelSwitchNotification = event[ - "notification" - ] + notification: ( + EntryControlNotification + | NotificationNotification + | PowerLevelNotification + | MultilevelSwitchNotification + ) = event["notification"] device = self.dev_reg.async_get_device( identifiers={get_device_id(driver, notification.node)} ) @@ -984,6 +1009,39 @@ async def async_remove_entry(hass: HomeAssistant, entry: ConfigEntry) -> None: LOGGER.error(err) +async def async_remove_config_entry_device( + hass: HomeAssistant, config_entry: ConfigEntry, device_entry: dr.DeviceEntry +) -> bool: + """Remove a config entry from a device.""" + entry_hass_data = hass.data[DOMAIN][config_entry.entry_id] + client: ZwaveClient = entry_hass_data[DATA_CLIENT] + + # Driver may not be ready yet so we can't allow users to remove a device since + # we need to check if the device is still known to the controller + if (driver := client.driver) is None: + LOGGER.error("Driver for %s is not ready", config_entry.title) + return False + + # If a node is found on the controller that matches the hardware based identifier + # on the device, prevent the device from being removed. + if next( + ( + node + for node in driver.controller.nodes.values() + if get_device_id_ext(driver, node) in device_entry.identifiers + ), + None, + ): + return False + + controller_events: ControllerEvents = entry_hass_data[ + DATA_DRIVER_EVENTS + ].controller_events + controller_events.registered_unique_ids.pop(device_entry.id, None) + controller_events.discovered_value_ids.pop(device_entry.id, None) + return True + + async def async_ensure_addon_running(hass: HomeAssistant, entry: ConfigEntry) -> None: """Ensure that Z-Wave JS add-on is installed and running.""" addon_manager = _get_addon_manager(hass) diff --git a/tests/components/zwave_js/test_init.py b/tests/components/zwave_js/test_init.py index 77b1fcb8b3a..7f3a9428dad 100644 --- a/tests/components/zwave_js/test_init.py +++ b/tests/components/zwave_js/test_init.py @@ -30,6 +30,7 @@ from homeassistant.setup import async_setup_component from .common import AIR_TEMPERATURE_SENSOR, EATON_RF9640_ENTITY from tests.common import MockConfigEntry, async_get_persistent_notifications +from tests.typing import WebSocketGenerator @pytest.fixture(name="connect_timeout") @@ -1008,6 +1009,7 @@ async def test_node_removed( client.driver.controller.receive_event(Event("node added", event)) await hass.async_block_till_done() old_device = dev_reg.async_get_device(identifiers={(DOMAIN, device_id)}) + assert old_device assert old_device.id event = {"node": node, "reason": 0} @@ -1131,6 +1133,7 @@ async def test_replace_different_node( hank_binary_switch_state, client, integration, + hass_ws_client: WebSocketGenerator, ) -> None: """Test when a node is replaced with a different node.""" dev_reg = dr.async_get(hass) @@ -1139,11 +1142,11 @@ async def test_replace_different_node( state["nodeId"] = node_id device_id = f"{client.driver.controller.home_id}-{node_id}" - multisensor_6_device_id = ( + multisensor_6_device_id_ext = ( f"{device_id}-{multisensor_6.manufacturer_id}:" f"{multisensor_6.product_type}:{multisensor_6.product_id}" ) - hank_device_id = ( + hank_device_id_ext = ( f"{device_id}-{state['manufacturerId']}:" f"{state['productType']}:" f"{state['productId']}" @@ -1152,7 +1155,7 @@ async def test_replace_different_node( device = dev_reg.async_get_device(identifiers={(DOMAIN, device_id)}) assert device assert device == dev_reg.async_get_device( - identifiers={(DOMAIN, multisensor_6_device_id)} + identifiers={(DOMAIN, multisensor_6_device_id_ext)} ) assert device.manufacturer == "AEON Labs" assert device.model == "ZW100" @@ -1160,8 +1163,7 @@ async def test_replace_different_node( assert hass.states.get(AIR_TEMPERATURE_SENSOR) - # A replace node event has the extra field "replaced" set to True - # to distinguish it from an exclusion + # Remove existing node event = Event( type="node removed", data={ @@ -1175,8 +1177,11 @@ async def test_replace_different_node( await hass.async_block_till_done() # Device should still be there after the node was removed - device = dev_reg.async_get(dev_id) + device = dev_reg.async_get_device( + identifiers={(DOMAIN, multisensor_6_device_id_ext)} + ) assert device + assert len(device.identifiers) == 2 # When the node is replaced, a non-ready node added event is emitted event = Event( @@ -1230,18 +1235,164 @@ async def test_replace_different_node( client.driver.receive_event(event) await hass.async_block_till_done() - # Old device and entities were removed, but the ID is re-used - device = dev_reg.async_get(dev_id) - assert device - assert device == dev_reg.async_get_device(identifiers={(DOMAIN, device_id)}) - assert device == dev_reg.async_get_device(identifiers={(DOMAIN, hank_device_id)}) - assert not dev_reg.async_get_device(identifiers={(DOMAIN, multisensor_6_device_id)}) - assert device.manufacturer == "HANK Electronics Ltd." - assert device.model == "HKZW-SO01" + # node ID based device identifier should be moved from the old multisensor device + # to the new hank device and both the old and new devices should exist. + new_device = dev_reg.async_get_device(identifiers={(DOMAIN, device_id)}) + assert new_device + hank_device = dev_reg.async_get_device(identifiers={(DOMAIN, hank_device_id_ext)}) + assert hank_device + assert hank_device == new_device + assert hank_device.identifiers == { + (DOMAIN, device_id), + (DOMAIN, hank_device_id_ext), + } + multisensor_6_device = dev_reg.async_get_device( + identifiers={(DOMAIN, multisensor_6_device_id_ext)} + ) + assert multisensor_6_device + assert multisensor_6_device != new_device + assert multisensor_6_device.identifiers == {(DOMAIN, multisensor_6_device_id_ext)} - assert not hass.states.get(AIR_TEMPERATURE_SENSOR) + assert new_device.manufacturer == "HANK Electronics Ltd." + assert new_device.model == "HKZW-SO01" + + # We keep the old entities in case there are customizations that a user wants to + # keep. They can always delete the device and that will remove the entities as well. + assert hass.states.get(AIR_TEMPERATURE_SENSOR) assert hass.states.get("switch.smart_plug_with_two_usb_ports") + # Try to add back the first node to see if the device IDs are correct + + # Remove existing node + event = Event( + type="node removed", + data={ + "source": "controller", + "event": "node removed", + "reason": 3, + "node": state, + }, + ) + client.driver.receive_event(event) + await hass.async_block_till_done() + + # Device should still be there after the node was removed + device = dev_reg.async_get_device(identifiers={(DOMAIN, hank_device_id_ext)}) + assert device + assert len(device.identifiers) == 2 + + # When the node is replaced, a non-ready node added event is emitted + event = Event( + type="node added", + data={ + "source": "controller", + "event": "node added", + "node": { + "nodeId": multisensor_6.node_id, + "index": 0, + "status": 4, + "ready": False, + "isSecure": False, + "interviewAttempts": 1, + "endpoints": [ + {"nodeId": multisensor_6.node_id, "index": 0, "deviceClass": None} + ], + "values": [], + "deviceClass": None, + "commandClasses": [], + "interviewStage": "None", + "statistics": { + "commandsTX": 0, + "commandsRX": 0, + "commandsDroppedRX": 0, + "commandsDroppedTX": 0, + "timeoutResponse": 0, + }, + "isControllerNode": False, + }, + "result": {}, + }, + ) + + client.driver.receive_event(event) + await hass.async_block_till_done() + + # Mark node as ready + event = Event( + type="ready", + data={ + "source": "node", + "event": "ready", + "nodeId": node_id, + "nodeState": multisensor_6_state, + }, + ) + client.driver.receive_event(event) + await hass.async_block_till_done() + + assert await async_setup_component(hass, "config", {}) + + # node ID based device identifier should be moved from the new hank device + # to the old multisensor device and both the old and new devices should exist. + old_device = dev_reg.async_get_device(identifiers={(DOMAIN, device_id)}) + assert old_device + hank_device = dev_reg.async_get_device(identifiers={(DOMAIN, hank_device_id_ext)}) + assert hank_device + assert hank_device != old_device + assert hank_device.identifiers == {(DOMAIN, hank_device_id_ext)} + multisensor_6_device = dev_reg.async_get_device( + identifiers={(DOMAIN, multisensor_6_device_id_ext)} + ) + assert multisensor_6_device + assert multisensor_6_device == old_device + assert multisensor_6_device.identifiers == { + (DOMAIN, device_id), + (DOMAIN, multisensor_6_device_id_ext), + } + + ws_client = await hass_ws_client(hass) + + # Simulate the driver not being ready to ensure that the device removal handler + # does not crash + driver = client.driver + client.driver = None + + await ws_client.send_json( + { + "id": 1, + "type": "config/device_registry/remove_config_entry", + "config_entry_id": integration.entry_id, + "device_id": hank_device.id, + } + ) + response = await ws_client.receive_json() + assert not response["success"] + + client.driver = driver + + # Attempting to remove the hank device should pass, but removing the multisensor should not + await ws_client.send_json( + { + "id": 2, + "type": "config/device_registry/remove_config_entry", + "config_entry_id": integration.entry_id, + "device_id": hank_device.id, + } + ) + response = await ws_client.receive_json() + assert response["success"] + + await ws_client.send_json( + { + "id": 3, + "type": "config/device_registry/remove_config_entry", + "config_entry_id": integration.entry_id, + "device_id": multisensor_6_device.id, + } + ) + response = await ws_client.receive_json() + assert not response["success"] + async def test_node_model_change( hass: HomeAssistant, zp3111, client, integration