From 7fe7990fba5ec56828d3647043e5715673d8c8a7 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Thu, 19 May 2022 15:26:26 -0400 Subject: [PATCH 1/4] Allow zwave_js/network_status WS API to accept device or entry ID --- homeassistant/components/zwave_js/api.py | 89 +++++++++++++++++------- tests/components/zwave_js/test_api.py | 85 ++++++++++++++++++++-- 2 files changed, 144 insertions(+), 30 deletions(-) diff --git a/homeassistant/components/zwave_js/api.py b/homeassistant/components/zwave_js/api.py index 1ab8e831b742c..6de571614b043 100644 --- a/homeassistant/components/zwave_js/api.py +++ b/homeassistant/components/zwave_js/api.py @@ -78,6 +78,8 @@ # general API constants ID = "id" +DEVICE_OR_ENTRY_ID = "device_or_entry_id" +ID_TYPE = "id_type" ENTRY_ID = "entry_id" ERR_NOT_LOADED = "not_loaded" NODE_ID = "node_id" @@ -236,6 +238,28 @@ def convert_qr_provisioning_information(info: dict) -> QRProvisioningInformation QR_CODE_STRING_SCHEMA = vol.All(str, vol.Length(min=MINIMUM_QR_STRING_LENGTH)) +async def _async_get_entry( + hass: HomeAssistant, connection: ActiveConnection, msg: dict, entry_id: str +) -> tuple[ConfigEntry | None, Client | None]: + """Get config entry and client from message data.""" + entry = hass.config_entries.async_get_entry(entry_id) + if entry is None: + connection.send_error( + msg[ID], ERR_NOT_FOUND, f"Config entry {entry_id} not found" + ) + return None, None + + if entry.state is not ConfigEntryState.LOADED: + connection.send_error( + msg[ID], ERR_NOT_LOADED, f"Config entry {entry_id} not loaded" + ) + return None, None + + client: Client = hass.data[DOMAIN][entry_id][DATA_CLIENT] + + return entry, client + + def async_get_entry(orig_func: Callable) -> Callable: """Decorate async function to get entry.""" @@ -244,22 +268,11 @@ async def async_get_entry_func( hass: HomeAssistant, connection: ActiveConnection, msg: dict ) -> None: """Provide user specific data and store to function.""" - entry_id = msg[ENTRY_ID] - entry = hass.config_entries.async_get_entry(entry_id) - if entry is None: - connection.send_error( - msg[ID], ERR_NOT_FOUND, f"Config entry {entry_id} not found" - ) - return + entry, client = await _async_get_entry(hass, connection, msg, msg[ENTRY_ID]) - if entry.state is not ConfigEntryState.LOADED: - connection.send_error( - msg[ID], ERR_NOT_LOADED, f"Config entry {entry_id} not loaded" - ) + if not entry and not client: return - client: Client = hass.data[DOMAIN][entry_id][DATA_CLIENT] - if client.driver is None: connection.send_error( msg[ID], @@ -273,6 +286,21 @@ async def async_get_entry_func( return async_get_entry_func +async def _async_get_node( + hass: HomeAssistant, connection: ActiveConnection, msg: dict, device_id: str +) -> Node | None: + """Get node from message data.""" + try: + node = async_get_node_from_device_id(hass, device_id) + except ValueError as err: + error_code = ERR_NOT_FOUND + if "loaded" in err.args[0]: + error_code = ERR_NOT_LOADED + connection.send_error(msg[ID], error_code, err.args[0]) + return None + return node + + def async_get_node(orig_func: Callable) -> Callable: """Decorate async function to get node.""" @@ -281,15 +309,8 @@ async def async_get_node_func( hass: HomeAssistant, connection: ActiveConnection, msg: dict ) -> None: """Provide user specific data and store to function.""" - device_id = msg[DEVICE_ID] - - try: - node = async_get_node_from_device_id(hass, device_id) - except ValueError as err: - error_code = ERR_NOT_FOUND - if "loaded" in err.args[0]: - error_code = ERR_NOT_LOADED - connection.send_error(msg[ID], error_code, err.args[0]) + node = await _async_get_node(hass, connection, msg, msg[DEVICE_ID]) + if not node: return await orig_func(hass, connection, msg, node) @@ -388,10 +409,13 @@ def async_register_api(hass: HomeAssistant) -> None: @websocket_api.require_admin @websocket_api.websocket_command( - {vol.Required(TYPE): "zwave_js/network_status", vol.Required(ENTRY_ID): str} + { + vol.Required(TYPE): "zwave_js/network_status", + vol.Required(DEVICE_OR_ENTRY_ID): str, + vol.Required(ID_TYPE): vol.In([ENTRY_ID, DEVICE_ID]), + } ) @websocket_api.async_response -@async_get_entry async def websocket_network_status( hass: HomeAssistant, connection: ActiveConnection, @@ -402,10 +426,21 @@ async def websocket_network_status( ) -> None: """Get the status of the Z-Wave JS network.""" controller = driver.controller + if msg[ID_TYPE] == ENTRY_ID: + _, client, driver = await _async_get_entry( + hass, connection, msg, msg[DEVICE_OR_ENTRY_ID] + ) + if not client: + return + else: + node = await _async_get_node(hass, connection, msg, msg[DEVICE_OR_ENTRY_ID]) + if not node: + return + client = node.client + controller = driver.controller + await controller.async_get_state() client_version_info = client.version assert client_version_info # When client is connected version info is set. - - await controller.async_get_state() data = { "client": { "ws_server_url": client.ws_server_url, @@ -1723,6 +1758,7 @@ async def websocket_get_log_config( driver: Driver, ) -> None: """Get log configuration for the Z-Wave JS driver.""" + assert client and client.driver connection.send_result( msg[ID], dataclasses.asdict(driver.log_config), @@ -1781,6 +1817,7 @@ async def websocket_data_collection_status( driver: Driver, ) -> None: """Return data collection preference and status.""" + assert client and client.driver result = { OPTED_IN: entry.data.get(CONF_DATA_COLLECTION_OPTED_IN), ENABLED: await driver.async_is_statistics_enabled(), diff --git a/tests/components/zwave_js/test_api.py b/tests/components/zwave_js/test_api.py index 2309e7b02bf53..c63fdf714d73d 100644 --- a/tests/components/zwave_js/test_api.py +++ b/tests/components/zwave_js/test_api.py @@ -36,6 +36,7 @@ COMMAND_CLASS_ID, CONFIG, DEVICE_ID, + DEVICE_OR_ENTRY_ID, DSK, ENABLED, ENTRY_ID, @@ -45,6 +46,7 @@ FORCE_CONSOLE, GENERIC_DEVICE_CLASS, ID, + ID_TYPE, INCLUSION_STRATEGY, INSTALLER_ICON_TYPE, LEVEL, @@ -82,14 +84,20 @@ def get_device(hass, node): return dev_reg.async_get_device({device_id}) -async def test_network_status(hass, integration, hass_ws_client): +async def test_network_status(hass, multisensor_6, integration, hass_ws_client): """Test the network status websocket command.""" entry = integration ws_client = await hass_ws_client(hass) + # Try API call with entry ID with patch("zwave_js_server.model.controller.Controller.async_get_state"): await ws_client.send_json( - {ID: 2, TYPE: "zwave_js/network_status", ENTRY_ID: entry.entry_id} + { + ID: 1, + TYPE: "zwave_js/network_status", + DEVICE_OR_ENTRY_ID: entry.entry_id, + ID_TYPE: ENTRY_ID, + } ) msg = await ws_client.receive_json() result = msg["result"] @@ -98,12 +106,81 @@ async def test_network_status(hass, integration, hass_ws_client): assert result["client"]["server_version"] == "1.0.0" assert result["controller"]["inclusion_state"] == InclusionState.IDLE - # Test sending command with not loaded entry fails + # Try API call with device ID + dev_reg = dr.async_get(hass) + device = dev_reg.async_get_device( + identifiers={(DOMAIN, "3245146787-52")}, + ) + assert device + with patch("zwave_js_server.model.controller.Controller.async_get_state"): + await ws_client.send_json( + { + ID: 2, + TYPE: "zwave_js/network_status", + DEVICE_OR_ENTRY_ID: device.id, + ID_TYPE: DEVICE_ID, + } + ) + msg = await ws_client.receive_json() + result = msg["result"] + + assert result["client"]["ws_server_url"] == "ws://test:3000/zjs" + assert result["client"]["server_version"] == "1.0.0" + assert result["controller"]["inclusion_state"] == InclusionState.IDLE + + # Test sending command with invalid config entry ID fails + await ws_client.send_json( + { + ID: 3, + TYPE: "zwave_js/network_status", + DEVICE_OR_ENTRY_ID: "fake_id", + ID_TYPE: ENTRY_ID, + } + ) + msg = await ws_client.receive_json() + + assert not msg["success"] + assert msg["error"]["code"] == ERR_NOT_FOUND + + # Test sending command with invalid device ID fails + await ws_client.send_json( + { + ID: 4, + TYPE: "zwave_js/network_status", + DEVICE_OR_ENTRY_ID: "fake_id", + ID_TYPE: DEVICE_ID, + } + ) + msg = await ws_client.receive_json() + + assert not msg["success"] + assert msg["error"]["code"] == ERR_NOT_FOUND + + # Test sending command with not loaded entry fails with config entry ID await hass.config_entries.async_unload(entry.entry_id) await hass.async_block_till_done() await ws_client.send_json( - {ID: 3, TYPE: "zwave_js/network_status", ENTRY_ID: entry.entry_id} + { + ID: 5, + TYPE: "zwave_js/network_status", + DEVICE_OR_ENTRY_ID: entry.entry_id, + ID_TYPE: ENTRY_ID, + } + ) + msg = await ws_client.receive_json() + + assert not msg["success"] + assert msg["error"]["code"] == ERR_NOT_LOADED + + # Test sending command with not loaded entry fails with device ID + await ws_client.send_json( + { + ID: 6, + TYPE: "zwave_js/network_status", + DEVICE_OR_ENTRY_ID: device.id, + ID_TYPE: DEVICE_ID, + } ) msg = await ws_client.receive_json() From 98462275f05d5040fe8ad5cb61db8d01cb29f110 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Wed, 25 May 2022 11:49:54 -0400 Subject: [PATCH 2/4] Fix based on upstream feedback --- homeassistant/components/zwave_js/api.py | 51 ++++++++++++------------ tests/components/zwave_js/test_api.py | 37 ++++++++++------- 2 files changed, 48 insertions(+), 40 deletions(-) diff --git a/homeassistant/components/zwave_js/api.py b/homeassistant/components/zwave_js/api.py index 6de571614b043..02578d4f87fca 100644 --- a/homeassistant/components/zwave_js/api.py +++ b/homeassistant/components/zwave_js/api.py @@ -78,8 +78,6 @@ # general API constants ID = "id" -DEVICE_OR_ENTRY_ID = "device_or_entry_id" -ID_TYPE = "id_type" ENTRY_ID = "entry_id" ERR_NOT_LOADED = "not_loaded" NODE_ID = "node_id" @@ -240,7 +238,7 @@ def convert_qr_provisioning_information(info: dict) -> QRProvisioningInformation async def _async_get_entry( hass: HomeAssistant, connection: ActiveConnection, msg: dict, entry_id: str -) -> tuple[ConfigEntry | None, Client | None]: +) -> tuple[ConfigEntry | None, Client | None, Driver | None]: """Get config entry and client from message data.""" entry = hass.config_entries.async_get_entry(entry_id) if entry is None: @@ -257,7 +255,15 @@ async def _async_get_entry( client: Client = hass.data[DOMAIN][entry_id][DATA_CLIENT] - return entry, client + if client.driver is None: + connection.send_error( + msg[ID], + ERR_NOT_LOADED, + f"Config entry {msg[ENTRY_ID]} not loaded, driver not ready", + ) + return + + return entry, client, client.driver def async_get_entry(orig_func: Callable) -> Callable: @@ -268,20 +274,12 @@ async def async_get_entry_func( hass: HomeAssistant, connection: ActiveConnection, msg: dict ) -> None: """Provide user specific data and store to function.""" - entry, client = await _async_get_entry(hass, connection, msg, msg[ENTRY_ID]) + entry, client, driver = await _async_get_entry(hass, connection, msg, msg[ENTRY_ID]) - if not entry and not client: - return - - if client.driver is None: - connection.send_error( - msg[ID], - ERR_NOT_LOADED, - f"Config entry {entry_id} not loaded, driver not ready", - ) + if not entry and not client and not driver: return - await orig_func(hass, connection, msg, entry, client, client.driver) + await orig_func(hass, connection, msg, entry, client, driver) return async_get_entry_func @@ -411,8 +409,8 @@ def async_register_api(hass: HomeAssistant) -> None: @websocket_api.websocket_command( { vol.Required(TYPE): "zwave_js/network_status", - vol.Required(DEVICE_OR_ENTRY_ID): str, - vol.Required(ID_TYPE): vol.In([ENTRY_ID, DEVICE_ID]), + vol.Exclusive(DEVICE_ID, "id"): str, + vol.Exclusive(ENTRY_ID, "id"): str, } ) @websocket_api.async_response @@ -425,19 +423,22 @@ async def websocket_network_status( driver: Driver, ) -> None: """Get the status of the Z-Wave JS network.""" - controller = driver.controller - if msg[ID_TYPE] == ENTRY_ID: - _, client, driver = await _async_get_entry( - hass, connection, msg, msg[DEVICE_OR_ENTRY_ID] - ) + if ENTRY_ID in msg: + _, client = await _async_get_entry(hass, connection, msg, msg[ENTRY_ID]) if not client: return - else: - node = await _async_get_node(hass, connection, msg, msg[DEVICE_OR_ENTRY_ID]) + elif DEVICE_ID in msg: + node = await _async_get_node(hass, connection, msg, msg[DEVICE_ID]) if not node: return client = node.client - controller = driver.controller + else: + connection.send_error( + msg[ID], ERR_INVALID_FORMAT, "Must specify either device_id or entry_id" + ) + return + assert client and client.driver and client.driver.controller + controller = client.driver.controller await controller.async_get_state() client_version_info = client.version assert client_version_info # When client is connected version info is set. diff --git a/tests/components/zwave_js/test_api.py b/tests/components/zwave_js/test_api.py index c63fdf714d73d..ba677e114e397 100644 --- a/tests/components/zwave_js/test_api.py +++ b/tests/components/zwave_js/test_api.py @@ -28,7 +28,10 @@ ) from zwave_js_server.model.node import Node -from homeassistant.components.websocket_api.const import ERR_NOT_FOUND +from homeassistant.components.websocket_api.const import ( + ERR_INVALID_FORMAT, + ERR_NOT_FOUND, +) from homeassistant.components.zwave_js.api import ( ADDITIONAL_PROPERTIES, APPLICATION_VERSION, @@ -36,7 +39,6 @@ COMMAND_CLASS_ID, CONFIG, DEVICE_ID, - DEVICE_OR_ENTRY_ID, DSK, ENABLED, ENTRY_ID, @@ -46,7 +48,6 @@ FORCE_CONSOLE, GENERIC_DEVICE_CLASS, ID, - ID_TYPE, INCLUSION_STRATEGY, INSTALLER_ICON_TYPE, LEVEL, @@ -95,8 +96,7 @@ async def test_network_status(hass, multisensor_6, integration, hass_ws_client): { ID: 1, TYPE: "zwave_js/network_status", - DEVICE_OR_ENTRY_ID: entry.entry_id, - ID_TYPE: ENTRY_ID, + ENTRY_ID: entry.entry_id, } ) msg = await ws_client.receive_json() @@ -117,8 +117,7 @@ async def test_network_status(hass, multisensor_6, integration, hass_ws_client): { ID: 2, TYPE: "zwave_js/network_status", - DEVICE_OR_ENTRY_ID: device.id, - ID_TYPE: DEVICE_ID, + DEVICE_ID: device.id, } ) msg = await ws_client.receive_json() @@ -133,8 +132,7 @@ async def test_network_status(hass, multisensor_6, integration, hass_ws_client): { ID: 3, TYPE: "zwave_js/network_status", - DEVICE_OR_ENTRY_ID: "fake_id", - ID_TYPE: ENTRY_ID, + ENTRY_ID: "fake_id", } ) msg = await ws_client.receive_json() @@ -147,8 +145,7 @@ async def test_network_status(hass, multisensor_6, integration, hass_ws_client): { ID: 4, TYPE: "zwave_js/network_status", - DEVICE_OR_ENTRY_ID: "fake_id", - ID_TYPE: DEVICE_ID, + DEVICE_ID: "fake_id", } ) msg = await ws_client.receive_json() @@ -164,8 +161,7 @@ async def test_network_status(hass, multisensor_6, integration, hass_ws_client): { ID: 5, TYPE: "zwave_js/network_status", - DEVICE_OR_ENTRY_ID: entry.entry_id, - ID_TYPE: ENTRY_ID, + ENTRY_ID: entry.entry_id, } ) msg = await ws_client.receive_json() @@ -178,8 +174,7 @@ async def test_network_status(hass, multisensor_6, integration, hass_ws_client): { ID: 6, TYPE: "zwave_js/network_status", - DEVICE_OR_ENTRY_ID: device.id, - ID_TYPE: DEVICE_ID, + DEVICE_ID: device.id, } ) msg = await ws_client.receive_json() @@ -187,6 +182,18 @@ async def test_network_status(hass, multisensor_6, integration, hass_ws_client): assert not msg["success"] assert msg["error"]["code"] == ERR_NOT_LOADED + # Test sending command with no device ID or entry ID fails + await ws_client.send_json( + { + ID: 6, + TYPE: "zwave_js/network_status", + } + ) + msg = await ws_client.receive_json() + + assert not msg["success"] + assert msg["error"]["code"] == ERR_INVALID_FORMAT + async def test_node_ready( hass, From ad6c0d4b7d68e3c9a55a3fa746d55b4eb6adac90 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Wed, 25 May 2022 11:53:00 -0400 Subject: [PATCH 3/4] Fixt ests --- tests/components/zwave_js/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/components/zwave_js/test_api.py b/tests/components/zwave_js/test_api.py index ba677e114e397..ee6a85db70fdf 100644 --- a/tests/components/zwave_js/test_api.py +++ b/tests/components/zwave_js/test_api.py @@ -185,7 +185,7 @@ async def test_network_status(hass, multisensor_6, integration, hass_ws_client): # Test sending command with no device ID or entry ID fails await ws_client.send_json( { - ID: 6, + ID: 7, TYPE: "zwave_js/network_status", } ) From e236e366cd8df9dd94b41b4612a251f0bcc495c2 Mon Sep 17 00:00:00 2001 From: Raman Gupta <7243222+raman325@users.noreply.github.com> Date: Wed, 25 May 2022 12:34:38 -0400 Subject: [PATCH 4/4] Fixes --- homeassistant/components/zwave_js/api.py | 26 +++++++++++------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/homeassistant/components/zwave_js/api.py b/homeassistant/components/zwave_js/api.py index 02578d4f87fca..05591e8ecfbc4 100644 --- a/homeassistant/components/zwave_js/api.py +++ b/homeassistant/components/zwave_js/api.py @@ -245,13 +245,13 @@ async def _async_get_entry( connection.send_error( msg[ID], ERR_NOT_FOUND, f"Config entry {entry_id} not found" ) - return None, None + return None, None, None if entry.state is not ConfigEntryState.LOADED: connection.send_error( msg[ID], ERR_NOT_LOADED, f"Config entry {entry_id} not loaded" ) - return None, None + return None, None, None client: Client = hass.data[DOMAIN][entry_id][DATA_CLIENT] @@ -261,7 +261,7 @@ async def _async_get_entry( ERR_NOT_LOADED, f"Config entry {msg[ENTRY_ID]} not loaded, driver not ready", ) - return + return None, None, None return entry, client, client.driver @@ -274,7 +274,9 @@ async def async_get_entry_func( hass: HomeAssistant, connection: ActiveConnection, msg: dict ) -> None: """Provide user specific data and store to function.""" - entry, client, driver = await _async_get_entry(hass, connection, msg, msg[ENTRY_ID]) + entry, client, driver = await _async_get_entry( + hass, connection, msg, msg[ENTRY_ID] + ) if not entry and not client and not driver: return @@ -415,30 +417,26 @@ def async_register_api(hass: HomeAssistant) -> None: ) @websocket_api.async_response async def websocket_network_status( - hass: HomeAssistant, - connection: ActiveConnection, - msg: dict, - entry: ConfigEntry, - client: Client, - driver: Driver, + hass: HomeAssistant, connection: ActiveConnection, msg: dict ) -> None: """Get the status of the Z-Wave JS network.""" if ENTRY_ID in msg: - _, client = await _async_get_entry(hass, connection, msg, msg[ENTRY_ID]) - if not client: + _, client, driver = await _async_get_entry(hass, connection, msg, msg[ENTRY_ID]) + if not client or not driver: return elif DEVICE_ID in msg: node = await _async_get_node(hass, connection, msg, msg[DEVICE_ID]) if not node: return client = node.client + assert client.driver + driver = client.driver else: connection.send_error( msg[ID], ERR_INVALID_FORMAT, "Must specify either device_id or entry_id" ) return - assert client and client.driver and client.driver.controller - controller = client.driver.controller + controller = driver.controller await controller.async_get_state() client_version_info = client.version assert client_version_info # When client is connected version info is set.