From e7f07a20faaff85f6bf13b496b9be001a4a64ffc Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Tue, 4 Jan 2022 22:39:57 -0500 Subject: [PATCH 1/4] Fix bug with zwave-js device actions --- homeassistant/components/zwave_js/device_action.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/homeassistant/components/zwave_js/device_action.py b/homeassistant/components/zwave_js/device_action.py index f819a33f1d492d..be57db561a426b 100644 --- a/homeassistant/components/zwave_js/device_action.py +++ b/homeassistant/components/zwave_js/device_action.py @@ -21,6 +21,7 @@ CONF_DOMAIN, CONF_ENTITY_ID, CONF_TYPE, + STATE_UNAVAILABLE, ) from homeassistant.core import Context, HomeAssistant from homeassistant.exceptions import HomeAssistantError @@ -173,6 +174,12 @@ async def async_get_actions(hass: HomeAssistant, device_id: str) -> list[dict]: meter_endpoints: dict[int, dict[str, Any]] = defaultdict(dict) for entry in entity_registry.async_entries_for_device(registry, device_id): + # If an entry is unavailable, it is possible that the underlying value + # is no longer valid. Additionally, if an entry is disabled, its + # underlying value is not being monitored by HA so we shouldn't allow + # actions against it. + if hass.states.get(entry.entity_id) == STATE_UNAVAILABLE or entry.disabled: + continue entity_action = {**base_action, CONF_ENTITY_ID: entry.entity_id} actions.append({**entity_action, CONF_TYPE: SERVICE_REFRESH_VALUE}) if entry.domain == LOCK_DOMAIN: From 03d83f4f26ed61a53a8d95975fb733179d75a402 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Sun, 16 Jan 2022 00:42:57 -0500 Subject: [PATCH 2/4] outdent --- homeassistant/components/zwave_js/device_action.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/zwave_js/device_action.py b/homeassistant/components/zwave_js/device_action.py index be57db561a426b..da766e3ccfb5b8 100644 --- a/homeassistant/components/zwave_js/device_action.py +++ b/homeassistant/components/zwave_js/device_action.py @@ -194,10 +194,9 @@ async def async_get_actions(hass: HomeAssistant, device_id: str) -> list[dict]: value_id = entry.unique_id.split(".")[1] # If this unique ID doesn't have a value ID, we know it is the node status # sensor which doesn't have any relevant actions - if re.match(VALUE_ID_REGEX, value_id): - value = node.values[value_id] - else: + if not re.match(VALUE_ID_REGEX, value_id): continue + value = node.values[value_id] # If the value has the meterType CC specific value, we can add a reset_meter # action for it if CC_SPECIFIC_METER_TYPE in value.metadata.cc_specific: From 92c763eb4cd145449bbd4a08d68f169f5b50f934 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Sun, 16 Jan 2022 00:54:04 -0500 Subject: [PATCH 3/4] Add test and fix bug --- .../components/zwave_js/device_action.py | 5 +++- .../components/zwave_js/test_device_action.py | 25 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/zwave_js/device_action.py b/homeassistant/components/zwave_js/device_action.py index da766e3ccfb5b8..eb94fe7134ebf8 100644 --- a/homeassistant/components/zwave_js/device_action.py +++ b/homeassistant/components/zwave_js/device_action.py @@ -178,7 +178,10 @@ async def async_get_actions(hass: HomeAssistant, device_id: str) -> list[dict]: # is no longer valid. Additionally, if an entry is disabled, its # underlying value is not being monitored by HA so we shouldn't allow # actions against it. - if hass.states.get(entry.entity_id) == STATE_UNAVAILABLE or entry.disabled: + if ( + (state := hass.states.get(entry.entity_id)) + and state.state == STATE_UNAVAILABLE + ) or entry.disabled: continue entity_action = {**base_action, CONF_ENTITY_ID: entry.entity_id} actions.append({**entity_action, CONF_TYPE: SERVICE_REFRESH_VALUE}) diff --git a/tests/components/zwave_js/test_device_action.py b/tests/components/zwave_js/test_device_action.py index 58f8091e2a1edd..d061ea6c4d2207 100644 --- a/tests/components/zwave_js/test_device_action.py +++ b/tests/components/zwave_js/test_device_action.py @@ -12,6 +12,7 @@ from homeassistant.components.zwave_js import DOMAIN, device_action from homeassistant.components.zwave_js.helpers import get_device_id from homeassistant.config_entries import ConfigEntry +from homeassistant.const import STATE_UNAVAILABLE from homeassistant.core import HomeAssistant from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers import config_validation as cv, device_registry @@ -588,3 +589,27 @@ async def test_failure_scenarios( ) == {} ) + + +async def test_disabled_entity_actions( + hass: HomeAssistant, + client: Client, + lock_schlage_be469: Node, + integration: ConfigEntry, +) -> None: + """Test disabled and unavailable entities are not included in actions list.""" + entity_id_disabled = "sensor.touchscreen_deadbolt_access_control_lock_state" + entity_id_unavailable = "binary_sensor.touchscreen_deadbolt_home_security_intrusion" + hass.states.async_set(entity_id_unavailable, STATE_UNAVAILABLE) + await hass.async_block_till_done() + node = lock_schlage_be469 + dev_reg = device_registry.async_get(hass) + device = dev_reg.async_get_device({get_device_id(client, node)}) + assert device + actions = await async_get_device_automations( + hass, DeviceAutomationType.ACTION, device.id + ) + assert not any(action.get("entity_id") == entity_id_disabled for action in actions) + assert not any( + action.get("entity_id") == entity_id_unavailable for action in actions + ) From f61f41b16c9c51090194b60058246fd4b4a01912 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Sun, 16 Jan 2022 01:10:48 -0500 Subject: [PATCH 4/4] fix --- homeassistant/components/zwave_js/device_action.py | 9 +++++---- tests/components/zwave_js/test_device_action.py | 8 +++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/homeassistant/components/zwave_js/device_action.py b/homeassistant/components/zwave_js/device_action.py index eb94fe7134ebf8..9f6fa7fc35cf78 100644 --- a/homeassistant/components/zwave_js/device_action.py +++ b/homeassistant/components/zwave_js/device_action.py @@ -173,15 +173,16 @@ async def async_get_actions(hass: HomeAssistant, device_id: str) -> list[dict]: meter_endpoints: dict[int, dict[str, Any]] = defaultdict(dict) - for entry in entity_registry.async_entries_for_device(registry, device_id): + for entry in entity_registry.async_entries_for_device( + registry, device_id, include_disabled_entities=False + ): # If an entry is unavailable, it is possible that the underlying value # is no longer valid. Additionally, if an entry is disabled, its # underlying value is not being monitored by HA so we shouldn't allow # actions against it. if ( - (state := hass.states.get(entry.entity_id)) - and state.state == STATE_UNAVAILABLE - ) or entry.disabled: + state := hass.states.get(entry.entity_id) + ) and state.state == STATE_UNAVAILABLE: continue entity_action = {**base_action, CONF_ENTITY_ID: entry.entity_id} actions.append({**entity_action, CONF_TYPE: SERVICE_REFRESH_VALUE}) diff --git a/tests/components/zwave_js/test_device_action.py b/tests/components/zwave_js/test_device_action.py index d061ea6c4d2207..5377d420268c1a 100644 --- a/tests/components/zwave_js/test_device_action.py +++ b/tests/components/zwave_js/test_device_action.py @@ -591,16 +591,15 @@ async def test_failure_scenarios( ) -async def test_disabled_entity_actions( +async def test_unavailable_entity_actions( hass: HomeAssistant, client: Client, lock_schlage_be469: Node, integration: ConfigEntry, ) -> None: - """Test disabled and unavailable entities are not included in actions list.""" - entity_id_disabled = "sensor.touchscreen_deadbolt_access_control_lock_state" + """Test unavailable entities are not included in actions list.""" entity_id_unavailable = "binary_sensor.touchscreen_deadbolt_home_security_intrusion" - hass.states.async_set(entity_id_unavailable, STATE_UNAVAILABLE) + hass.states.async_set(entity_id_unavailable, STATE_UNAVAILABLE, force_update=True) await hass.async_block_till_done() node = lock_schlage_be469 dev_reg = device_registry.async_get(hass) @@ -609,7 +608,6 @@ async def test_disabled_entity_actions( actions = await async_get_device_automations( hass, DeviceAutomationType.ACTION, device.id ) - assert not any(action.get("entity_id") == entity_id_disabled for action in actions) assert not any( action.get("entity_id") == entity_id_unavailable for action in actions )