diff --git a/homeassistant/components/zwave_js/helpers.py b/homeassistant/components/zwave_js/helpers.py index f8e8dab2b46..50d57d9a6e4 100644 --- a/homeassistant/components/zwave_js/helpers.py +++ b/homeassistant/components/zwave_js/helpers.py @@ -89,7 +89,7 @@ def async_get_node_from_device_id( device_entry = dev_reg.async_get(device_id) if not device_entry: - raise ValueError("Device ID is not valid") + raise ValueError(f"Device ID {device_id} is not valid") # Use device config entry ID's to validate that this is a valid zwave_js device # and to get the client @@ -107,7 +107,9 @@ def async_get_node_from_device_id( None, ) if config_entry_id is None or config_entry_id not in hass.data[DOMAIN]: - raise ValueError("Device is not from an existing zwave_js config entry") + raise ValueError( + f"Device {device_id} is not from an existing zwave_js config entry" + ) client = hass.data[DOMAIN][config_entry_id][DATA_CLIENT] @@ -125,7 +127,7 @@ def async_get_node_from_device_id( node_id = int(identifier[1]) if identifier is not None else None if node_id is None or node_id not in client.driver.controller.nodes: - raise ValueError("Device node can't be found") + raise ValueError(f"Node for device {device_id} can't be found") return client.driver.controller.nodes[node_id] diff --git a/homeassistant/components/zwave_js/services.py b/homeassistant/components/zwave_js/services.py index 47709a908ed..1204f458bff 100644 --- a/homeassistant/components/zwave_js/services.py +++ b/homeassistant/components/zwave_js/services.py @@ -34,8 +34,9 @@ def parameter_name_does_not_need_bitmask( val: dict[str, int | str | list[str]] ) -> dict[str, int | str | list[str]]: """Validate that if a parameter name is provided, bitmask is not as well.""" - if isinstance(val[const.ATTR_CONFIG_PARAMETER], str) and ( - val.get(const.ATTR_CONFIG_PARAMETER_BITMASK) + if ( + isinstance(val[const.ATTR_CONFIG_PARAMETER], str) + and const.ATTR_CONFIG_PARAMETER_BITMASK in val ): raise vol.Invalid( "Don't include a bitmask when a parameter name is specified", @@ -94,25 +95,24 @@ class ZWaveServices: def get_nodes_from_service_data(val: dict[str, Any]) -> dict[str, Any]: """Get nodes set from service data.""" nodes: set[ZwaveNode] = set() - try: - if ATTR_ENTITY_ID in val: - nodes |= { + for entity_id in val.pop(ATTR_ENTITY_ID, []): + try: + nodes.add( async_get_node_from_entity_id( self._hass, entity_id, self._ent_reg, self._dev_reg ) - for entity_id in val[ATTR_ENTITY_ID] - } - val.pop(ATTR_ENTITY_ID) - if ATTR_DEVICE_ID in val: - nodes |= { + ) + except ValueError as err: + const.LOGGER.warning(err.args[0]) + for device_id in val.pop(ATTR_DEVICE_ID, []): + try: + nodes.add( async_get_node_from_device_id( self._hass, device_id, self._dev_reg ) - for device_id in val[ATTR_DEVICE_ID] - } - val.pop(ATTR_DEVICE_ID) - except ValueError as err: - raise vol.Invalid(err.args[0]) from err + ) + except ValueError as err: + const.LOGGER.warning(err.args[0]) val[const.ATTR_NODES] = nodes return val @@ -124,22 +124,16 @@ class ZWaveServices: broadcast: bool = val[const.ATTR_BROADCAST] # User must specify a node if they are attempting a broadcast and have more - # than one zwave-js network. We know it's a broadcast if the nodes list is - # empty because of schema validation. + # than one zwave-js network. if ( - not nodes + broadcast + and not nodes and len(self._hass.config_entries.async_entries(const.DOMAIN)) > 1 ): raise vol.Invalid( "You must include at least one entity or device in the service call" ) - # When multicasting, user must specify at least two nodes - if not broadcast and len(nodes) < 2: - raise vol.Invalid( - "To set a value on a single node, use the zwave_js.set_value service" - ) - first_node = next((node for node in nodes), None) # If any nodes don't have matching home IDs, we can't run the command because @@ -413,6 +407,14 @@ class ZWaveServices: nodes = service.data[const.ATTR_NODES] broadcast: bool = service.data[const.ATTR_BROADCAST] + if not broadcast and len(nodes) == 1: + const.LOGGER.warning( + "Passing the zwave_js.multicast_set_value service call to the " + "zwave_js.set_value service since only one node was targeted" + ) + await self.async_set_value(service) + return + value = { "commandClass": service.data[const.ATTR_COMMAND_CLASS], "property": service.data[const.ATTR_PROPERTY], diff --git a/tests/components/zwave_js/test_services.py b/tests/components/zwave_js/test_services.py index dfc7ddaa85d..25373bcb026 100644 --- a/tests/components/zwave_js/test_services.py +++ b/tests/components/zwave_js/test_services.py @@ -261,31 +261,7 @@ async def test_set_config_parameter(hass, client, multisensor_6, integration): } assert args["value"] == 1 - # Test that an invalid entity ID raises a MultipleInvalid - with pytest.raises(vol.MultipleInvalid): - await hass.services.async_call( - DOMAIN, - SERVICE_SET_CONFIG_PARAMETER, - { - ATTR_ENTITY_ID: "sensor.fake_entity", - ATTR_CONFIG_PARAMETER: "Temperature Threshold (Unit)", - ATTR_CONFIG_VALUE: "Fahrenheit", - }, - blocking=True, - ) - - # Test that an invalid device ID raises a MultipleInvalid - with pytest.raises(vol.MultipleInvalid): - await hass.services.async_call( - DOMAIN, - SERVICE_SET_CONFIG_PARAMETER, - { - ATTR_DEVICE_ID: "fake_device_id", - ATTR_CONFIG_PARAMETER: "Temperature Threshold (Unit)", - ATTR_CONFIG_VALUE: "Fahrenheit", - }, - blocking=True, - ) + client.async_send_command_no_wait.reset_mock() # Test that we can't include a bitmask value if parameter is a string with pytest.raises(vol.Invalid): @@ -308,36 +284,10 @@ async def test_set_config_parameter(hass, client, multisensor_6, integration): identifiers={("test", "test")}, ) - # Test that a non Z-Wave JS device raises a MultipleInvalid - with pytest.raises(vol.MultipleInvalid): - await hass.services.async_call( - DOMAIN, - SERVICE_SET_CONFIG_PARAMETER, - { - ATTR_DEVICE_ID: non_zwave_js_device.id, - ATTR_CONFIG_PARAMETER: "Temperature Threshold (Unit)", - ATTR_CONFIG_VALUE: "Fahrenheit", - }, - blocking=True, - ) - zwave_js_device_with_invalid_node_id = dev_reg.async_get_or_create( config_entry_id=integration.entry_id, identifiers={(DOMAIN, "500-500")} ) - # Test that a Z-Wave JS device with an invalid node ID raises a MultipleInvalid - with pytest.raises(vol.MultipleInvalid): - await hass.services.async_call( - DOMAIN, - SERVICE_SET_CONFIG_PARAMETER, - { - ATTR_DEVICE_ID: zwave_js_device_with_invalid_node_id.id, - ATTR_CONFIG_PARAMETER: "Temperature Threshold (Unit)", - ATTR_CONFIG_VALUE: "Fahrenheit", - }, - blocking=True, - ) - non_zwave_js_entity = ent_reg.async_get_or_create( "test", "sensor", @@ -346,18 +296,59 @@ async def test_set_config_parameter(hass, client, multisensor_6, integration): config_entry=non_zwave_js_config_entry, ) - # Test that a non Z-Wave JS entity raises a MultipleInvalid - with pytest.raises(vol.MultipleInvalid): - await hass.services.async_call( - DOMAIN, - SERVICE_SET_CONFIG_PARAMETER, - { - ATTR_ENTITY_ID: non_zwave_js_entity.entity_id, - ATTR_CONFIG_PARAMETER: "Temperature Threshold (Unit)", - ATTR_CONFIG_VALUE: "Fahrenheit", - }, - blocking=True, - ) + # Test that a Z-Wave JS device with an invalid node ID, non Z-Wave JS entity, + # non Z-Wave JS device, invalid device_id, and invalid node_id gets filtered out. + await hass.services.async_call( + DOMAIN, + SERVICE_SET_CONFIG_PARAMETER, + { + ATTR_ENTITY_ID: [ + AIR_TEMPERATURE_SENSOR, + non_zwave_js_entity.entity_id, + "sensor.fake", + ], + ATTR_DEVICE_ID: [ + zwave_js_device_with_invalid_node_id.id, + non_zwave_js_device.id, + "fake_device_id", + ], + ATTR_CONFIG_PARAMETER: 102, + ATTR_CONFIG_PARAMETER_BITMASK: "0x01", + ATTR_CONFIG_VALUE: 1, + }, + blocking=True, + ) + + assert len(client.async_send_command_no_wait.call_args_list) == 1 + args = client.async_send_command_no_wait.call_args[0][0] + assert args["command"] == "node.set_value" + assert args["nodeId"] == 52 + assert args["valueId"] == { + "commandClassName": "Configuration", + "commandClass": 112, + "endpoint": 0, + "property": 102, + "propertyName": "Group 2: Send battery reports", + "propertyKey": 1, + "metadata": { + "type": "number", + "readable": True, + "writeable": True, + "valueSize": 4, + "min": 0, + "max": 1, + "default": 1, + "format": 0, + "allowManualEntry": True, + "label": "Group 2: Send battery reports", + "description": "Include battery information in periodic reports to Group 2", + "isFromConfig": True, + }, + "value": 0, + } + assert args["value"] == 1 + + client.async_send_command_no_wait.reset_mock() # Test that when a device is awake, we call async_send_command instead of # async_send_command_no_wait @@ -862,19 +853,24 @@ async def test_multicast_set_value( client.async_send_command.reset_mock() - # Test sending one node without broadcast fails - with pytest.raises(vol.Invalid): - await hass.services.async_call( - DOMAIN, - SERVICE_MULTICAST_SET_VALUE, - { - ATTR_ENTITY_ID: CLIMATE_DANFOSS_LC13_ENTITY, - ATTR_COMMAND_CLASS: 117, - ATTR_PROPERTY: "local", - ATTR_VALUE: 2, - }, - blocking=True, - ) + # Test sending one node without broadcast uses the node.set_value command instead + await hass.services.async_call( + DOMAIN, + SERVICE_MULTICAST_SET_VALUE, + { + ATTR_ENTITY_ID: CLIMATE_DANFOSS_LC13_ENTITY, + ATTR_COMMAND_CLASS: 117, + ATTR_PROPERTY: "local", + ATTR_VALUE: 2, + }, + blocking=True, + ) + + assert len(client.async_send_command_no_wait.call_args_list) == 1 + args = client.async_send_command_no_wait.call_args[0][0] + assert args["command"] == "node.set_value" + + client.async_send_command_no_wait.reset_mock() # Test no device, entity, or broadcast flag raises error with pytest.raises(vol.Invalid):