From 2ecfd74fa48ee6db74491f465914c4552c6892db Mon Sep 17 00:00:00 2001 From: jan iversen Date: Mon, 12 Jul 2021 07:58:45 +0200 Subject: [PATCH] Add more data_types to modbus (#52423) * Add more data_types. * Use new struct when writing temperature. --- .coveragerc | 1 + homeassistant/components/modbus/__init__.py | 19 +++ homeassistant/components/modbus/climate.py | 4 +- homeassistant/components/modbus/const.py | 22 +++- homeassistant/components/modbus/validators.py | 120 +++++++++++------- tests/components/modbus/test_init.py | 3 +- tests/components/modbus/test_sensor.py | 55 +------- 7 files changed, 123 insertions(+), 101 deletions(-) diff --git a/.coveragerc b/.coveragerc index 3c7f7640623..d7eae65becf 100644 --- a/.coveragerc +++ b/.coveragerc @@ -634,6 +634,7 @@ omit = homeassistant/components/mochad/* homeassistant/components/modbus/climate.py homeassistant/components/modbus/modbus.py + homeassistant/components/modbus/validators.py homeassistant/components/modem_callerid/sensor.py homeassistant/components/motion_blinds/__init__.py homeassistant/components/motion_blinds/const.py diff --git a/homeassistant/components/modbus/__init__.py b/homeassistant/components/modbus/__init__.py index e60bbbda78b..8ccfe45f86c 100644 --- a/homeassistant/components/modbus/__init__.py +++ b/homeassistant/components/modbus/__init__.py @@ -94,9 +94,18 @@ from .const import ( CONF_WRITE_TYPE, DATA_TYPE_CUSTOM, DATA_TYPE_FLOAT, + DATA_TYPE_FLOAT16, + DATA_TYPE_FLOAT32, + DATA_TYPE_FLOAT64, DATA_TYPE_INT, + DATA_TYPE_INT16, + DATA_TYPE_INT32, + DATA_TYPE_INT64, DATA_TYPE_STRING, DATA_TYPE_UINT, + DATA_TYPE_UINT16, + DATA_TYPE_UINT32, + DATA_TYPE_UINT64, DEFAULT_HUB, DEFAULT_SCAN_INTERVAL, DEFAULT_TEMP_UNIT, @@ -137,6 +146,16 @@ BASE_STRUCT_SCHEMA = BASE_COMPONENT_SCHEMA.extend( vol.Optional(CONF_COUNT, default=1): cv.positive_int, vol.Optional(CONF_DATA_TYPE, default=DATA_TYPE_INT): vol.In( [ + DATA_TYPE_INT16, + DATA_TYPE_INT32, + DATA_TYPE_INT64, + DATA_TYPE_UINT16, + DATA_TYPE_UINT32, + DATA_TYPE_UINT64, + DATA_TYPE_FLOAT16, + DATA_TYPE_FLOAT32, + DATA_TYPE_FLOAT64, + DATA_TYPE_STRING, DATA_TYPE_INT, DATA_TYPE_UINT, DATA_TYPE_FLOAT, diff --git a/homeassistant/components/modbus/climate.py b/homeassistant/components/modbus/climate.py index 5c99ac86d6c..bfc04024e45 100644 --- a/homeassistant/components/modbus/climate.py +++ b/homeassistant/components/modbus/climate.py @@ -40,6 +40,7 @@ from .const import ( CONF_SWAP_WORD, CONF_SWAP_WORD_BYTE, CONF_TARGET_TEMP, + DEFAULT_STRUCT_FORMAT, MODBUS_DOMAIN, ) from .modbus import ModbusHub @@ -156,7 +157,8 @@ class ModbusThermostat(BasePlatform, RestoreEntity, ClimateEntity): (kwargs.get(ATTR_TEMPERATURE) - self._offset) / self._scale ) byte_string = struct.pack(self._structure, target_temperature) - register_value = struct.unpack(">h", byte_string[0:2])[0] + struct_string = f">{DEFAULT_STRUCT_FORMAT[self._data_type]}" + register_value = struct.unpack(struct_string, byte_string)[0] result = await self._hub.async_pymodbus_call( self._slave, self._target_temperature_register, diff --git a/homeassistant/components/modbus/const.py b/homeassistant/components/modbus/const.py index 8fb4626d2fe..2607fd2bdf0 100644 --- a/homeassistant/components/modbus/const.py +++ b/homeassistant/components/modbus/const.py @@ -79,6 +79,15 @@ DATA_TYPE_FLOAT = "float" DATA_TYPE_INT = "int" DATA_TYPE_UINT = "uint" DATA_TYPE_STRING = "string" +DATA_TYPE_INT16 = "int16" +DATA_TYPE_INT32 = "int32" +DATA_TYPE_INT64 = "int64" +DATA_TYPE_UINT16 = "uint16" +DATA_TYPE_UINT32 = "uint32" +DATA_TYPE_UINT64 = "uint64" +DATA_TYPE_FLOAT16 = "float16" +DATA_TYPE_FLOAT32 = "float32" +DATA_TYPE_FLOAT64 = "float64" # call types CALL_TYPE_COIL = "coil" @@ -100,9 +109,16 @@ DEFAULT_SCAN_INTERVAL = 15 # seconds DEFAULT_SLAVE = 1 DEFAULT_STRUCTURE_PREFIX = ">f" DEFAULT_STRUCT_FORMAT = { - DATA_TYPE_INT: {1: "h", 2: "i", 4: "q"}, - DATA_TYPE_UINT: {1: "H", 2: "I", 4: "Q"}, - DATA_TYPE_FLOAT: {1: "e", 2: "f", 4: "d"}, + DATA_TYPE_INT16: "h", + DATA_TYPE_INT32: "i", + DATA_TYPE_INT64: "q", + DATA_TYPE_UINT16: "H", + DATA_TYPE_UINT32: "I", + DATA_TYPE_UINT64: "Q", + DATA_TYPE_FLOAT16: "e", + DATA_TYPE_FLOAT32: "f", + DATA_TYPE_FLOAT64: "d", + DATA_TYPE_STRING: "s", } DEFAULT_TEMP_UNIT = "C" MODBUS_DOMAIN = "modbus" diff --git a/homeassistant/components/modbus/validators.py b/homeassistant/components/modbus/validators.py index 03f27dd461b..6e8b740c415 100644 --- a/homeassistant/components/modbus/validators.py +++ b/homeassistant/components/modbus/validators.py @@ -21,7 +21,18 @@ from .const import ( CONF_SWAP_BYTE, CONF_SWAP_NONE, DATA_TYPE_CUSTOM, - DATA_TYPE_STRING, + DATA_TYPE_FLOAT, + DATA_TYPE_FLOAT16, + DATA_TYPE_FLOAT32, + DATA_TYPE_FLOAT64, + DATA_TYPE_INT, + DATA_TYPE_INT16, + DATA_TYPE_INT32, + DATA_TYPE_INT64, + DATA_TYPE_UINT, + DATA_TYPE_UINT16, + DATA_TYPE_UINT32, + DATA_TYPE_UINT64, DEFAULT_SCAN_INTERVAL, DEFAULT_STRUCT_FORMAT, PLATFORMS, @@ -29,58 +40,79 @@ from .const import ( _LOGGER = logging.getLogger(__name__) +old_data_types = { + DATA_TYPE_INT: { + 1: DATA_TYPE_INT16, + 2: DATA_TYPE_INT32, + 4: DATA_TYPE_INT64, + }, + DATA_TYPE_UINT: { + 1: DATA_TYPE_UINT16, + 2: DATA_TYPE_UINT32, + 4: DATA_TYPE_UINT64, + }, + DATA_TYPE_FLOAT: { + 1: DATA_TYPE_FLOAT16, + 2: DATA_TYPE_FLOAT32, + 4: DATA_TYPE_FLOAT64, + }, +} + def sensor_schema_validator(config): """Sensor schema validator.""" - if config[CONF_DATA_TYPE] == DATA_TYPE_STRING: - structure = str(config[CONF_COUNT] * 2) + "s" - elif config[CONF_DATA_TYPE] != DATA_TYPE_CUSTOM: - try: - structure = ( - f">{DEFAULT_STRUCT_FORMAT[config[CONF_DATA_TYPE]][config[CONF_COUNT]]}" - ) - except KeyError as key: - raise vol.Invalid( - f"Unable to detect data type for {config[CONF_NAME]} sensor, try a custom type" - ) from key - else: - structure = config.get(CONF_STRUCTURE) - - if not structure: - raise vol.Invalid( - f"Error in sensor {config[CONF_NAME]}. The `{CONF_STRUCTURE}` field can not be empty " - f"if the parameter `{CONF_DATA_TYPE}` is set to the `{DATA_TYPE_CUSTOM}`" - ) - - try: - size = struct.calcsize(structure) - except struct.error as err: - raise vol.Invalid( - f"Error in sensor {config[CONF_NAME]} structure: {str(err)}" - ) from err - - bytecount = config[CONF_COUNT] * 2 - if bytecount != size: - raise vol.Invalid( - f"Structure request {size} bytes, " - f"but {config[CONF_COUNT]} registers have a size of {bytecount} bytes" - ) - + data_type = config[CONF_DATA_TYPE] + count = config[CONF_COUNT] + name = config[CONF_NAME] + structure = config.get(CONF_STRUCTURE) swap_type = config.get(CONF_SWAP) + if data_type in [DATA_TYPE_INT, DATA_TYPE_UINT, DATA_TYPE_FLOAT]: + error = f"{name} {name} with {data_type} is not valid, trying to convert" + _LOGGER.warning(error) + try: + data_type = old_data_types[data_type][count] + except KeyError as exp: + raise vol.Invalid("cannot convert automatically") from exp - if config.get(CONF_SWAP) != CONF_SWAP_NONE: - if swap_type == CONF_SWAP_BYTE: - regs_needed = 1 - else: # CONF_SWAP_WORD_BYTE, CONF_SWAP_WORD - regs_needed = 2 - if config[CONF_COUNT] < regs_needed or (config[CONF_COUNT] % regs_needed) != 0: + if config[CONF_DATA_TYPE] != DATA_TYPE_CUSTOM: + try: + structure = f">{DEFAULT_STRUCT_FORMAT[data_type]}" + except KeyError: raise vol.Invalid( - f"Error in sensor {config[CONF_NAME]} swap({swap_type}) " - f"not possible due to the registers " - f"count: {config[CONF_COUNT]}, needed: {regs_needed}" + f"Modbus error {data_type} unknown in {name}" + ) from KeyError + else: + if not structure: + raise vol.Invalid( + f"Error in sensor {config[CONF_NAME]}. The `{CONF_STRUCTURE}` field can not be empty " + f"if the parameter `{CONF_DATA_TYPE}` is set to the `{DATA_TYPE_CUSTOM}`" ) + try: + size = struct.calcsize(structure) + except struct.error as err: + raise vol.Invalid(f"Error in {name} structure: {str(err)}") from err + + bytecount = count * 2 + if bytecount != size: + raise vol.Invalid( + f"Structure request {size} bytes, " + f"but {count} registers have a size of {bytecount} bytes" + ) + + 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}) " + f"not possible due to the registers " + f"count: {count}, needed: {regs_needed}" + ) + return { **config, CONF_STRUCTURE: structure, diff --git a/tests/components/modbus/test_init.py b/tests/components/modbus/test_init.py index 435b8446b6b..7dda164e5eb 100644 --- a/tests/components/modbus/test_init.py +++ b/tests/components/modbus/test_init.py @@ -180,7 +180,8 @@ async def test_ok_sensor_schema_validator(do_config): { CONF_NAME: TEST_SENSOR_NAME, CONF_COUNT: 1, - CONF_DATA_TYPE: DATA_TYPE_INT, + CONF_DATA_TYPE: DATA_TYPE_CUSTOM, + CONF_STRUCTURE: ">f", CONF_SWAP: CONF_SWAP_WORD, }, ], diff --git a/tests/components/modbus/test_sensor.py b/tests/components/modbus/test_sensor.py index f9bc8454281..a3433a504b8 100644 --- a/tests/components/modbus/test_sensor.py +++ b/tests/components/modbus/test_sensor.py @@ -135,15 +135,6 @@ async def test_config_sensor(hass, mock_modbus): @pytest.mark.parametrize( "do_config,error_message", [ - ( - { - CONF_ADDRESS: 1234, - CONF_COUNT: 8, - CONF_PRECISION: 2, - CONF_DATA_TYPE: DATA_TYPE_INT, - }, - "Unable to detect data type for test_sensor sensor, try a custom type", - ), ( { CONF_ADDRESS: 1234, @@ -152,7 +143,7 @@ async def test_config_sensor(hass, mock_modbus): CONF_DATA_TYPE: DATA_TYPE_CUSTOM, CONF_STRUCTURE: ">no struct", }, - "Error in sensor test_sensor structure: bad char in struct format", + "bad char in struct format", ), ( { @@ -172,7 +163,7 @@ async def test_config_sensor(hass, mock_modbus): CONF_SWAP: CONF_SWAP_NONE, CONF_STRUCTURE: "invalid", }, - "Error in sensor test_sensor structure: bad char in struct format", + "bad char in struct format", ), ( { @@ -229,7 +220,7 @@ async def test_config_wrong_struct_sensor( expect_setup_to_fail=True, ) - assert error_message in caplog.text + assert caplog.text.count(error_message) @pytest.mark.parametrize( @@ -597,46 +588,6 @@ async def test_restore_state_sensor(hass, mock_test_state, mock_modbus): assert hass.states.get(ENTITY_ID).state == mock_test_state[0].state -@pytest.mark.parametrize( - "swap_type, error_message", - [ - ( - CONF_SWAP_WORD, - f"Error in sensor {SENSOR_NAME} swap(word) not possible due to the registers count: 1, needed: 2", - ), - ( - CONF_SWAP_WORD_BYTE, - f"Error in sensor {SENSOR_NAME} swap(word_byte) not possible due to the registers count: 1, needed: 2", - ), - ], -) -async def test_swap_sensor_wrong_config( - hass, caplog, swap_type, error_message, mock_pymodbus -): - """Run test for sensor swap.""" - config = { - CONF_NAME: SENSOR_NAME, - CONF_ADDRESS: 1234, - CONF_COUNT: 1, - CONF_SWAP: swap_type, - CONF_DATA_TYPE: DATA_TYPE_INT, - } - - caplog.set_level(logging.ERROR) - caplog.clear() - await base_config_test( - hass, - config, - SENSOR_NAME, - SENSOR_DOMAIN, - CONF_SENSORS, - None, - method_discovery=True, - expect_setup_to_fail=True, - ) - assert error_message in "".join(caplog.messages) - - async def test_service_sensor_update(hass, mock_pymodbus): """Run test for service homeassistant.update_entity.""" config = {