From 1a791186006a59c3bd330be86dc4d207e99a5e7e Mon Sep 17 00:00:00 2001 From: jan iversen Date: Sat, 12 Mar 2022 13:12:38 +0100 Subject: [PATCH] Fix modbus reload service (#68040) * Fix modbus reload service. * Please coverage. * Resolve difference between local pytest and github. --- homeassistant/components/modbus/__init__.py | 13 ++++++ homeassistant/components/modbus/modbus.py | 15 +------ .../modbus/fixtures/configuration.yaml | 6 +++ tests/components/modbus/test_init.py | 44 ++++++++++++++++++- 4 files changed, 64 insertions(+), 14 deletions(-) create mode 100644 tests/components/modbus/fixtures/configuration.yaml diff --git a/homeassistant/components/modbus/__init__.py b/homeassistant/components/modbus/__init__.py index 41a1e37425e..17a4acc1742 100644 --- a/homeassistant/components/modbus/__init__.py +++ b/homeassistant/components/modbus/__init__.py @@ -1,6 +1,7 @@ """Support for Modbus.""" from __future__ import annotations +import logging from typing import cast import voluptuous as vol @@ -110,6 +111,9 @@ from .validators import ( struct_validator, ) +_LOGGER = logging.getLogger(__name__) + + BASE_SCHEMA = vol.Schema({vol.Optional(CONF_NAME, default=DEFAULT_HUB): cv.string}) @@ -342,3 +346,12 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: hass, config, ) + + +async def async_reset_platform(hass: HomeAssistant, integration_name: str) -> None: + """Release modbus resources.""" + _LOGGER.info("Modbus reloading") + hubs = hass.data[DOMAIN] + for name in hubs: + await hubs[name].async_close() + del hass.data[DOMAIN] diff --git a/homeassistant/components/modbus/modbus.py b/homeassistant/components/modbus/modbus.py index 3fa4a74a932..5e15f65035a 100644 --- a/homeassistant/components/modbus/modbus.py +++ b/homeassistant/components/modbus/modbus.py @@ -130,10 +130,7 @@ async def async_modbus_setup( ) -> bool: """Set up Modbus component.""" - platform_names = [] - for entry in PLATFORMS: - platform_names.append(entry[1]) - await async_setup_reload_service(hass, DOMAIN, platform_names) + await async_setup_reload_service(hass, DOMAIN, [DOMAIN]) hass.data[DOMAIN] = hub_collect = {} for conf_hub in config[DOMAIN]: @@ -245,14 +242,6 @@ async def async_modbus_setup( return True -async def async_reset_platform(hass: HomeAssistant, integration_name: str) -> None: - """Release modbus resources.""" - _LOGGER.info("Modbus reloading") - for hub in hass.data[DOMAIN]: - await hub.async_close() - del hass.data[DOMAIN] - - class ModbusHub: """Thread safe wrapper class for pymodbus.""" @@ -410,7 +399,7 @@ class ModbusHub: return None async with self._lock: if not self._client: - return None # pragma: no cover + return None result = await self.hass.async_add_executor_job( self._pymodbus_call, unit, address, value, use_call ) diff --git a/tests/components/modbus/fixtures/configuration.yaml b/tests/components/modbus/fixtures/configuration.yaml new file mode 100644 index 00000000000..0a2f46b3151 --- /dev/null +++ b/tests/components/modbus/fixtures/configuration.yaml @@ -0,0 +1,6 @@ +modbus: + type: "tcp" + host: "testHost" + port: 5001 + name: "testModbus" + diff --git a/tests/components/modbus/test_init.py b/tests/components/modbus/test_init.py index ac9aa964316..11ddf4bc426 100644 --- a/tests/components/modbus/test_init.py +++ b/tests/components/modbus/test_init.py @@ -21,6 +21,7 @@ from pymodbus.pdu import ExceptionResponse, IllegalFunctionRequest import pytest import voluptuous as vol +from homeassistant import config as hass_config from homeassistant.components.binary_sensor import DOMAIN as BINARY_SENSOR_DOMAIN from homeassistant.components.modbus.const import ( ATTR_ADDRESS, @@ -83,6 +84,7 @@ from homeassistant.const import ( CONF_TIMEOUT, CONF_TYPE, EVENT_HOMEASSISTANT_STOP, + SERVICE_RELOAD, STATE_ON, STATE_UNAVAILABLE, STATE_UNKNOWN, @@ -99,7 +101,7 @@ from .conftest import ( ReadResult, ) -from tests.common import async_fire_time_changed +from tests.common import async_fire_time_changed, get_fixture_path @pytest.fixture(name="mock_modbus_with_pymodbus") @@ -824,3 +826,43 @@ async def test_stop_restart(hass, caplog, mock_modbus): assert mock_modbus.connect.called assert f"modbus {TEST_MODBUS_NAME} communication closed" in caplog.text assert f"modbus {TEST_MODBUS_NAME} communication open" in caplog.text + + +@pytest.mark.parametrize("do_config", [{}]) +async def test_write_no_client(hass, mock_modbus): + """Run test for service stop and write without client.""" + + mock_modbus.reset() + data = { + ATTR_HUB: TEST_MODBUS_NAME, + } + await hass.services.async_call(DOMAIN, SERVICE_STOP, data, blocking=True) + await hass.async_block_till_done() + assert mock_modbus.close.called + + data = { + ATTR_HUB: TEST_MODBUS_NAME, + ATTR_UNIT: 17, + ATTR_ADDRESS: 16, + ATTR_STATE: True, + } + await hass.services.async_call(DOMAIN, SERVICE_WRITE_COIL, data, blocking=True) + + +@pytest.mark.parametrize("do_config", [{}]) +async def test_integration_reload(hass, caplog, mock_modbus): + """Run test for integration reload.""" + + caplog.set_level(logging.INFO) + caplog.clear() + + yaml_path = get_fixture_path("configuration.yaml", "modbus") + now = dt_util.utcnow() + with mock.patch.object(hass_config, "YAML_CONFIG_FILE", yaml_path): + await hass.services.async_call(DOMAIN, SERVICE_RELOAD, blocking=True) + await hass.async_block_till_done() + for i in range(4): + now = now + timedelta(seconds=1) + async_fire_time_changed(hass, now) + await hass.async_block_till_done() + assert "Modbus reloading" in caplog.text