Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid removing zwave_js devices for non-ready nodes #59964

Merged
merged 28 commits into from Dec 27, 2021
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
5630e6f
Only replace a node if the mfgr id / prod id / prod type differ
kpine Nov 14, 2021
8fb93d7
Prefer original device name for unready node
kpine Nov 14, 2021
30ec1e6
move register_node_in_dev_reg into async_setup_entry
kpine Nov 14, 2021
064fc37
simplify get_device_id_ext
kpine Nov 19, 2021
db0a1f3
Don't need hex ids
kpine Nov 19, 2021
54c6478
Revert "move register_node_in_dev_reg into async_setup_entry"
kpine Nov 20, 2021
bba258b
Revert Callable change
kpine Nov 20, 2021
b3bfe41
Revert device backup name
kpine Nov 27, 2021
e5f5e91
Add test fixtures
kpine Nov 10, 2021
d4c3e19
Update existing not ready test with new fixture data
kpine Nov 22, 2021
39e09ea
Check device properties after node added event
kpine Nov 22, 2021
06cceae
Add entity check
kpine Nov 22, 2021
b26cce3
Check for extended device id
kpine Nov 27, 2021
5232794
better device info checks
kpine Nov 27, 2021
506b2a9
Use receive_event to properly setup components
kpine Nov 27, 2021
901833c
Cleanup tests
kpine Nov 27, 2021
16a3cf8
improve test_replace_different_node
kpine Nov 27, 2021
dc7329f
improve test_replace_same_node
kpine Nov 27, 2021
0e86a4b
add test test_node_model_change
kpine Nov 28, 2021
2c553f1
Clean up long comments and strings
MartinHjelmare Dec 1, 2021
be6c406
Format
MartinHjelmare Dec 1, 2021
ec015dc
Reload integration to detect node device config changes
kpine Dec 4, 2021
7f4e5c1
update assertions
kpine Dec 4, 2021
efa60cc
Disable entities on "value removed" event
kpine Dec 19, 2021
b3523b1
Disable node status sensor on node replacement
kpine Dec 19, 2021
6c93810
Add test for disabling entities on remove value event
kpine Dec 25, 2021
8617df1
Add test for disabling node status sensor on node replacement
kpine Dec 26, 2021
f22bc41
disable entity -> remove entity
kpine Dec 26, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 32 additions & 10 deletions homeassistant/components/zwave_js/__init__.py
Expand Up @@ -88,7 +88,12 @@
async_discover_node_values,
async_discover_single_value,
)
from .helpers import async_enable_statistics, get_device_id, get_unique_id
from .helpers import (
async_enable_statistics,
get_device_id,
get_device_id_ext,
get_unique_id,
)
from .migrate import async_migrate_discovered_value
from .services import ZWaveServices

Expand Down Expand Up @@ -116,17 +121,27 @@ def register_node_in_dev_reg(
) -> device_registry.DeviceEntry:
"""Register node in dev reg."""
device_id = get_device_id(client, node)
# If a device already exists but it doesn't match the new node, it means the node
# was replaced with a different device and the device needs to be removeed so the
# new device can be created. Otherwise if the device exists and the node is the same,
# the node was replaced with the same device model and we can reuse the device.
if (device := dev_reg.async_get_device({device_id})) and (
device.model != node.device_config.label
or device.manufacturer != node.device_config.manufacturer
device_id_ext = get_device_id_ext(client, node)
device = dev_reg.async_get_device({device_id})

# 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
):
remove_device_func(device)
device = None

if device_id_ext:
ids = {device_id, device_id_ext}
else:
ids = {device_id}

params = {
ATTR_IDENTIFIERS: {device_id},
ATTR_IDENTIFIERS: ids,
ATTR_SW_VERSION: node.firmware_version,
ATTR_NAME: node.name
or node.device_config.description
Expand Down Expand Up @@ -338,7 +353,14 @@ def async_on_node_removed(event: dict) -> None:
device = dev_reg.async_get_device({dev_id})
# We assert because we know the device exists
assert device
if not replaced:
if replaced:
discovered_value_ids.pop(device.id, None)

async_dispatcher_send(
hass,
f"{DOMAIN}_{client.driver.controller.home_id}.{node.node_id}.node_status_remove_entity",
)
else:
remove_device(device)

@callback
Expand Down
26 changes: 25 additions & 1 deletion homeassistant/components/zwave_js/entity.py
Expand Up @@ -20,6 +20,7 @@
LOGGER = logging.getLogger(__name__)

EVENT_VALUE_UPDATED = "value updated"
EVENT_VALUE_REMOVED = "value removed"
EVENT_DEAD = "dead"
EVENT_ALIVE = "alive"

Expand Down Expand Up @@ -99,6 +100,10 @@ async def async_added_to_hass(self) -> None:
self.async_on_remove(
self.info.node.on(EVENT_VALUE_UPDATED, self._value_changed)
)
self.async_on_remove(
self.info.node.on(EVENT_VALUE_REMOVED, self._value_removed)
)

for status_event in (EVENT_ALIVE, EVENT_DEAD):
self.async_on_remove(
self.info.node.on(status_event, self._node_status_alive_or_dead)
Expand Down Expand Up @@ -171,7 +176,7 @@ def _node_status_alive_or_dead(self, event_data: dict) -> None:

@callback
def _value_changed(self, event_data: dict) -> None:
"""Call when (one of) our watched values changes.
"""Call when a value associated with our node changes.

Should not be overridden by subclasses.
"""
Expand All @@ -193,6 +198,25 @@ def _value_changed(self, event_data: dict) -> None:
self.on_value_update()
self.async_write_ha_state()

@callback
def _value_removed(self, event_data: dict) -> None:
"""Call when a value associated with our node is removed.

Should not be overridden by subclasses.
"""
value_id = event_data["value"].value_id

if value_id != self.info.primary_value.value_id:
return

LOGGER.debug(
"[%s] Primary value %s is being removed",
self.entity_id,
value_id,
)

self.hass.async_create_task(self.async_remove())

@callback
def get_zwave_value(
self,
Expand Down
13 changes: 13 additions & 0 deletions homeassistant/components/zwave_js/helpers.py
Expand Up @@ -66,6 +66,19 @@ def get_device_id(client: ZwaveClient, node: ZwaveNode) -> tuple[str, str]:
return (DOMAIN, f"{client.driver.controller.home_id}-{node.node_id}")


@callback
def get_device_id_ext(client: ZwaveClient, node: ZwaveNode) -> tuple[str, str] | None:
"""Get extended device registry identifier for Z-Wave node."""
if None in (node.manufacturer_id, node.product_type, node.product_id):
return None

domain, dev_id = get_device_id(client, node)
return (
domain,
f"{dev_id}-{node.manufacturer_id}:{node.product_type}:{node.product_id}",
kpine marked this conversation as resolved.
Show resolved Hide resolved
)


@callback
def get_home_and_node_id_from_device_id(device_id: tuple[str, ...]) -> list[str]:
"""
Expand Down
7 changes: 7 additions & 0 deletions homeassistant/components/zwave_js/sensor.py
Expand Up @@ -502,4 +502,11 @@ async def async_added_to_hass(self) -> None:
self.async_poll_value,
)
)
self.async_on_remove(
async_dispatcher_connect(
self.hass,
f"{DOMAIN}_{self.unique_id}_remove_entity",
self.async_remove,
)
)
self.async_write_ha_state()
28 changes: 28 additions & 0 deletions tests/components/zwave_js/conftest.py
Expand Up @@ -479,6 +479,18 @@ def fortrezz_ssa3_siren_state_fixture():
return json.loads(load_fixture("zwave_js/fortrezz_ssa3_siren_state.json"))


@pytest.fixture(name="zp3111_not_ready_state", scope="session")
def zp3111_not_ready_state_fixture():
"""Load the zp3111 4-in-1 sensor not-ready node state fixture data."""
return json.loads(load_fixture("zwave_js/zp3111-5_not_ready_state.json"))


@pytest.fixture(name="zp3111_state", scope="session")
def zp3111_state_fixture():
"""Load the zp3111 4-in-1 sensor node state fixture data."""
return json.loads(load_fixture("zwave_js/zp3111-5_state.json"))


@pytest.fixture(name="client")
def mock_client_fixture(controller_state, version_state, log_config_state):
"""Mock a client."""
Expand Down Expand Up @@ -919,3 +931,19 @@ def fortrezz_ssa3_siren_fixture(client, fortrezz_ssa3_siren_state):
def firmware_file_fixture():
"""Return mock firmware file stream."""
return io.BytesIO(bytes(10))


@pytest.fixture(name="zp3111_not_ready")
def zp3111_not_ready_fixture(client, zp3111_not_ready_state):
"""Mock a zp3111 4-in-1 sensor node in a not-ready state."""
node = Node(client, copy.deepcopy(zp3111_not_ready_state))
client.driver.controller.nodes[node.node_id] = node
return node


@pytest.fixture(name="zp3111")
def zp3111_fixture(client, zp3111_state):
"""Mock a zp3111 4-in-1 sensor node."""
node = Node(client, copy.deepcopy(zp3111_state))
client.driver.controller.nodes[node.node_id] = node
return node
@@ -0,0 +1,68 @@
{
"nodeId": 22,
"index": 0,
"status": 1,
"ready": false,
"isListening": false,
"isRouting": true,
"isSecure": "unknown",
"interviewAttempts": 1,
"endpoints": [
{
"nodeId": 22,
"index": 0,
"deviceClass": {
"basic": {
"key": 4,
"label": "Routing Slave"
},
"generic": {
"key": 7,
"label": "Notification Sensor"
},
"specific": {
"key": 1,
"label": "Notification Sensor"
},
"mandatorySupportedCCs": [],
"mandatoryControlledCCs": []
}
}
],
"values": [],
"isFrequentListening": false,
"maxDataRate": 100000,
"supportedDataRates": [
40000,
100000
],
"protocolVersion": 3,
"supportsBeaming": true,
"supportsSecurity": false,
"nodeType": 1,
"deviceClass": {
"basic": {
"key": 4,
"label": "Routing Slave"
},
"generic": {
"key": 7,
"label": "Notification Sensor"
},
"specific": {
"key": 1,
"label": "Notification Sensor"
},
"mandatorySupportedCCs": [],
"mandatoryControlledCCs": []
},
"commandClasses": [],
"interviewStage": "ProtocolInfo",
"statistics": {
"commandsTX": 0,
"commandsRX": 0,
"commandsDroppedRX": 0,
"commandsDroppedTX": 0,
"timeoutResponse": 0
}
}