From 363f35da4ed8cb3d3018412f5bd415a61d6ddd31 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sun, 13 Sep 2026 03:08:15 -0400 Subject: [PATCH] Include YAML hubs in the Modbus connections list (#182024) Co-authored-by: Claude Opus 5 --- homeassistant/components/modbus/entity.py | 9 +- homeassistant/components/modbus/modbus.py | 31 +++ homeassistant/components/modbus/services.py | 5 +- .../components/modbus/websocket_api.py | 22 +- .../fixtures/configuration_no_entities.yaml | 5 + tests/components/modbus/test_websocket_api.py | 190 ++++++++++++++++++ 6 files changed, 253 insertions(+), 9 deletions(-) create mode 100644 tests/components/modbus/fixtures/configuration_no_entities.yaml diff --git a/homeassistant/components/modbus/entity.py b/homeassistant/components/modbus/entity.py index 5e6934099a11..ee56e011a75c 100644 --- a/homeassistant/components/modbus/entity.py +++ b/homeassistant/components/modbus/entity.py @@ -16,7 +16,6 @@ from homeassistant.const import ( CONF_DEVICE_CLASS, CONF_NAME, CONF_SCAN_INTERVAL, - CONF_SLAVE, CONF_STRUCTURE, CONF_UNIQUE_ID, STATE_OFF, @@ -40,7 +39,6 @@ from .const import ( CALL_TYPE_X_COILS, CALL_TYPE_X_REGISTER_HOLDINGS, CONF_DATA_TYPE, - CONF_DEVICE_ADDRESS, CONF_INPUT_TYPE, CONF_MAX_VALUE, CONF_MIN_VALUE, @@ -63,7 +61,7 @@ from .const import ( SIGNAL_STOP_ENTITY, DataType, ) -from .modbus import ModbusHub +from .modbus import ModbusHub, entity_unit_id class ModbusBaseEntity(Entity): @@ -80,10 +78,7 @@ class ModbusBaseEntity(Entity): """Initialize the Modbus binary sensor.""" self._hub = hub - if (conf_slave := entry.get(CONF_SLAVE)) is not None: - self._device_address = conf_slave - else: - self._device_address = entry.get(CONF_DEVICE_ADDRESS, 1) + self._device_address = entity_unit_id(entry) self._address = int(entry[CONF_ADDRESS]) self._input_type = entry[CONF_INPUT_TYPE] self._scan_interval = int(entry[CONF_SCAN_INTERVAL]) diff --git a/homeassistant/components/modbus/modbus.py b/homeassistant/components/modbus/modbus.py index 6a7982757ffa..da8771ed65e4 100644 --- a/homeassistant/components/modbus/modbus.py +++ b/homeassistant/components/modbus/modbus.py @@ -19,6 +19,7 @@ from homeassistant.const import ( CONF_METHOD, CONF_NAME, CONF_PORT, + CONF_SLAVE, CONF_TIMEOUT, CONF_TYPE, EVENT_HOMEASSISTANT_STOP, @@ -27,6 +28,7 @@ from homeassistant.core import Event, HomeAssistant from homeassistant.helpers.discovery import async_load_platform from homeassistant.helpers.typing import ConfigType +from .connection import ModbusEndpoint from .const import ( CALL_TYPE_COIL, CALL_TYPE_DISCRETE, @@ -38,6 +40,7 @@ from .const import ( CALL_TYPE_WRITE_REGISTERS, CONF_BAUDRATE, CONF_BYTESIZE, + CONF_DEVICE_ADDRESS, CONF_MSG_WAIT, CONF_PARITY, CONF_STOPBITS, @@ -109,6 +112,13 @@ PB_CALL = [ ] +def entity_unit_id(entity_config: dict[str, Any]) -> int: + """Return the unit an entity config addresses, defaulting to 1.""" + if (conf_slave := entity_config.get(CONF_SLAVE)) is not None: + return int(conf_slave) + return int(entity_config.get(CONF_DEVICE_ADDRESS, 1)) + + async def async_modbus_setup( hass: HomeAssistant, config: ConfigType, @@ -198,8 +208,11 @@ class ModbusHub: "timeout": client_config[CONF_TIMEOUT], "retries": 3, } + # The endpoint is keyed like `ModbusParams.endpoint`, so that a hub and + # a shared connection to one device can be told apart from two devices if self._config_type == SERIAL: # serial configuration + self.endpoint: ModbusEndpoint = ("serial", client_config[CONF_PORT]) if client_config[CONF_METHOD] == "ascii": self._pb_params["framer"] = FramerType.ASCII else: @@ -215,6 +228,11 @@ class ModbusHub: else: # network configuration self._pb_params["host"] = client_config[CONF_HOST] + self.endpoint = ( + "udp" if self._config_type == UDP else "tcp", + client_config[CONF_HOST].lower(), + client_config[CONF_PORT], + ) if self._config_type == RTUOVERTCP: self._pb_params["framer"] = FramerType.RTU else: @@ -227,6 +245,19 @@ class ModbusHub: else: self._msg_wait = 0 + self.units = sorted( + { + entity_unit_id(entity_config) + for _, conf_key in PLATFORMS + for entity_config in client_config.get(conf_key, []) + } + ) + + @property + def connected(self) -> bool: + """Return whether the client currently holds a link to the device.""" + return self._client is not None and self._client.connected + def _log_error(self, text: str) -> None: if text == self._last_log_error: return diff --git a/homeassistant/components/modbus/services.py b/homeassistant/components/modbus/services.py index 834707d48c8c..2abb1963e7f4 100644 --- a/homeassistant/components/modbus/services.py +++ b/homeassistant/components/modbus/services.py @@ -121,9 +121,12 @@ async def _async_reload_config(call: ServiceCall) -> None: reload_config = await async_integration_yaml_config(hass, DOMAIN) if not reload_config: LOGGER.debug("Modbus not present anymore") + hubs.clear() return LOGGER.debug("Modbus reloading") - await async_modbus_setup(hass, reload_config) + # Setup replaces the hubs only once it has new ones to replace them with + if not await async_modbus_setup(hass, reload_config): + hubs.clear() @callback diff --git a/homeassistant/components/modbus/websocket_api.py b/homeassistant/components/modbus/websocket_api.py index 7a027702d4fb..57d388327ea2 100644 --- a/homeassistant/components/modbus/websocket_api.py +++ b/homeassistant/components/modbus/websocket_api.py @@ -8,9 +8,13 @@ from homeassistant.components import websocket_api from homeassistant.core import HomeAssistant, callback from .connection import async_get_connection_info +from .const import DATA_MODBUS_HUBS TYPE_LIST_CONNECTIONS: Final = "modbus/connections/list" +SOURCE_CONFIG_ENTRY: Final = "config_entry" +SOURCE_YAML: Final = "yaml" + @callback def async_setup(hass: HomeAssistant) -> None: @@ -26,7 +30,13 @@ def websocket_list_connections( connection: websocket_api.ActiveConnection, msg: dict[str, Any], ) -> None: - """List the connections, and which config entries hold units on each.""" + """List the connections held over config entries, then those from YAML. + + The unit ids of a connection are keyed by config entry id, or by hub name + for one from YAML. A YAML hub is a link of its own rather than a hold on a + shared connection, so it is listed separately even when it addresses a + device a config entry also talks to. + """ connection.send_result( msg["id"], { @@ -34,9 +44,19 @@ def websocket_list_connections( { "endpoint": list(info.endpoint), "connected": info.connected, + "source": SOURCE_CONFIG_ENTRY, "units": info.units, } for info in async_get_connection_info(hass) ] + + [ + { + "endpoint": list(hub.endpoint), + "connected": hub.connected, + "source": SOURCE_YAML, + "units": {name: hub.units}, + } + for name, hub in hass.data.get(DATA_MODBUS_HUBS, {}).items() + ] }, ) diff --git a/tests/components/modbus/fixtures/configuration_no_entities.yaml b/tests/components/modbus/fixtures/configuration_no_entities.yaml new file mode 100644 index 000000000000..0f12ac886860 --- /dev/null +++ b/tests/components/modbus/fixtures/configuration_no_entities.yaml @@ -0,0 +1,5 @@ +modbus: + type: "tcp" + host: "testHost" + port: 5001 + name: "testModbus" diff --git a/tests/components/modbus/test_websocket_api.py b/tests/components/modbus/test_websocket_api.py index 65941f19720d..a82796120949 100644 --- a/tests/components/modbus/test_websocket_api.py +++ b/tests/components/modbus/test_websocket_api.py @@ -1,20 +1,25 @@ """Test the Modbus websocket API.""" from collections.abc import Callable, Generator +from typing import Any from unittest.mock import AsyncMock, patch from modbus_connection import ModbusTcpParams from modbus_connection.tmodbus import ModbusConnection import pytest +from homeassistant import config as hass_config from homeassistant.components.modbus import async_get_unit +from homeassistant.components.modbus.const import DATA_MODBUS_HUBS from homeassistant.config_entries import ConfigFlow +from homeassistant.const import SERVICE_RELOAD from homeassistant.core import HomeAssistant from homeassistant.setup import async_setup_component from tests.common import ( MockConfigEntry, MockModule, + get_fixture_path, mock_config_flow, mock_integration, mock_platform, @@ -23,6 +28,35 @@ from tests.typing import WebSocketGenerator type ConsumerFactory = Callable[[], MockConfigEntry] +YAML_HUB_NAME = "yaml_hub" + +# The host is given in mixed case, as a shared connection folds the one it is +# keyed by to lower case +TCP_TRANSPORT = {"type": "tcp", "host": "Device.Local", "port": 502} + +SERIAL_TRANSPORT = { + "type": "serial", + "port": "/dev/ttyUSB0", + "baudrate": 9600, + "bytesize": 8, + "method": "rtu", + "parity": "E", + "stopbits": 1, +} + + +def yaml_hub(transport: dict[str, Any]) -> dict[str, Any]: + """Return a hub config on *transport*, with sensors on three units.""" + return { + "name": YAML_HUB_NAME, + "sensors": [ + {"name": "on unit 3", "address": 10, "slave": 3}, + {"name": "on unit 2", "address": 11, "device_address": 2}, + {"name": "on the default unit", "address": 12}, + ], + **transport, + } + class MockFlow(ConfigFlow): """A config flow for the integration standing in for a consumer.""" @@ -76,6 +110,7 @@ async def test_list_connections( { "endpoint": ["tcp", "device.local", 502], "connected": False, + "source": "config_entry", "units": {first.entry_id: [1], second.entry_id: [2]}, } ] @@ -104,6 +139,7 @@ async def test_a_connection_that_is_up_reports_itself_connected( { "endpoint": ["tcp", "device.local", 502], "connected": True, + "source": "config_entry", "units": {entry.entry_id: [1]}, } ] @@ -185,3 +221,157 @@ async def test_one_entry_holding_two_units( result = (await client.receive_json())["result"] assert result["connections"][0]["units"] == {entry.entry_id: [1, 2]} + + +async def test_a_yaml_hub_is_listed_with_the_units_its_entities_address( + hass: HomeAssistant, + hass_ws_client: WebSocketGenerator, + mock_pymodbus: AsyncMock, +) -> None: + """A hub is flagged as YAML and keyed by its name, having no config entry.""" + mock_pymodbus.connected = True + assert await async_setup_component( + hass, "modbus", {"modbus": [yaml_hub(TCP_TRANSPORT)]} + ) + + client = await hass_ws_client(hass) + await client.send_json_auto_id({"type": "modbus/connections/list"}) + result = (await client.receive_json())["result"] + + assert result == { + "connections": [ + { + "endpoint": ["tcp", "device.local", 502], + "connected": True, + "source": "yaml", + "units": {YAML_HUB_NAME: [1, 2, 3]}, + } + ] + } + + +async def test_a_closed_yaml_hub_reports_itself_not_connected( + hass: HomeAssistant, + hass_ws_client: WebSocketGenerator, + mock_pymodbus: AsyncMock, +) -> None: + """The stop action drops the client, which is no longer a link.""" + mock_pymodbus.connected = True + assert await async_setup_component( + hass, "modbus", {"modbus": [yaml_hub(TCP_TRANSPORT)]} + ) + await hass.data[DATA_MODBUS_HUBS][YAML_HUB_NAME].async_close() + + client = await hass_ws_client(hass) + await client.send_json_auto_id({"type": "modbus/connections/list"}) + result = (await client.receive_json())["result"] + + assert result["connections"][0]["connected"] is False + + +async def test_a_yaml_hub_is_listed_beside_a_connection_to_the_same_device( + hass: HomeAssistant, + hass_ws_client: WebSocketGenerator, + consumer: ConsumerFactory, + mock_pymodbus: AsyncMock, +) -> None: + """A hub is a link of its own, so it is never folded into a shared one.""" + mock_pymodbus.connected = False + assert await async_setup_component( + hass, "modbus", {"modbus": [yaml_hub(TCP_TRANSPORT)]} + ) + + entry = consumer() + await hass.config_entries.async_setup(entry.entry_id) + async_get_unit(hass, entry, ModbusTcpParams(host="device.local", port=502), 7) + + client = await hass_ws_client(hass) + await client.send_json_auto_id({"type": "modbus/connections/list"}) + result = (await client.receive_json())["result"] + + assert result["connections"] == [ + { + "endpoint": ["tcp", "device.local", 502], + "connected": False, + "source": "config_entry", + "units": {entry.entry_id: [7]}, + }, + { + "endpoint": ["tcp", "device.local", 502], + "connected": False, + "source": "yaml", + "units": {YAML_HUB_NAME: [1, 2, 3]}, + }, + ] + + +@pytest.mark.parametrize( + ("transport", "endpoint"), + [ + pytest.param(TCP_TRANSPORT, ["tcp", "device.local", 502], id="tcp"), + pytest.param( + {**TCP_TRANSPORT, "type": "rtuovertcp"}, + ["tcp", "device.local", 502], + id="rtuovertcp", + ), + pytest.param( + {**TCP_TRANSPORT, "type": "udp"}, ["udp", "device.local", 502], id="udp" + ), + pytest.param(SERIAL_TRANSPORT, ["serial", "/dev/ttyUSB0"], id="serial"), + ], +) +async def test_the_endpoint_of_a_yaml_hub_follows_its_transport( + hass: HomeAssistant, + hass_ws_client: WebSocketGenerator, + mock_pymodbus: AsyncMock, + transport: dict[str, Any], + endpoint: list[str | int], +) -> None: + """A hub is keyed by the device it addresses, as a shared connection is. + + An RTU-over-TCP hub keys as TCP: the framing differs, the device does not. + """ + mock_pymodbus.connected = True + assert await async_setup_component( + hass, "modbus", {"modbus": [yaml_hub(transport)]} + ) + + client = await hass_ws_client(hass) + await client.send_json_auto_id({"type": "modbus/connections/list"}) + result = (await client.receive_json())["result"] + + assert result["connections"][0]["endpoint"] == endpoint + + +@pytest.mark.parametrize( + "fixture", + [ + pytest.param("configuration_empty.yaml", id="modbus gone from yaml"), + pytest.param("configuration_no_entities.yaml", id="hub without entities"), + ], +) +async def test_a_yaml_hub_a_reload_leaves_behind_is_not_listed( + hass: HomeAssistant, + hass_ws_client: WebSocketGenerator, + mock_pymodbus: AsyncMock, + fixture: str, +) -> None: + """A reload that sets no hub up again leaves no connection behind. + + The reload closes the hubs before reading the new config, so one it does + not set up again is a link to a device nothing talks to. + """ + mock_pymodbus.connected = True + assert await async_setup_component( + hass, "modbus", {"modbus": [yaml_hub(TCP_TRANSPORT)]} + ) + + yaml_path = get_fixture_path(fixture, "modbus") + with patch.object(hass_config, "YAML_CONFIG_FILE", yaml_path): + await hass.services.async_call("modbus", SERVICE_RELOAD, blocking=True) + await hass.async_block_till_done() + + client = await hass_ws_client(hass) + await client.send_json_auto_id({"type": "modbus/connections/list"}) + + assert (await client.receive_json())["result"] == {"connections": []}