From 00507539c1b5f34c6ac1b3a30acdace9d29788c0 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Fri, 28 May 2021 13:23:44 +0200 Subject: [PATCH] Change Cover to use address/input_type (#51154) * Change Cover to use address/input_type. * Flake. --- homeassistant/components/modbus/__init__.py | 39 +++++++------ homeassistant/components/modbus/cover.py | 64 ++++++--------------- tests/components/modbus/test_cover.py | 25 +++++--- 3 files changed, 55 insertions(+), 73 deletions(-) diff --git a/homeassistant/components/modbus/__init__.py b/homeassistant/components/modbus/__init__.py index d4b70796c1a..f60c765d80c 100644 --- a/homeassistant/components/modbus/__init__.py +++ b/homeassistant/components/modbus/__init__.py @@ -68,7 +68,6 @@ from .const import ( CONF_MIN_TEMP, CONF_PARITY, CONF_PRECISION, - CONF_REGISTER, CONF_REVERSE_ORDER, CONF_SCALE, CONF_STATE_CLOSED, @@ -145,24 +144,26 @@ CLIMATE_SCHEMA = BASE_COMPONENT_SCHEMA.extend( } ) -COVERS_SCHEMA = vol.All( - cv.has_at_least_one_key(CALL_TYPE_COIL, CONF_REGISTER), - BASE_COMPONENT_SCHEMA.extend( - { - vol.Optional(CONF_DEVICE_CLASS): COVER_DEVICE_CLASSES_SCHEMA, - vol.Optional(CONF_STATE_CLOSED, default=0): cv.positive_int, - vol.Optional(CONF_STATE_CLOSING, default=3): cv.positive_int, - vol.Optional(CONF_STATE_OPEN, default=1): cv.positive_int, - vol.Optional(CONF_STATE_OPENING, default=2): cv.positive_int, - vol.Optional(CONF_STATUS_REGISTER): cv.positive_int, - vol.Optional( - CONF_STATUS_REGISTER_TYPE, - default=CALL_TYPE_REGISTER_HOLDING, - ): vol.In([CALL_TYPE_REGISTER_HOLDING, CALL_TYPE_REGISTER_INPUT]), - vol.Exclusive(CALL_TYPE_COIL, CONF_INPUT_TYPE): cv.positive_int, - vol.Exclusive(CONF_REGISTER, CONF_INPUT_TYPE): cv.positive_int, - } - ), +COVERS_SCHEMA = BASE_COMPONENT_SCHEMA.extend( + { + vol.Required(CONF_ADDRESS): cv.positive_int, + vol.Optional(CONF_INPUT_TYPE, default=CALL_TYPE_REGISTER_HOLDING,): vol.In( + [ + CALL_TYPE_REGISTER_HOLDING, + CALL_TYPE_COIL, + ] + ), + vol.Optional(CONF_DEVICE_CLASS): COVER_DEVICE_CLASSES_SCHEMA, + vol.Optional(CONF_STATE_CLOSED, default=0): cv.positive_int, + vol.Optional(CONF_STATE_CLOSING, default=3): cv.positive_int, + vol.Optional(CONF_STATE_OPEN, default=1): cv.positive_int, + vol.Optional(CONF_STATE_OPENING, default=2): cv.positive_int, + vol.Optional(CONF_STATUS_REGISTER): cv.positive_int, + vol.Optional( + CONF_STATUS_REGISTER_TYPE, + default=CALL_TYPE_REGISTER_HOLDING, + ): vol.In([CALL_TYPE_REGISTER_HOLDING, CALL_TYPE_REGISTER_INPUT]), + } ) SWITCH_SCHEMA = BASE_COMPONENT_SCHEMA.extend( diff --git a/homeassistant/components/modbus/cover.py b/homeassistant/components/modbus/cover.py index 2317bc5401f..88c8fd77ae8 100644 --- a/homeassistant/components/modbus/cover.py +++ b/homeassistant/components/modbus/cover.py @@ -6,7 +6,6 @@ from typing import Any from homeassistant.components.cover import SUPPORT_CLOSE, SUPPORT_OPEN, CoverEntity from homeassistant.const import ( - CONF_ADDRESS, CONF_COVERS, CONF_NAME, STATE_CLOSED, @@ -23,11 +22,8 @@ from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType from .base_platform import BasePlatform from .const import ( CALL_TYPE_COIL, - CALL_TYPE_REGISTER_HOLDING, CALL_TYPE_WRITE_COIL, CALL_TYPE_WRITE_REGISTER, - CONF_INPUT_TYPE, - CONF_REGISTER, CONF_STATE_CLOSED, CONF_STATE_CLOSING, CONF_STATE_OPEN, @@ -69,11 +65,7 @@ class ModbusCover(BasePlatform, CoverEntity, RestoreEntity): config: dict[str, Any], ) -> None: """Initialize the modbus cover.""" - config[CONF_ADDRESS] = "0" - config[CONF_INPUT_TYPE] = "" super().__init__(hub, config) - self._coil = config.get(CALL_TYPE_COIL) - self._register = config.get(CONF_REGISTER) self._state_closed = config[CONF_STATE_CLOSED] self._state_closing = config[CONF_STATE_CLOSING] self._state_open = config[CONF_STATE_OPEN] @@ -84,22 +76,23 @@ class ModbusCover(BasePlatform, CoverEntity, RestoreEntity): # If we read cover status from coil, and not from optional status register, # we interpret boolean value False as closed cover, and value True as open cover. # Intermediate states are not supported in such a setup. - if self._coil is not None: + if self._input_type == CALL_TYPE_COIL: self._write_type = CALL_TYPE_WRITE_COIL + self._write_address = self._address if self._status_register is None: self._state_closed = False self._state_open = True self._state_closing = None self._state_opening = None - - # If we read cover status from the main register (i.e., an optional - # status register is not specified), we need to make sure the register_type - # is set to "holding". - if self._register is not None: + else: + # If we read cover status from the main register (i.e., an optional + # status register is not specified), we need to make sure the register_type + # is set to "holding". self._write_type = CALL_TYPE_WRITE_REGISTER - if self._status_register is None: - self._status_register = self._register - self._status_register_type = CALL_TYPE_REGISTER_HOLDING + self._write_address = self._address + if self._status_register: + self._address = self._status_register + self._input_type = self._status_register_type async def async_added_to_hass(self): """Handle entity which will be added.""" @@ -139,7 +132,7 @@ class ModbusCover(BasePlatform, CoverEntity, RestoreEntity): async def async_open_cover(self, **kwargs: Any) -> None: """Open cover.""" result = await self._hub.async_pymodbus_call( - self._slave, self._register, self._state_open, self._write_type + self._slave, self._write_address, self._state_open, self._write_type ) self._available = result is not None await self.async_update() @@ -147,7 +140,7 @@ class ModbusCover(BasePlatform, CoverEntity, RestoreEntity): async def async_close_cover(self, **kwargs: Any) -> None: """Close cover.""" result = await self._hub.async_pymodbus_call( - self._slave, self._register, self._state_closed, self._write_type + self._slave, self._write_address, self._state_closed, self._write_type ) self._available = result is not None await self.async_update() @@ -156,35 +149,16 @@ class ModbusCover(BasePlatform, CoverEntity, RestoreEntity): """Update the state of the cover.""" # remark "now" is a dummy parameter to avoid problems with # async_track_time_interval - if self._coil is not None and self._status_register is None: - self._value = await self._async_read_coil() - else: - self._value = await self._async_read_status_register() - - self.async_write_ha_state() - - async def _async_read_status_register(self) -> int | None: - """Read status register using the Modbus hub slave.""" result = await self._hub.async_pymodbus_call( - self._slave, self._status_register, 1, self._status_register_type + self._slave, self._address, 1, self._input_type ) if result is None: self._available = False + self.async_write_ha_state() return None - - value = int(result.registers[0]) self._available = True - - return value - - async def _async_read_coil(self) -> bool | None: - """Read coil using the Modbus hub slave.""" - result = await self._hub.async_pymodbus_call( - self._slave, self._coil, 1, CALL_TYPE_COIL - ) - if result is None: - self._available = False - return None - - value = bool(result.bits[0] & 1) - return value + if self._input_type == CALL_TYPE_COIL: + self._value = bool(result.bits[0] & 1) + else: + self._value = int(result.registers[0]) + self.async_write_ha_state() diff --git a/tests/components/modbus/test_cover.py b/tests/components/modbus/test_cover.py index 4073e46a86e..98c4e84699f 100644 --- a/tests/components/modbus/test_cover.py +++ b/tests/components/modbus/test_cover.py @@ -7,7 +7,7 @@ from homeassistant.components.cover import DOMAIN as COVER_DOMAIN from homeassistant.components.modbus.const import ( CALL_TYPE_COIL, CALL_TYPE_REGISTER_HOLDING, - CONF_REGISTER, + CONF_INPUT_TYPE, CONF_STATE_CLOSED, CONF_STATE_CLOSING, CONF_STATE_OPEN, @@ -16,6 +16,7 @@ from homeassistant.components.modbus.const import ( CONF_STATUS_REGISTER_TYPE, ) from homeassistant.const import ( + CONF_ADDRESS, CONF_COVERS, CONF_NAME, CONF_SCAN_INTERVAL, @@ -43,13 +44,14 @@ from tests.common import mock_restore_cache }, ], ) -@pytest.mark.parametrize("read_type", [CALL_TYPE_COIL, CONF_REGISTER]) +@pytest.mark.parametrize("read_type", [CALL_TYPE_COIL, CALL_TYPE_REGISTER_HOLDING]) async def test_config_cover(hass, do_options, read_type): """Run test for cover.""" device_name = "test_cover" device_config = { CONF_NAME: device_name, - read_type: 1234, + CONF_ADDRESS: 1234, + CONF_INPUT_TYPE: read_type, **do_options, } await base_config_test( @@ -95,7 +97,8 @@ async def test_coil_cover(hass, regs, expected): hass, { CONF_NAME: cover_name, - CALL_TYPE_COIL: 1234, + CONF_INPUT_TYPE: CALL_TYPE_COIL, + CONF_ADDRESS: 1234, CONF_SLAVE: 1, }, cover_name, @@ -142,7 +145,7 @@ async def test_register_cover(hass, regs, expected): hass, { CONF_NAME: cover_name, - CONF_REGISTER: 1234, + CONF_ADDRESS: 1234, CONF_SLAVE: 1, }, cover_name, @@ -165,7 +168,7 @@ async def test_service_cover_update(hass, mock_pymodbus): CONF_COVERS: [ { CONF_NAME: "test", - CONF_REGISTER: 1234, + CONF_ADDRESS: 1234, CONF_STATUS_REGISTER_TYPE: CALL_TYPE_REGISTER_HOLDING, } ] @@ -196,7 +199,8 @@ async def test_restore_state_cover(hass, state): cover_name = "test" config = { CONF_NAME: cover_name, - CALL_TYPE_COIL: 1234, + CONF_INPUT_TYPE: CALL_TYPE_COIL, + CONF_ADDRESS: 1234, CONF_STATE_OPEN: 1, CONF_STATE_CLOSED: 0, CONF_STATE_OPENING: 2, @@ -229,12 +233,13 @@ async def test_service_cover_move(hass, mock_pymodbus): CONF_COVERS: [ { CONF_NAME: "test", - CONF_REGISTER: 1234, + CONF_ADDRESS: 1234, CONF_STATUS_REGISTER_TYPE: CALL_TYPE_REGISTER_HOLDING, }, { CONF_NAME: "test2", - CALL_TYPE_COIL: 1234, + CONF_INPUT_TYPE: CALL_TYPE_COIL, + CONF_ADDRESS: 1234, }, ] } @@ -254,10 +259,12 @@ async def test_service_cover_move(hass, mock_pymodbus): ) assert hass.states.get(entity_id).state == STATE_CLOSED + mock_pymodbus.reset() mock_pymodbus.read_holding_registers.side_effect = ModbusException("fail write_") await hass.services.async_call( "cover", "close_cover", {"entity_id": entity_id}, blocking=True ) + assert mock_pymodbus.read_holding_registers.called assert hass.states.get(entity_id).state == STATE_UNAVAILABLE mock_pymodbus.read_coils.side_effect = ModbusException("fail write_")