From 59d37f65d5d59e2077ccc35e5440cbf56fdfeffe Mon Sep 17 00:00:00 2001 From: jan iversen Date: Fri, 18 Aug 2023 10:55:39 +0200 Subject: [PATCH] Correct modbus config validator: slave/swap (#97798) --- homeassistant/components/modbus/validators.py | 71 +++++++++++-------- tests/components/modbus/test_init.py | 11 ++- tests/components/modbus/test_sensor.py | 2 +- 3 files changed, 53 insertions(+), 31 deletions(-) diff --git a/homeassistant/components/modbus/validators.py b/homeassistant/components/modbus/validators.py index ee9d40dd874..40461e3effd 100644 --- a/homeassistant/components/modbus/validators.py +++ b/homeassistant/components/modbus/validators.py @@ -65,25 +65,14 @@ def struct_validator(config: dict[str, Any]) -> dict[str, Any]: name = config[CONF_NAME] structure = config.get(CONF_STRUCTURE) slave_count = config.get(CONF_SLAVE_COUNT, 0) + 1 - swap_type = config.get(CONF_SWAP) - if config[CONF_DATA_TYPE] != DataType.CUSTOM: - if structure: - error = f"{name} structure: cannot be mixed with {data_type}" + slave = config.get(CONF_SLAVE, 0) + swap_type = config.get(CONF_SWAP, CONF_SWAP_NONE) + if config[CONF_DATA_TYPE] == DataType.CUSTOM: + if slave or slave_count > 1: + error = f"{name}: `{CONF_STRUCTURE}` illegal with `{CONF_SLAVE_COUNT}` / `{CONF_SLAVE}`" raise vol.Invalid(error) - if data_type not in DEFAULT_STRUCT_FORMAT: - error = f"Error in sensor {name}. data_type `{data_type}` not supported" - raise vol.Invalid(error) - - structure = f">{DEFAULT_STRUCT_FORMAT[data_type].struct_id}" - if CONF_COUNT not in config: - config[CONF_COUNT] = DEFAULT_STRUCT_FORMAT[data_type].register_count - if slave_count > 1: - structure = f">{slave_count}{DEFAULT_STRUCT_FORMAT[data_type].struct_id}" - else: - structure = f">{DEFAULT_STRUCT_FORMAT[data_type].struct_id}" - else: - if slave_count > 1: - error = f"{name} structure: cannot be mixed with {CONF_SLAVE_COUNT}" + if swap_type != CONF_SWAP_NONE: + error = f"{name}: `{CONF_STRUCTURE}` illegal with `{CONF_SWAP}`" raise vol.Invalid(error) if not structure: error = ( @@ -102,19 +91,43 @@ def struct_validator(config: dict[str, Any]) -> dict[str, Any]: f"Structure request {size} bytes, " f"but {count} registers have a size of {bytecount} bytes" ) + return { + **config, + CONF_STRUCTURE: structure, + CONF_SWAP: swap_type, + } - if swap_type != CONF_SWAP_NONE: - if swap_type == CONF_SWAP_BYTE: - regs_needed = 1 - else: # CONF_SWAP_WORD_BYTE, CONF_SWAP_WORD - regs_needed = 2 - if count < regs_needed or (count % regs_needed) != 0: - raise vol.Invalid( - f"Error in sensor {name} swap({swap_type}) " - "not possible due to the registers " - f"count: {count}, needed: {regs_needed}" - ) + if structure: + error = f"{name} structure: cannot be mixed with {data_type}" + raise vol.Invalid(error) + if data_type not in DEFAULT_STRUCT_FORMAT: + error = f"Error in sensor {name}. data_type `{data_type}` not supported" + raise vol.Invalid(error) + if (slave or slave_count > 1) and data_type == DataType.STRING: + error = ( + f"{name}: `{data_type}` illegal with `{CONF_SLAVE_COUNT}` / `{CONF_SLAVE}`" + ) + raise vol.Invalid(error) + if CONF_COUNT not in config: + config[CONF_COUNT] = DEFAULT_STRUCT_FORMAT[data_type].register_count + if swap_type != CONF_SWAP_NONE: + if swap_type == CONF_SWAP_BYTE: + regs_needed = 1 + else: # CONF_SWAP_WORD_BYTE, CONF_SWAP_WORD + regs_needed = 2 + count = config[CONF_COUNT] + if count < regs_needed or (count % regs_needed) != 0: + raise vol.Invalid( + f"Error in sensor {name} swap({swap_type}) " + "not possible due to the registers " + f"count: {count}, needed: {regs_needed}" + ) + structure = f">{DEFAULT_STRUCT_FORMAT[data_type].struct_id}" + if slave_count > 1: + structure = f">{slave_count}{DEFAULT_STRUCT_FORMAT[data_type].struct_id}" + else: + structure = f">{DEFAULT_STRUCT_FORMAT[data_type].struct_id}" return { **config, CONF_STRUCTURE: structure, diff --git a/tests/components/modbus/test_init.py b/tests/components/modbus/test_init.py index 35c01ec478b..6ad1e33821c 100644 --- a/tests/components/modbus/test_init.py +++ b/tests/components/modbus/test_init.py @@ -181,7 +181,6 @@ async def test_nan_validator() -> None: CONF_COUNT: 2, CONF_DATA_TYPE: DataType.CUSTOM, CONF_STRUCTURE: ">i", - CONF_SWAP: CONF_SWAP_BYTE, }, ], ) @@ -239,6 +238,16 @@ async def test_ok_struct_validator(do_config) -> None: CONF_STRUCTURE: ">f", CONF_SLAVE_COUNT: 5, }, + { + CONF_NAME: TEST_ENTITY_NAME, + CONF_DATA_TYPE: DataType.STRING, + CONF_SLAVE_COUNT: 2, + }, + { + CONF_NAME: TEST_ENTITY_NAME, + CONF_DATA_TYPE: DataType.INT16, + CONF_SWAP: CONF_SWAP_WORD, + }, ], ) async def test_exception_struct_validator(do_config) -> None: diff --git a/tests/components/modbus/test_sensor.py b/tests/components/modbus/test_sensor.py index 06b0b68a746..e7d15c971c9 100644 --- a/tests/components/modbus/test_sensor.py +++ b/tests/components/modbus/test_sensor.py @@ -246,7 +246,7 @@ async def test_config_sensor(hass: HomeAssistant, mock_modbus) -> None: }, ] }, - f"Error in sensor {TEST_ENTITY_NAME} swap(word) not possible due to the registers count: 1, needed: 2", + f"{TEST_ENTITY_NAME}: `structure` illegal with `swap`", ), ], )