From 5685ba7f5575108236729a5fd57caaeef38359d5 Mon Sep 17 00:00:00 2001 From: Andre Lengwenus Date: Sat, 14 Sep 2024 09:21:15 +0200 Subject: [PATCH] Make acknowledge requests from LCN modules optional (#125765) * Add acknowledge flag to config_entry * Add acknowledge option to lcn configuration * Fix tests * Bump pypck to 0.7.23 * Add entry fixture for config_entry version 1.1 to test migration * Add data_description to strings.json * Create versioned config_entry in tests --- homeassistant/components/lcn/__init__.py | 28 +++ homeassistant/components/lcn/config_flow.py | 6 +- homeassistant/components/lcn/const.py | 1 + homeassistant/components/lcn/helpers.py | 3 + homeassistant/components/lcn/manifest.json | 2 +- homeassistant/components/lcn/strings.json | 16 +- requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/lcn/conftest.py | 9 +- .../lcn/fixtures/config_entry_myhome.json | 1 + .../lcn/fixtures/config_entry_pchk.json | 1 + .../lcn/fixtures/config_entry_pchk_v1_1.json | 230 ++++++++++++++++++ tests/components/lcn/test_config_flow.py | 8 +- tests/components/lcn/test_init.py | 17 ++ 14 files changed, 317 insertions(+), 9 deletions(-) create mode 100644 tests/components/lcn/fixtures/config_entry_pchk_v1_1.json diff --git a/homeassistant/components/lcn/__init__.py b/homeassistant/components/lcn/__init__.py index 96ffaddfb93..a8d75fe5635 100644 --- a/homeassistant/components/lcn/__init__.py +++ b/homeassistant/components/lcn/__init__.py @@ -22,6 +22,7 @@ from homeassistant.helpers.typing import ConfigType from .const import ( ADD_ENTITIES_CALLBACKS, + CONF_ACKNOWLEDGE, CONF_DIM_MODE, CONF_SK_NUM_TRIES, CONNECTION, @@ -73,6 +74,7 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b settings = { "SK_NUM_TRIES": config_entry.data[CONF_SK_NUM_TRIES], "DIM_MODE": pypck.lcn_defs.OutputPortDimMode[config_entry.data[CONF_DIM_MODE]], + "ACKNOWLEDGE": config_entry.data[CONF_ACKNOWLEDGE], } # connect to PCHK @@ -137,6 +139,32 @@ async def async_setup_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> b return True +async def async_migrate_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: + """Migrate old entry.""" + _LOGGER.debug( + "Migrating configuration from version %s.%s", + config_entry.version, + config_entry.minor_version, + ) + + if config_entry.version == 1: + new_data = {**config_entry.data} + + if config_entry.minor_version < 2: + new_data[CONF_ACKNOWLEDGE] = False + + hass.config_entries.async_update_entry( + config_entry, data=new_data, minor_version=2, version=1 + ) + + _LOGGER.debug( + "Migration to configuration version %s.%s successful", + config_entry.version, + config_entry.minor_version, + ) + return True + + async def async_unload_entry(hass: HomeAssistant, config_entry: ConfigEntry) -> bool: """Close connection to PCHK host represented by config_entry.""" # forward unloading to platforms diff --git a/homeassistant/components/lcn/config_flow.py b/homeassistant/components/lcn/config_flow.py index e3979effc07..a1a98a39db3 100644 --- a/homeassistant/components/lcn/config_flow.py +++ b/homeassistant/components/lcn/config_flow.py @@ -26,7 +26,7 @@ from homeassistant.helpers.issue_registry import IssueSeverity, async_create_iss from homeassistant.helpers.typing import ConfigType from . import PchkConnectionManager -from .const import CONF_DIM_MODE, CONF_SK_NUM_TRIES, DIM_MODES, DOMAIN +from .const import CONF_ACKNOWLEDGE, CONF_DIM_MODE, CONF_SK_NUM_TRIES, DIM_MODES, DOMAIN from .helpers import purge_device_registry, purge_entity_registry _LOGGER = logging.getLogger(__name__) @@ -38,6 +38,7 @@ CONFIG_DATA = { vol.Required(CONF_PASSWORD, default=""): str, vol.Required(CONF_SK_NUM_TRIES, default=0): cv.positive_int, vol.Required(CONF_DIM_MODE, default="STEPS200"): vol.In(DIM_MODES), + vol.Required(CONF_ACKNOWLEDGE, default=False): cv.boolean, } USER_DATA = {vol.Required(CONF_HOST, default="pchk"): str, **CONFIG_DATA} @@ -71,10 +72,12 @@ async def validate_connection(data: ConfigType) -> str | None: password = data[CONF_PASSWORD] sk_num_tries = data[CONF_SK_NUM_TRIES] dim_mode = data[CONF_DIM_MODE] + acknowledge = data[CONF_ACKNOWLEDGE] settings = { "SK_NUM_TRIES": sk_num_tries, "DIM_MODE": pypck.lcn_defs.OutputPortDimMode[dim_mode], + "ACKNOWLEDGE": acknowledge, } _LOGGER.debug("Validating connection parameters to PCHK host '%s'", host_name) @@ -108,6 +111,7 @@ class LcnFlowHandler(config_entries.ConfigFlow, domain=DOMAIN): """Handle a LCN config flow.""" VERSION = 1 + MINOR_VERSION = 2 async def async_step_import(self, import_data: dict[str, Any]) -> ConfigFlowResult: """Import existing configuration from LCN.""" diff --git a/homeassistant/components/lcn/const.py b/homeassistant/components/lcn/const.py index 24d2e68495c..707d0f29ba3 100644 --- a/homeassistant/components/lcn/const.py +++ b/homeassistant/components/lcn/const.py @@ -25,6 +25,7 @@ CONF_SOFTWARE_SERIAL = "software_serial" CONF_HARDWARE_TYPE = "hardware_type" CONF_DOMAIN_DATA = "domain_data" +CONF_ACKNOWLEDGE = "acknowledge" CONF_CONNECTIONS = "connections" CONF_SK_NUM_TRIES = "sk_num_tries" CONF_OUTPUT = "output" diff --git a/homeassistant/components/lcn/helpers.py b/homeassistant/components/lcn/helpers.py index 70034e9020a..7da047682ac 100644 --- a/homeassistant/components/lcn/helpers.py +++ b/homeassistant/components/lcn/helpers.py @@ -37,6 +37,7 @@ from homeassistant.helpers.typing import ConfigType from .const import ( BINSENSOR_PORTS, + CONF_ACKNOWLEDGE, CONF_CLIMATES, CONF_CONNECTIONS, CONF_DIM_MODE, @@ -158,6 +159,7 @@ def import_lcn_config(lcn_config: ConfigType) -> list[ConfigType]: "password": "lcn, "sk_num_tries: 0, "dim_mode: "STEPS200", + "acknowledge": False, "devices": [ { "address": (0, 7, False) @@ -192,6 +194,7 @@ def import_lcn_config(lcn_config: ConfigType) -> list[ConfigType]: CONF_PASSWORD: connection[CONF_PASSWORD], CONF_SK_NUM_TRIES: connection[CONF_SK_NUM_TRIES], CONF_DIM_MODE: connection[CONF_DIM_MODE], + CONF_ACKNOWLEDGE: False, CONF_DEVICES: [], CONF_ENTITIES: [], } diff --git a/homeassistant/components/lcn/manifest.json b/homeassistant/components/lcn/manifest.json index 9023941277f..43a34291138 100644 --- a/homeassistant/components/lcn/manifest.json +++ b/homeassistant/components/lcn/manifest.json @@ -8,5 +8,5 @@ "documentation": "https://www.home-assistant.io/integrations/lcn", "iot_class": "local_push", "loggers": ["pypck"], - "requirements": ["pypck==0.7.22", "lcn-frontend==0.1.6"] + "requirements": ["pypck==0.7.23", "lcn-frontend==0.1.6"] } diff --git a/homeassistant/components/lcn/strings.json b/homeassistant/components/lcn/strings.json index a5f303c6392..9b5ce8c9cc0 100644 --- a/homeassistant/components/lcn/strings.json +++ b/homeassistant/components/lcn/strings.json @@ -26,7 +26,12 @@ "username": "[%key:common::config_flow::data::username%]", "password": "[%key:common::config_flow::data::password%]", "sk_num_tries": "Segment coupler scan attempts", - "dim_mode": "Dimming mode" + "dim_mode": "Dimming mode", + "acknowledge": "Request acknowledgement from modules" + }, + "data_description": { + "dim_mode": "The number of steps used for dimming outputs.", + "acknowledge": "Retry sendig commands if no response is received (increases bus traffic)." } }, "reconfigure": { @@ -37,8 +42,13 @@ "port": "[%key:common::config_flow::data::port%]", "username": "[%key:common::config_flow::data::username%]", "password": "[%key:common::config_flow::data::password%]", - "sk_num_tries": "Segment coupler scan attempts", - "dim_mode": "Dimming mode" + "sk_num_tries": "[%key:component::lcn::config::step::user::data::sk_num_tries%]", + "dim_mode": "[%key:component::lcn::config::step::user::data::dim_mode%]", + "acknowledge": "[%key:component::lcn::config::step::user::data::acknowledge%]" + }, + "data_description": { + "dim_mode": "[%key:component::lcn::config::step::user::data_description::dim_mode%]", + "acknowledge": "[%key:component::lcn::config::step::user::data_description::acknowledge%]" } } }, diff --git a/requirements_all.txt b/requirements_all.txt index e6efd168596..ca1c008aad7 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -2130,7 +2130,7 @@ pyownet==0.10.0.post1 pypca==0.0.7 # homeassistant.components.lcn -pypck==0.7.22 +pypck==0.7.23 # homeassistant.components.pjlink pypjlink2==1.2.1 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 9b8a379ecd0..b0e034ebf72 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -1711,7 +1711,7 @@ pyoverkiz==1.13.14 pyownet==0.10.0.post1 # homeassistant.components.lcn -pypck==0.7.22 +pypck==0.7.23 # homeassistant.components.pjlink pypjlink2==1.2.1 diff --git a/tests/components/lcn/conftest.py b/tests/components/lcn/conftest.py index 16797f6065d..3c5979c3c36 100644 --- a/tests/components/lcn/conftest.py +++ b/tests/components/lcn/conftest.py @@ -10,6 +10,7 @@ from pypck.module import GroupConnection, ModuleConnection import pytest from homeassistant.components.lcn import PchkConnectionManager +from homeassistant.components.lcn.config_flow import LcnFlowHandler from homeassistant.components.lcn.const import DOMAIN from homeassistant.components.lcn.helpers import AddressType, generate_unique_id from homeassistant.const import CONF_ADDRESS, CONF_DEVICES, CONF_ENTITIES, CONF_HOST @@ -19,6 +20,8 @@ from homeassistant.setup import async_setup_component from tests.common import MockConfigEntry, load_fixture +LATEST_CONFIG_ENTRY_VERSION = (LcnFlowHandler.VERSION, LcnFlowHandler.MINOR_VERSION) + class MockModuleConnection(ModuleConnection): """Fake a LCN module connection.""" @@ -71,7 +74,9 @@ class MockPchkConnectionManager(PchkConnectionManager): send_command = AsyncMock() -def create_config_entry(name: str) -> MockConfigEntry: +def create_config_entry( + name: str, version: tuple[int, int] = LATEST_CONFIG_ENTRY_VERSION +) -> MockConfigEntry: """Set up config entries with configuration data.""" fixture_filename = f"lcn/config_entry_{name}.json" entry_data = json.loads(load_fixture(fixture_filename)) @@ -89,6 +94,8 @@ def create_config_entry(name: str) -> MockConfigEntry: title=title, data=entry_data, options=options, + version=version[0], + minor_version=version[1], ) diff --git a/tests/components/lcn/fixtures/config_entry_myhome.json b/tests/components/lcn/fixtures/config_entry_myhome.json index a0f8e7d3e10..5abc9749b46 100644 --- a/tests/components/lcn/fixtures/config_entry_myhome.json +++ b/tests/components/lcn/fixtures/config_entry_myhome.json @@ -6,6 +6,7 @@ "password": "lcn", "sk_num_tries": 0, "dim_mode": "STEPS200", + "acknowledge": false, "devices": [], "entities": [ { diff --git a/tests/components/lcn/fixtures/config_entry_pchk.json b/tests/components/lcn/fixtures/config_entry_pchk.json index 9a8095ff16d..d8eef6d1eb3 100644 --- a/tests/components/lcn/fixtures/config_entry_pchk.json +++ b/tests/components/lcn/fixtures/config_entry_pchk.json @@ -6,6 +6,7 @@ "password": "lcn", "sk_num_tries": 0, "dim_mode": "STEPS200", + "acknowledge": false, "devices": [ { "address": [0, 7, false], diff --git a/tests/components/lcn/fixtures/config_entry_pchk_v1_1.json b/tests/components/lcn/fixtures/config_entry_pchk_v1_1.json new file mode 100644 index 00000000000..9a8095ff16d --- /dev/null +++ b/tests/components/lcn/fixtures/config_entry_pchk_v1_1.json @@ -0,0 +1,230 @@ +{ + "host": "pchk", + "ip_address": "192.168.2.41", + "port": 4114, + "username": "lcn", + "password": "lcn", + "sk_num_tries": 0, + "dim_mode": "STEPS200", + "devices": [ + { + "address": [0, 7, false], + "name": "TestModule", + "hardware_serial": -1, + "software_serial": -1, + "hardware_type": -1 + }, + { + "address": [0, 5, true], + "name": "TestGroup", + "hardware_serial": -1, + "software_serial": -1, + "hardware_type": -1 + } + ], + "entities": [ + { + "address": [0, 7, false], + "name": "Light_Output1", + "resource": "output1", + "domain": "light", + "domain_data": { + "output": "OUTPUT1", + "dimmable": true, + "transition": 5000.0 + } + }, + { + "address": [0, 7, false], + "name": "Light_Output2", + "resource": "output2", + "domain": "light", + "domain_data": { + "output": "OUTPUT2", + "dimmable": false, + "transition": 0 + } + }, + { + "address": [0, 7, false], + "name": "Light_Relay1", + "resource": "relay1", + "domain": "light", + "domain_data": { + "output": "RELAY1", + "dimmable": false, + "transition": 0.0 + } + }, + { + "address": [0, 7, false], + "name": "Switch_Output1", + "resource": "output1", + "domain": "switch", + "domain_data": { + "output": "OUTPUT1" + } + }, + { + "address": [0, 7, false], + "name": "Switch_Output2", + "resource": "output2", + "domain": "switch", + "domain_data": { + "output": "OUTPUT2" + } + }, + { + "address": [0, 7, false], + "name": "Switch_Relay1", + "resource": "relay1", + "domain": "switch", + "domain_data": { + "output": "RELAY1" + } + }, + { + "address": [0, 7, false], + "name": "Switch_Relay2", + "resource": "relay2", + "domain": "switch", + "domain_data": { + "output": "RELAY2" + } + }, + { + "address": [0, 5, true], + "name": "Switch_Group5", + "resource": "relay1", + "domain": "switch", + "domain_data": { + "output": "RELAY1" + } + }, + { + "address": [0, 7, false], + "name": "Cover_Outputs", + "resource": "outputs", + "domain": "cover", + "domain_data": { + "motor": "OUTPUTS", + "reverse_time": "RT1200" + } + }, + { + "address": [0, 7, false], + "name": "Cover_Relays", + "resource": "motor1", + "domain": "cover", + "domain_data": { + "motor": "MOTOR1", + "reverse_time": "RT1200" + } + }, + { + "address": [0, 7, false], + "name": "Climate1", + "resource": "var1.r1varsetpoint", + "domain": "climate", + "domain_data": { + "source": "VAR1", + "setpoint": "R1VARSETPOINT", + "lockable": true, + "min_temp": 0.0, + "max_temp": 40.0, + "unit_of_measurement": "°C" + } + }, + { + "address": [0, 7, false], + "name": "Romantic", + "resource": "0.0", + "domain": "scene", + "domain_data": { + "register": 0, + "scene": 0, + "outputs": ["OUTPUT1", "OUTPUT2", "RELAY1"], + "transition": null + } + }, + { + "address": [0, 7, false], + "name": "Romantic Transition", + "resource": "0.1", + "domain": "scene", + "domain_data": { + "register": 0, + "scene": 1, + "outputs": ["OUTPUT1", "OUTPUT2", "RELAY1"], + "transition": 10 + } + }, + { + "address": [0, 7, false], + "name": "Sensor_LockRegulator1", + "resource": "r1varsetpoint", + "domain": "binary_sensor", + "domain_data": { + "source": "R1VARSETPOINT" + } + }, + { + "address": [0, 7, false], + "name": "Binary_Sensor1", + "resource": "binsensor1", + "domain": "binary_sensor", + "domain_data": { + "source": "BINSENSOR1" + } + }, + { + "address": [0, 7, false], + "name": "Sensor_KeyLock", + "resource": "a5", + "domain": "binary_sensor", + "domain_data": { + "source": "A5" + } + }, + { + "address": [0, 7, false], + "name": "Sensor_Var1", + "resource": "var1", + "domain": "sensor", + "domain_data": { + "source": "VAR1", + "unit_of_measurement": "°C" + } + }, + { + "address": [0, 7, false], + "name": "Sensor_Setpoint1", + "resource": "r1varsetpoint", + "domain": "sensor", + "domain_data": { + "source": "R1VARSETPOINT", + "unit_of_measurement": "°C" + } + }, + { + "address": [0, 7, false], + "name": "Sensor_Led6", + "resource": "led6", + "domain": "sensor", + "domain_data": { + "source": "LED6", + "unit_of_measurement": "NATIVE" + } + }, + { + "address": [0, 7, false], + "name": "Sensor_LogicOp1", + "resource": "logicop1", + "domain": "sensor", + "domain_data": { + "source": "LOGICOP1", + "unit_of_measurement": "NATIVE" + } + } + ] +} diff --git a/tests/components/lcn/test_config_flow.py b/tests/components/lcn/test_config_flow.py index 9f46202ac8a..a34592a4f87 100644 --- a/tests/components/lcn/test_config_flow.py +++ b/tests/components/lcn/test_config_flow.py @@ -7,7 +7,12 @@ import pytest from homeassistant import config_entries, data_entry_flow from homeassistant.components.lcn.config_flow import LcnFlowHandler, validate_connection -from homeassistant.components.lcn.const import CONF_DIM_MODE, CONF_SK_NUM_TRIES, DOMAIN +from homeassistant.components.lcn.const import ( + CONF_ACKNOWLEDGE, + CONF_DIM_MODE, + CONF_SK_NUM_TRIES, + DOMAIN, +) from homeassistant.const import ( CONF_BASE, CONF_DEVICES, @@ -31,6 +36,7 @@ CONFIG_DATA = { CONF_PASSWORD: "lcn", CONF_SK_NUM_TRIES: 0, CONF_DIM_MODE: "STEPS200", + CONF_ACKNOWLEDGE: False, } CONNECTION_DATA = {CONF_HOST: "pchk", **CONFIG_DATA} diff --git a/tests/components/lcn/test_init.py b/tests/components/lcn/test_init.py index ece0e95e501..62fa79961cb 100644 --- a/tests/components/lcn/test_init.py +++ b/tests/components/lcn/test_init.py @@ -14,6 +14,7 @@ from homeassistant.helpers import device_registry as dr, entity_registry as er from .conftest import ( MockConfigEntry, MockPchkConnectionManager, + create_config_entry, init_integration, setup_component, ) @@ -125,3 +126,19 @@ async def test_async_setup_from_configuration_yaml(hass: HomeAssistant) -> None: await setup_component(hass) assert async_setup_entry.await_count == 2 + + +@patch("homeassistant.components.lcn.PchkConnectionManager", MockPchkConnectionManager) +async def test_migrate_1_1(hass: HomeAssistant, entry) -> None: + """Test migration config entry.""" + entry_v1_1 = create_config_entry("pchk_v1_1", version=(1, 1)) + entry_v1_1.add_to_hass(hass) + + await hass.config_entries.async_setup(entry_v1_1.entry_id) + await hass.async_block_till_done() + + entry_migrated = hass.config_entries.async_get_entry(entry_v1_1.entry_id) + assert entry_migrated.state is ConfigEntryState.LOADED + assert entry_migrated.version == 1 + assert entry_migrated.minor_version == 2 + assert entry_migrated.data == entry.data