From 61a4722c9370d1bc527d9a70a1cab961d6e5f360 Mon Sep 17 00:00:00 2001 From: Bonne Eggleston Date: Tue, 4 Aug 2026 07:15:31 -0700 Subject: [PATCH] [modbus_controller] Add bus-fairness integration test (xfail until #11781) (#17345) Co-authored-by: Claude Opus 4.8 --- .../fixtures/uart_mock_modbus_fairness.yaml | 125 ++++++++++++++++++ tests/integration/test_uart_mock_modbus.py | 67 +++++++++- 2 files changed, 191 insertions(+), 1 deletion(-) create mode 100644 tests/integration/fixtures/uart_mock_modbus_fairness.yaml diff --git a/tests/integration/fixtures/uart_mock_modbus_fairness.yaml b/tests/integration/fixtures/uart_mock_modbus_fairness.yaml new file mode 100644 index 0000000000..e2918c82dd --- /dev/null +++ b/tests/integration/fixtures/uart_mock_modbus_fairness.yaml @@ -0,0 +1,125 @@ +esphome: + name: uart-mock-modbus-fairness + +host: +api: +logger: + # DEBUG (not VERBOSE) keeps the log volume manageable while both controllers + # hammer the bus at a high rate. + level: DEBUG + +external_components: + - source: + type: local + path: EXTERNAL_COMPONENT_PATH + +# Dummy uart entry to satisfy modbus's DEPENDENCIES = ["uart"] +# The actual UART bus used is the uart_mock component below +uart: + baud_rate: 115200 + port: /dev/null + +# Counters for the number of requests seen on the bus for each device address. +globals: + - id: req_count_1 + type: int + initial_value: "0" + - id: req_count_2 + type: int + initial_value: "0" + +uart_mock: + - id: virtual_uart + baud_rate: 9600 + # auto_start so the mock is ready to deliver injected responses. Polling + # itself is gated by the Start button below. + auto_start: true + debug: + on_tx: + - then: + # Count each outgoing request by device address (byte 0 of the frame). + - lambda: |- + if (data.empty()) + return; + if (data[0] == 0x01) { + id(req_count_1) += 1; + id(requests_1).publish_state(id(req_count_1)); + } else if (data[0] == 0x02) { + id(req_count_2) += 1; + id(requests_2).publish_state(id(req_count_2)); + } + # Reply directly with a canned, CRC-correct "read holding register" + # response for whichever device was addressed (both controllers only + # ever issue this one fixed request, so the responses are constant). + - uart_mock.inject_rx: + id: virtual_uart + data: !lambda |- + if (!data.empty() && data[0] == 0x01) + return {0x01, 0x03, 0x02, 0x00, 0x6F, 0xF8, 0x68}; // value 111 + if (!data.empty() && data[0] == 0x02) + return {0x02, 0x03, 0x02, 0x00, 0xDE, 0x7C, 0x1C}; // value 222 + return {}; + +modbus: + - uart_id: virtual_uart + id: virtual_modbus_client + role: client + turnaround_time: 15ms #This is longer than the polling interval to cause contention + +# Two controllers sharing one client bus, each polling a different device. +# Polling is started by the test (update_interval: never until then) so counting +# only begins once the API client has subscribed. +modbus_controller: + - address: 1 + modbus_id: virtual_modbus_client + id: modbus_controller_1 + update_interval: never + - address: 2 + modbus_id: virtual_modbus_client + id: modbus_controller_2 + update_interval: never + +sensor: + # These sensors define the register range each controller polls (and so drive + # the requests). Their values are not checked by the test. + - platform: modbus_controller + modbus_controller_id: modbus_controller_1 + name: reg_1 + address: 0x01 + register_type: holding + value_type: U_WORD + - platform: modbus_controller + modbus_controller_id: modbus_controller_2 + name: reg_2 + address: 0x01 + register_type: holding + value_type: U_WORD + # Request counters exposed to the test. Updated manually from the on_tx hook. + - platform: template + name: requests_1 + id: requests_1 + update_interval: never + - platform: template + name: requests_2 + id: requests_2 + update_interval: never + +button: + - platform: template + name: "Start Scenario" + id: start_scenario_btn + on_press: + - lambda: |- + // Poll much faster than the bus can service so both controllers always + // have a request pending and must contend for the bus. + id(modbus_controller_1).set_update_interval(10); + id(modbus_controller_1).start_poller(); + id(modbus_controller_2).set_update_interval(10); + id(modbus_controller_2).start_poller(); + - platform: template + name: "Stop Scenario" + id: stop_scenario_btn + on_press: + - lambda: |- + id(modbus_controller_1).stop_poller(); + id(modbus_controller_2).stop_poller(); diff --git a/tests/integration/test_uart_mock_modbus.py b/tests/integration/test_uart_mock_modbus.py index ce707fb0e0..b7103b62dd 100644 --- a/tests/integration/test_uart_mock_modbus.py +++ b/tests/integration/test_uart_mock_modbus.py @@ -9,6 +9,10 @@ test_uart_mock_modbus_no_threshold : Test modbus with no rx_full_threshold set (simulating USB UART / non-hardware UART). Verifies the 50ms fallback timeout handles chunked data with USB packet gaps. +test_uart_mock_modbus_fairness : + Two controllers sharing one client bus, both polling far faster than the bus + can service. Verifies the hub schedules them fairly (request counts within 1). + """ from __future__ import annotations @@ -17,7 +21,7 @@ import asyncio from collections.abc import Callable from dataclasses import dataclass -from aioesphomeapi import NumberInfo +from aioesphomeapi import ButtonInfo, NumberInfo import pytest from .state_utils import SensorTracker, find_entity @@ -446,3 +450,64 @@ async def test_uart_mock_modbus_shared_address( await tracker.setup_and_start_scenario(client) await tracker.await_all(futures) _assert_no_modbus_errors(error_log_lines, warning_log_lines) + + +@pytest.mark.xfail( + strict=True, + reason="Fair bus scheduling across controllers sharing one client hub " + "requires the modbus_controller refactor in esphome#11781. On dev the " + "controllers each queue independently and contend for the bus, so the " + "request counts diverge. Expected to XPASS (and this marker removed) once " + "that refactor lands.", +) +@pytest.mark.asyncio +async def test_uart_mock_modbus_fairness( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Two controllers sharing one bus should get a fair share of it. + + Both controllers poll different devices (addresses 1 and 2) on the same + client hub, far faster than the bus can service, so they continually + contend for it. The on_tx hook in the fixture counts the requests issued + for each address. With fair scheduling in the modbus hub, neither + controller should starve the other: the two request counts must end up + within 1 of each other. + """ + + tracker = SensorTracker(["requests_1", "requests_2"]) + + async with ( + run_compiled(yaml_config), + api_client_connected() as client, + ): + entities = await tracker.setup_and_start_scenario(client) + + # Let both controllers hammer the bus for a while. + await asyncio.sleep(2.0) + + # Stop polling so the counters settle to a final, stable value (state + # coalescing means intermediate values may be skipped, but the final + # value is always delivered once changes stop). + stop_btn = find_entity(entities, "stop_scenario", ButtonInfo) + assert stop_btn is not None, "Stop Scenario button not found" + client.button_command(stop_btn.key) + await asyncio.sleep(0.5) + + assert tracker.sensor_states["requests_1"], "controller 1 issued no requests" + assert tracker.sensor_states["requests_2"], "controller 2 issued no requests" + count_1 = tracker.sensor_states["requests_1"][-1] + count_2 = tracker.sensor_states["requests_2"][-1] + + # Both must have polled repeatedly, otherwise "fairness" is meaningless. + assert count_1 >= 5 and count_2 >= 5, ( + f"expected both controllers to poll repeatedly, " + f"got controller 1={count_1}, controller 2={count_2}" + ) + # Fair scheduling: the bus alternates between the two pending requests, + # so the counts can differ by at most one in-flight request. + assert abs(count_1 - count_2) <= 1, ( + f"controllers did not get a fair share of the bus: " + f"controller 1 issued {count_1}, controller 2 issued {count_2}" + )