[modbus] Add server support for read/write multiple registers (0x17) (#17357)

Co-authored-by: J. Nick Koston <nick@koston.org>

tests/components/modbus/modbus_helpers_test.cpp

@@ -92,6 +92,18 @@ TEST(ModbusClientFrameLength, ReadWriteMultipleByteCountCappedAtSpecLimit) {
   EXPECT_EQ(client_pdu_length(pdu, sizeof(pdu)), 10 + MAX_NUM_OF_REGISTERS_TO_WRITE_RW * 2);
 }
 
+TEST(ModbusClientFrameLength, ReadWriteMultipleUsesByteCount) {
+  // read start(2) + read qty(2) + write start(2) + write qty(2) + byte count(1) then data
+  const uint8_t frame[] = {0x01, 0x17, 0x9C, 0xB9, 0x00, 0x02, 0x9C, 0x41, 0x00, 0x02, 0x04, 0xAA, 0xBB, 0xCC, 0xDD};
+  EXPECT_EQ(client_frame_length(frame, sizeof(frame)), 13 + 4);
+}
+
+TEST(ModbusClientFrameLength, ReadWriteMultipleMissingByteCount) {
+  // header present up to the write quantity but the byte count byte (frame[10]) is absent
+  const uint8_t frame[] = {0x01, 0x17, 0x9C, 0xB9, 0x00, 0x02, 0x9C, 0x41, 0x00, 0x02};
+  EXPECT_EQ(client_frame_length(frame, sizeof(frame)), 13);
+}
+
 TEST(ModbusClientFrameLength, WriteMultipleMissingByteCount) {
   const uint8_t frame[] = {0x01, 0x10, 0x00, 0x00, 0x00, 0x02};
   EXPECT_EQ(client_frame_length(frame, sizeof(frame)), 9);

tests/integration/fixtures/uart_mock_modbus_server_read_write.yaml

@@ -0,0 +1,106 @@
+esphome:
+  name: uart-mock-modbus-srv-rw
+
+host:
+api:
+logger:
+  level: VERBOSE
+
+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
+
+uart_mock:
+  - id: virtual_uart_dev
+    baud_rate: 9600
+    rx_full_threshold: 120
+    rx_timeout: 2
+    auto_start: false
+    debug:
+    injections:
+      # FC 0x17 Read/Write Multiple Registers on device 1:
+      #   write reg 0x0001 = 0x1234 (qty 1), then read regs 0x0001..0x0002 (qty 2).
+      # Per Modbus 6.17 the write is performed before the read, so reg 0x0001 must
+      # read back the just-written 0x1234 in the same request.
+      - delay: 100ms
+        inject_rx:
+          [0x01, 0x17, 0x00, 0x01, 0x00, 0x02, 0x00, 0x01, 0x00, 0x01, 0x02, 0x12, 0x34, 0x49, 0xD8]
+      # FC 0x17: write reg 0x0003 = 0x5678 (qty 1), then read reg 0x0003 (qty 1) -
+      # a write and read targeting a different register block.
+      - delay: 100ms
+        inject_rx:
+          [0x01, 0x17, 0x00, 0x03, 0x00, 0x01, 0x00, 0x03, 0x00, 0x01, 0x02, 0x56, 0x78, 0x9B, 0x10]
+
+globals:
+  - id: stored_1
+    type: uint16_t
+    initial_value: "0"
+  - id: stored_3
+    type: uint16_t
+    initial_value: "0"
+
+modbus:
+  uart_id: virtual_uart_dev
+  role: server
+
+modbus_server:
+  - address: 1
+    registers:
+      # Writable + readable register backed by a global. The read publishes what it
+      # returns so the test can confirm the write half ran before the read half.
+      - address: 0x01
+        value_type: U_WORD
+        read_lambda: |-
+          id(rw_read_1).publish_state(id(stored_1));
+          return id(stored_1);
+        write_lambda: |-
+          id(stored_1) = x;
+          id(rw_write_1).publish_state(x);
+          return true;
+      # Read-only register, read together with 0x01 by the first request's 2-register read.
+      - address: 0x02
+        value_type: U_WORD
+        read_lambda: |-
+          id(rw_read_2).publish_state(0x00AA);
+          return 0x00AA;
+      # Second writable + readable register, targeted by the second request.
+      - address: 0x03
+        value_type: U_WORD
+        read_lambda: |-
+          id(rw_read_3).publish_state(id(stored_3));
+          return id(stored_3);
+        write_lambda: |-
+          id(stored_3) = x;
+          id(rw_write_3).publish_state(x);
+          return true;
+
+sensor:
+  - platform: template
+    name: "rw_write_1"
+    id: rw_write_1
+  - platform: template
+    name: "rw_read_1"
+    id: rw_read_1
+  - platform: template
+    name: "rw_read_2"
+    id: rw_read_2
+  - platform: template
+    name: "rw_write_3"
+    id: rw_write_3
+  - platform: template
+    name: "rw_read_3"
+    id: rw_read_3
+
+button:
+  - platform: template
+    name: "Start Scenario"
+    id: start_scenario_btn
+    on_press:
+      - lambda: "id(virtual_uart_dev).start_scenario();"

tests/integration/fixtures/uart_mock_modbus_server_read_write_invalid.yaml

@@ -0,0 +1,81 @@
+esphome:
+  name: uart-mock-modbus-srv-rw-inv
+
+host:
+api:
+logger:
+  level: VERBOSE
+
+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
+
+uart_mock:
+  - id: virtual_uart_dev
+    baud_rate: 9600
+    rx_full_threshold: 120
+    rx_timeout: 2
+    auto_start: false
+    debug:
+    injections:
+      # Malformed FC 0x17 Read/Write Multiple Registers, otherwise well formed (valid CRC): write
+      # quantity 2 but byte count 2 (2 registers need 4 bytes), i.e. byte count != 2x write quantity.
+      # The hub must reject it (ILLEGAL_DATA_VALUE) before touching any register.
+      - delay: 100ms
+        inject_rx:
+          [0x01, 0x17, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x02, 0x02, 0x12, 0x34, 0x09, 0x89]
+      # A valid FC 0x03 read of reg 0x0A injected afterwards. Its read_lambda fires the "probe"
+      # sensor, which (because injections run in order) signals the malformed frame was processed.
+      - delay: 100ms
+        inject_rx: [0x01, 0x03, 0x00, 0x0A, 0x00, 0x01, 0xA4, 0x08]
+
+modbus:
+  uart_id: virtual_uart_dev
+  role: server
+
+modbus_server:
+  - address: 1
+    registers:
+      # The malformed request's write half spans 0x01-0x02. Both are registered so modbus_server's
+      # address pre-flight cannot reject the frame on its own: if the hub wrongly accepted it, these
+      # write_lambdas would fire the "write_seen" sensor.
+      - address: 0x01
+        value_type: U_WORD
+        read_lambda: return 0;
+        write_lambda: |-
+          id(write_seen).publish_state(1);
+          return true;
+      - address: 0x02
+        value_type: U_WORD
+        read_lambda: return 0;
+        write_lambda: |-
+          id(write_seen).publish_state(1);
+          return true;
+      # Processing probe: a valid read of this register fires after the malformed frame.
+      - address: 0x0A
+        value_type: U_WORD
+        read_lambda: |-
+          id(probe).publish_state(1);
+          return 1;
+
+sensor:
+  - platform: template
+    name: "write_seen"
+    id: write_seen
+  - platform: template
+    name: "probe"
+    id: probe
+
+button:
+  - platform: template
+    name: "Start Scenario"
+    id: start_scenario_btn
+    on_press:
+      - lambda: "id(virtual_uart_dev).start_scenario();"

tests/integration/test_uart_mock_modbus.py

@@ -203,6 +203,99 @@ async def test_uart_mock_modbus_server(
         _assert_no_modbus_errors(error_log_lines, warning_log_lines)
 
 
+@pytest.mark.asyncio
+async def test_uart_mock_modbus_server_read_write(
+    yaml_config: str,
+    run_compiled: RunCompiledFunction,
+    api_client_connected: APIClientConnectedFactory,
+) -> None:
+    """Test modbus server FC 0x17 (read/write multiple registers).
+
+    Injects raw 0x17 request frames and checks the round-trip through the
+    server's read_lambda/write_lambda, independent of how the hub dispatches
+    0x17 internally:
+      * one request writes reg 0x01 then reads regs 0x01+0x02 -- reg 0x01 reads
+        back the just-written value (the write happens before the read per
+        Modbus 6.17), and the second register is returned by the same
+        multi-register read;
+      * a second request writes and reads a different register block.
+    """
+
+    line_callback, error_log_lines, warning_log_lines = _make_modbus_line_callback()
+
+    tracker = SensorTracker(
+        ["rw_write_1", "rw_read_1", "rw_read_2", "rw_write_3", "rw_read_3"]
+    )
+    futures = tracker.expect_all(
+        {
+            "rw_write_1": 4660,  # 0x1234 written to reg 0x0001
+            "rw_read_1": 4660,  # reg 0x0001 reads back the just-written value
+            "rw_read_2": 170,  # 0x00AA read from reg 0x0002 in the same request
+            "rw_write_3": 22136,  # 0x5678 written to reg 0x0003
+            "rw_read_3": 22136,  # reg 0x0003 reads back the just-written value
+        }
+    )
+
+    async with (
+        run_compiled(yaml_config, line_callback=line_callback),
+        api_client_connected() as client,
+    ):
+        await tracker.setup_and_start_scenario(client)
+        await tracker.await_all(futures)
+        _assert_no_modbus_errors(error_log_lines, warning_log_lines)
+
+
+@pytest.mark.asyncio
+async def test_uart_mock_modbus_server_read_write_invalid(
+    yaml_config: str,
+    run_compiled: RunCompiledFunction,
+    api_client_connected: APIClientConnectedFactory,
+) -> None:
+    """Test modbus server FC 0x17 invalid-frame handling.
+
+    Injects a well-formed (valid CRC) 0x17 request whose write byte count (2)
+    does not match 2x the write quantity (2 registers need 4 bytes), so the hub
+    must reject it with ILLEGAL_DATA_VALUE before touching any register. A valid
+    read is injected right after as a processing marker.
+
+    The invalid frame is verified via bus-level signals rather than the reply
+    frame on the wire: the mock UART cannot observe the server's TX reliably on
+    the host platform (the server's transmission is gated by a millis()-based tx
+    delay), so instead we assert the request is rejected exactly once and never
+    applied to a register.
+    """
+
+    line_callback, error_log_lines, warning_log_lines = _make_modbus_line_callback()
+
+    tracker = SensorTracker(["write_seen", "probe"])
+    probe_seen = tracker.expect("probe", 1)
+
+    async with (
+        run_compiled(yaml_config, line_callback=line_callback),
+        api_client_connected() as client,
+    ):
+        await tracker.setup_and_start_scenario(client)
+        # The probe read is injected after the malformed frame, so once it fires
+        # the malformed frame has already been processed.
+        await tracker.await_change(probe_seen, "probe")
+
+    # Exactly one bus-level rejection for the malformed frame (no cascade)...
+    invalid_warnings = [
+        line for line in warning_log_lines if "Invalid number of registers" in line
+    ]
+    assert len(invalid_warnings) == 1, (
+        "Expected exactly one invalid-frame rejection, got warnings:\n"
+        + "\n".join(warning_log_lines)
+    )
+    assert len(error_log_lines) == 0, (
+        "Expected no modbus errors, but got:\n" + "\n".join(error_log_lines)
+    )
+    # ...and the rejected write is never applied to the target register.
+    assert not tracker.sensor_states["write_seen"], (
+        f"malformed 0x17 must not write, but write_seen fired: {tracker.sensor_states['write_seen']}"
+    )
+
+
 @pytest.mark.asyncio
 async def test_uart_mock_modbus_server_controller(
     yaml_config: str,