Change behavior of Z-Wave JS services (#52941)

* Change behavior of Z-Wave JS services

* pop parameters in for loop

* Update logger message

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]
 

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],

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):