[modbus_controller] Add bus-fairness integration test (xfail until #11781) (#17345)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-10-07 03:16:37 +00:00 · 2026-08-04 09:15:31 -05:00
parent dbf7167622
commit 61a4722c93
2 changed files with 191 additions and 1 deletions
@@ -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();
@@ -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}"
+        )