From 6db4075a230d3bd427431358dc22f8822e5890f0 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Mon, 5 Oct 2020 15:47:28 -0400 Subject: [PATCH] Add OZW node config parameters websocket commands (#40527) * add websocket commands to retrieve and set config parameters for a node * move set_config_parameter into generic function and refactor service and WS API * add payload to return to make service call behave the same way it did before * create response class * update error message to pass tests * move things a bit to reduce LOC * add tests * handle logging errors better and make new response class more generic to prepare for lock user code work * remove unused function parameter * invert check * add additional error checking * refactor a bit to remove repeat code * revert log msg change * one more refactor to create generic get_config_parameters function * change if logic for consistency * fix test * add support to provide bool value in set_config_parameter service call * standardize parameter names on service call * add test coverage * fix tests and message sending * remove unnecessary logging import * fix one test to get missing coverage * update per martin and kpines reviews * remove false assertion * string line length * add support for Decimal config param, remove node instance ID as input, and move helper functions to node.py * cast Decimal appropriately * revert change to support Decimal for config params since they are not supported as a config param type * revert to using error arguments to make next PR for WS lock commands easier * switch to class method and add guard for list Value not being a number * update logic to use new openzwavemqtt util methods * add support for bitsets * use parent exception class * bump openzwavemqtt version, remove node.py from .coveragerc and put file references in the right place * add comment * improve config validation * remove bitset support from config validation * re-add bitset support with some additional tests * move send_result out of try block --- .coveragerc | 6 +- homeassistant/components/ozw/manifest.json | 2 +- homeassistant/components/ozw/services.py | 75 +++------ homeassistant/components/ozw/websocket_api.py | 151 ++++++++++++++--- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/ozw/test_services.py | 64 ++++--- tests/components/ozw/test_websocket_api.py | 159 +++++++++++++++++- 8 files changed, 357 insertions(+), 104 deletions(-) diff --git a/.coveragerc b/.coveragerc index f843b280c21..8d3aeb916d1 100644 --- a/.coveragerc +++ b/.coveragerc @@ -651,6 +651,9 @@ omit = homeassistant/components/ovo_energy/__init__.py homeassistant/components/ovo_energy/const.py homeassistant/components/ovo_energy/sensor.py + homeassistant/components/ozw/__init__.py + homeassistant/components/ozw/entity.py + homeassistant/components/ozw/services.py homeassistant/components/panasonic_bluray/media_player.py homeassistant/components/panasonic_viera/media_player.py homeassistant/components/pandora/media_player.py @@ -1052,9 +1055,6 @@ omit = homeassistant/components/ziggo_mediabox_xl/media_player.py homeassistant/components/supla/* homeassistant/components/zwave/util.py - homeassistant/components/ozw/__init__.py - homeassistant/components/ozw/entity.py - homeassistant/components/ozw/services.py [report] # Regexes for lines to exclude from consideration diff --git a/homeassistant/components/ozw/manifest.json b/homeassistant/components/ozw/manifest.json index 6fa4174da4a..667a50428e1 100644 --- a/homeassistant/components/ozw/manifest.json +++ b/homeassistant/components/ozw/manifest.json @@ -4,7 +4,7 @@ "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/ozw", "requirements": [ - "python-openzwave-mqtt==1.0.5" + "python-openzwave-mqtt==1.2.0" ], "after_dependencies": [ "mqtt" diff --git a/homeassistant/components/ozw/services.py b/homeassistant/components/ozw/services.py index 500289cec21..fc3a9ee5ed4 100644 --- a/homeassistant/components/ozw/services.py +++ b/homeassistant/components/ozw/services.py @@ -1,7 +1,8 @@ """Methods and classes related to executing Z-Wave commands and publishing these to hass.""" import logging -from openzwavemqtt.const import CommandClass, ValueType +from openzwavemqtt.const import ATTR_LABEL, ATTR_POSITION, ATTR_VALUE +from openzwavemqtt.util.node import get_node_from_manager, set_config_parameter import voluptuous as vol from homeassistant.core import callback @@ -53,7 +54,24 @@ class ZWaveServices: vol.Required(const.ATTR_NODE_ID): vol.Coerce(int), vol.Required(const.ATTR_CONFIG_PARAMETER): vol.Coerce(int), vol.Required(const.ATTR_CONFIG_VALUE): vol.Any( - vol.Coerce(int), cv.string + vol.All( + cv.ensure_list, + [ + vol.All( + { + vol.Exclusive(ATTR_LABEL, "bit"): cv.string, + vol.Exclusive(ATTR_POSITION, "bit"): vol.Coerce( + int + ), + vol.Required(ATTR_VALUE): bool, + }, + cv.has_at_least_one_key(ATTR_LABEL, ATTR_POSITION), + ) + ], + ), + vol.Coerce(int), + bool, + cv.string, ), } ), @@ -66,57 +84,12 @@ class ZWaveServices: node_id = service.data[const.ATTR_NODE_ID] param = service.data[const.ATTR_CONFIG_PARAMETER] selection = service.data[const.ATTR_CONFIG_VALUE] - payload = None - value = ( - self._manager.get_instance(instance_id) - .get_node(node_id) - .get_value(CommandClass.CONFIGURATION, param) - ) + # These function calls may raise an exception but that's ok because + # the exception will show in the UI to the user + node = get_node_from_manager(self._manager, instance_id, node_id) + payload = set_config_parameter(node, param, selection) - if value.type == ValueType.BOOL: - payload = selection == "True" - - if value.type == ValueType.LIST: - # accept either string from the list value OR the int value - for selected in value.value["List"]: - if selection not in (selected["Label"], selected["Value"]): - continue - payload = int(selected["Value"]) - - if payload is None: - _LOGGER.error( - "Invalid value %s for parameter %s", - selection, - param, - ) - return - - if value.type == ValueType.BUTTON: - # Unsupported at this time - _LOGGER.info("Button type not supported yet") - return - - if value.type == ValueType.STRING: - payload = selection - - if ( - value.type == ValueType.INT - or value.type == ValueType.BYTE - or value.type == ValueType.SHORT - ): - if selection > value.max or selection < value.min: - _LOGGER.error( - "Value %s out of range for parameter %s (Min: %s Max: %s)", - selection, - param, - value.min, - value.max, - ) - return - payload = int(selection) - - value.send_value(payload) # send the payload _LOGGER.info( "Setting configuration parameter %s on Node %s with value %s", param, diff --git a/homeassistant/components/ozw/websocket_api.py b/homeassistant/components/ozw/websocket_api.py index da9c2b1f598..f0233bf6bd4 100644 --- a/homeassistant/components/ozw/websocket_api.py +++ b/homeassistant/components/ozw/websocket_api.py @@ -1,21 +1,31 @@ """Web socket API for OpenZWave.""" - -import logging - -from openzwavemqtt.const import EVENT_NODE_ADDED, EVENT_NODE_CHANGED +from openzwavemqtt.const import ( + ATTR_LABEL, + ATTR_POSITION, + ATTR_VALUE, + EVENT_NODE_ADDED, + EVENT_NODE_CHANGED, +) +from openzwavemqtt.exceptions import NotFoundError, NotSupportedError +from openzwavemqtt.util.node import ( + get_config_parameters, + get_node_from_manager, + set_config_parameter, +) import voluptuous as vol from homeassistant.components import websocket_api from homeassistant.core import callback +from homeassistant.helpers import config_validation as cv -from .const import DOMAIN, MANAGER, OPTIONS - -_LOGGER = logging.getLogger(__name__) +from .const import ATTR_CONFIG_PARAMETER, ATTR_CONFIG_VALUE, DOMAIN, MANAGER, OPTIONS TYPE = "type" ID = "id" OZW_INSTANCE = "ozw_instance" NODE_ID = "node_id" +PARAMETER = ATTR_CONFIG_PARAMETER +VALUE = ATTR_CONFIG_VALUE ATTR_NODE_QUERY_STAGE = "node_query_stage" ATTR_IS_ZWAVE_PLUS = "is_zwave_plus" @@ -45,6 +55,8 @@ def async_register_api(hass): websocket_api.async_register_command(hass, websocket_node_status) websocket_api.async_register_command(hass, websocket_node_statistics) websocket_api.async_register_command(hass, websocket_refresh_node_info) + websocket_api.async_register_command(hass, websocket_get_config_parameters) + websocket_api.async_register_command(hass, websocket_set_config_parameter) @websocket_api.websocket_command({vol.Required(TYPE): "ozw/get_instances"}) @@ -102,6 +114,97 @@ def websocket_get_nodes(hass, connection, msg): ) +@websocket_api.websocket_command( + { + vol.Required(TYPE): "ozw/get_config_parameters", + vol.Required(NODE_ID): vol.Coerce(int), + vol.Optional(OZW_INSTANCE, default=1): vol.Coerce(int), + } +) +def websocket_get_config_parameters(hass, connection, msg): + """Get a list of configuration parameters for an OZW node instance.""" + try: + node = get_node_from_manager( + hass.data[DOMAIN][MANAGER], msg[OZW_INSTANCE], msg[NODE_ID] + ) + except NotFoundError as err: + connection.send_error( + msg[ID], + websocket_api.const.ERR_NOT_FOUND, + err.args[0], + ) + return + + connection.send_result( + msg[ID], + get_config_parameters(node), + ) + + +@websocket_api.websocket_command( + { + vol.Required(TYPE): "ozw/set_config_parameter", + vol.Required(NODE_ID): vol.Coerce(int), + vol.Optional(OZW_INSTANCE, default=1): vol.Coerce(int), + vol.Required(PARAMETER): vol.Coerce(int), + vol.Required(VALUE): vol.Any( + vol.All( + cv.ensure_list, + [ + vol.All( + { + vol.Exclusive(ATTR_LABEL, "bit"): cv.string, + vol.Exclusive(ATTR_POSITION, "bit"): vol.Coerce(int), + vol.Required(ATTR_VALUE): bool, + }, + cv.has_at_least_one_key(ATTR_LABEL, ATTR_POSITION), + ) + ], + ), + vol.Coerce(int), + bool, + cv.string, + ), + } +) +def websocket_set_config_parameter(hass, connection, msg): + """Set a config parameter to a node.""" + try: + node = get_node_from_manager( + hass.data[DOMAIN][MANAGER], msg[OZW_INSTANCE], msg[NODE_ID] + ) + except NotFoundError as err: + connection.send_error( + msg[ID], + websocket_api.const.ERR_NOT_FOUND, + err.args[0], + ) + return + + try: + set_config_parameter( + node, + msg[PARAMETER], + msg[VALUE], + ) + except NotFoundError as err: + connection.send_error( + msg[ID], + websocket_api.const.ERR_NOT_FOUND, + err.args[0], + ) + return + except NotSupportedError as err: + connection.send_error( + msg[ID], + websocket_api.const.ERR_NOT_SUPPORTED, + err.args[0], + ) + return + + connection.send_result(msg[ID]) + + @websocket_api.websocket_command( { vol.Required(TYPE): "ozw/network_status", @@ -148,14 +251,15 @@ def websocket_network_statistics(hass, connection, msg): ) def websocket_node_status(hass, connection, msg): """Get the status for a Z-Wave node.""" - manager = hass.data[DOMAIN][MANAGER] - node = manager.get_instance(msg[OZW_INSTANCE]).get_node(msg[NODE_ID]) - - if not node: - connection.send_message( - websocket_api.error_message( - msg[ID], websocket_api.const.ERR_NOT_FOUND, "OZW Node not found" - ) + try: + node = get_node_from_manager( + hass.data[DOMAIN][MANAGER], msg[OZW_INSTANCE], msg[NODE_ID] + ) + except NotFoundError as err: + connection.send_error( + msg[ID], + websocket_api.const.ERR_NOT_FOUND, + err.args[0], ) return @@ -192,14 +296,15 @@ def websocket_node_status(hass, connection, msg): ) def websocket_node_metadata(hass, connection, msg): """Get the metadata for a Z-Wave node.""" - manager = hass.data[DOMAIN][MANAGER] - node = manager.get_instance(msg[OZW_INSTANCE]).get_node(msg[NODE_ID]) - - if not node: - connection.send_message( - websocket_api.error_message( - msg[ID], websocket_api.const.ERR_NOT_FOUND, "OZW Node not found" - ) + try: + node = get_node_from_manager( + hass.data[DOMAIN][MANAGER], msg[OZW_INSTANCE], msg[NODE_ID] + ) + except NotFoundError as err: + connection.send_error( + msg[ID], + websocket_api.const.ERR_NOT_FOUND, + err.args[0], ) return diff --git a/requirements_all.txt b/requirements_all.txt index 7e5ae3b956e..8a4160024b8 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -1764,7 +1764,7 @@ python-nest==4.1.0 python-nmap==0.6.1 # homeassistant.components.ozw -python-openzwave-mqtt==1.0.5 +python-openzwave-mqtt==1.2.0 # homeassistant.components.qbittorrent python-qbittorrent==0.4.1 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index cb7f801a501..f9195a3b2fd 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -839,7 +839,7 @@ python-miio==0.5.3 python-nest==4.1.0 # homeassistant.components.ozw -python-openzwave-mqtt==1.0.5 +python-openzwave-mqtt==1.2.0 # homeassistant.components.songpal python-songpal==0.12 diff --git a/tests/components/ozw/test_services.py b/tests/components/ozw/test_services.py index 1460e69b9c9..7c71b234242 100644 --- a/tests/components/ozw/test_services.py +++ b/tests/components/ozw/test_services.py @@ -1,8 +1,12 @@ """Test Z-Wave Services.""" +from openzwavemqtt.const import ATTR_POSITION, ATTR_VALUE +from openzwavemqtt.exceptions import InvalidValueError, NotFoundError, WrongTypeError +import pytest + from .common import setup_ozw -async def test_services(hass, light_data, sent_messages, light_msg, caplog): +async def test_services(hass, light_data, sent_messages): """Test services on lock.""" await setup_ozw(hass, fixture=light_data) @@ -43,34 +47,48 @@ async def test_services(hass, light_data, sent_messages, light_msg, caplog): assert msg["payload"] == {"Value": 55, "ValueIDKey": 844425594667027} # Test set_config_parameter invalid list int - await hass.services.async_call( - "ozw", - "set_config_parameter", - {"node_id": 39, "parameter": 1, "value": 12}, - blocking=True, - ) + with pytest.raises(NotFoundError): + assert await hass.services.async_call( + "ozw", + "set_config_parameter", + {"node_id": 39, "parameter": 1, "value": 12}, + blocking=True, + ) assert len(sent_messages) == 3 - assert "Invalid value 12 for parameter 1" in caplog.text - # Test set_config_parameter invalid list string - await hass.services.async_call( - "ozw", - "set_config_parameter", - {"node_id": 39, "parameter": 1, "value": "Blah"}, - blocking=True, - ) + # Test set_config_parameter invalid list value + with pytest.raises(NotFoundError): + assert await hass.services.async_call( + "ozw", + "set_config_parameter", + {"node_id": 39, "parameter": 1, "value": "Blah"}, + blocking=True, + ) + assert len(sent_messages) == 3 + + # Test set_config_parameter invalid list value type + with pytest.raises(WrongTypeError): + assert await hass.services.async_call( + "ozw", + "set_config_parameter", + { + "node_id": 39, + "parameter": 1, + "value": {ATTR_VALUE: True, ATTR_POSITION: 1}, + }, + blocking=True, + ) assert len(sent_messages) == 3 - assert "Invalid value Blah for parameter 1" in caplog.text # Test set_config_parameter int out of range - await hass.services.async_call( - "ozw", - "set_config_parameter", - {"node_id": 39, "parameter": 3, "value": 2147483657}, - blocking=True, - ) + with pytest.raises(InvalidValueError): + assert await hass.services.async_call( + "ozw", + "set_config_parameter", + {"node_id": 39, "parameter": 3, "value": 2147483657}, + blocking=True, + ) assert len(sent_messages) == 3 - assert "Value 2147483657 out of range for parameter 3" in caplog.text # Test set_config_parameter short await hass.services.async_call( diff --git a/tests/components/ozw/test_websocket_api.py b/tests/components/ozw/test_websocket_api.py index 353615c1812..9bceab567c3 100644 --- a/tests/components/ozw/test_websocket_api.py +++ b/tests/components/ozw/test_websocket_api.py @@ -1,5 +1,13 @@ """Test OpenZWave Websocket API.""" +from openzwavemqtt.const import ( + ATTR_LABEL, + ATTR_OPTIONS, + ATTR_POSITION, + ATTR_VALUE, + ValueType, +) +from homeassistant.components.ozw.const import ATTR_CONFIG_PARAMETER from homeassistant.components.ozw.websocket_api import ( ATTR_IS_AWAKE, ATTR_IS_BEAMING, @@ -17,9 +25,15 @@ from homeassistant.components.ozw.websocket_api import ( ID, NODE_ID, OZW_INSTANCE, + PARAMETER, TYPE, + VALUE, +) +from homeassistant.components.websocket_api.const import ( + ERR_INVALID_FORMAT, + ERR_NOT_FOUND, + ERR_NOT_SUPPORTED, ) -from homeassistant.components.websocket_api.const import ERR_NOT_FOUND from .common import MQTTMessage, setup_ozw @@ -119,6 +133,149 @@ async def test_websocket_api(hass, generic_data, hass_ws_client): assert result[2][ATTR_IS_AWAKE] assert not result[1][ATTR_IS_FAILED] + # Test get config parameters + await client.send_json({ID: 13, TYPE: "ozw/get_config_parameters", NODE_ID: 39}) + msg = await client.receive_json() + result = msg["result"] + assert len(result) == 8 + for config_param in result: + assert config_param["type"] in ( + ValueType.LIST.value, + ValueType.BOOL.value, + ValueType.INT.value, + ValueType.BYTE.value, + ValueType.SHORT.value, + ValueType.BITSET.value, + ) + + # Test set config parameter + config_param = result[0] + current_val = config_param[ATTR_VALUE] + new_val = next( + option["Value"] + for option in config_param[ATTR_OPTIONS] + if option["Label"] != current_val + ) + new_label = next( + option["Label"] + for option in config_param[ATTR_OPTIONS] + if option["Label"] != current_val and option["Value"] != new_val + ) + await client.send_json( + { + ID: 14, + TYPE: "ozw/set_config_parameter", + NODE_ID: 39, + PARAMETER: config_param[ATTR_CONFIG_PARAMETER], + VALUE: new_val, + } + ) + msg = await client.receive_json() + assert msg["success"] + await client.send_json( + { + ID: 15, + TYPE: "ozw/set_config_parameter", + NODE_ID: 39, + PARAMETER: config_param[ATTR_CONFIG_PARAMETER], + VALUE: new_label, + } + ) + msg = await client.receive_json() + assert msg["success"] + + # Test OZW Instance not found error + await client.send_json( + {ID: 16, TYPE: "ozw/get_config_parameters", OZW_INSTANCE: 999, NODE_ID: 1} + ) + msg = await client.receive_json() + result = msg["error"] + assert result["code"] == ERR_NOT_FOUND + + # Test OZW Node not found error + await client.send_json( + { + ID: 18, + TYPE: "ozw/set_config_parameter", + NODE_ID: 999, + PARAMETER: 0, + VALUE: "test", + } + ) + msg = await client.receive_json() + result = msg["error"] + assert result["code"] == ERR_NOT_FOUND + + # Test parameter not found + await client.send_json( + { + ID: 19, + TYPE: "ozw/set_config_parameter", + NODE_ID: 39, + PARAMETER: 45, + VALUE: "test", + } + ) + msg = await client.receive_json() + result = msg["error"] + assert result["code"] == ERR_NOT_FOUND + + # Test list value not found + await client.send_json( + { + ID: 20, + TYPE: "ozw/set_config_parameter", + NODE_ID: 39, + PARAMETER: config_param[ATTR_CONFIG_PARAMETER], + VALUE: "test", + } + ) + msg = await client.receive_json() + result = msg["error"] + assert result["code"] == ERR_NOT_FOUND + + # Test value type invalid + await client.send_json( + { + ID: 21, + TYPE: "ozw/set_config_parameter", + NODE_ID: 39, + PARAMETER: 3, + VALUE: 0, + } + ) + msg = await client.receive_json() + result = msg["error"] + assert result["code"] == ERR_NOT_SUPPORTED + + # Test invalid bitset format + await client.send_json( + { + ID: 22, + TYPE: "ozw/set_config_parameter", + NODE_ID: 39, + PARAMETER: 3, + VALUE: {ATTR_POSITION: 1, ATTR_VALUE: True, ATTR_LABEL: "test"}, + } + ) + msg = await client.receive_json() + result = msg["error"] + assert result["code"] == ERR_INVALID_FORMAT + + # Test valid bitset format passes validation + await client.send_json( + { + ID: 23, + TYPE: "ozw/set_config_parameter", + NODE_ID: 39, + PARAMETER: 10000, + VALUE: {ATTR_POSITION: 1, ATTR_VALUE: True}, + } + ) + msg = await client.receive_json() + result = msg["error"] + assert result["code"] == ERR_NOT_FOUND + async def test_refresh_node(hass, generic_data, sent_messages, hass_ws_client): """Test the ozw refresh node api."""