From 0de52c173cd51abc87ca9fb52009d29fe92a5672 Mon Sep 17 00:00:00 2001 From: epenet <6771947+epenet@users.noreply.github.com> Date: Mon, 26 Oct 2020 16:33:13 +0100 Subject: [PATCH] Implement PlatformNotReady on onewire integration (#42395) --- homeassistant/components/onewire/__init__.py | 23 ++++++- .../components/onewire/config_flow.py | 20 ++---- .../components/onewire/onewirehub.py | 44 ++++++++---- homeassistant/components/onewire/sensor.py | 26 ++++--- tests/components/onewire/__init__.py | 11 ++- tests/components/onewire/test_init.py | 69 ++++++++++++++++++- 6 files changed, 146 insertions(+), 47 deletions(-) diff --git a/homeassistant/components/onewire/__init__.py b/homeassistant/components/onewire/__init__.py index e228856a7fd..6d64478aa72 100644 --- a/homeassistant/components/onewire/__init__.py +++ b/homeassistant/components/onewire/__init__.py @@ -1,7 +1,12 @@ """The 1-Wire component.""" import asyncio -from .const import SUPPORTED_PLATFORMS +from homeassistant.config_entries import ConfigEntry +from homeassistant.exceptions import ConfigEntryNotReady +from homeassistant.helpers.typing import HomeAssistantType + +from .const import DOMAIN, SUPPORTED_PLATFORMS +from .onewirehub import CannotConnect, OneWireHub async def async_setup(hass, config): @@ -9,8 +14,18 @@ async def async_setup(hass, config): return True -async def async_setup_entry(hass, config_entry): +async def async_setup_entry(hass: HomeAssistantType, config_entry: ConfigEntry): """Set up a 1-Wire proxy for a config entry.""" + hass.data.setdefault(DOMAIN, {}) + + onewirehub = OneWireHub(hass) + try: + await onewirehub.initialize(config_entry) + except CannotConnect as exc: + raise ConfigEntryNotReady() from exc + + hass.data[DOMAIN][config_entry.unique_id] = onewirehub + for component in SUPPORTED_PLATFORMS: hass.async_create_task( hass.config_entries.async_forward_entry_setup(config_entry, component) @@ -18,7 +33,7 @@ async def async_setup_entry(hass, config_entry): return True -async def async_unload_entry(hass, config_entry): +async def async_unload_entry(hass: HomeAssistantType, config_entry: ConfigEntry): """Unload a config entry.""" unload_ok = all( await asyncio.gather( @@ -28,4 +43,6 @@ async def async_unload_entry(hass, config_entry): ] ) ) + if unload_ok: + hass.data[DOMAIN].pop(config_entry.unique_id) return unload_ok diff --git a/homeassistant/components/onewire/config_flow.py b/homeassistant/components/onewire/config_flow.py index 7cd963c6996..f83431111d8 100644 --- a/homeassistant/components/onewire/config_flow.py +++ b/homeassistant/components/onewire/config_flow.py @@ -1,7 +1,6 @@ """Config flow for 1-Wire component.""" import voluptuous as vol -from homeassistant import exceptions from homeassistant.config_entries import CONN_CLASS_LOCAL_POLL, ConfigFlow from homeassistant.const import CONF_HOST, CONF_PORT, CONF_TYPE from homeassistant.helpers.typing import HomeAssistantType @@ -16,7 +15,7 @@ from .const import ( # pylint: disable=unused-import DEFAULT_SYSBUS_MOUNT_DIR, DOMAIN, ) -from .onewirehub import OneWireHub +from .onewirehub import CannotConnect, InvalidPath, OneWireHub DATA_SCHEMA_USER = vol.Schema( {vol.Required(CONF_TYPE): vol.In([CONF_TYPE_OWSERVER, CONF_TYPE_SYSBUS])} @@ -44,8 +43,8 @@ async def validate_input_owserver(hass: HomeAssistantType, data): host = data[CONF_HOST] port = data[CONF_PORT] - if not await hub.can_connect(host, port): - raise CannotConnect + # Raises CannotConnect exception on failure + await hub.connect(host, port) # Return info that you want to store in the config entry. return {"title": host} @@ -71,8 +70,9 @@ async def validate_input_mount_dir(hass: HomeAssistantType, data): hub = OneWireHub(hass) mount_dir = data[CONF_MOUNT_DIR] - if not await hub.is_valid_mount_dir(mount_dir): - raise InvalidPath + + # Raises InvalidDir exception on failure + await hub.check_mount_dir(mount_dir) # Return info that you want to store in the config entry. return {"title": mount_dir} @@ -185,11 +185,3 @@ class OneWireFlowHandler(ConfigFlow, domain=DOMAIN): if CONF_MOUNT_DIR not in platform_config: platform_config[CONF_MOUNT_DIR] = DEFAULT_SYSBUS_MOUNT_DIR return await self.async_step_mount_dir(platform_config) - - -class CannotConnect(exceptions.HomeAssistantError): - """Error to indicate we cannot connect.""" - - -class InvalidPath(exceptions.HomeAssistantError): - """Error to indicate the path is invalid.""" diff --git a/homeassistant/components/onewire/onewirehub.py b/homeassistant/components/onewire/onewirehub.py index 22bb0098f92..1d2aef27b09 100644 --- a/homeassistant/components/onewire/onewirehub.py +++ b/homeassistant/components/onewire/onewirehub.py @@ -1,12 +1,14 @@ """Hub for communication with 1-Wire server or mount_dir.""" -import logging import os from pyownet import protocol +from homeassistant.config_entries import ConfigEntry +from homeassistant.const import CONF_HOST, CONF_PORT, CONF_TYPE +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.typing import HomeAssistantType -_LOGGER = logging.getLogger(__name__) +from .const import CONF_MOUNT_DIR, CONF_TYPE_OWSERVER, CONF_TYPE_SYSBUS class OneWireHub: @@ -15,21 +17,35 @@ class OneWireHub: def __init__(self, hass: HomeAssistantType): """Initialize.""" self.hass = hass + self.owproxy: protocol._Proxy = None - async def can_connect(self, host, port) -> bool: - """Test if we can authenticate with the host.""" + async def connect(self, host: str, port: int) -> None: + """Connect to the owserver host.""" try: - await self.hass.async_add_executor_job(protocol.proxy, host, port) - except (protocol.Error, protocol.ConnError) as exc: - _LOGGER.error( - "Cannot connect to owserver on %s:%d, got: %s", host, port, exc + self.owproxy = await self.hass.async_add_executor_job( + protocol.proxy, host, port ) - return False - return True + except protocol.ConnError as exc: + raise CannotConnect from exc - async def is_valid_mount_dir(self, mount_dir) -> bool: + async def check_mount_dir(self, mount_dir: str) -> None: """Test that the mount_dir is a valid path.""" if not await self.hass.async_add_executor_job(os.path.isdir, mount_dir): - _LOGGER.error("Cannot find directory %s", mount_dir) - return False - return True + raise InvalidPath + + async def initialize(self, config_entry: ConfigEntry) -> None: + """Initialize a config entry.""" + if config_entry.data[CONF_TYPE] == CONF_TYPE_SYSBUS: + await self.check_mount_dir(config_entry.data[CONF_MOUNT_DIR]) + elif config_entry.data[CONF_TYPE] == CONF_TYPE_OWSERVER: + host = config_entry.data[CONF_HOST] + port = config_entry.data[CONF_PORT] + await self.connect(host, port) + + +class CannotConnect(HomeAssistantError): + """Error to indicate we cannot connect.""" + + +class InvalidPath(HomeAssistantError): + """Error to indicate the path is invalid.""" diff --git a/homeassistant/components/onewire/sensor.py b/homeassistant/components/onewire/sensor.py index 31bafdb483a..e1b6d736194 100644 --- a/homeassistant/components/onewire/sensor.py +++ b/homeassistant/components/onewire/sensor.py @@ -8,6 +8,7 @@ from pi1wire import InvalidCRCException, Pi1Wire, UnsupportResponseException from pyownet import protocol import voluptuous as vol +from homeassistant.components.onewire.onewirehub import OneWireHub from homeassistant.components.sensor import PLATFORM_SCHEMA from homeassistant.config_entries import SOURCE_IMPORT from homeassistant.const import ( @@ -156,11 +157,14 @@ async def async_setup_platform(hass, config, async_add_entities, discovery_info= async def async_setup_entry(hass, config_entry, async_add_entities): """Set up 1-Wire platform.""" - entities = await hass.async_add_executor_job(get_entities, config_entry.data) + onewirehub = hass.data[DOMAIN][config_entry.unique_id] + entities = await hass.async_add_executor_job( + get_entities, onewirehub, config_entry.data + ) async_add_entities(entities, True) -def get_entities(config): +def get_entities(onewirehub: OneWireHub, config): """Get a list of entities.""" entities = [] device_names = {} @@ -174,19 +178,17 @@ def get_entities(config): owhost = config[CONF_HOST] owport = config[CONF_PORT] - _LOGGER.debug("Initializing using %s:%s", owhost, owport) try: - owproxy = protocol.proxy(host=owhost, port=owport) - devices = owproxy.dir() - except protocol.Error as exc: + devices = onewirehub.owproxy.dir() + except protocol.OwnetError as exc: _LOGGER.error( - "Cannot connect to owserver on %s:%d, got: %s", owhost, owport, exc + "Failed to list devices on %s:%d, got: %s", owhost, owport, exc ) return entities for device in devices: _LOGGER.debug("Found device: %s", device) - family = owproxy.read(f"{device}family").decode() - device_type = owproxy.read(f"{device}type").decode() + family = onewirehub.owproxy.read(f"{device}family").decode() + device_type = onewirehub.owproxy.read(f"{device}type").decode() sensor_id = os.path.split(os.path.split(device)[0])[1] dev_type = "std" if "EF" in family: @@ -210,7 +212,9 @@ def get_entities(config): if "moisture" in sensor_key: s_id = sensor_key.split("_")[1] is_leaf = int( - owproxy.read(f"{device}moisture/is_leaf.{s_id}").decode() + onewirehub.owproxy.read( + f"{device}moisture/is_leaf.{s_id}" + ).decode() ) if is_leaf: sensor_key = f"wetness_{s_id}" @@ -221,7 +225,7 @@ def get_entities(config): device_file, sensor_key, device_info, - owproxy, + onewirehub.owproxy, ) ) diff --git a/tests/components/onewire/__init__.py b/tests/components/onewire/__init__.py index 9d9f4148fb4..7074bc01a7c 100644 --- a/tests/components/onewire/__init__.py +++ b/tests/components/onewire/__init__.py @@ -2,6 +2,7 @@ from asynctest.mock import patch from homeassistant.components.onewire.const import ( + CONF_MOUNT_DIR, CONF_TYPE_OWSERVER, CONF_TYPE_SYSBUS, DEFAULT_SYSBUS_MOUNT_DIR, @@ -20,6 +21,7 @@ async def setup_onewire_sysbus_integration(hass): source="user", data={ CONF_TYPE: CONF_TYPE_SYSBUS, + CONF_MOUNT_DIR: DEFAULT_SYSBUS_MOUNT_DIR, }, unique_id=f"{CONF_TYPE_SYSBUS}:{DEFAULT_SYSBUS_MOUNT_DIR}", connection_class=CONN_CLASS_LOCAL_POLL, @@ -28,8 +30,11 @@ async def setup_onewire_sysbus_integration(hass): ) config_entry.add_to_hass(hass) - await hass.config_entries.async_setup(config_entry.entry_id) - await hass.async_block_till_done() + with patch( + "homeassistant.components.onewire.onewirehub.os.path.isdir", return_value=True + ): + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() return config_entry @@ -52,7 +57,7 @@ async def setup_onewire_owserver_integration(hass): config_entry.add_to_hass(hass) with patch( - "homeassistant.components.onewire.sensor.protocol.proxy", + "homeassistant.components.onewire.onewirehub.protocol.proxy", ): await hass.config_entries.async_setup(config_entry.entry_id) await hass.async_block_till_done() diff --git a/tests/components/onewire/test_init.py b/tests/components/onewire/test_init.py index 6c09d0a595f..a371b8c6b2b 100644 --- a/tests/components/onewire/test_init.py +++ b/tests/components/onewire/test_init.py @@ -1,9 +1,74 @@ """Tests for 1-Wire config flow.""" -from homeassistant.components.onewire.const import DOMAIN -from homeassistant.config_entries import ENTRY_STATE_LOADED, ENTRY_STATE_NOT_LOADED +from asynctest.mock import patch +from pyownet.protocol import ConnError, OwnetError + +from homeassistant.components.onewire.const import CONF_TYPE_OWSERVER, DOMAIN +from homeassistant.config_entries import ( + CONN_CLASS_LOCAL_POLL, + ENTRY_STATE_LOADED, + ENTRY_STATE_NOT_LOADED, + ENTRY_STATE_SETUP_RETRY, +) +from homeassistant.const import CONF_HOST, CONF_PORT, CONF_TYPE from . import setup_onewire_owserver_integration, setup_onewire_sysbus_integration +from tests.common import MockConfigEntry + + +async def test_owserver_platform_not_ready(hass): + """Create the 1-Wire integration.""" + config_entry_owserver = MockConfigEntry( + domain=DOMAIN, + source="user", + data={ + CONF_TYPE: CONF_TYPE_OWSERVER, + CONF_HOST: "1.2.3.4", + CONF_PORT: "1234", + }, + unique_id=f"{CONF_TYPE_OWSERVER}:1.2.3.4:1234", + connection_class=CONN_CLASS_LOCAL_POLL, + options={}, + entry_id="2", + ) + config_entry_owserver.add_to_hass(hass) + + with patch( + "homeassistant.components.onewire.onewirehub.protocol.proxy", + side_effect=ConnError, + ): + await hass.config_entries.async_setup(config_entry_owserver.entry_id) + await hass.async_block_till_done() + + assert len(hass.config_entries.async_entries(DOMAIN)) == 1 + assert config_entry_owserver.state == ENTRY_STATE_SETUP_RETRY + assert not hass.data.get(DOMAIN) + + +async def test_failed_owserver_listing(hass): + """Create the 1-Wire integration.""" + config_entry_owserver = MockConfigEntry( + domain=DOMAIN, + source="user", + data={ + CONF_TYPE: CONF_TYPE_OWSERVER, + CONF_HOST: "1.2.3.4", + CONF_PORT: "1234", + }, + unique_id=f"{CONF_TYPE_OWSERVER}:1.2.3.4:1234", + connection_class=CONN_CLASS_LOCAL_POLL, + options={}, + entry_id="2", + ) + config_entry_owserver.add_to_hass(hass) + + with patch("homeassistant.components.onewire.onewirehub.protocol.proxy") as owproxy: + owproxy.return_value.dir.side_effect = OwnetError + await hass.config_entries.async_setup(config_entry_owserver.entry_id) + await hass.async_block_till_done() + + return config_entry_owserver + async def test_unload_entry(hass): """Test being able to unload an entry."""