From d4c5a93b638356686d003321919a1639a4c6f4e6 Mon Sep 17 00:00:00 2001
From: FredericMa <FredericMa@users.noreply.github.com>
Date: Mon, 13 Nov 2023 12:24:20 +0100
Subject: [PATCH] Add Risco communication delay (#101349)
* Add optional communication delay in case a panel responds slow.
* Add migration test
* Connect by increasing the communication delay
* Update init to use option instead of config
* Updated strings
* Fix migration and tests
* Review changes
* Update connect strategy
* Update tests
* Changes after review
* Change typing from object to Any.
* Add test to validate communication delay update.
* Move test to separate file
* Change filename.
---
homeassistant/components/risco/__init__.py | 36 ++++++++++++++-----
homeassistant/components/risco/config_flow.py | 32 ++++++++++++++---
homeassistant/components/risco/const.py | 3 ++
homeassistant/components/risco/manifest.json | 2 +-
requirements_all.txt | 2 +-
requirements_test_all.txt | 2 +-
tests/components/risco/conftest.py | 10 ++++++
tests/components/risco/test_config_flow.py | 7 ++--
tests/components/risco/test_init.py | 21 +++++++++++
9 files changed, 96 insertions(+), 19 deletions(-)
create mode 100644 tests/components/risco/test_init.py
diff --git a/homeassistant/components/risco/__init__.py b/homeassistant/components/risco/__init__.py
index 88f8ba9bdfa..9c62447ee04 100644
--- a/homeassistant/components/risco/__init__.py
+++ b/homeassistant/components/risco/__init__.py
@@ -35,10 +35,12 @@ from homeassistant.helpers.storage import Store
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
from .const import (
+ CONF_COMMUNICATION_DELAY,
DATA_COORDINATOR,
DEFAULT_SCAN_INTERVAL,
DOMAIN,
EVENTS_COORDINATOR,
+ MAX_COMMUNICATION_DELAY,
TYPE_LOCAL,
)
@@ -81,15 +83,31 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
async def _async_setup_local_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
data = entry.data
- risco = RiscoLocal(data[CONF_HOST], data[CONF_PORT], data[CONF_PIN])
-
- try:
- await risco.connect()
- except CannotConnectError as error:
- raise ConfigEntryNotReady() from error
- except UnauthorizedError:
- _LOGGER.exception("Failed to login to Risco cloud")
- return False
+ comm_delay = initial_delay = data.get(CONF_COMMUNICATION_DELAY, 0)
+
+ while True:
+ risco = RiscoLocal(
+ data[CONF_HOST],
+ data[CONF_PORT],
+ data[CONF_PIN],
+ communication_delay=comm_delay,
+ )
+ try:
+ await risco.connect()
+ except CannotConnectError as error:
+ if comm_delay >= MAX_COMMUNICATION_DELAY:
+ raise ConfigEntryNotReady() from error
+ comm_delay += 1
+ except UnauthorizedError:
+ _LOGGER.exception("Failed to login to Risco cloud")
+ return False
+ else:
+ break
+
+ if comm_delay > initial_delay:
+ new_data = data.copy()
+ new_data[CONF_COMMUNICATION_DELAY] = comm_delay
+ hass.config_entries.async_update_entry(entry, data=new_data)
async def _error(error: Exception) -> None:
_LOGGER.error("Error in Risco library: %s", error)
diff --git a/homeassistant/components/risco/config_flow.py b/homeassistant/components/risco/config_flow.py
index 0f532a376a1..ef96714742d 100644
--- a/homeassistant/components/risco/config_flow.py
+++ b/homeassistant/components/risco/config_flow.py
@@ -28,10 +28,12 @@ from homeassistant.helpers.aiohttp_client import async_get_clientsession
from .const import (
CONF_CODE_ARM_REQUIRED,
CONF_CODE_DISARM_REQUIRED,
+ CONF_COMMUNICATION_DELAY,
CONF_HA_STATES_TO_RISCO,
CONF_RISCO_STATES_TO_HA,
DEFAULT_OPTIONS,
DOMAIN,
+ MAX_COMMUNICATION_DELAY,
RISCO_STATES,
TYPE_LOCAL,
)
@@ -78,16 +80,31 @@ async def validate_cloud_input(hass: core.HomeAssistant, data) -> dict[str, str]
async def validate_local_input(
hass: core.HomeAssistant, data: Mapping[str, str]
-) -> dict[str, str]:
+) -> dict[str, Any]:
"""Validate the user input allows us to connect to a local panel.
Data has the keys from LOCAL_SCHEMA with values provided by the user.
"""
- risco = RiscoLocal(data[CONF_HOST], data[CONF_PORT], data[CONF_PIN])
- await risco.connect()
+ comm_delay = 0
+ while True:
+ risco = RiscoLocal(
+ data[CONF_HOST],
+ data[CONF_PORT],
+ data[CONF_PIN],
+ communication_delay=comm_delay,
+ )
+ try:
+ await risco.connect()
+ except CannotConnectError as e:
+ if comm_delay >= MAX_COMMUNICATION_DELAY:
+ raise e
+ comm_delay += 1
+ else:
+ break
+
site_id = risco.id
await risco.disconnect()
- return {"title": site_id}
+ return {"title": site_id, "comm_delay": comm_delay}
class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
@@ -170,7 +187,12 @@ class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
self._abort_if_unique_id_configured()
return self.async_create_entry(
- title=info["title"], data={**user_input, **{CONF_TYPE: TYPE_LOCAL}}
+ title=info["title"],
+ data={
+ **user_input,
+ **{CONF_TYPE: TYPE_LOCAL},
+ **{CONF_COMMUNICATION_DELAY: info["comm_delay"]},
+ },
)
return self.async_show_form(
diff --git a/homeassistant/components/risco/const.py b/homeassistant/components/risco/const.py
index 9f0e71701c6..800003d2384 100644
--- a/homeassistant/components/risco/const.py
+++ b/homeassistant/components/risco/const.py
@@ -17,10 +17,13 @@ DEFAULT_SCAN_INTERVAL = 30
TYPE_LOCAL = "local"
+MAX_COMMUNICATION_DELAY = 3
+
CONF_CODE_ARM_REQUIRED = "code_arm_required"
CONF_CODE_DISARM_REQUIRED = "code_disarm_required"
CONF_RISCO_STATES_TO_HA = "risco_states_to_ha"
CONF_HA_STATES_TO_RISCO = "ha_states_to_risco"
+CONF_COMMUNICATION_DELAY = "communication_delay"
RISCO_GROUPS = ["A", "B", "C", "D"]
RISCO_ARM = "arm"
diff --git a/homeassistant/components/risco/manifest.json b/homeassistant/components/risco/manifest.json
index 5b208d1fc18..ca28af3d8e5 100644
--- a/homeassistant/components/risco/manifest.json
+++ b/homeassistant/components/risco/manifest.json
@@ -7,5 +7,5 @@
"iot_class": "local_push",
"loggers": ["pyrisco"],
"quality_scale": "platinum",
- "requirements": ["pyrisco==0.5.7"]
+ "requirements": ["pyrisco==0.5.8"]
}
diff --git a/requirements_all.txt b/requirements_all.txt
index 3cb6eaff697..3a2758e3a9a 100644
--- a/requirements_all.txt
+++ b/requirements_all.txt
@@ -2001,7 +2001,7 @@ pyrecswitch==1.0.2
pyrepetierng==0.1.0
# homeassistant.components.risco
-pyrisco==0.5.7
+pyrisco==0.5.8
# homeassistant.components.rituals_perfume_genie
pyrituals==0.0.6
diff --git a/requirements_test_all.txt b/requirements_test_all.txt
index 5ece8f95a86..aa187d8c886 100644
--- a/requirements_test_all.txt
+++ b/requirements_test_all.txt
@@ -1508,7 +1508,7 @@ pyqwikswitch==0.93
pyrainbird==4.0.0
# homeassistant.components.risco
-pyrisco==0.5.7
+pyrisco==0.5.8
# homeassistant.components.rituals_perfume_genie
pyrituals==0.0.6
diff --git a/tests/components/risco/conftest.py b/tests/components/risco/conftest.py
index cb3b3dd929e..325e787bb4f 100644
--- a/tests/components/risco/conftest.py
+++ b/tests/components/risco/conftest.py
@@ -171,6 +171,16 @@ def connect_with_error(exception):
yield
+@pytest.fixture
+def connect_with_single_error(exception):
+ """Fixture to simulate error on connect."""
+ with patch(
+ "homeassistant.components.risco.RiscoLocal.connect",
+ side_effect=[exception, None],
+ ):
+ yield
+
+
@pytest.fixture
async def setup_risco_local(hass, local_config_entry):
"""Set up a local Risco integration for testing."""
diff --git a/tests/components/risco/test_config_flow.py b/tests/components/risco/test_config_flow.py
index 5a9b60ed130..fdb51c65dda 100644
--- a/tests/components/risco/test_config_flow.py
+++ b/tests/components/risco/test_config_flow.py
@@ -9,7 +9,7 @@ from homeassistant.components.risco.config_flow import (
CannotConnectError,
UnauthorizedError,
)
-from homeassistant.components.risco.const import DOMAIN
+from homeassistant.components.risco.const import CONF_COMMUNICATION_DELAY, DOMAIN
from homeassistant.const import CONF_PASSWORD, CONF_USERNAME
from homeassistant.core import HomeAssistant
from homeassistant.data_entry_flow import FlowResultType
@@ -246,7 +246,10 @@ async def test_local_form(hass: HomeAssistant) -> None:
)
await hass.async_block_till_done()
- expected_data = {**TEST_LOCAL_DATA, **{"type": "local"}}
+ expected_data = {
+ **TEST_LOCAL_DATA,
+ **{"type": "local", CONF_COMMUNICATION_DELAY: 0},
+ }
assert result3["type"] == FlowResultType.CREATE_ENTRY
assert result3["title"] == TEST_SITE_NAME
assert result3["data"] == expected_data
diff --git a/tests/components/risco/test_init.py b/tests/components/risco/test_init.py
new file mode 100644
index 00000000000..a1a9e3bd6a7
--- /dev/null
+++ b/tests/components/risco/test_init.py
@@ -0,0 +1,21 @@
+"""Tests for the Risco initialization."""
+import pytest
+
+from homeassistant.components.risco import CannotConnectError
+from homeassistant.components.risco.const import CONF_COMMUNICATION_DELAY
+from homeassistant.core import HomeAssistant
+
+
+@pytest.mark.parametrize("exception", [CannotConnectError])
+async def test_single_error_on_connect(
+ hass: HomeAssistant, connect_with_single_error, local_config_entry
+) -> None:
+ """Test single error on connect to validate communication delay update from 0 (default) to 1."""
+ expected_data = {
+ **local_config_entry.data,
+ **{"type": "local", CONF_COMMUNICATION_DELAY: 1},
+ }
+
+ await hass.config_entries.async_setup(local_config_entry.entry_id)
+ await hass.async_block_till_done()
+ assert local_config_entry.data == expected_data
--
GitLab