From 3c3860c923b34cdd7b5c8442a0b9e9daefb14ba6 Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Tue, 28 Mar 2023 12:34:25 +0200 Subject: [PATCH] Make OTBR use same channel as ZHA (#88546) --- .../silabs_multiprotocol_addon.py | 8 +++ homeassistant/components/otbr/config_flow.py | 23 ++++---- homeassistant/components/otbr/manifest.json | 4 +- homeassistant/components/otbr/util.py | 43 ++++++++++++++ .../components/otbr/websocket_api.py | 19 +++--- .../test_silabs_multiprotocol_addon.py | 11 ++++ tests/components/otbr/test_config_flow.py | 13 ++++- tests/components/otbr/test_util.py | 58 +++++++++++++++++++ tests/components/otbr/test_websocket_api.py | 26 ++++++--- 9 files changed, 171 insertions(+), 34 deletions(-) create mode 100644 homeassistant/components/otbr/util.py create mode 100644 tests/components/otbr/test_util.py diff --git a/homeassistant/components/homeassistant_hardware/silabs_multiprotocol_addon.py b/homeassistant/components/homeassistant_hardware/silabs_multiprotocol_addon.py index 41f16462cd8..ff2bf9138f5 100644 --- a/homeassistant/components/homeassistant_hardware/silabs_multiprotocol_addon.py +++ b/homeassistant/components/homeassistant_hardware/silabs_multiprotocol_addon.py @@ -8,6 +8,7 @@ import logging from typing import Any import voluptuous as vol +import yarl from homeassistant import config_entries from homeassistant.components.hassio import ( @@ -74,6 +75,13 @@ def get_zigbee_socket() -> str: return f"socket://{hostname}:9999" +def is_multiprotocol_url(url: str) -> bool: + """Return if the URL points at the Multiprotocol add-on.""" + parsed = yarl.URL(url) + hostname = hostname_from_addon_slug(SILABS_MULTIPROTOCOL_ADDON_SLUG) + return parsed.host == hostname + + class BaseMultiPanFlow(FlowHandler, ABC): """Support configuring the Silicon Labs Multiprotocol add-on.""" diff --git a/homeassistant/components/otbr/config_flow.py b/homeassistant/components/otbr/config_flow.py index 4247d5dbd65..434b9026ae2 100644 --- a/homeassistant/components/otbr/config_flow.py +++ b/homeassistant/components/otbr/config_flow.py @@ -18,6 +18,7 @@ from homeassistant.data_entry_flow import FlowResult from homeassistant.helpers.aiohttp_client import async_get_clientsession from .const import DEFAULT_CHANNEL, DOMAIN +from .util import get_allowed_channel _LOGGER = logging.getLogger(__name__) @@ -27,13 +28,12 @@ class OTBRConfigFlow(ConfigFlow, domain=DOMAIN): VERSION = 1 - async def _connect_and_create_dataset(self, url: str) -> None: - """Connect to the OTBR and create a dataset if it doesn't have one.""" - api = python_otbr_api.OTBR(url, async_get_clientsession(self.hass), 10) + async def _connect_and_set_dataset(self, otbr_url: str) -> None: + """Connect to the OTBR and create or apply a dataset if it doesn't have one.""" + api = python_otbr_api.OTBR(otbr_url, async_get_clientsession(self.hass), 10) if await api.get_active_dataset_tlvs() is None: - # We currently have no way to know which channel zha is using, assume it's - # the default - zha_channel = DEFAULT_CHANNEL + allowed_channel = await get_allowed_channel(self.hass, otbr_url) + thread_dataset_channel = None thread_dataset_tlv = await async_get_preferred_dataset(self.hass) if thread_dataset_tlv: @@ -41,7 +41,9 @@ class OTBRConfigFlow(ConfigFlow, domain=DOMAIN): if channel_str := dataset.get(tlv_parser.MeshcopTLVType.CHANNEL): thread_dataset_channel = int(channel_str, base=16) - if thread_dataset_tlv is not None and zha_channel == thread_dataset_channel: + if thread_dataset_tlv is not None and ( + not allowed_channel or allowed_channel == thread_dataset_channel + ): await api.set_active_dataset_tlvs(bytes.fromhex(thread_dataset_tlv)) else: _LOGGER.debug( @@ -49,7 +51,8 @@ class OTBRConfigFlow(ConfigFlow, domain=DOMAIN): ) await api.create_active_dataset( python_otbr_api.OperationalDataSet( - channel=zha_channel, network_name="home-assistant" + channel=allowed_channel if allowed_channel else DEFAULT_CHANNEL, + network_name="home-assistant", ) ) await api.set_enabled(True) @@ -66,7 +69,7 @@ class OTBRConfigFlow(ConfigFlow, domain=DOMAIN): if user_input is not None: url = user_input[CONF_URL] try: - await self._connect_and_create_dataset(url) + await self._connect_and_set_dataset(url) except ( python_otbr_api.OTBRError, aiohttp.ClientError, @@ -108,7 +111,7 @@ class OTBRConfigFlow(ConfigFlow, domain=DOMAIN): return self.async_abort(reason="single_instance_allowed") try: - await self._connect_and_create_dataset(url) + await self._connect_and_set_dataset(url) except python_otbr_api.OTBRError as exc: _LOGGER.warning("Failed to communicate with OTBR@%s: %s", url, exc) return self.async_abort(reason="unknown") diff --git a/homeassistant/components/otbr/manifest.json b/homeassistant/components/otbr/manifest.json index 2590e92210f..8e9050ca9f4 100644 --- a/homeassistant/components/otbr/manifest.json +++ b/homeassistant/components/otbr/manifest.json @@ -1,10 +1,10 @@ { "domain": "otbr", "name": "Open Thread Border Router", - "after_dependencies": ["hassio"], + "after_dependencies": ["hassio", "zha"], "codeowners": ["@home-assistant/core"], "config_flow": true, - "dependencies": ["thread"], + "dependencies": ["homeassistant_hardware", "thread"], "documentation": "https://www.home-assistant.io/integrations/otbr", "integration_type": "service", "iot_class": "local_polling", diff --git a/homeassistant/components/otbr/util.py b/homeassistant/components/otbr/util.py new file mode 100644 index 00000000000..b1a3ee11b82 --- /dev/null +++ b/homeassistant/components/otbr/util.py @@ -0,0 +1,43 @@ +"""Utility functions for the Open Thread Border Router integration.""" +from __future__ import annotations + +import contextlib + +from homeassistant.components.homeassistant_hardware.silabs_multiprotocol_addon import ( + is_multiprotocol_url, +) +from homeassistant.components.zha import api as zha_api +from homeassistant.core import HomeAssistant + + +def _get_zha_url(hass: HomeAssistant) -> str | None: + """Get ZHA radio path, or None if there's no ZHA config entry.""" + with contextlib.suppress(ValueError): + return zha_api.async_get_radio_path(hass) + return None + + +async def _get_zha_channel(hass: HomeAssistant) -> int | None: + """Get ZHA channel, or None if there's no ZHA config entry.""" + zha_network_settings: zha_api.NetworkBackup | None + with contextlib.suppress(ValueError): + zha_network_settings = await zha_api.async_get_network_settings(hass) + if not zha_network_settings: + return None + channel: int = zha_network_settings.network_info.channel + # ZHA uses channel 0 when no channel is set + return channel or None + + +async def get_allowed_channel(hass: HomeAssistant, otbr_url: str) -> int | None: + """Return the allowed channel, or None if there's no restriction.""" + if not is_multiprotocol_url(otbr_url): + # The OTBR is not sharing the radio, no restriction + return None + + zha_url = _get_zha_url(hass) + if not zha_url or not is_multiprotocol_url(zha_url): + # ZHA is not configured or not sharing the radio with this OTBR, no restriction + return None + + return await _get_zha_channel(hass) diff --git a/homeassistant/components/otbr/websocket_api.py b/homeassistant/components/otbr/websocket_api.py index aa8c1dd2dd9..cd4f8875e7b 100644 --- a/homeassistant/components/otbr/websocket_api.py +++ b/homeassistant/components/otbr/websocket_api.py @@ -11,6 +11,7 @@ from homeassistant.core import HomeAssistant, callback from homeassistant.exceptions import HomeAssistantError from .const import DEFAULT_CHANNEL, DOMAIN +from .util import get_allowed_channel if TYPE_CHECKING: from . import OTBRData @@ -72,11 +73,8 @@ async def websocket_create_network( connection.send_error(msg["id"], "not_loaded", "No OTBR API loaded") return - # We currently have no way to know which channel zha is using, assume it's - # the default - zha_channel = DEFAULT_CHANNEL - data: OTBRData = hass.data[DOMAIN] + channel = await get_allowed_channel(hass, data.url) or DEFAULT_CHANNEL try: await data.set_enabled(False) @@ -87,7 +85,7 @@ async def websocket_create_network( try: await data.create_active_dataset( python_otbr_api.OperationalDataSet( - channel=zha_channel, network_name="home-assistant" + channel=channel, network_name="home-assistant" ) ) except HomeAssistantError as exc: @@ -139,21 +137,18 @@ async def websocket_set_network( if channel_str := dataset.get(tlv_parser.MeshcopTLVType.CHANNEL): thread_dataset_channel = int(channel_str, base=16) - # We currently have no way to know which channel zha is using, assume it's - # the default - zha_channel = DEFAULT_CHANNEL + data: OTBRData = hass.data[DOMAIN] + allowed_channel = await get_allowed_channel(hass, data.url) - if thread_dataset_channel != zha_channel: + if allowed_channel and thread_dataset_channel != allowed_channel: connection.send_error( msg["id"], "channel_conflict", f"Can't connect to network on channel {thread_dataset_channel}, ZHA is " - f"using channel {zha_channel}", + f"using channel {allowed_channel}", ) return - data: OTBRData = hass.data[DOMAIN] - try: await data.set_enabled(False) except HomeAssistantError as exc: diff --git a/tests/components/homeassistant_hardware/test_silabs_multiprotocol_addon.py b/tests/components/homeassistant_hardware/test_silabs_multiprotocol_addon.py index 424e4126e05..a195899136d 100644 --- a/tests/components/homeassistant_hardware/test_silabs_multiprotocol_addon.py +++ b/tests/components/homeassistant_hardware/test_silabs_multiprotocol_addon.py @@ -795,3 +795,14 @@ async def test_option_flow_install_multi_pan_addon_zha_migration_fails_step_2( result = await hass.config_entries.options.async_configure(result["flow_id"]) assert result["type"] == FlowResultType.ABORT assert result["reason"] == "zha_migration_failed" + + +def test_is_multiprotocol_url() -> None: + """Test is_multiprotocol_url.""" + assert silabs_multiprotocol_addon.is_multiprotocol_url( + "socket://core-silabs-multiprotocol:9999" + ) + assert silabs_multiprotocol_addon.is_multiprotocol_url( + "http://core-silabs-multiprotocol:8081" + ) + assert not silabs_multiprotocol_addon.is_multiprotocol_url("/dev/ttyAMA1") diff --git a/tests/components/otbr/test_config_flow.py b/tests/components/otbr/test_config_flow.py index ae49c63002a..b788c93610d 100644 --- a/tests/components/otbr/test_config_flow.py +++ b/tests/components/otbr/test_config_flow.py @@ -2,7 +2,7 @@ import asyncio from http import HTTPStatus from typing import Any -from unittest.mock import patch +from unittest.mock import Mock, patch import aiohttp import pytest @@ -320,13 +320,22 @@ async def test_hassio_discovery_flow_router_not_setup_has_preferred_2( aioclient_mock.post(f"{url}/node/dataset/active", status=HTTPStatus.ACCEPTED) aioclient_mock.post(f"{url}/node/state", status=HTTPStatus.OK) + networksettings = Mock() + networksettings.network_info.channel = 15 + with patch( "homeassistant.components.otbr.config_flow.async_get_preferred_dataset", return_value=DATASET_CH16.hex(), ), patch( "homeassistant.components.otbr.async_setup_entry", return_value=True, - ) as mock_setup_entry: + ) as mock_setup_entry, patch( + "homeassistant.components.otbr.util.zha_api.async_get_radio_path", + return_value="socket://core-silabs-multiprotocol:9999", + ), patch( + "homeassistant.components.otbr.util.zha_api.async_get_network_settings", + return_value=networksettings, + ): result = await hass.config_entries.flow.async_init( otbr.DOMAIN, context={"source": "hassio"}, data=HASSIO_DATA ) diff --git a/tests/components/otbr/test_util.py b/tests/components/otbr/test_util.py new file mode 100644 index 00000000000..af5306b3581 --- /dev/null +++ b/tests/components/otbr/test_util.py @@ -0,0 +1,58 @@ +"""Test OTBR Utility functions.""" +from unittest.mock import Mock, patch + +from homeassistant.components import otbr +from homeassistant.core import HomeAssistant + +OTBR_MULTIPAN_URL = "http://core-silabs-multiprotocol:8081" +OTBR_NON_MULTIPAN_URL = "/dev/ttyAMA1" + + +async def test_get_allowed_channel(hass: HomeAssistant) -> None: + """Test get_allowed_channel.""" + + zha_networksettings = Mock() + zha_networksettings.network_info.channel = 15 + + # OTBR multipan + No ZHA -> no restriction + assert await otbr.util.get_allowed_channel(hass, OTBR_MULTIPAN_URL) is None + + # OTBR multipan + ZHA multipan empty settings -> no restriction + with patch( + "homeassistant.components.otbr.util.zha_api.async_get_radio_path", + return_value="socket://core-silabs-multiprotocol:9999", + ), patch( + "homeassistant.components.otbr.util.zha_api.async_get_network_settings", + return_value=None, + ): + assert await otbr.util.get_allowed_channel(hass, OTBR_MULTIPAN_URL) is None + + # OTBR multipan + ZHA not multipan using channel 15 -> no restriction + with patch( + "homeassistant.components.otbr.util.zha_api.async_get_radio_path", + return_value="/dev/ttyAMA1", + ), patch( + "homeassistant.components.otbr.util.zha_api.async_get_network_settings", + return_value=zha_networksettings, + ): + assert await otbr.util.get_allowed_channel(hass, OTBR_MULTIPAN_URL) is None + + # OTBR multipan + ZHA multipan using channel 15 -> 15 + with patch( + "homeassistant.components.otbr.util.zha_api.async_get_radio_path", + return_value="socket://core-silabs-multiprotocol:9999", + ), patch( + "homeassistant.components.otbr.util.zha_api.async_get_network_settings", + return_value=zha_networksettings, + ): + assert await otbr.util.get_allowed_channel(hass, OTBR_MULTIPAN_URL) == 15 + + # OTBR not multipan + ZHA multipan using channel 15 -> no restriction + with patch( + "homeassistant.components.otbr.util.zha_api.async_get_radio_path", + return_value="socket://core-silabs-multiprotocol:9999", + ), patch( + "homeassistant.components.otbr.util.zha_api.async_get_network_settings", + return_value=zha_networksettings, + ): + assert await otbr.util.get_allowed_channel(hass, OTBR_NON_MULTIPAN_URL) is None diff --git a/tests/components/otbr/test_websocket_api.py b/tests/components/otbr/test_websocket_api.py index 84421622570..e6f492f5e5f 100644 --- a/tests/components/otbr/test_websocket_api.py +++ b/tests/components/otbr/test_websocket_api.py @@ -1,5 +1,5 @@ """Test OTBR Websocket API.""" -from unittest.mock import patch +from unittest.mock import Mock, patch import pytest import python_otbr_api @@ -283,14 +283,24 @@ async def test_set_network_channel_conflict( dataset_store = await thread.dataset_store.async_get_store(hass) dataset_id = list(dataset_store.datasets)[0] - await websocket_client.send_json_auto_id( - { - "type": "otbr/set_network", - "dataset_id": dataset_id, - } - ) + networksettings = Mock() + networksettings.network_info.channel = 15 - msg = await websocket_client.receive_json() + with patch( + "homeassistant.components.otbr.util.zha_api.async_get_radio_path", + return_value="socket://core-silabs-multiprotocol:9999", + ), patch( + "homeassistant.components.otbr.util.zha_api.async_get_network_settings", + return_value=networksettings, + ): + await websocket_client.send_json_auto_id( + { + "type": "otbr/set_network", + "dataset_id": dataset_id, + } + ) + + msg = await websocket_client.receive_json() assert not msg["success"] assert msg["error"]["code"] == "channel_conflict"