From 86176f1bf9ed90650693d608de4255bcbdb2f4a7 Mon Sep 17 00:00:00 2001 From: epenet <6771947+epenet@users.noreply.github.com> Date: Sat, 3 Apr 2021 23:08:35 +0200 Subject: [PATCH] Add retry mechanism on onewire sysbus devices (#48614) * Add retry mechanism on sysbus * Update tests * Move to async * Move blocking calls on the executor --- homeassistant/components/onewire/sensor.py | 25 ++++++++- tests/components/onewire/__init__.py | 30 ++++++++++- tests/components/onewire/const.py | 60 +++++++++++++++++----- tests/components/onewire/test_sensor.py | 29 ++++++----- 4 files changed, 115 insertions(+), 29 deletions(-) diff --git a/homeassistant/components/onewire/sensor.py b/homeassistant/components/onewire/sensor.py index 02af7a89ae3..b3a5be0a1ca 100644 --- a/homeassistant/components/onewire/sensor.py +++ b/homeassistant/components/onewire/sensor.py @@ -1,6 +1,7 @@ """Support for 1-Wire environment sensors.""" from __future__ import annotations +import asyncio from glob import glob import logging import os @@ -426,11 +427,31 @@ class OneWireDirectSensor(OneWireSensor): """Return the state of the entity.""" return self._state - def update(self): + async def get_temperature(self): + """Get the latest data from the device.""" + attempts = 1 + while True: + try: + return await self.hass.async_add_executor_job( + self._owsensor.get_temperature + ) + except UnsupportResponseException as ex: + _LOGGER.debug( + "Cannot read from sensor %s (retry attempt %s): %s", + self._device_file, + attempts, + ex, + ) + await asyncio.sleep(0.2) + attempts += 1 + if attempts > 10: + raise + + async def async_update(self): """Get the latest data from the device.""" value = None try: - self._value_raw = self._owsensor.get_temperature() + self._value_raw = await self.get_temperature() value = round(float(self._value_raw), 1) except ( FileNotFoundError, diff --git a/tests/components/onewire/__init__.py b/tests/components/onewire/__init__.py index 7b85c16d4c8..f133f89d5d6 100644 --- a/tests/components/onewire/__init__.py +++ b/tests/components/onewire/__init__.py @@ -1,5 +1,6 @@ """Tests for 1-Wire integration.""" +from typing import Any, List, Tuple from unittest.mock import patch from pyownet.protocol import ProtocolError @@ -15,7 +16,7 @@ from homeassistant.components.onewire.const import ( from homeassistant.config_entries import CONN_CLASS_LOCAL_POLL from homeassistant.const import CONF_HOST, CONF_PORT, CONF_TYPE -from .const import MOCK_OWPROXY_DEVICES +from .const import MOCK_OWPROXY_DEVICES, MOCK_SYSBUS_DEVICES from tests.common import MockConfigEntry @@ -125,3 +126,30 @@ def setup_owproxy_mock_devices(owproxy, domain, device_ids) -> None: ) owproxy.return_value.dir.return_value = dir_return_value owproxy.return_value.read.side_effect = read_side_effect + + +def setup_sysbus_mock_devices( + domain: str, device_ids: List[str] +) -> Tuple[List[str], List[Any]]: + """Set up mock for sysbus.""" + glob_result = [] + read_side_effect = [] + + for device_id in device_ids: + mock_device = MOCK_SYSBUS_DEVICES[device_id] + + # Setup directory listing + glob_result += [f"/{DEFAULT_SYSBUS_MOUNT_DIR}/{device_id}"] + + # Setup sub-device reads + device_sensors = mock_device.get(domain, []) + for expected_sensor in device_sensors: + if isinstance(expected_sensor["injected_value"], list): + read_side_effect += expected_sensor["injected_value"] + else: + read_side_effect.append(expected_sensor["injected_value"]) + + # Ensure enough read side effect + read_side_effect.extend([FileNotFoundError("Missing injected value")] * 20) + + return (glob_result, read_side_effect) diff --git a/tests/components/onewire/const.py b/tests/components/onewire/const.py index 8fa149c7adc..ccae8e695ce 100644 --- a/tests/components/onewire/const.py +++ b/tests/components/onewire/const.py @@ -778,7 +778,7 @@ MOCK_OWPROXY_DEVICES = { } MOCK_SYSBUS_DEVICES = { - "00-111111111111": {"sensors": []}, + "00-111111111111": {SENSOR_DOMAIN: []}, "10-111111111111": { "device_info": { "identifiers": {(DOMAIN, "10-111111111111")}, @@ -786,7 +786,7 @@ MOCK_SYSBUS_DEVICES = { "model": "10", "name": "10-111111111111", }, - "sensors": [ + SENSOR_DOMAIN: [ { "entity_id": "sensor.my_ds18b20_temperature", "unique_id": "/sys/bus/w1/devices/10-111111111111/w1_slave", @@ -797,8 +797,8 @@ MOCK_SYSBUS_DEVICES = { }, ], }, - "12-111111111111": {"sensors": []}, - "1D-111111111111": {"sensors": []}, + "12-111111111111": {SENSOR_DOMAIN: []}, + "1D-111111111111": {SENSOR_DOMAIN: []}, "22-111111111111": { "device_info": { "identifiers": {(DOMAIN, "22-111111111111")}, @@ -806,7 +806,7 @@ MOCK_SYSBUS_DEVICES = { "model": "22", "name": "22-111111111111", }, - "sensors": [ + "sensor": [ { "entity_id": "sensor.22_111111111111_temperature", "unique_id": "/sys/bus/w1/devices/22-111111111111/w1_slave", @@ -817,7 +817,7 @@ MOCK_SYSBUS_DEVICES = { }, ], }, - "26-111111111111": {"sensors": []}, + "26-111111111111": {SENSOR_DOMAIN: []}, "28-111111111111": { "device_info": { "identifiers": {(DOMAIN, "28-111111111111")}, @@ -825,7 +825,7 @@ MOCK_SYSBUS_DEVICES = { "model": "28", "name": "28-111111111111", }, - "sensors": [ + SENSOR_DOMAIN: [ { "entity_id": "sensor.28_111111111111_temperature", "unique_id": "/sys/bus/w1/devices/28-111111111111/w1_slave", @@ -836,7 +836,7 @@ MOCK_SYSBUS_DEVICES = { }, ], }, - "29-111111111111": {"sensors": []}, + "29-111111111111": {SENSOR_DOMAIN: []}, "3B-111111111111": { "device_info": { "identifiers": {(DOMAIN, "3B-111111111111")}, @@ -844,7 +844,7 @@ MOCK_SYSBUS_DEVICES = { "model": "3B", "name": "3B-111111111111", }, - "sensors": [ + SENSOR_DOMAIN: [ { "entity_id": "sensor.3b_111111111111_temperature", "unique_id": "/sys/bus/w1/devices/3B-111111111111/w1_slave", @@ -862,7 +862,7 @@ MOCK_SYSBUS_DEVICES = { "model": "42", "name": "42-111111111111", }, - "sensors": [ + SENSOR_DOMAIN: [ { "entity_id": "sensor.42_111111111111_temperature", "unique_id": "/sys/bus/w1/devices/42-111111111111/w1_slave", @@ -873,10 +873,46 @@ MOCK_SYSBUS_DEVICES = { }, ], }, + "42-111111111112": { + "device_info": { + "identifiers": {(DOMAIN, "42-111111111112")}, + "manufacturer": "Maxim Integrated", + "model": "42", + "name": "42-111111111112", + }, + SENSOR_DOMAIN: [ + { + "entity_id": "sensor.42_111111111112_temperature", + "unique_id": "/sys/bus/w1/devices/42-111111111112/w1_slave", + "injected_value": [UnsupportResponseException] * 9 + ["27.993"], + "result": "28.0", + "unit": TEMP_CELSIUS, + "class": DEVICE_CLASS_TEMPERATURE, + }, + ], + }, + "42-111111111113": { + "device_info": { + "identifiers": {(DOMAIN, "42-111111111113")}, + "manufacturer": "Maxim Integrated", + "model": "42", + "name": "42-111111111113", + }, + SENSOR_DOMAIN: [ + { + "entity_id": "sensor.42_111111111113_temperature", + "unique_id": "/sys/bus/w1/devices/42-111111111113/w1_slave", + "injected_value": [UnsupportResponseException] * 10 + ["27.993"], + "result": "unknown", + "unit": TEMP_CELSIUS, + "class": DEVICE_CLASS_TEMPERATURE, + }, + ], + }, "EF-111111111111": { - "sensors": [], + SENSOR_DOMAIN: [], }, "EF-111111111112": { - "sensors": [], + SENSOR_DOMAIN: [], }, } diff --git a/tests/components/onewire/test_sensor.py b/tests/components/onewire/test_sensor.py index f81044eb86d..f3063dfc128 100644 --- a/tests/components/onewire/test_sensor.py +++ b/tests/components/onewire/test_sensor.py @@ -12,7 +12,11 @@ from homeassistant.components.onewire.const import ( from homeassistant.components.sensor import DOMAIN as SENSOR_DOMAIN from homeassistant.setup import async_setup_component -from . import setup_onewire_patched_owserver_integration, setup_owproxy_mock_devices +from . import ( + setup_onewire_patched_owserver_integration, + setup_owproxy_mock_devices, + setup_sysbus_mock_devices, +) from .const import MOCK_OWPROXY_DEVICES, MOCK_SYSBUS_DEVICES from tests.common import assert_setup_component, mock_device_registry, mock_registry @@ -185,19 +189,16 @@ async def test_owserver_setup_valid_device(owproxy, hass, device_id, platform): @pytest.mark.parametrize("device_id", MOCK_SYSBUS_DEVICES.keys()) async def test_onewiredirect_setup_valid_device(hass, device_id): """Test that sysbus config entry works correctly.""" + await async_setup_component(hass, "persistent_notification", {}) entity_registry = mock_registry(hass) device_registry = mock_device_registry(hass) - mock_device_sensor = MOCK_SYSBUS_DEVICES[device_id] + glob_result, read_side_effect = setup_sysbus_mock_devices( + SENSOR_DOMAIN, [device_id] + ) - glob_result = [f"/{DEFAULT_SYSBUS_MOUNT_DIR}/{device_id}"] - read_side_effect = [] - expected_sensors = mock_device_sensor["sensors"] - for expected_sensor in expected_sensors: - read_side_effect.append(expected_sensor["injected_value"]) - - # Ensure enough read side effect - read_side_effect.extend([FileNotFoundError("Missing injected value")] * 20) + mock_device = MOCK_SYSBUS_DEVICES[device_id] + expected_entities = mock_device.get(SENSOR_DOMAIN, []) with patch( "homeassistant.components.onewire.onewirehub.os.path.isdir", return_value=True @@ -208,10 +209,10 @@ async def test_onewiredirect_setup_valid_device(hass, device_id): assert await async_setup_component(hass, SENSOR_DOMAIN, MOCK_SYSBUS_CONFIG) await hass.async_block_till_done() - assert len(entity_registry.entities) == len(expected_sensors) + assert len(entity_registry.entities) == len(expected_entities) - if len(expected_sensors) > 0: - device_info = mock_device_sensor["device_info"] + if len(expected_entities) > 0: + device_info = mock_device["device_info"] assert len(device_registry.devices) == 1 registry_entry = device_registry.async_get_device({(DOMAIN, device_id)}) assert registry_entry is not None @@ -220,7 +221,7 @@ async def test_onewiredirect_setup_valid_device(hass, device_id): assert registry_entry.name == device_info["name"] assert registry_entry.model == device_info["model"] - for expected_sensor in expected_sensors: + for expected_sensor in expected_entities: entity_id = expected_sensor["entity_id"] registry_entry = entity_registry.entities.get(entity_id) assert registry_entry is not None