From e122fd548c94cb5a08f8504cad29deea027a455b Mon Sep 17 00:00:00 2001 From: FredericMa Date: Wed, 27 Sep 2023 20:31:30 +0000 Subject: [PATCH 01/14] Add optional communication delay in case a panel responds slow. --- homeassistant/components/risco/__init__.py | 8 +++++++- homeassistant/components/risco/config_flow.py | 10 +++++++++- homeassistant/components/risco/const.py | 1 + homeassistant/components/risco/manifest.json | 2 +- homeassistant/components/risco/strings.json | 3 ++- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- 7 files changed, 22 insertions(+), 6 deletions(-) diff --git a/homeassistant/components/risco/__init__.py b/homeassistant/components/risco/__init__.py index 88f8ba9bdfa79b..40a057b351737c 100644 --- a/homeassistant/components/risco/__init__.py +++ b/homeassistant/components/risco/__init__.py @@ -35,6 +35,7 @@ from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed from .const import ( + CONF_COMMUNICATION_DELAY, DATA_COORDINATOR, DEFAULT_SCAN_INTERVAL, DOMAIN, @@ -81,7 +82,12 @@ 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]) + risco = RiscoLocal( + data[CONF_HOST], + data[CONF_PORT], + data[CONF_PIN], + **{CONF_COMMUNICATION_DELAY: data[CONF_COMMUNICATION_DELAY]}, + ) try: await risco.connect() diff --git a/homeassistant/components/risco/config_flow.py b/homeassistant/components/risco/config_flow.py index 0f532a376a1f96..5377f2baa5d528 100644 --- a/homeassistant/components/risco/config_flow.py +++ b/homeassistant/components/risco/config_flow.py @@ -28,6 +28,7 @@ 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, @@ -51,6 +52,7 @@ vol.Required(CONF_HOST): str, vol.Required(CONF_PORT, default=1000): int, vol.Required(CONF_PIN): str, + vol.Optional(CONF_COMMUNICATION_DELAY, default=0): int, } ) HA_STATES = [ @@ -83,7 +85,13 @@ async def validate_local_input( Data has the keys from LOCAL_SCHEMA with values provided by the user. """ - risco = RiscoLocal(data[CONF_HOST], data[CONF_PORT], data[CONF_PIN]) + + risco = RiscoLocal( + data[CONF_HOST], + data[CONF_PORT], + data[CONF_PIN], + **{CONF_COMMUNICATION_DELAY: data[CONF_COMMUNICATION_DELAY]}, + ) await risco.connect() site_id = risco.id await risco.disconnect() diff --git a/homeassistant/components/risco/const.py b/homeassistant/components/risco/const.py index 9f0e71701c65b9..f5ab63f14280e9 100644 --- a/homeassistant/components/risco/const.py +++ b/homeassistant/components/risco/const.py @@ -21,6 +21,7 @@ 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 5b208d1fc187d1..ca28af3d8e55c5 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/homeassistant/components/risco/strings.json b/homeassistant/components/risco/strings.json index 13dfd60b5b661f..41b6ef033f3b22 100644 --- a/homeassistant/components/risco/strings.json +++ b/homeassistant/components/risco/strings.json @@ -18,7 +18,8 @@ "data": { "host": "[%key:common::config_flow::data::host%]", "port": "[%key:common::config_flow::data::port%]", - "pin": "[%key:common::config_flow::data::pin%]" + "pin": "[%key:common::config_flow::data::pin%]", + "communication_delay": "Communication delay (seconds)" } } }, diff --git a/requirements_all.txt b/requirements_all.txt index 3d127d30b15897..4104b2e7eddb66 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 5ece8f95a86fd6..aa187d8c886902 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 From 430f3d2f060549c80ce95eadaada8d350ed1e0ef Mon Sep 17 00:00:00 2001 From: FredericMa Date: Mon, 2 Oct 2023 21:17:26 +0000 Subject: [PATCH 02/14] Add migration test --- homeassistant/components/risco/__init__.py | 16 ++++++++ homeassistant/components/risco/config_flow.py | 2 +- tests/components/risco/test_config_flow.py | 38 ++++++++++++++++++- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/risco/__init__.py b/homeassistant/components/risco/__init__.py index 40a057b351737c..9daaea1eab3f1b 100644 --- a/homeassistant/components/risco/__init__.py +++ b/homeassistant/components/risco/__init__.py @@ -183,6 +183,22 @@ async def _update_listener(hass: HomeAssistant, entry: ConfigEntry) -> None: await hass.config_entries.async_reload(entry.entry_id) +async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: + """Migrate old entry.""" + _LOGGER.debug("Migrating from version %s", config_entry.version) + + if config_entry.version == 1: + new = {**config_entry.data} + new[CONF_COMMUNICATION_DELAY] = 0 + + config_entry.version = 2 + hass.config_entries.async_update_entry(config_entry, data=new) + + _LOGGER.debug("Migration to version %s successful", config_entry.version) + + return True + + class RiscoDataUpdateCoordinator(DataUpdateCoordinator[Alarm]): """Class to manage fetching risco data.""" diff --git a/homeassistant/components/risco/config_flow.py b/homeassistant/components/risco/config_flow.py index 5377f2baa5d528..352ecc0e7427e2 100644 --- a/homeassistant/components/risco/config_flow.py +++ b/homeassistant/components/risco/config_flow.py @@ -101,7 +101,7 @@ async def validate_local_input( class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Handle a config flow for Risco.""" - VERSION = 1 + VERSION = 2 def __init__(self) -> None: """Init the config flow.""" diff --git a/tests/components/risco/test_config_flow.py b/tests/components/risco/test_config_flow.py index 5a9b60ed13010b..bf9f93a9385bfc 100644 --- a/tests/components/risco/test_config_flow.py +++ b/tests/components/risco/test_config_flow.py @@ -29,6 +29,13 @@ "pin": "1234", } +TEST_LOCAL_DATA_V2 = { + "host": "test-host", + "port": 5004, + "pin": "1234", + "communication_delay": 0, +} + TEST_RISCO_TO_HA = { "arm": "armed_away", "partial_arm": "armed_home", @@ -246,7 +253,7 @@ 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_V2, **{"type": "local"}} assert result3["type"] == FlowResultType.CREATE_ENTRY assert result3["title"] == TEST_SITE_NAME assert result3["data"] == expected_data @@ -392,3 +399,32 @@ async def test_ha_to_risco_schema(hass: HomeAssistant) -> None: result["flow_id"], user_input={**TEST_HA_TO_RISCO, "armed_night": "A"}, ) + + +async def test_migration_1_2(hass: HomeAssistant) -> None: + """Test migrating from version 1 to 2.""" + test_data = {**TEST_LOCAL_DATA, **{"type": "local"}} + config_entry = MockConfigEntry( + domain=DOMAIN, unique_id=TEST_SITE_NAME, data=test_data, version=1 + ) + config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.risco.RiscoLocal.connect", + return_value=True, + ), patch( + "homeassistant.components.risco.RiscoLocal.id", + new_callable=PropertyMock(return_value=TEST_SITE_NAME), + ), patch( + "homeassistant.components.risco.RiscoLocal.disconnect" + ): + await hass.config_entries.async_setup(config_entry.entry_id) + await hass.async_block_till_done() + + config_entries = hass.config_entries.async_entries(DOMAIN) + assert len(config_entries) == 1 + + expected_data = {**TEST_LOCAL_DATA_V2, **{"type": "local"}} + assert config_entries[0].unique_id == TEST_SITE_NAME + assert config_entries[0].data == expected_data + assert config_entries[0].version == 2 From 646bee1a6b2eeca370ce2d5792dcfe434a43d15e Mon Sep 17 00:00:00 2001 From: FredericMa Date: Wed, 4 Oct 2023 22:33:22 +0000 Subject: [PATCH 03/14] Connect by increasing the communication delay --- homeassistant/components/risco/config_flow.py | 34 +++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/homeassistant/components/risco/config_flow.py b/homeassistant/components/risco/config_flow.py index 352ecc0e7427e2..a0be85787afeea 100644 --- a/homeassistant/components/risco/config_flow.py +++ b/homeassistant/components/risco/config_flow.py @@ -52,7 +52,6 @@ vol.Required(CONF_HOST): str, vol.Required(CONF_PORT, default=1000): int, vol.Required(CONF_PIN): str, - vol.Optional(CONF_COMMUNICATION_DELAY, default=0): int, } ) HA_STATES = [ @@ -85,17 +84,26 @@ async def validate_local_input( Data has the keys from LOCAL_SCHEMA with values provided by the user. """ + attempts = 0 + + while True: + try: + risco = RiscoLocal( + data[CONF_HOST], + data[CONF_PORT], + data[CONF_PIN], + **{CONF_COMMUNICATION_DELAY: attempts}, + ) + await risco.connect() + break + except CannotConnectError as e: + if attempts >= 3: + raise e + attempts += 1 - risco = RiscoLocal( - data[CONF_HOST], - data[CONF_PORT], - data[CONF_PIN], - **{CONF_COMMUNICATION_DELAY: data[CONF_COMMUNICATION_DELAY]}, - ) - await risco.connect() site_id = risco.id await risco.disconnect() - return {"title": site_id} + return {"title": site_id, "attempts": str(attempts)} class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): @@ -178,7 +186,9 @@ async def async_step_local(self, user_input=None): 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}}, + options={**{CONF_COMMUNICATION_DELAY: int(info["attempts"])}}, ) return self.async_show_form( @@ -207,6 +217,10 @@ def _options_schema(self): CONF_CODE_DISARM_REQUIRED, default=self._data[CONF_CODE_DISARM_REQUIRED], ): bool, + vol.Required( + CONF_COMMUNICATION_DELAY, + default=self._data[CONF_COMMUNICATION_DELAY], + ): int, } ) From 0558fe4942c1e120b436b20e318fd35191ce6097 Mon Sep 17 00:00:00 2001 From: FredericMa Date: Wed, 4 Oct 2023 22:40:23 +0000 Subject: [PATCH 04/14] Update init to use option instead of config --- homeassistant/components/risco/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/homeassistant/components/risco/__init__.py b/homeassistant/components/risco/__init__.py index 9daaea1eab3f1b..8580ed11ac923b 100644 --- a/homeassistant/components/risco/__init__.py +++ b/homeassistant/components/risco/__init__.py @@ -82,11 +82,12 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: async def _async_setup_local_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: data = entry.data + options = entry.options risco = RiscoLocal( data[CONF_HOST], data[CONF_PORT], data[CONF_PIN], - **{CONF_COMMUNICATION_DELAY: data[CONF_COMMUNICATION_DELAY]}, + **{CONF_COMMUNICATION_DELAY: options[CONF_COMMUNICATION_DELAY]}, ) try: From c6e5fc214de24386d99bb6ed072f0da33ed8a5d5 Mon Sep 17 00:00:00 2001 From: FredericMa Date: Thu, 19 Oct 2023 19:14:26 +0000 Subject: [PATCH 05/14] Updated strings --- homeassistant/components/risco/strings.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/homeassistant/components/risco/strings.json b/homeassistant/components/risco/strings.json index 41b6ef033f3b22..6b83f9ed594134 100644 --- a/homeassistant/components/risco/strings.json +++ b/homeassistant/components/risco/strings.json @@ -18,8 +18,7 @@ "data": { "host": "[%key:common::config_flow::data::host%]", "port": "[%key:common::config_flow::data::port%]", - "pin": "[%key:common::config_flow::data::pin%]", - "communication_delay": "Communication delay (seconds)" + "pin": "[%key:common::config_flow::data::pin%]" } } }, @@ -39,7 +38,8 @@ "data": { "scan_interval": "How often to poll Risco (in seconds)", "code_arm_required": "Require PIN to arm", - "code_disarm_required": "Require PIN to disarm" + "code_disarm_required": "Require PIN to disarm", + "communication_delay": "Communication delay (in seconds)" } }, "risco_to_ha": { From 8e772e6bd27f3a49b2e20d7b21188024e2122d19 Mon Sep 17 00:00:00 2001 From: FredericMa Date: Thu, 19 Oct 2023 20:07:49 +0000 Subject: [PATCH 06/14] Fix migration and tests --- homeassistant/components/risco/__init__.py | 4 +-- tests/components/risco/test_config_flow.py | 38 ++++++++++++++-------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/risco/__init__.py b/homeassistant/components/risco/__init__.py index 8580ed11ac923b..c3893fa4722790 100644 --- a/homeassistant/components/risco/__init__.py +++ b/homeassistant/components/risco/__init__.py @@ -189,11 +189,11 @@ async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> _LOGGER.debug("Migrating from version %s", config_entry.version) if config_entry.version == 1: - new = {**config_entry.data} + new = {**config_entry.options} new[CONF_COMMUNICATION_DELAY] = 0 config_entry.version = 2 - hass.config_entries.async_update_entry(config_entry, data=new) + hass.config_entries.async_update_entry(config_entry, options=new) _LOGGER.debug("Migration to version %s successful", config_entry.version) diff --git a/tests/components/risco/test_config_flow.py b/tests/components/risco/test_config_flow.py index bf9f93a9385bfc..fd2ceba5812833 100644 --- a/tests/components/risco/test_config_flow.py +++ b/tests/components/risco/test_config_flow.py @@ -29,13 +29,6 @@ "pin": "1234", } -TEST_LOCAL_DATA_V2 = { - "host": "test-host", - "port": 5004, - "pin": "1234", - "communication_delay": 0, -} - TEST_RISCO_TO_HA = { "arm": "armed_away", "partial_arm": "armed_home", @@ -57,6 +50,17 @@ "code_disarm_required": True, } +TEST_OPTIONS_V2 = { + "scan_interval": 10, + "code_arm_required": True, + "code_disarm_required": True, + "communication_delay": 0, +} + +TEST_OPTIONS_INITIAL = { + "communication_delay": 0, +} + async def test_cloud_form(hass: HomeAssistant) -> None: """Test we get the cloud form.""" @@ -253,10 +257,11 @@ async def test_local_form(hass: HomeAssistant) -> None: ) await hass.async_block_till_done() - expected_data = {**TEST_LOCAL_DATA_V2, **{"type": "local"}} + expected_data = {**TEST_LOCAL_DATA, **{"type": "local"}} assert result3["type"] == FlowResultType.CREATE_ENTRY assert result3["title"] == TEST_SITE_NAME assert result3["data"] == expected_data + assert result3["options"] == TEST_OPTIONS_INITIAL assert len(mock_setup_entry.mock_calls) == 1 mock_close.assert_awaited_once() @@ -327,6 +332,7 @@ async def test_options_flow(hass: HomeAssistant) -> None: domain=DOMAIN, unique_id=TEST_CLOUD_DATA["username"], data=TEST_CLOUD_DATA, + options=TEST_OPTIONS_INITIAL, ) entry.add_to_hass(hass) @@ -338,7 +344,7 @@ async def test_options_flow(hass: HomeAssistant) -> None: result = await hass.config_entries.options.async_configure( result["flow_id"], - user_input=TEST_OPTIONS, + user_input=TEST_OPTIONS_V2, ) assert result["type"] == FlowResultType.FORM assert result["step_id"] == "risco_to_ha" @@ -359,7 +365,7 @@ async def test_options_flow(hass: HomeAssistant) -> None: assert result["type"] == FlowResultType.CREATE_ENTRY assert entry.options == { - **TEST_OPTIONS, + **TEST_OPTIONS_V2, "risco_states_to_ha": TEST_RISCO_TO_HA, "ha_states_to_risco": TEST_HA_TO_RISCO, } @@ -371,6 +377,7 @@ async def test_ha_to_risco_schema(hass: HomeAssistant) -> None: domain=DOMAIN, unique_id=TEST_CLOUD_DATA["username"], data=TEST_CLOUD_DATA, + options=TEST_OPTIONS_INITIAL, ) entry.add_to_hass(hass) @@ -379,7 +386,7 @@ async def test_ha_to_risco_schema(hass: HomeAssistant) -> None: result = await hass.config_entries.options.async_configure( result["flow_id"], - user_input=TEST_OPTIONS, + user_input=TEST_OPTIONS_V2, ) result = await hass.config_entries.options.async_configure( result["flow_id"], @@ -405,7 +412,11 @@ async def test_migration_1_2(hass: HomeAssistant) -> None: """Test migrating from version 1 to 2.""" test_data = {**TEST_LOCAL_DATA, **{"type": "local"}} config_entry = MockConfigEntry( - domain=DOMAIN, unique_id=TEST_SITE_NAME, data=test_data, version=1 + domain=DOMAIN, + unique_id=TEST_SITE_NAME, + data=test_data, + options=TEST_OPTIONS_INITIAL, + version=1, ) config_entry.add_to_hass(hass) @@ -424,7 +435,8 @@ async def test_migration_1_2(hass: HomeAssistant) -> None: config_entries = hass.config_entries.async_entries(DOMAIN) assert len(config_entries) == 1 - expected_data = {**TEST_LOCAL_DATA_V2, **{"type": "local"}} + expected_data = {**TEST_LOCAL_DATA, **{"type": "local"}} assert config_entries[0].unique_id == TEST_SITE_NAME assert config_entries[0].data == expected_data + assert config_entries[0].options == TEST_OPTIONS_INITIAL assert config_entries[0].version == 2 From b300b2652c928a00b9c6ef1e8cbe966931425ce3 Mon Sep 17 00:00:00 2001 From: FredericMa Date: Thu, 19 Oct 2023 22:06:45 +0000 Subject: [PATCH 07/14] Review changes --- homeassistant/components/risco/__init__.py | 18 +--------- homeassistant/components/risco/config_flow.py | 4 +-- tests/components/risco/test_config_flow.py | 34 ------------------- 3 files changed, 3 insertions(+), 53 deletions(-) diff --git a/homeassistant/components/risco/__init__.py b/homeassistant/components/risco/__init__.py index c3893fa4722790..67804dca02a369 100644 --- a/homeassistant/components/risco/__init__.py +++ b/homeassistant/components/risco/__init__.py @@ -87,7 +87,7 @@ async def _async_setup_local_entry(hass: HomeAssistant, entry: ConfigEntry) -> b data[CONF_HOST], data[CONF_PORT], data[CONF_PIN], - **{CONF_COMMUNICATION_DELAY: options[CONF_COMMUNICATION_DELAY]}, + **{CONF_COMMUNICATION_DELAY: options.get(CONF_COMMUNICATION_DELAY, 0)}, ) try: @@ -184,22 +184,6 @@ async def _update_listener(hass: HomeAssistant, entry: ConfigEntry) -> None: await hass.config_entries.async_reload(entry.entry_id) -async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: - """Migrate old entry.""" - _LOGGER.debug("Migrating from version %s", config_entry.version) - - if config_entry.version == 1: - new = {**config_entry.options} - new[CONF_COMMUNICATION_DELAY] = 0 - - config_entry.version = 2 - hass.config_entries.async_update_entry(config_entry, options=new) - - _LOGGER.debug("Migration to version %s successful", config_entry.version) - - return True - - class RiscoDataUpdateCoordinator(DataUpdateCoordinator[Alarm]): """Class to manage fetching risco data.""" diff --git a/homeassistant/components/risco/config_flow.py b/homeassistant/components/risco/config_flow.py index a0be85787afeea..93beb1e90e56af 100644 --- a/homeassistant/components/risco/config_flow.py +++ b/homeassistant/components/risco/config_flow.py @@ -109,7 +109,7 @@ async def validate_local_input( class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): """Handle a config flow for Risco.""" - VERSION = 2 + VERSION = 1 def __init__(self) -> None: """Init the config flow.""" @@ -219,7 +219,7 @@ def _options_schema(self): ): bool, vol.Required( CONF_COMMUNICATION_DELAY, - default=self._data[CONF_COMMUNICATION_DELAY], + default=self._data.get(CONF_COMMUNICATION_DELAY, 0), ): int, } ) diff --git a/tests/components/risco/test_config_flow.py b/tests/components/risco/test_config_flow.py index fd2ceba5812833..0661be278feadb 100644 --- a/tests/components/risco/test_config_flow.py +++ b/tests/components/risco/test_config_flow.py @@ -406,37 +406,3 @@ async def test_ha_to_risco_schema(hass: HomeAssistant) -> None: result["flow_id"], user_input={**TEST_HA_TO_RISCO, "armed_night": "A"}, ) - - -async def test_migration_1_2(hass: HomeAssistant) -> None: - """Test migrating from version 1 to 2.""" - test_data = {**TEST_LOCAL_DATA, **{"type": "local"}} - config_entry = MockConfigEntry( - domain=DOMAIN, - unique_id=TEST_SITE_NAME, - data=test_data, - options=TEST_OPTIONS_INITIAL, - version=1, - ) - config_entry.add_to_hass(hass) - - with patch( - "homeassistant.components.risco.RiscoLocal.connect", - return_value=True, - ), patch( - "homeassistant.components.risco.RiscoLocal.id", - new_callable=PropertyMock(return_value=TEST_SITE_NAME), - ), patch( - "homeassistant.components.risco.RiscoLocal.disconnect" - ): - await hass.config_entries.async_setup(config_entry.entry_id) - await hass.async_block_till_done() - - config_entries = hass.config_entries.async_entries(DOMAIN) - assert len(config_entries) == 1 - - expected_data = {**TEST_LOCAL_DATA, **{"type": "local"}} - assert config_entries[0].unique_id == TEST_SITE_NAME - assert config_entries[0].data == expected_data - assert config_entries[0].options == TEST_OPTIONS_INITIAL - assert config_entries[0].version == 2 From 30786a9dadd64079fb83bd201c233f56ead320de Mon Sep 17 00:00:00 2001 From: FredericMa Date: Sun, 22 Oct 2023 21:45:57 +0000 Subject: [PATCH 08/14] Update connect strategy --- homeassistant/components/risco/__init__.py | 38 ++++++++++++------- homeassistant/components/risco/config_flow.py | 14 +++---- homeassistant/components/risco/const.py | 2 + homeassistant/components/risco/strings.json | 3 +- 4 files changed, 34 insertions(+), 23 deletions(-) diff --git a/homeassistant/components/risco/__init__.py b/homeassistant/components/risco/__init__.py index 67804dca02a369..83c201dfd2782d 100644 --- a/homeassistant/components/risco/__init__.py +++ b/homeassistant/components/risco/__init__.py @@ -40,6 +40,7 @@ DEFAULT_SCAN_INTERVAL, DOMAIN, EVENTS_COORDINATOR, + MAX_COMMUNICATION_DELAY, TYPE_LOCAL, ) @@ -82,21 +83,30 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: async def _async_setup_local_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: data = entry.data - options = entry.options - risco = RiscoLocal( - data[CONF_HOST], - data[CONF_PORT], - data[CONF_PIN], - **{CONF_COMMUNICATION_DELAY: options.get(CONF_COMMUNICATION_DELAY, 0)}, - ) + comm_delay = initial_delay = data.get(CONF_COMMUNICATION_DELAY, 0) - try: - await risco.connect() - except CannotConnectError as error: - raise ConfigEntryNotReady() from error - except UnauthorizedError: - _LOGGER.exception("Failed to login to Risco cloud") - return False + while True: + try: + risco = RiscoLocal( + data[CONF_HOST], + data[CONF_PORT], + data[CONF_PIN], + **{CONF_COMMUNICATION_DELAY: comm_delay}, + ) + await risco.connect() + break + 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 + + 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 93beb1e90e56af..2d051cc3b7ebca 100644 --- a/homeassistant/components/risco/config_flow.py +++ b/homeassistant/components/risco/config_flow.py @@ -33,6 +33,7 @@ CONF_RISCO_STATES_TO_HA, DEFAULT_OPTIONS, DOMAIN, + MAX_COMMUNICATION_DELAY, RISCO_STATES, TYPE_LOCAL, ) @@ -97,7 +98,7 @@ async def validate_local_input( await risco.connect() break except CannotConnectError as e: - if attempts >= 3: + if attempts >= MAX_COMMUNICATION_DELAY: raise e attempts += 1 @@ -187,8 +188,11 @@ async def async_step_local(self, user_input=None): return self.async_create_entry( title=info["title"], - data={**user_input, **{CONF_TYPE: TYPE_LOCAL}}, - options={**{CONF_COMMUNICATION_DELAY: int(info["attempts"])}}, + data={ + **user_input, + **{CONF_TYPE: TYPE_LOCAL}, + **{CONF_COMMUNICATION_DELAY: int(info["attempts"])}, + }, ) return self.async_show_form( @@ -217,10 +221,6 @@ def _options_schema(self): CONF_CODE_DISARM_REQUIRED, default=self._data[CONF_CODE_DISARM_REQUIRED], ): bool, - vol.Required( - CONF_COMMUNICATION_DELAY, - default=self._data.get(CONF_COMMUNICATION_DELAY, 0), - ): int, } ) diff --git a/homeassistant/components/risco/const.py b/homeassistant/components/risco/const.py index f5ab63f14280e9..800003d2384b84 100644 --- a/homeassistant/components/risco/const.py +++ b/homeassistant/components/risco/const.py @@ -17,6 +17,8 @@ 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" diff --git a/homeassistant/components/risco/strings.json b/homeassistant/components/risco/strings.json index 6b83f9ed594134..13dfd60b5b661f 100644 --- a/homeassistant/components/risco/strings.json +++ b/homeassistant/components/risco/strings.json @@ -38,8 +38,7 @@ "data": { "scan_interval": "How often to poll Risco (in seconds)", "code_arm_required": "Require PIN to arm", - "code_disarm_required": "Require PIN to disarm", - "communication_delay": "Communication delay (in seconds)" + "code_disarm_required": "Require PIN to disarm" } }, "risco_to_ha": { From 388e822e7459ca4e97c16662e1c62a083ed5664b Mon Sep 17 00:00:00 2001 From: FredericMa Date: Tue, 7 Nov 2023 20:49:48 +0000 Subject: [PATCH 09/14] Update tests --- tests/components/risco/test_config_flow.py | 27 +++++++--------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/tests/components/risco/test_config_flow.py b/tests/components/risco/test_config_flow.py index 0661be278feadb..fdb51c65ddab61 100644 --- a/tests/components/risco/test_config_flow.py +++ b/tests/components/risco/test_config_flow.py @@ -9,7 +9,7 @@ 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 @@ -50,17 +50,6 @@ "code_disarm_required": True, } -TEST_OPTIONS_V2 = { - "scan_interval": 10, - "code_arm_required": True, - "code_disarm_required": True, - "communication_delay": 0, -} - -TEST_OPTIONS_INITIAL = { - "communication_delay": 0, -} - async def test_cloud_form(hass: HomeAssistant) -> None: """Test we get the cloud form.""" @@ -257,11 +246,13 @@ 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 - assert result3["options"] == TEST_OPTIONS_INITIAL assert len(mock_setup_entry.mock_calls) == 1 mock_close.assert_awaited_once() @@ -332,7 +323,6 @@ async def test_options_flow(hass: HomeAssistant) -> None: domain=DOMAIN, unique_id=TEST_CLOUD_DATA["username"], data=TEST_CLOUD_DATA, - options=TEST_OPTIONS_INITIAL, ) entry.add_to_hass(hass) @@ -344,7 +334,7 @@ async def test_options_flow(hass: HomeAssistant) -> None: result = await hass.config_entries.options.async_configure( result["flow_id"], - user_input=TEST_OPTIONS_V2, + user_input=TEST_OPTIONS, ) assert result["type"] == FlowResultType.FORM assert result["step_id"] == "risco_to_ha" @@ -365,7 +355,7 @@ async def test_options_flow(hass: HomeAssistant) -> None: assert result["type"] == FlowResultType.CREATE_ENTRY assert entry.options == { - **TEST_OPTIONS_V2, + **TEST_OPTIONS, "risco_states_to_ha": TEST_RISCO_TO_HA, "ha_states_to_risco": TEST_HA_TO_RISCO, } @@ -377,7 +367,6 @@ async def test_ha_to_risco_schema(hass: HomeAssistant) -> None: domain=DOMAIN, unique_id=TEST_CLOUD_DATA["username"], data=TEST_CLOUD_DATA, - options=TEST_OPTIONS_INITIAL, ) entry.add_to_hass(hass) @@ -386,7 +375,7 @@ async def test_ha_to_risco_schema(hass: HomeAssistant) -> None: result = await hass.config_entries.options.async_configure( result["flow_id"], - user_input=TEST_OPTIONS_V2, + user_input=TEST_OPTIONS, ) result = await hass.config_entries.options.async_configure( result["flow_id"], From 459a7c38358a4f49cf1ab80d01341b8fecd51243 Mon Sep 17 00:00:00 2001 From: FredericMa Date: Thu, 9 Nov 2023 21:15:02 +0000 Subject: [PATCH 10/14] Changes after review --- homeassistant/components/risco/__init__.py | 15 +++++----- homeassistant/components/risco/config_flow.py | 28 +++++++++---------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/homeassistant/components/risco/__init__.py b/homeassistant/components/risco/__init__.py index 83c201dfd2782d..9c62447ee047f2 100644 --- a/homeassistant/components/risco/__init__.py +++ b/homeassistant/components/risco/__init__.py @@ -86,15 +86,14 @@ async def _async_setup_local_entry(hass: HomeAssistant, entry: ConfigEntry) -> b 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: - risco = RiscoLocal( - data[CONF_HOST], - data[CONF_PORT], - data[CONF_PIN], - **{CONF_COMMUNICATION_DELAY: comm_delay}, - ) await risco.connect() - break except CannotConnectError as error: if comm_delay >= MAX_COMMUNICATION_DELAY: raise ConfigEntryNotReady() from error @@ -102,6 +101,8 @@ async def _async_setup_local_entry(hass: HomeAssistant, entry: ConfigEntry) -> b except UnauthorizedError: _LOGGER.exception("Failed to login to Risco cloud") return False + else: + break if comm_delay > initial_delay: new_data = data.copy() diff --git a/homeassistant/components/risco/config_flow.py b/homeassistant/components/risco/config_flow.py index 2d051cc3b7ebca..08031e50ff5d78 100644 --- a/homeassistant/components/risco/config_flow.py +++ b/homeassistant/components/risco/config_flow.py @@ -80,31 +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, object]: """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. """ - attempts = 0 - + comm_delay = 0 while True: + risco = RiscoLocal( + data[CONF_HOST], + data[CONF_PORT], + data[CONF_PIN], + communication_delay=comm_delay, + ) try: - risco = RiscoLocal( - data[CONF_HOST], - data[CONF_PORT], - data[CONF_PIN], - **{CONF_COMMUNICATION_DELAY: attempts}, - ) await risco.connect() - break except CannotConnectError as e: - if attempts >= MAX_COMMUNICATION_DELAY: + if comm_delay >= MAX_COMMUNICATION_DELAY: raise e - attempts += 1 + comm_delay += 1 + else: + break site_id = risco.id await risco.disconnect() - return {"title": site_id, "attempts": str(attempts)} + return {"title": site_id, "comm_delay": comm_delay} class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN): @@ -191,7 +191,7 @@ async def async_step_local(self, user_input=None): data={ **user_input, **{CONF_TYPE: TYPE_LOCAL}, - **{CONF_COMMUNICATION_DELAY: int(info["attempts"])}, + **{CONF_COMMUNICATION_DELAY: info["comm_delay"]}, }, ) From 839760f730c86817cfb5a6da239c7a1af02d6afb Mon Sep 17 00:00:00 2001 From: FredericMa Date: Thu, 9 Nov 2023 21:59:36 +0000 Subject: [PATCH 11/14] Change typing from object to Any. --- homeassistant/components/risco/config_flow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/risco/config_flow.py b/homeassistant/components/risco/config_flow.py index 08031e50ff5d78..ef96714742d0c6 100644 --- a/homeassistant/components/risco/config_flow.py +++ b/homeassistant/components/risco/config_flow.py @@ -80,7 +80,7 @@ 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, object]: +) -> 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. From a334c9e47371a8a2b9d2f0afbe2205f5fed663d7 Mon Sep 17 00:00:00 2001 From: FredericMa Date: Sun, 12 Nov 2023 22:34:52 +0000 Subject: [PATCH 12/14] Add test to validate communication delay update. --- tests/components/risco/conftest.py | 10 ++++++++++ tests/components/risco/test_binary_sensor.py | 17 ++++++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/components/risco/conftest.py b/tests/components/risco/conftest.py index cb3b3dd929e761..325e787bb4f148 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_binary_sensor.py b/tests/components/risco/test_binary_sensor.py index ee74dbbedc8a28..88c07dd68063ba 100644 --- a/tests/components/risco/test_binary_sensor.py +++ b/tests/components/risco/test_binary_sensor.py @@ -4,7 +4,7 @@ import pytest from homeassistant.components.risco import CannotConnectError, UnauthorizedError -from homeassistant.components.risco.const import DOMAIN +from homeassistant.components.risco.const import CONF_COMMUNICATION_DELAY, DOMAIN from homeassistant.const import STATE_OFF, STATE_ON from homeassistant.core import HomeAssistant from homeassistant.helpers import device_registry as dr, entity_registry as er @@ -92,6 +92,21 @@ async def test_error_on_connect( assert not registry.async_is_registered(SECOND_ALARMED_ENTITY_ID) +@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.""" + 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 + + async def test_local_setup( hass: HomeAssistant, two_zone_local, setup_risco_local ) -> None: From caf1347b394c8a318eb812d306d1cbee04edc931 Mon Sep 17 00:00:00 2001 From: FredericMa Date: Mon, 13 Nov 2023 07:53:56 +0000 Subject: [PATCH 13/14] Move test to separate file --- tests/components/risco/test_binary_sensor.py | 17 +--------------- tests/components/risco/test_connect.py | 21 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 16 deletions(-) create mode 100644 tests/components/risco/test_connect.py diff --git a/tests/components/risco/test_binary_sensor.py b/tests/components/risco/test_binary_sensor.py index 88c07dd68063ba..ee74dbbedc8a28 100644 --- a/tests/components/risco/test_binary_sensor.py +++ b/tests/components/risco/test_binary_sensor.py @@ -4,7 +4,7 @@ import pytest from homeassistant.components.risco import CannotConnectError, UnauthorizedError -from homeassistant.components.risco.const import CONF_COMMUNICATION_DELAY, DOMAIN +from homeassistant.components.risco.const import DOMAIN from homeassistant.const import STATE_OFF, STATE_ON from homeassistant.core import HomeAssistant from homeassistant.helpers import device_registry as dr, entity_registry as er @@ -92,21 +92,6 @@ async def test_error_on_connect( assert not registry.async_is_registered(SECOND_ALARMED_ENTITY_ID) -@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.""" - 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 - - async def test_local_setup( hass: HomeAssistant, two_zone_local, setup_risco_local ) -> None: diff --git a/tests/components/risco/test_connect.py b/tests/components/risco/test_connect.py new file mode 100644 index 00000000000000..8f4d4a18e2b1df --- /dev/null +++ b/tests/components/risco/test_connect.py @@ -0,0 +1,21 @@ +"""Tests for the Risco connect logic.""" +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 From 5976e1234ab1d76181fac882ad3edc27d321cbca Mon Sep 17 00:00:00 2001 From: FredericMa Date: Mon, 13 Nov 2023 08:04:13 +0000 Subject: [PATCH 14/14] Change filename. --- tests/components/risco/{test_connect.py => test_init.py} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/components/risco/{test_connect.py => test_init.py} (94%) diff --git a/tests/components/risco/test_connect.py b/tests/components/risco/test_init.py similarity index 94% rename from tests/components/risco/test_connect.py rename to tests/components/risco/test_init.py index 8f4d4a18e2b1df..a1a9e3bd6a7ac8 100644 --- a/tests/components/risco/test_connect.py +++ b/tests/components/risco/test_init.py @@ -1,4 +1,4 @@ -"""Tests for the Risco connect logic.""" +"""Tests for the Risco initialization.""" import pytest from homeassistant.components.risco import CannotConnectError