[modbus] Properly support client-mode broadcast sends (#17467)

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

tests/components/modbus/modbus_client_hub_test.cpp

@@ -684,6 +684,155 @@ TEST(ModbusClientHubSent, FiresOnWireNotOnQueue) {
   EXPECT_TRUE(hub.waiting());
 }
 
+namespace {
+// Records on_sent / on_response / on_no_response so a broadcast's fire-and-forget completion
+// (on_sent, and no terminal) can be asserted.
+class BroadcastProbeDevice : public ModbusClientDevice {
+ public:
+  BroadcastProbeDevice(ModbusClientHub *hub, uint8_t address) : ModbusClientDevice(hub, address) {}
+  void on_sent(std::span<const uint8_t> request_pdu) override { this->sent_count_++; }
+  void on_response(std::span<const uint8_t> request_pdu, std::span<const uint8_t> response_pdu) override {
+    this->response_count_++;
+    this->last_response_size_ = response_pdu.size();
+  }
+  bool on_no_response(std::span<const uint8_t> request_pdu) override {
+    this->no_response_count_++;
+    return false;
+  }
+  int sent_count_{0};
+  int response_count_{0};
+  int no_response_count_{0};
+  size_t last_response_size_{0};
+};
+}  // namespace
+
+// A broadcast (address 0) is never answered (Modbus 4.1), so the client treats it as fire-and-forget:
+// on_sent fires as the frame goes out, NO terminal (on_response/on_error/on_no_response) is delivered,
+// the hub is left NOT waiting - no timeout is burned - and the sweep erases the entry.
+TEST(ModbusClientHubBroadcast, CompletesAtTransmissionWithoutWaiting) {
+  NullUART uart;
+  NoResponseProbeHub hub;
+  hub.set_uart_parent(&uart);
+  hub.setup();
+  BroadcastProbeDevice device(&hub, BROADCAST_ADDRESS);
+
+  const uint8_t write[] = {0x06, 0x00, 0x10, 0x00, 0x01};  // write single register 0x0010 = 0x0001
+  ASSERT_TRUE(device.queue_pdu(write));
+  EXPECT_EQ(hub.queued_frames(), 1u);
+
+  hub.send_next_for_test();  // transmit + sweep
+
+  EXPECT_EQ(device.sent_count_, 1);         // the frame went on the wire
+  EXPECT_EQ(device.response_count_, 0);     // fire-and-forget: no terminal callback
+  EXPECT_EQ(device.no_response_count_, 0);  // and it never waited for a reply
+  EXPECT_FALSE(hub.waiting());              // no waiting slot occupied
+  EXPECT_EQ(hub.queued_frames(), 0u);       // and the entry is gone
+  EXPECT_EQ(hub.entries(), 0u);
+}
+
+namespace {
+// Keeps the DEFAULT on_response() (so the base typed dispatcher runs) and records the typed write
+// callback and the catch-all, to prove a broadcast reaches neither - only on_sent.
+class BroadcastTypedProbeDevice : public ModbusClientDevice {
+ public:
+  BroadcastTypedProbeDevice(ModbusClientHub *hub, uint8_t address) : ModbusClientDevice(hub, address) {}
+  void on_sent(std::span<const uint8_t> request_pdu) override { this->sent_count_++; }
+  void on_write_single_register(uint16_t address, uint16_t value, ResponseStatus status) override {
+    this->write_single_count_++;
+  }
+  void on_custom_response(std::span<const uint8_t> request_pdu, std::span<const uint8_t> response_pdu,
+                          ResponseStatus status) override {
+    this->custom_count_++;
+  }
+  int sent_count_{0};
+  int write_single_count_{0};
+  int custom_count_{0};
+};
+}  // namespace
+
+// Completing a broadcast with an empty response({}) used to fall, for a device on the default
+// on_response(), through the typed dispatcher to on_custom_response() - firing the wrong callback and
+// logging a spurious "non-standard" warning. Fire-and-forget delivers no terminal at all, so a broadcast
+// write reaches neither the typed write callback nor the catch-all: only on_sent.
+TEST(ModbusClientHubBroadcast, DeliversNoTerminalToTypedDevice) {
+  NullUART uart;
+  NoResponseProbeHub hub;
+  hub.set_uart_parent(&uart);
+  hub.setup();
+  BroadcastTypedProbeDevice device(&hub, BROADCAST_ADDRESS);
+
+  const uint8_t write[] = {0x06, 0x00, 0x10, 0x00, 0x01};  // write single register 0x0010 = 0x0001
+  ASSERT_TRUE(device.queue_pdu(write));
+
+  hub.send_next_for_test();  // transmit + sweep
+
+  EXPECT_EQ(device.sent_count_, 1);          // on_sent still reports the transmission
+  EXPECT_EQ(device.write_single_count_, 0);  // no terminal: the typed write callback never fires
+  EXPECT_EQ(device.custom_count_, 0);        // and it is NOT diverted to the catch-all (no false warning)
+  EXPECT_FALSE(hub.waiting());
+  EXPECT_EQ(hub.entries(), 0u);
+}
+
+// A broadcast is only meaningful for a command that changes state; a broadcast READ could never be
+// answered, so the hub refuses it at the door (false return, no entry queued) rather than silently
+// retiring it. Writes, 0x17, and custom codes still go through (covered above).
+TEST(ModbusClientHubBroadcast, RefusesReadBroadcast) {
+  NullUART uart;
+  NoResponseProbeHub hub;
+  hub.set_uart_parent(&uart);
+  hub.setup();
+  BroadcastProbeDevice device(&hub, BROADCAST_ADDRESS);
+
+  const uint8_t read[] = {0x03, 0x00, 0x10, 0x00, 0x02};  // read holding registers 0x0010, count 2
+  EXPECT_FALSE(device.queue_pdu(read));                   // refused: a broadcast read is never answered
+  EXPECT_EQ(hub.entries(), 0u);                           // nothing entered the machine
+  EXPECT_FALSE(hub.waiting());
+
+  hub.send_next_for_test();          // nothing to send
+  EXPECT_EQ(device.sent_count_, 0);  // never transmitted
+}
+
+// The counterpart to RefusesReadBroadcast: a custom (user-defined) function code carries no reply the
+// hub knows how to expect, so a broadcast of one is accepted and completes fire-and-forget like a write.
+TEST(ModbusClientHubBroadcast, AcceptsCustomBroadcast) {
+  NullUART uart;
+  NoResponseProbeHub hub;
+  hub.set_uart_parent(&uart);
+  hub.setup();
+  BroadcastProbeDevice device(&hub, BROADCAST_ADDRESS);
+
+  const uint8_t custom[] = {0x41, 0x01, 0x02};  // FC 0x41: first user-defined function code space
+  ASSERT_TRUE(device.queue_pdu(custom));        // accepted: a custom code is not a read
+  EXPECT_EQ(hub.queued_frames(), 1u);
+
+  hub.send_next_for_test();  // transmit + sweep
+
+  EXPECT_EQ(device.sent_count_, 1);         // the frame went on the wire
+  EXPECT_EQ(device.response_count_, 0);     // fire-and-forget: no terminal callback
+  EXPECT_EQ(device.no_response_count_, 0);  // and it never waited for a reply
+  EXPECT_FALSE(hub.waiting());
+  EXPECT_EQ(hub.entries(), 0u);  // the entry is gone
+}
+
+// An exception-flagged custom code (0x80 bit set) is not a real request: is_function_code_custom() masks
+// the bit away and would accept it, but the broadcast guard excludes it, matching classify()'s handling
+// of an exception-flagged write.
+TEST(ModbusClientHubBroadcast, RefusesExceptionFlaggedCustomBroadcast) {
+  NullUART uart;
+  NoResponseProbeHub hub;
+  hub.set_uart_parent(&uart);
+  hub.setup();
+  BroadcastProbeDevice device(&hub, BROADCAST_ADDRESS);
+
+  const uint8_t exception_custom[] = {0xC1, 0x01, 0x02};  // 0x41 | 0x80: custom code with the exception bit
+  EXPECT_FALSE(device.queue_pdu(exception_custom));       // refused: exception-flagged, never a real broadcast
+  EXPECT_EQ(hub.entries(), 0u);                           // nothing entered the machine
+  EXPECT_FALSE(hub.waiting());
+
+  hub.send_next_for_test();          // nothing to send
+  EXPECT_EQ(device.sent_count_, 0);  // never transmitted
+}
+
 namespace {
 // tx_blocked() clear for send_next_frame_'s gate, then blocked for send_frame_'s post-delay re-check.
 class RejectPostDelayHub : public NoResponseProbeHub {

tests/integration/fixtures/uart_mock_modbus_broadcast_write.yaml

@@ -0,0 +1,150 @@
+esphome:
+  name: uart-mock-modbus-broadcast
+
+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_server
+    baud_rate: 9600
+    auto_start: true  # controller polls at boot; forwarding must already be active
+    debug:
+    on_tx:
+      - then:
+          - uart_mock.inject_rx:
+              id: virtual_uart_controller
+              data: !lambda return data;
+          - uart_mock.inject_rx:
+              id: virtual_uart_server_2
+              data: !lambda return data;
+  - id: virtual_uart_server_2
+    baud_rate: 9600
+    auto_start: true
+    debug:
+    on_tx:
+      - then:
+          - uart_mock.inject_rx:
+              id: virtual_uart_server
+              data: !lambda return data;
+          - uart_mock.inject_rx:
+              id: virtual_uart_controller
+              data: !lambda return data;
+  - id: virtual_uart_controller
+    baud_rate: 9600
+    auto_start: true
+    debug:
+    on_tx:
+      - then:
+          - uart_mock.inject_rx:
+              id: virtual_uart_server
+              data: !lambda return data;
+          - uart_mock.inject_rx:
+              id: virtual_uart_server_2
+              data: !lambda return data;
+
+modbus:
+  - uart_id: virtual_uart_server
+    id: virtual_modbus_server
+    role: server
+  - uart_id: virtual_uart_server_2
+    id: virtual_modbus_server_2
+    role: server
+  - uart_id: virtual_uart_controller
+    id: virtual_modbus_client
+    role: client
+    turnaround_time: 10ms
+
+globals:
+  - id: srv1_reg
+    type: int
+    initial_value: "0"
+  - id: srv2_reg
+    type: int
+    initial_value: "0"
+
+modbus_controller:
+  - address: 1
+    modbus_id: virtual_modbus_client
+    # Polling is off until the test has subscribed; the Start Scenario button starts it, so the
+    # first poll is never lost to a boot-time race ahead of the API subscription.
+    update_interval: never
+    id: modbus_controller_1
+
+modbus_server:
+  - address: 1
+    modbus_id: virtual_modbus_server
+    registers:
+      - address: 0x01
+        value_type: U_WORD
+        read_lambda: return 919;
+      - address: 0x10
+        value_type: U_WORD
+        read_lambda: return id(srv1_reg);
+        write_lambda: |-
+          id(srv1_reg) = x;
+          return true;
+  - address: 2
+    modbus_id: virtual_modbus_server_2
+    registers:
+      - address: 0x10
+        value_type: U_WORD
+        read_lambda: return id(srv2_reg);
+        write_lambda: |-
+          id(srv2_reg) = x;
+          return true;
+
+sensor:
+  # Normal polling continues before and after the broadcast: the old behavior burned a
+  # timeout per broadcast, which surfaces as modbus warnings and failed expectations here.
+  - platform: modbus_controller
+    modbus_controller_id: modbus_controller_1
+    name: "reg_u_word"
+    address: 0x01
+    register_type: holding
+    value_type: U_WORD
+    # Republish every poll (the value is constant 919): the test observes successive publishes to
+    # prove polling continues before and after the broadcast, which dedup would otherwise hide.
+    force_update: true
+  # The servers' written values, published locally.
+  - platform: template
+    name: "srv1_written"
+    lambda: return id(srv1_reg);
+    update_interval: 0.2s
+  - platform: template
+    name: "srv2_written"
+    lambda: return id(srv2_reg);
+    update_interval: 0.2s
+  # Whether the hub accepted the broadcast into the transmit queue (the bool queue_pdu() returns).
+  - platform: template
+    name: "broadcast_accepted"
+    id: broadcast_accepted
+
+button:
+  - platform: template
+    name: "Start Scenario"
+    id: start_scenario_btn
+    on_press:
+      - lambda: |-
+          // Start polling now that the test has subscribed.
+          id(modbus_controller_1).set_update_interval(1000);
+          id(modbus_controller_1).start_poller();
+          // Broadcast (address 0) write single register: reg 0x10 = 777 on every server.
+          // PDU is function code + data (no address/CRC); the hub prepends address 0 and appends CRC.
+          const uint8_t pdu[] = {0x06, 0x00, 0x10, 0x03, 0x09};
+          // queue_pdu() returns whether the broadcast was accepted into the machine (the answer this PR
+          // makes meaningful); publish it so the test asserts the accept, not just the servers' writes.
+          bool accepted = id(virtual_modbus_client)->queue_pdu(0x00, pdu);
+          id(broadcast_accepted).publish_state(accepted ? 1.0f : 0.0f);

tests/integration/test_uart_mock_modbus.py

@@ -836,6 +836,44 @@ async def test_uart_mock_modbus_fairness(
         )
 
 
+@pytest.mark.asyncio
+async def test_uart_mock_modbus_broadcast_write(
+    yaml_config: str,
+    run_compiled: RunCompiledFunction,
+    api_client_connected: APIClientConnectedFactory,
+) -> None:
+    """A client broadcast write (address 0) reaches every server and costs no timeout.
+
+    The scenario button sends a broadcast single-register write of 777 to register
+    0x10; both servers must apply it. The client's normal polling sensor must keep
+    updating, and no modbus warnings may appear - the pre-broadcast-support behavior
+    parked the frame in the waiting slot until the send-wait timeout, which surfaced
+    here as 'Stop waiting for response' warnings and a stalled poll.
+    """
+    line_callback, error_log_lines, warning_log_lines = _make_modbus_line_callback()
+
+    tracker = SensorTracker(
+        ["reg_u_word", "srv1_written", "srv2_written", "broadcast_accepted"]
+    )
+    poll_before = tracker.expect("reg_u_word", 919)
+    written = tracker.expect_all({"srv1_written": 777, "srv2_written": 777})
+    # queue_pdu() must accept the broadcast into the machine (return true), the answer this PR adds.
+    accepted = tracker.expect("broadcast_accepted", 1)
+
+    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_change(accepted, "broadcast_accepted")
+        await tracker.await_change(poll_before, "reg_u_word")
+        await tracker.await_all(written)
+        # Polling must continue after the broadcast (a burned timeout stalls it).
+        poll_after = tracker.expect("reg_u_word", 919)
+        await tracker.await_change(poll_after, "reg_u_word", timeout=3.0)
+        _assert_no_modbus_errors(error_log_lines, warning_log_lines)
+
+
 @pytest.mark.asyncio
 async def test_uart_mock_modbus_client_read_write(
     yaml_config: str,