From e891df0ff3f51809f8e2186e9ff8845de8410099 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Mon, 28 Feb 2022 20:07:55 +0100 Subject: [PATCH] Allow multi read of Modbus sensor (#67378) --- homeassistant/components/modbus/__init__.py | 1 + homeassistant/components/modbus/sensor.py | 86 ++++++++++++++++-- homeassistant/components/modbus/validators.py | 9 ++ tests/components/modbus/test_init.py | 8 ++ tests/components/modbus/test_sensor.py | 90 ++++++++++++++++++- 5 files changed, 185 insertions(+), 9 deletions(-) diff --git a/homeassistant/components/modbus/__init__.py b/homeassistant/components/modbus/__init__.py index fd8e15b9b7e..eb55066390b 100644 --- a/homeassistant/components/modbus/__init__.py +++ b/homeassistant/components/modbus/__init__.py @@ -253,6 +253,7 @@ SENSOR_SCHEMA = vol.All( vol.Optional(CONF_DEVICE_CLASS): SENSOR_DEVICE_CLASSES_SCHEMA, vol.Optional(CONF_STATE_CLASS): SENSOR_STATE_CLASSES_SCHEMA, vol.Optional(CONF_UNIT_OF_MEASUREMENT): cv.string, + vol.Optional(CONF_SLAVE_COUNT, default=0): cv.positive_int, } ), ) diff --git a/homeassistant/components/modbus/sensor.py b/homeassistant/components/modbus/sensor.py index e4249594940..8363de3adf1 100644 --- a/homeassistant/components/modbus/sensor.py +++ b/homeassistant/components/modbus/sensor.py @@ -2,19 +2,27 @@ from __future__ import annotations from datetime import datetime +import logging from typing import Any from homeassistant.components.sensor import CONF_STATE_CLASS, SensorEntity from homeassistant.const import CONF_NAME, CONF_SENSORS, CONF_UNIT_OF_MEASUREMENT -from homeassistant.core import HomeAssistant +from homeassistant.core import HomeAssistant, callback from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.helpers.restore_state import RestoreEntity from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType +from homeassistant.helpers.update_coordinator import ( + CoordinatorEntity, + DataUpdateCoordinator, +) from . import get_hub from .base_platform import BaseStructPlatform +from .const import CONF_SLAVE_COUNT from .modbus import ModbusHub +_LOGGER = logging.getLogger(__name__) + PARALLEL_UPDATES = 1 @@ -25,15 +33,18 @@ async def async_setup_platform( discovery_info: DiscoveryInfoType | None = None, ) -> None: """Set up the Modbus sensors.""" - sensors = [] if discovery_info is None: # pragma: no cover return + sensors: list[ModbusRegisterSensor | SlaveSensor] = [] + hub = get_hub(hass, discovery_info[CONF_NAME]) for entry in discovery_info[CONF_SENSORS]: - hub = get_hub(hass, discovery_info[CONF_NAME]) - sensors.append(ModbusRegisterSensor(hub, entry)) - + slave_count = entry.get(CONF_SLAVE_COUNT, 0) + sensor = ModbusRegisterSensor(hub, entry) + if slave_count > 0: + sensors.extend(await sensor.async_setup_slaves(hass, slave_count, entry)) + sensors.append(sensor) async_add_entities(sensors) @@ -47,9 +58,30 @@ class ModbusRegisterSensor(BaseStructPlatform, RestoreEntity, SensorEntity): ) -> None: """Initialize the modbus register sensor.""" super().__init__(hub, entry) + self._coordinator: DataUpdateCoordinator[Any] | None = None self._attr_native_unit_of_measurement = entry.get(CONF_UNIT_OF_MEASUREMENT) self._attr_state_class = entry.get(CONF_STATE_CLASS) + async def async_setup_slaves( + self, hass: HomeAssistant, slave_count: int, entry: dict[str, Any] + ) -> list[SlaveSensor]: + """Add slaves as needed (1 read for multiple sensors).""" + + # Add a dataCoordinator for each sensor that have slaves + # this ensures that idx = bit position of value in result + # polling is done with the base class + name = self._attr_name if self._attr_name else "modbus_sensor" + self._coordinator = DataUpdateCoordinator( + hass, + _LOGGER, + name=name, + ) + + slaves: list[SlaveSensor] = [] + for idx in range(0, slave_count): + slaves.append(SlaveSensor(self._coordinator, idx, entry)) + return slaves + async def async_added_to_hass(self) -> None: """Handle entity which will be added.""" await self.async_base_added_to_hass() @@ -60,10 +92,10 @@ class ModbusRegisterSensor(BaseStructPlatform, RestoreEntity, SensorEntity): """Update the state of the sensor.""" # remark "now" is a dummy parameter to avoid problems with # async_track_time_interval - result = await self._hub.async_pymodbus_call( + raw_result = await self._hub.async_pymodbus_call( self._slave, self._address, self._count, self._input_type ) - if result is None: + if raw_result is None: if self._lazy_errors: self._lazy_errors -= 1 return @@ -72,10 +104,48 @@ class ModbusRegisterSensor(BaseStructPlatform, RestoreEntity, SensorEntity): self.async_write_ha_state() return - self._attr_native_value = self.unpack_structure_result(result.registers) + result = self.unpack_structure_result(raw_result.registers) + if self._coordinator: + if result: + result_array = result.split(",") + self._attr_native_value = result_array[0] + self._coordinator.async_set_updated_data(result_array) + else: + self._attr_native_value = None + self._coordinator.async_set_updated_data(None) + else: + self._attr_native_value = result if self._attr_native_value is None: self._attr_available = False else: self._attr_available = True self._lazy_errors = self._lazy_error_count self.async_write_ha_state() + + +class SlaveSensor(CoordinatorEntity, RestoreEntity, SensorEntity): + """Modbus slave binary sensor.""" + + def __init__( + self, coordinator: DataUpdateCoordinator[Any], idx: int, entry: dict[str, Any] + ) -> None: + """Initialize the Modbus binary sensor.""" + idx += 1 + self._idx = idx + self._attr_name = f"{entry[CONF_NAME]} {idx}" + self._attr_available = False + super().__init__(coordinator) + + async def async_added_to_hass(self) -> None: + """Handle entity which will be added.""" + if state := await self.async_get_last_state(): + self._attr_native_value = state.state + await super().async_added_to_hass() + + @callback + def _handle_coordinator_update(self) -> None: + """Handle updated data from the coordinator.""" + result = self.coordinator.data + if result: + self._attr_native_value = result[self._idx] + super()._handle_coordinator_update() diff --git a/homeassistant/components/modbus/validators.py b/homeassistant/components/modbus/validators.py index 646e1129c4e..35fc9cf63a9 100644 --- a/homeassistant/components/modbus/validators.py +++ b/homeassistant/components/modbus/validators.py @@ -26,6 +26,7 @@ from homeassistant.const import ( from .const import ( CONF_DATA_TYPE, CONF_INPUT_TYPE, + CONF_SLAVE_COUNT, CONF_SWAP, CONF_SWAP_BYTE, CONF_SWAP_NONE, @@ -63,6 +64,7 @@ def struct_validator(config: dict[str, Any]) -> dict[str, Any]: count = config.get(CONF_COUNT, 1) 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: @@ -75,7 +77,14 @@ def struct_validator(config: dict[str, Any]) -> dict[str, Any]: 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}" + raise vol.Invalid(error) if not structure: error = ( f"Error in sensor {name}. The `{CONF_STRUCTURE}` field can not be empty" diff --git a/tests/components/modbus/test_init.py b/tests/components/modbus/test_init.py index 127389ee50f..ac9aa964316 100644 --- a/tests/components/modbus/test_init.py +++ b/tests/components/modbus/test_init.py @@ -42,6 +42,7 @@ from homeassistant.components.modbus.const import ( CONF_INPUT_TYPE, CONF_MSG_WAIT, CONF_PARITY, + CONF_SLAVE_COUNT, CONF_STOPBITS, CONF_SWAP, CONF_SWAP_BYTE, @@ -209,6 +210,13 @@ async def test_ok_struct_validator(do_config): CONF_STRUCTURE: ">f", CONF_SWAP: CONF_SWAP_WORD, }, + { + CONF_NAME: TEST_ENTITY_NAME, + CONF_COUNT: 2, + CONF_DATA_TYPE: DataType.CUSTOM, + CONF_STRUCTURE: ">f", + CONF_SLAVE_COUNT: 5, + }, ], ) async def test_exception_struct_validator(do_config): diff --git a/tests/components/modbus/test_sensor.py b/tests/components/modbus/test_sensor.py index e3085ecf91e..aa513d1c473 100644 --- a/tests/components/modbus/test_sensor.py +++ b/tests/components/modbus/test_sensor.py @@ -9,6 +9,7 @@ from homeassistant.components.modbus.const import ( CONF_LAZY_ERROR, CONF_PRECISION, CONF_SCALE, + CONF_SLAVE_COUNT, CONF_SWAP, CONF_SWAP_BYTE, CONF_SWAP_NONE, @@ -124,6 +125,16 @@ ENTITY_ID = f"{SENSOR_DOMAIN}.{TEST_ENTITY_NAME}".replace(" ", "_") } ] }, + { + CONF_SENSORS: [ + { + CONF_NAME: TEST_ENTITY_NAME, + CONF_ADDRESS: 51, + CONF_DATA_TYPE: DataType.INT32, + CONF_SLAVE_COUNT: 5, + } + ] + }, ], ) async def test_config_sensor(hass, mock_modbus): @@ -535,6 +546,73 @@ async def test_all_sensor(hass, mock_do_cycle, expected): assert hass.states.get(ENTITY_ID).state == expected +@pytest.mark.parametrize( + "do_config", + [ + { + CONF_SENSORS: [ + { + CONF_NAME: TEST_ENTITY_NAME, + CONF_ADDRESS: 51, + CONF_INPUT_TYPE: CALL_TYPE_REGISTER_HOLDING, + CONF_DATA_TYPE: DataType.UINT32, + CONF_SCALE: 1, + CONF_OFFSET: 0, + CONF_PRECISION: 0, + }, + ], + }, + ], +) +@pytest.mark.parametrize( + "config_addon,register_words,expected", + [ + ( + { + CONF_SLAVE_COUNT: 0, + }, + [0x0102, 0x0304], + ["16909060"], + ), + ( + { + CONF_SLAVE_COUNT: 1, + }, + [0x0102, 0x0304, 0x0403, 0x0201], + ["16909060", "67305985"], + ), + ( + { + CONF_SLAVE_COUNT: 3, + }, + [ + 0x0102, + 0x0304, + 0x0506, + 0x0708, + 0x090A, + 0x0B0C, + 0x0D0E, + 0x0F00, + ], + [ + "16909060", + "84281096", + "151653132", + "219025152", + ], + ), + ], +) +async def test_slave_sensor(hass, mock_do_cycle, expected): + """Run test for sensor.""" + assert hass.states.get(ENTITY_ID).state == expected[0] + + for i in range(1, len(expected)): + entity_id = f"{SENSOR_DOMAIN}.{TEST_ENTITY_NAME}_{i}".replace(" ", "_") + assert hass.states.get(entity_id).state == expected[i] + + @pytest.mark.parametrize( "do_config", [ @@ -664,7 +742,7 @@ async def test_struct_sensor(hass, mock_do_cycle, expected): @pytest.mark.parametrize( "mock_test_state", - [(State(ENTITY_ID, "117"),)], + [(State(ENTITY_ID, "117"), State(f"{ENTITY_ID}_1", "119"))], indirect=True, ) @pytest.mark.parametrize( @@ -679,6 +757,16 @@ async def test_struct_sensor(hass, mock_do_cycle, expected): } ] }, + { + CONF_SENSORS: [ + { + CONF_NAME: TEST_ENTITY_NAME, + CONF_ADDRESS: 51, + CONF_SCAN_INTERVAL: 0, + CONF_SLAVE_COUNT: 1, + } + ] + }, ], ) async def test_restore_state_sensor(hass, mock_test_state, mock_modbus):