Fix zwave_js device re-interview (#78046)

* Handle stale node and entity info on re-interview

* Add test

* Unsubscribe on config entry unload

homeassistant/components/zwave_js/__init__.py

@@ -313,19 +313,24 @@ class ControllerEvents:
                 node,
             )
 
-        # we only want to run discovery when the node has reached ready state,
-        # otherwise we'll have all kinds of missing info issues.
-        if node.ready:
-            await self.node_events.async_on_node_ready(node)
-            return
-        # if node is not yet ready, register one-time callback for ready state
-        LOGGER.debug("Node added: %s - waiting for it to become ready", node.node_id)
-        node.once(
+        LOGGER.debug("Node added: %s", node.node_id)
+
+        # Listen for ready node events, both new and re-interview.
+        self.config_entry.async_on_unload(
+            node.on(
                 "ready",
                 lambda event: self.hass.async_create_task(
                     self.node_events.async_on_node_ready(event["node"])
                 ),
             )
+        )
+
+        # we only want to run discovery when the node has reached ready state,
+        # otherwise we'll have all kinds of missing info issues.
+        if node.ready:
+            await self.node_events.async_on_node_ready(node)
+            return
+
         # we do submit the node to device registry so user has
         # some visual feedback that something is (in the process of) being added
         self.register_node_in_dev_reg(node)
@@ -414,12 +419,25 @@ class NodeEvents:
     async def async_on_node_ready(self, node: ZwaveNode) -> None:
         """Handle node ready event."""
         LOGGER.debug("Processing node %s", node)
+        driver = self.controller_events.driver_events.driver
         # register (or update) node in device registry
         device = self.controller_events.register_node_in_dev_reg(node)
         # We only want to create the defaultdict once, even on reinterviews
         if device.id not in self.controller_events.registered_unique_ids:
             self.controller_events.registered_unique_ids[device.id] = defaultdict(set)
 
+        # Remove any old value ids if this is a reinterview.
+        self.controller_events.discovered_value_ids.pop(device.id, None)
+        # Remove stale entities that may exist from a previous interview.
+        async_dispatcher_send(
+            self.hass,
+            (
+                f"{DOMAIN}_"
+                f"{get_valueless_base_unique_id(driver, node)}_"
+                "remove_entity_on_ready_node"
+            ),
+        )
+
         value_updates_disc_info: dict[str, ZwaveDiscoveryInfo] = {}
 
         # run discovery on all node values and create/update entities

homeassistant/components/zwave_js/entity.py

@@ -12,7 +12,7 @@ from homeassistant.helpers.entity import DeviceInfo, Entity
 
 from .const import DOMAIN, LOGGER
 from .discovery import ZwaveDiscoveryInfo
-from .helpers import get_device_id, get_unique_id
+from .helpers import get_device_id, get_unique_id, get_valueless_base_unique_id
 
 EVENT_VALUE_UPDATED = "value updated"
 EVENT_VALUE_REMOVED = "value removed"
@@ -96,6 +96,17 @@ class ZWaveBaseEntity(Entity):
         self.async_on_remove(
             self.info.node.on(EVENT_VALUE_REMOVED, self._value_removed)
         )
+        self.async_on_remove(
+            async_dispatcher_connect(
+                self.hass,
+                (
+                    f"{DOMAIN}_"
+                    f"{get_valueless_base_unique_id(self.driver, self.info.node)}_"
+                    "remove_entity_on_ready_node"
+                ),
+                self.async_remove,
+            )
+        )
 
         for status_event in (EVENT_ALIVE, EVENT_DEAD):
             self.async_on_remove(

homeassistant/components/zwave_js/update.py

@@ -189,6 +189,14 @@ class ZWaveNodeFirmwareUpdate(UpdateEntity):
             )
         )
 
+        self.async_on_remove(
+            async_dispatcher_connect(
+                self.hass,
+                f"{DOMAIN}_{self._base_unique_id}_remove_entity_on_ready_node",
+                self.async_remove,
+            )
+        )
+
         self.async_on_remove(async_at_start(self.hass, self._async_update))
 
     async def async_will_remove_from_hass(self) -> None:

tests/components/zwave_js/test_init.py

@@ -3,6 +3,7 @@ from copy import deepcopy
 from unittest.mock import call, patch
 
 import pytest
+from zwave_js_server.client import Client
 from zwave_js_server.event import Event
 from zwave_js_server.exceptions import BaseZwaveJSServerError, InvalidServerVersion
 from zwave_js_server.model.node import Node
@@ -12,6 +13,7 @@ from homeassistant.components.zwave_js.const import DOMAIN
 from homeassistant.components.zwave_js.helpers import get_device_id
 from homeassistant.config_entries import ConfigEntryDisabler, ConfigEntryState
 from homeassistant.const import STATE_UNAVAILABLE
+from homeassistant.core import HomeAssistant
 from homeassistant.helpers import (
     area_registry as ar,
     device_registry as dr,
@@ -242,6 +244,61 @@ async def test_existing_node_ready(hass, client, multisensor_6, integration):
     )
 
 
+async def test_existing_node_reinterview(
+    hass: HomeAssistant,
+    client: Client,
+    multisensor_6_state: dict,
+    multisensor_6: Node,
+    integration: MockConfigEntry,
+) -> None:
+    """Test we handle a node re-interview firing a node ready event."""
+    dev_reg = dr.async_get(hass)
+    node = multisensor_6
+    assert client.driver is not None
+    air_temperature_device_id = f"{client.driver.controller.home_id}-{node.node_id}"
+    air_temperature_device_id_ext = (
+        f"{air_temperature_device_id}-{node.manufacturer_id}:"
+        f"{node.product_type}:{node.product_id}"
+    )
+
+    state = hass.states.get(AIR_TEMPERATURE_SENSOR)
+
+    assert state  # entity and device added
+    assert state.state != STATE_UNAVAILABLE
+
+    device = dev_reg.async_get_device(identifiers={(DOMAIN, air_temperature_device_id)})
+    assert device
+    assert device == dev_reg.async_get_device(
+        identifiers={(DOMAIN, air_temperature_device_id_ext)}
+    )
+    assert device.sw_version == "1.12"
+
+    node_state = deepcopy(multisensor_6_state)
+    node_state["firmwareVersion"] = "1.13"
+    event = Event(
+        type="ready",
+        data={
+            "source": "node",
+            "event": "ready",
+            "nodeId": node.node_id,
+            "nodeState": node_state,
+        },
+    )
+    client.driver.receive_event(event)
+    await hass.async_block_till_done()
+
+    state = hass.states.get(AIR_TEMPERATURE_SENSOR)
+
+    assert state
+    assert state.state != STATE_UNAVAILABLE
+    device = dev_reg.async_get_device(identifiers={(DOMAIN, air_temperature_device_id)})
+    assert device
+    assert device == dev_reg.async_get_device(
+        identifiers={(DOMAIN, air_temperature_device_id_ext)}
+    )
+    assert device.sw_version == "1.13"
+
+
 async def test_existing_node_not_ready(hass, zp3111_not_ready, client, integration):
     """Test we handle a non-ready node that exists during integration setup."""
     dev_reg = dr.async_get(hass)