[modbus] Rework the client queue as a per-frame state machine (#17922)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: J. Nick Koston <nick@koston.org>

esphome/components/modbus/modbus.cpp

@@ -45,26 +45,46 @@ void Modbus::loop() {
 }
 
 void ModbusClientHub::loop() {
-  // Call base class to receive bytes and parse frames
+  // Drain anything owed since the last loop (e.g. an external clear) before the watchdog runs, so it
-  this->Modbus::loop();
+  // never times out an entry whose pending count has not been drained. No-op when nothing is owed.
+  this->sweep_();
 
-  //  If we're past the send_wait_time timeout and response buffer doesn't have the start of the expected response
+  this->Modbus::loop();  // receive bytes and parse frames
-  if (this->waiting_for_response_.has_value()) {
+
-    ModbusDeviceCommand &wfr = this->waiting_for_response_.value();
+  // Send-wait watchdog: only the cheap time check runs at loop rate; expire_waiting_() looks the
-    uint8_t expected_address = wfr.frame.address();
+  // entry up and holds off if the response has started arriving.
-    if (this->last_receive_check_ - this->last_send_ > this->last_send_tx_offset_ + this->send_wait_time_ &&
+  if (this->waiting_for_response_ &&
-        (this->rx_buffer_.empty() || this->rx_buffer_[0] != expected_address)) {
+      this->last_receive_check_ - this->last_send_ > this->last_send_tx_offset_ + this->send_wait_time_) {
-      ESP_LOGW(TAG, "Stop waiting for response from %" PRIu8 " %" PRIu32 "ms after last send", expected_address,
+    this->expire_waiting_();
-               this->last_receive_check_ - this->last_send_);
-      this->notify_no_response_(wfr);
-      this->waiting_for_response_.reset();
-    }
   }
 
-  //  If there's no response pending and there's commands in the buffer
+  this->sweep_();  // deliver owed callbacks with the hub quiescent
   this->send_next_frame_();
 }
 
+void ModbusClientHub::expire_waiting_() {
+  ModbusDeviceCommand *cmd = this->find_waiting_();
+  if (cmd == nullptr) {
+    this->waiting_for_response_ = false;
+    return;
+  }
+  if (!this->rx_buffer_.empty() && this->rx_buffer_[0] == cmd->frame.address()) {
+    // The start of the response is in the buffer: let the frame finish arriving.
+    return;
+  }
+  // Only a genuine WAITING entry warrants the log (a cleared or interrupted shell timing out is expected).
+  if (cmd->state == FrameState::WAITING) {
+    ESP_LOGW(TAG, "Stop waiting for response from %" PRIu8 " %" PRIu32 "ms after last send", cmd->frame.address(),
+             this->last_receive_check_ - this->last_send_);
+  }
+  // Deliver on_no_response directly, the way the parse path delivers response()/error(): the entry
+  // lands in TIMED_OUT and the following sweep reschedules a retry or erases it. Free the
+  // wire first so a resend from inside the callback sees it available.
+  this->waiting_for_response_ = false;
+  this->sweep_needed_ = true;
+  cmd->timed_out();
+}
+
 bool Modbus::timeout_() {
   // If the response frame is finished (including interframe delay) - we timeout.
   // The long_rx_buffer_delay accounts for long responses (larger than the UART rx_full_threshold) to avoid timeouts
@@ -110,12 +130,21 @@ bool Modbus::tx_blocked() {
 
 bool ModbusClientHub::tx_blocked() {
   // We block transmission in any of these case:
-  // 1. We're waiting for a response
+  // 1. We're waiting for a response (a waiting entry: WAITING/INTERRUPTED/WAITING_RETIRED/INTERRUPTED_RETIRED)
   // 2. Any of the base class tx_blocked conditions
-  return (this->waiting_for_response_.has_value()) || this->Modbus::tx_blocked();
+  return this->waiting_for_response_ || this->Modbus::tx_blocked();
 }
 
-bool ModbusClientHub::tx_buffer_empty() { return this->tx_buffer_.empty(); }
+bool ModbusClientHub::tx_buffer_empty() {
+  // "Empty" for ready_for_immediate_send(): no one-shot is queued ahead of the caller. Entries in
+  // other states are mid-transaction or owed bookkeeping, not queued sends - and a READY continuous
+  // poll does not count either, since it ranks below every one-shot, so a new send goes out first.
+  for (const auto &cmd : this->tx_buffer_) {
+    if (cmd.state == FrameState::READY && !cmd.continuous)
+      return false;
+  }
+  return true;
+}
 
 void Modbus::receive_bytes_() {
   this->last_receive_check_ = millis();
@@ -257,69 +286,57 @@ bool ModbusServerHub::parse_modbus_client_frame_() {
   return true;
 }
 
-// Bounds contract, enforced by the parser (parse_modbus_server_frame_) rather than locally:
+// The parser (parse_modbus_server_frame_) guarantees the bounds relied on here: pdu is never empty,
-// - pdu is never empty: helpers::server_pdu_length() returns at least MIN_PDU_SIZE (1) on every
+// and an exception-flagged pdu is at least 2 bytes. Keep that in mind when changing server_pdu_length().
-//   branch, and find_custom_frame_end_() only ever lengthens the frame, so the PDU always holds
-//   at least the function code.
-// - When the exception bit is set, pdu has at least 2 bytes: server_pdu_length() checks the
-//   exception bit before anything else and pins those PDUs to 2 bytes, so the exception code
-//   read below is always present.
-// Keep those guarantees in mind when changing server_pdu_length() or adding callers.
 void ModbusClientHub::process_modbus_server_frame(uint8_t address, std::span<const uint8_t> pdu) {
   const uint8_t function_code = pdu[0];
-  if (!this->waiting_for_response_.has_value()) {
+  ModbusDeviceCommand *cmd = this->waiting_for_response_ ? this->find_waiting_() : nullptr;
+  if (cmd == nullptr) {
     ESP_LOGW(TAG,
              "Received unexpected frame from address %" PRIu8 ", function code 0x%X, %" PRIu32 "ms after last send",
              address, function_code, this->last_modbus_byte_ - this->last_send_);
     return;
-  } else {  // We are waiting for a response
+  }
-    // Check if the response matches the expected address and function code
 
-    ModbusDeviceCommand &wfr = this->waiting_for_response_.value();
+  // Check if the response matches the expected address and function code
-    uint8_t expected_address = wfr.frame.address();
+  const uint8_t expected_address = cmd->frame.address();
-    uint8_t expected_function_code = wfr.frame.pdu()[0];
+  const uint8_t expected_function_code = cmd->frame.pdu()[0];
-    if (expected_address != address || expected_function_code != (function_code & FUNCTION_CODE_MASK)) {
+  if (expected_address != address || expected_function_code != (function_code & FUNCTION_CODE_MASK)) {
-      ESP_LOGW(TAG,
+    ESP_LOGW(TAG,
-               "Received incorrect frame address %" PRIu8 " <> %" PRIu8 " or function code 0x%X <> 0x%X, %" PRIu32
+             "Received incorrect frame address %" PRIu8 " <> %" PRIu8 " or function code 0x%X <> 0x%X, %" PRIu32
-               "ms after last send",
+             "ms after last send",
-               address, expected_address, (function_code & FUNCTION_CODE_MASK), expected_function_code,
+             address, expected_address, (function_code & FUNCTION_CODE_MASK), expected_function_code,
-               this->last_modbus_byte_ - this->last_send_);
+             this->last_modbus_byte_ - this->last_send_);
-      // Invalidate the device; the entry survives as an interrupted shell so the late response is ignored.
+    // Unexpected frame: flip a WAITING entry to an INTERRUPTED shell that ignores the rest of this
-      // A retry requested here stays queued behind the shell until the send-wait timeout clears it.
+    // transaction and blocks tx until the send-wait timeout, where it gets its on_no_response.
-      this->notify_no_response_(wfr);
+    cmd->interrupt();
-      wfr.interrupted = true;
+    return;
-      return;
+  }
-    }
 
-    if (wfr.interrupted) {
+  if (cmd->state == FrameState::INTERRUPTED || cmd->state == FrameState::INTERRUPTED_RETIRED) {
-      ESP_LOGW(TAG,
+    // An interrupted shell keeps blocking until the send-wait timeout; a late response for it is
-               "Ignoring response from %" PRIu8 " - transmission interrupted by previous unexpected response, %" PRIu32
+    // ignored and does NOT free the wire. The distrust survives a clear (INTERRUPTED_RETIRED), so a
-               "ms after last send",
+    // cleared-interrupted frame still ends in on_no_response rather than delivering a late response.
-               address, this->last_modbus_byte_ - this->last_send_);
+    ESP_LOGW(TAG,
-      return;
+             "Ignoring response from %" PRIu8 " - transmission interrupted by previous unexpected response, %" PRIu32
-    } else {  // We have a valid device waiting for this response
+             "ms after last send",
+             address, this->last_modbus_byte_ - this->last_send_);
+    return;
+  }
 
-      // Move the command out of the waiting slot so the request PDU stays alive for the callback.
+  // Deliver at parse time so the response span can point into the rx buffer (zero copy). error()/
-      ModbusDeviceCommand command = std::move(this->waiting_for_response_.value());
+  // response() set the state and consume the request BEFORE the callback, so a clear from inside it
-      this->waiting_for_response_.reset();
+  // ("stop polling now") wins. A device-less shell runs no callback and the sweep erases it.
-      ModbusClientDevice *device = command.device;
+  this->waiting_for_response_ = false;
-      // The request PDU is the sent frame without the leading address and the trailing CRC.
+  this->sweep_needed_ = true;
-      std::span<const uint8_t> request_pdu = command.frame.pdu();
+  if (helpers::is_function_code_exception(function_code)) {
-      // Is it an error response?
+    uint8_t exception = pdu[1];  // exception frames are fixed-length, so the code is always present
-      if (helpers::is_function_code_exception(function_code)) {
+    ESP_LOGW(TAG, "Error function code: 0x%X exception: %" PRIu8 ", address: %" PRIu8 ", %" PRIu32 "ms after last send",
-        uint8_t exception = pdu[1];  // exception frames are fixed-length, so the code is always present
+             function_code, exception, address, this->last_modbus_byte_ - this->last_send_);
-        ESP_LOGW(TAG,
+    cmd->error(static_cast<ExceptionCode>(exception));
-                 "Error function code: 0x%X exception: %" PRIu8 ", address: %" PRIu8 ", %" PRIu32 "ms after last send",
+  } else if (!cmd->response(pdu)) {
-                 function_code, exception, address, this->last_modbus_byte_ - this->last_send_);
+    ESP_LOGV(TAG, "Ignoring response from %" PRIu8 " - no callback device set, %" PRIu32 "ms after last send", address,
-        if (device)
+             this->last_modbus_byte_ - this->last_send_);
-          device->on_error(request_pdu, static_cast<ExceptionCode>(exception));
-      } else if (device) {  // Not an error response
-        device->on_response(request_pdu, pdu);
-      } else {  // Not an error response, but no device to respond to
-        ESP_LOGV(TAG, "Ignoring response from %" PRIu8 " - no callback device set, %" PRIu32 "ms after last send",
-                 address, this->last_modbus_byte_ - this->last_send_);
-      }
-    }
   }
 }
 
@@ -460,21 +477,20 @@ void ModbusServerHub::process_modbus_client_frame_(uint8_t address, uint8_t func
   }
 }
 
+// Callers gate on tx_blocked() first, but the pre-send delay below can span several ms, so re-check
+// after it and refuse (return false) if a byte arrived in that window rather than transmit over it.
 bool Modbus::send_frame_(const ModbusFrame &frame) {
-  if (this->tx_blocked()) {
-    ESP_LOGE(TAG, "Attempted to send while transmission blocked");
-    return false;
-  }
-  if (frame.size() > MAX_FRAME_SIZE) {
-    ESP_LOGE(TAG, "Attempted to send frame larger than max frame size of %" PRIu16 " bytes", MAX_FRAME_SIZE);
-    return false;
-  }
-
   const int32_t tx_delay_remaining = this->tx_delay_remaining();
   if (tx_delay_remaining > 0) {
     delay(tx_delay_remaining);
   }
 
+  // The delay above can span several ms; a byte arriving in that window blocks transmission after the
+  // caller's gate already passed. Don't collide with the incoming frame - leave the entry to retry.
+  if (this->tx_blocked()) {
+    return false;
+  }
+
   if (this->flow_control_pin_ != nullptr) {
     this->flow_control_pin_->digital_write(true);
     this->write_array(frame.data.data(), frame.size());
@@ -498,37 +514,21 @@ bool Modbus::send_frame_(const ModbusFrame &frame) {
 }
 
 void ModbusClientHub::send_next_frame_() {
-  if (this->tx_buffer_.empty()) {
+  if (this->tx_blocked())
+    return;
+
+  ModbusDeviceCommand *cmd = this->select_next_ready_();
+  if (cmd == nullptr)
+    return;
+
+  if (!this->send_frame_(cmd->frame)) {
+    ESP_LOGV(TAG, "Send deferred for %" PRIu8 ": a frame arrived during the send delay, will retry",
+             cmd->frame.address());
     return;
   }
 
-  if (this->tx_blocked()) {
+  cmd->sent();
-    return;
+  this->waiting_for_response_ = true;
-  }
-
-  // Move the command out and pop BEFORE attempting the send: no callback may run while the frame still
-  // sits in the queue (the same principle as the clear sweep). A failure callback that sends would
-  // otherwise queue a new frame and pop_front() could discard the wrong one - and the deque
-  // reference / PDU span could be invalidated mid-callback.
-  ModbusDeviceCommand command = std::move(this->tx_buffer_.front());
-  this->tx_buffer_.pop_front();
-  ModbusClientDevice *device = command.device;
-  const bool sent = this->send_frame_(command.frame);
-
-  if (sent) {
-    // The frame now lives in the waiting slot; its PDU is the frame without the leading address and
-    // trailing CRC.
-    ModbusDeviceCommand &wfr = this->waiting_for_response_.emplace(std::move(command));
-    if (device != nullptr)
-      device->on_sent(wfr.frame.pdu());
-  } else {
-    if (device != nullptr)
-      device->trigger_not_sent(command.frame.pdu());
-  }
-
-  if (!this->tx_buffer_.empty()) {
-    ESP_LOGV(TAG, "Write queue contains %zu items.", this->tx_buffer_.size());
-  }
 }
 
 void ModbusClientHub::dump_config() {
@@ -579,118 +579,258 @@ void ModbusServerHub::send_exception_(uint8_t address, uint8_t function_code, Ex
   this->send_raw_(raw_frame, 3);
 }
 
-void ModbusClientHub::notify_no_response_(ModbusDeviceCommand &wfr) {
+ModbusDeviceCommand *ModbusClientHub::find_waiting_() {
-  if (wfr.device == nullptr)
+  for (auto &cmd : this->tx_buffer_) {
-    return;
+    if (cmd.waiting_state())
-  const bool retry = wfr.device->on_no_response(wfr.frame.pdu());
+      return &cmd;
-  // The callback may have detached the device (e.g. clear_tx_queue_for_device()); honor the detach
+  }
-  // over the retry request rather than re-queueing a frame that can no longer be routed.
+  return nullptr;
-  if (retry && wfr.device != nullptr)
-    this->requeue_waiting_frame_(wfr);
-  // The old transaction is over either way; never deliver anything else to the device through it.
-  wfr.device = nullptr;
 }
 
-void ModbusClientHub::requeue_waiting_frame_(ModbusDeviceCommand &wfr) {
+ModbusDeviceCommand *ModbusClientHub::select_next_ready_() {
-  const ModbusFrame &frame = wfr.frame;
+  // Class first (WRITE, then one-shot READ, then CONTINUOUS), oldest within a class. seq is a
-  if (this->tx_buffer_.size() >= MODBUS_TX_BUFFER_SIZE) {
+  // free-running counter, so compare each entry's AGE against it (correct across the full range).
-    ESP_LOGE(TAG, "Write buffer full, dropped retry for address %" PRIu8, frame.address());
+  const uint16_t now = this->next_seq_;
-    if (wfr.device != nullptr)
+  const auto age = [now](const ModbusDeviceCommand &cmd) -> uint16_t { return now - cmd.seq; };
-      wfr.device->trigger_not_sent(frame.pdu());
+  const auto older = [&age](const ModbusDeviceCommand &a, const ModbusDeviceCommand &b) { return age(a) > age(b); };
+  ModbusDeviceCommand *best = nullptr;
+  for (auto &cmd : this->tx_buffer_) {
+    if (cmd.state != FrameState::READY)
+      continue;
+    if (best == nullptr || cmd.priority() > best->priority() ||
+        (cmd.priority() == best->priority() && older(cmd, *best))) {
+      best = &cmd;
+    }
+  }
+  return best;
+}
+
+bool ModbusDeviceCommand::sent() {
+  this->state = FrameState::WAITING;
+  // on_sent() is not a terminal, so nothing is consumed.
+  if (this->device == nullptr)
+    return false;
+  this->device->on_sent(this->frame.pdu());
+  return true;
+}
+
+bool ModbusDeviceCommand::notify_retired() {
+  if (!this->decrement_pending())
+    return false;  // nothing owed - stop the sweep draining this entry
+  if (this->device != nullptr)
+    this->device->on_not_sent(this->frame.pdu());
+  return true;  // consumed one debt (delivered, or silent when device-less) - keep draining to zero
+}
+
+bool ModbusDeviceCommand::response(std::span<const uint8_t> response_pdu) {
+  this->state = this->state == FrameState::WAITING_RETIRED ? FrameState::RETIRED : FrameState::RECEIVED_RESPONSE;
+  // A continuous poll is never consumed by its own response; a one-shot consumes one request here.
+  if (!this->continuous)
+    this->decrement_pending();
+  if (this->device == nullptr)
+    return false;
+  this->device->on_response(this->frame.pdu(), response_pdu);
+  return true;
+}
+
+bool ModbusDeviceCommand::error(ExceptionCode exception_code) {
+  this->state = this->state == FrameState::WAITING_RETIRED ? FrameState::RETIRED : FrameState::RECEIVED_EXCEPTION;
+  // An exception ends a continuous poll too, so decrement unconditionally.
+  this->decrement_pending();
+  if (this->device == nullptr)
+    return false;
+  this->device->on_error(this->frame.pdu(), exception_code);
+  return true;
+}
+
+bool ModbusDeviceCommand::interrupt() {
+  // An unexpected frame distrusts the transaction. A cleared-but-still-waiting shell distrusts too, so
+  // the interrupt survives the clear in either order (WAITING_RETIRED -> INTERRUPTED_RETIRED).
+  if (this->state == FrameState::WAITING) {
+    this->state = FrameState::INTERRUPTED;
+    return true;
+  }
+  if (this->state == FrameState::WAITING_RETIRED) {
+    this->state = FrameState::INTERRUPTED_RETIRED;
+    return true;
+  }
+  return false;
+}
+
+bool ModbusDeviceCommand::timed_out() {
+  this->state = FrameState::TIMED_OUT;  // advance BEFORE the callback so a clear from inside it wins
+  this->decrement_pending();            // resolve this request (WAITING-origin, so pending >= 1)
+  if (this->device == nullptr)
+    return false;  // resolved, no one to tell
+  if (this->device->on_no_response(this->frame.pdu()))
+    this->increment_pending();  // granted retry = re-request (capped)
+  return true;
+}
+
+void ModbusClientHub::sweep_() {
+  if (!this->sweep_needed_)
     return;
+  this->sweep_needed_ = false;
+  // Serve only the entries present now: a callback may append (a re-send), but those sit beyond
+  // work_set and are left for the next sweep, which bounds the work and is the termination argument.
+  // Entries leave the container only in the erase pass below, so indices/references stay valid.
+  const size_t work_set = this->tx_buffer_.size();
+  // Restart the walk after every callback: a handler may have moved any entry to any state.
+  bool callback_ran = true;
+  while (callback_ran) {
+    callback_ran = false;
+    for (size_t i = 0; i != work_set && !callback_ran; i++) {
+      ModbusDeviceCommand &cmd = this->tx_buffer_[i];
+      switch (cmd.state) {
+        case FrameState::RECEIVED_RESPONSE:
+        case FrameState::RECEIVED_EXCEPTION:
+        case FrameState::TIMED_OUT:
+          // Off the wire, callback already delivered: reschedule what is still pending, else erase.
+          if (cmd.pending)
+            cmd.requeue(this->next_seq_++);
+          break;
+        case FrameState::RETIRED:
+          // Owes one on_not_sent() per accepted request; notify_retired() consumes one and reports
+          // whether a debt remained, so the restart loop drains the entry to zero - even a device-less
+          // shell with pending > 1 (no callback fires, but it still drains rather than stranding).
+          callback_ran = cmd.notify_retired();
+          break;
+        case FrameState::WAITING_RETIRED:
+        case FrameState::INTERRUPTED_RETIRED:
+          // Cleared shell: drain only the un-run duplicates; the request in flight keeps pending 1
+          // and gets its usual callback when it resolves.
+          if (cmd.pending > 1)
+            callback_ran = cmd.notify_retired();
+          break;
+        default:  // READY / WAITING / INTERRUPTED: idle or waiting for a response, nothing owed until the timeout
+          break;
+      }
+    }
+  }
+  // Erase pass: the only place entries leave the container. Storage order carries no meaning, so a
+  // finished entry is swap-and-popped; walking backwards means a moved-down entry is already seen.
+  for (size_t i = this->tx_buffer_.size(); i-- > 0;) {
+    const ModbusDeviceCommand &cmd = this->tx_buffer_[i];
+    // pending == 0 is erasable, but shells still waiting for a response are exempt until it resolves.
+    if (cmd.pending != 0 || cmd.waiting_state())
+      continue;
+    if (i + 1 != this->tx_buffer_.size())
+      this->tx_buffer_[i] = std::move(this->tx_buffer_.back());
+    this->tx_buffer_.pop_back();
   }
-  // Re-queue a copy (not a move): the waiting entry may have to survive as an interrupted shell.
-  this->tx_buffer_.emplace_back(wfr.device, frame.address(), frame.pdu());
 }
 
 // Raw send for client: pushes to tx queue. Everything except the CRC must be contained in payload.
-void ModbusClientHub::send_pdu(uint8_t address, std::span<const uint8_t> pdu, ModbusClientDevice *device) {
+bool ModbusClientHub::send_pdu(uint8_t address, std::span<const uint8_t> pdu, ModbusClientDevice *device,
+                               CommandOptions options) {
+  // Requests refused here never enter the machine and get no callback - the false return is it.
   if (pdu.empty()) {
-    if (device != nullptr)
+    ESP_LOGW(TAG, "Empty PDU refused for address %" PRIu8, address);
-      device->trigger_not_sent(pdu);
+    return false;
-    return;
   }
-
   // Bound the PDU so the wire frame (address + pdu + CRC) stays within the Modbus RTU 256-byte limit.
   if (pdu.size() > MAX_PDU_SIZE) {
-    ESP_LOGE(TAG, "Frame too large, dropped: %" PRIu8 ":%zu bytes", address, pdu.size());
+    ESP_LOGE(TAG, "Frame too large, refused: %" PRIu8 ":%zu bytes", address, pdu.size());
-    if (device != nullptr)
+    return false;
-      device->trigger_not_sent(pdu);
-    return;
   }
 
-  if (this->tx_buffer_.size() < MODBUS_TX_BUFFER_SIZE) {
+  // continuous is ignored for every mutating code (re-writing a value forever is never intended).
-#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERBOSE
+  const bool mutates = ModbusDeviceCommand::classify(pdu[0]) == CommandPriority::WRITE;
-    char hex_buf[format_hex_pretty_size(MODBUS_MAX_LOG_BYTES)];
+  bool continuous = false;
-#endif
+  if (options.continuous) {
-    ESP_LOGV(TAG, "Adding frame to tx queue: %" PRIu8 ":%s", address,
+    if (mutates) {
-             format_hex_pretty_to(hex_buf, pdu.data(), pdu.size()));
+      ESP_LOGV(TAG, "continuous is ignored for a mutating function (0x%X, address %" PRIu8 ")", pdu[0], address);
-    this->tx_buffer_.emplace_back(device, address, pdu);
+    } else {
-  } else {
+      continuous = true;
+    }
+  }
+
+  // A duplicate of a live entry with the same owner is not queued twice; it resolves against that
+  // entry: anonymous -> dropped; continuous incoming -> convert the entry to a poll; one-shot onto a
+  // poll -> downgrade the poll to one-shot; both one-shots -> pending++ below the cap, else refused.
+  for (auto &item : this->tx_buffer_) {
+    if (item.state == FrameState::RETIRED || item.state == FrameState::WAITING_RETIRED ||
+        item.state == FrameState::INTERRUPTED_RETIRED)
+      continue;  // cleared, on their way out: a new identical send queues fresh, never absorbs
+    if (item.device != device || !item.same_frame(address, pdu))
+      continue;
+    if (device == nullptr) {
+      // A dropped read is routine (DEBUG); a dropped write/custom warns (unobservable without a device).
+      const bool requeueable = !helpers::is_function_code_exception(pdu[0]) && helpers::is_function_code_read(pdu[0]);
+      if (requeueable) {
+        ESP_LOGD(TAG, "Anonymous duplicate of active frame for %" PRIu8 " (function 0x%X), dropped", address, pdu[0]);
+      } else {
+        ESP_LOGW(TAG,
+                 "Anonymous duplicate of active frame for %" PRIu8 " (function 0x%X), dropped - register a "
+                 "device for delivery accounting",
+                 address, pdu[0]);
+      }
+      return false;  // dropped: no entry, no callbacks - the refusal is the return value
+    }
+    if (continuous) {
+      item.make_continuous(true);
+      ESP_LOGV(TAG, "Frame already active for %" PRIu8 ", now polled continuously", address);
+    } else if (item.continuous) {
+      // A one-shot duplicate downgrades the poll to a one-shot: it runs one more cycle to serve this
+      // request, then stops (mirrors continuous incoming converting a one-shot the other way).
+      item.make_continuous(false);
+      ESP_LOGV(TAG, "Frame already active for %" PRIu8 ", downgraded from continuous to one-shot", address);
+    } else if (!item.increment_pending()) {
+      // At the servable cap, so refused. (An absorbed duplicate leaves seq alone - the entry keeps
+      // its place in line, held by its oldest outstanding request.)
+      ESP_LOGD(TAG, "Frame already active for %" PRIu8 " with %" PRIu8 " requests pending, refused", address,
+               item.pending);
+      return false;
+    } else {
+      ESP_LOGV(TAG, "Frame already active for %" PRIu8 ", request absorbed (pending %" PRIu8 ")", address,
+               item.pending);
+    }
+    return true;
+  }
+
+  // Backstop counts every entry; dead ones are gone by the sweep's end, so at worst they cost one
+  // refusal at the very cap for one loop.
+  if (this->tx_buffer_.size() >= MODBUS_TX_BUFFER_SIZE) {
 #if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_ERROR
     char hex_buf[format_hex_pretty_size(MODBUS_MAX_LOG_BYTES)];
 #endif
-    ESP_LOGE(TAG, "Write buffer full, dropped: %" PRIu8 ":%s", address,
+    ESP_LOGE(TAG, "Write buffer full, refused: %" PRIu8 ":%s", address,
              format_hex_pretty_to(hex_buf, pdu.data(), pdu.size()));
-    if (device != nullptr)
+    return false;
-      device->trigger_not_sent(pdu);
   }
+#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERBOSE
+  char hex_buf[format_hex_pretty_size(MODBUS_MAX_LOG_BYTES)];
+#endif
+  ESP_LOGV(TAG, "Adding frame to tx queue: %" PRIu8 ":%s", address,
+           format_hex_pretty_to(hex_buf, pdu.data(), pdu.size()));
+  this->tx_buffer_.emplace_back(device, address, pdu, continuous, this->next_seq_++);
+  return true;
 }
 
-void ModbusClientHub::clear_tx_queue_for_address(uint8_t address, bool clear_sent) {
+void ModbusClientHub::clear_tx_queue_for_address(uint8_t address) {
-  // Drop the queued frames for this address, delivering on_not_sent() to each frame's owner: other
+  // A clear is a pure state flip; the sweep delivers every owed on_not_sent() from a quiescent hub.
-  // devices talking to the same physical device (e.g. a modbus_client action alongside a controller that
-  // just went offline) must observe the drop, or their command never resolves. Mark first, then sweep
-  // only marked frames: anything a callback re-queues is unmarked, so it
-  // is never swept - or re-notified - by the clear that triggered it. Each marked frame is moved out and erased BEFORE
-  // its callback runs, so handlers see a consistent queue; termination is guaranteed because only the initially-marked
-  // frames are ever swept.
   for (auto &cmd : this->tx_buffer_) {
-    if (cmd.frame.address() == address)
+    if (cmd.frame.address() != address)
-      cmd.marked_for_deletion = true;
+      continue;
-  }
+    cmd.retire();
-  for (;;) {
+    this->sweep_needed_ = true;
-    auto it = std::find_if(this->tx_buffer_.begin(), this->tx_buffer_.end(),
-                           [](const ModbusDeviceCommand &cmd) { return cmd.marked_for_deletion; });
-    if (it == this->tx_buffer_.end())
-      break;
-    ModbusDeviceCommand dropped = std::move(*it);
-    this->tx_buffer_.erase(it);
-    // The sweep delivers through the same per-device guard as refusals: a device clearing from inside
-    // its own on_not_sent() gets its remaining frames resolved silently (documented in the lifecycle
-    // contract), other owners are notified normally, and every nested clear stays bounded.
-    if (dropped.device != nullptr)
-      dropped.device->trigger_not_sent(dropped.frame.pdu());
-  }
-
-  if (clear_sent && this->waiting_for_response_.has_value() && this->waiting_for_response_.value().device) {
-    if (this->waiting_for_response_.value().frame.address() == address) {
-      ESP_LOGV(TAG, "Clearing waiting for response for address %" PRIu8, address);
-      // Invalidate the waiting device so it won't process a response.
-      this->waiting_for_response_.value().device = nullptr;
-    }
   }
 }
-void ModbusClientHub::clear_tx_queue_for_device(ModbusClientDevice *device) {
-  // Remove any pending commands for this address from the tx buffer
-  auto &tx_buffer = this->tx_buffer_;
-  tx_buffer.erase(std::remove_if(tx_buffer.begin(), tx_buffer.end(),
-                                 [device](const ModbusDeviceCommand &cmd) { return cmd.device == device; }),
-                  tx_buffer.end());
 
-  if (this->waiting_for_response_.has_value() && this->waiting_for_response_.value().device) {
+void ModbusClientHub::clear_tx_queue_for_device(ModbusClientDevice *device) {
-    if (this->waiting_for_response_.value().device == device) {
+  // Silent teardown (supersede semantics): the caller's own frames vanish without callbacks; see
-      ESP_LOGV(TAG, "Clearing waiting for response");
+  // the lifecycle note on ModbusClientDevice.
-      // Invalidate the waiting device so it won't process a response.
+  for (auto &cmd : this->tx_buffer_) {
-      this->waiting_for_response_.value().device = nullptr;
+    if (cmd.device != device)
-    }
+      continue;
+    cmd.silent_retire();
+    this->sweep_needed_ = true;
   }
 }
 
 void ModbusClientHub::send_raw(const std::vector<uint8_t> &payload, ModbusClientDevice *device) {
   if (payload.size() < 2) {
-    if (device != nullptr)
+    ESP_LOGW(TAG, "send_raw() payload too short to contain a PDU, refused");
-      device->trigger_not_sent({});  // too short to contain a PDU
     return;
   }
   this->send_pdu(payload[0], std::span<const uint8_t>(payload).subspan(1), device);
@@ -706,23 +846,26 @@ void ModbusServerHub::send_raw_(const uint8_t *payload, uint16_t len) {
     return;
   }
 
-  // In the rare case that the server is blocked (frame delay has not elapsed), we delay the send.
+  // If blocked now (frame delay not elapsed at low baud, or a frame arriving), defer rather than
-  // This should only happen at low baud rates with long frame delays.
+  // busy-waiting the loop; send_frame_ itself re-checks after its delay, so the deferred callback
+  // just reports whatever it returns.
   if (this->tx_blocked()) {
     // Stash the raw payload in a single member buffer so the deferred callback can rebuild the frame
-    // without a heap allocation. Only one server reply is ever in flight, and the named timeout ensures
+    // without a heap allocation. Only one server reply is ever waiting, so a single buffer suffices.
-    // only one deferred send is pending, so a single buffer is sufficient.
     std::memcpy(this->deferred_payload_.data(), payload, len);
     this->deferred_payload_len_ = len;
     this->set_timeout("deferred_send", this->tx_delay_remaining(), [this]() {
       ModbusFrame frame(this->deferred_payload_[0], this->deferred_payload_.data() + 1,
                         this->deferred_payload_len_ - 1);
-      this->send_frame_(frame);
+      if (!this->send_frame_(frame))
+        ESP_LOGE(TAG, "Deferred server reply dropped: transmission still blocked");
     });
-  } else {
+    return;
-    ModbusFrame frame(payload[0], payload + 1, len - 1);
-    this->send_frame_(frame);
   }
+
+  ModbusFrame frame(payload[0], payload + 1, len - 1);
+  if (!this->send_frame_(frame))
+    ESP_LOGE(TAG, "Server reply dropped: a frame arrived during the send delay");
 }
 
 void Modbus::clear_rx_buffer_(const LogString *reason, bool warn, size_t bytes_to_clear) {
@@ -778,7 +921,7 @@ void ModbusClientDevice::dispatch_response_(std::span<const uint8_t> request_pdu
       const bool bits =
           function_code == FunctionCode::READ_COILS || function_code == FunctionCode::READ_DISCRETE_INPUTS;
       const size_t expected_data_size =
-          bits ? (static_cast<size_t>(count_or_value) + 7) / 8 : static_cast<size_t>(count_or_value) * 2;
+          bits ? packed_bit_bytes(count_or_value) : static_cast<size_t>(count_or_value) * 2;
       if (response_pdu.size() != expected_data_size + 2) {
         ESP_LOGD(TAG, "Response length %zu does not match request (expected %zu) for function code 0x%X",
                  response_pdu.size(), expected_data_size + 2, static_cast<uint8_t>(function_code));

esphome/components/modbus/modbus.h

@@ -16,7 +16,13 @@
 
 namespace esphome::modbus {
 
-static constexpr uint16_t MODBUS_TX_BUFFER_SIZE = 15;
+// Tx queue backstop. Duplicate frames dedup into one entry, so reads can never approach this in a
+// sane config - it exists to stop a runaway generator of distinct frames (e.g. a loop writing a
+// changing value) from growing the heap unboundedly. The deque grows on demand; this reserves nothing.
+// Worst case the cap permits: 128 distinct max-size frames = ~32 kB of spilled frame data plus
+// ~3 kB of deque node storage (typical 8-byte frames stay inline; large PDUs spill to one
+// allocation each) - pathological configs only, but the numbers matter when tuning for ESP8266.
+static constexpr uint16_t MODBUS_TX_BUFFER_SIZE = 128;
 static constexpr uint16_t MODBUS_TX_MAX_DELAY_MS = 5;
 
 // Typical frames -- reads and single-register/coil writes -- are exactly 8 bytes
@@ -70,6 +76,9 @@ class Modbus : public uart::UARTDevice, public Component {
   // pdu is the whole PDU (function code + payload, no address/CRC); pdu[0] is the (standard or custom) function code.
   virtual void process_modbus_server_frame(uint8_t address, std::span<const uint8_t> pdu) = 0;
   void clear_rx_buffer_(const LogString *reason, bool warn = false, size_t bytes_to_clear = 0);
+  // Transmit a frame. Callers gate on tx_blocked() first, but the pre-send delay can span several ms,
+  // so this re-checks after the delay and returns false without transmitting if a byte arrived in that
+  // window (the caller then leaves its entry to retry). Returns true once the frame has been transmitted.
   bool send_frame_(const ModbusFrame &frame);
   // Scans forward from min_length to find a frame boundary by CRC match for custom function codes.
   // Returns the matched frame length, or 0 if no valid CRC was found within MAX_FRAME_SIZE.
@@ -90,20 +99,158 @@ class Modbus : public uart::UARTDevice, public Component {
 class ModbusClientDevice;
 class ModbusServerDevice;
 
+// Transmit ordering, highest first: writes before one-shot reads before continuous polls. Derived
+// at selection time, never caller-chosen or stored.
+enum class CommandPriority : uint8_t { CONTINUOUS = 0, READ, WRITE };
+
+// Per-entry lifecycle state. Waiting states (see waiting_state()) hold the bus; the sweep delivers owed
+// callbacks from a quiescent hub, and an entry is erased once pending == 0 && !waiting_state().
+enum class FrameState : uint8_t {
+  READY = 0,
+  WAITING,
+  RECEIVED_RESPONSE,
+  RECEIVED_EXCEPTION,
+  TIMED_OUT,            // on_no_response delivered at the send-wait timeout; awaiting reschedule/erase
+  INTERRUPTED,          // unexpected frame arrived; ignores this transaction, waits out the timeout
+  WAITING_RETIRED,      // cleared while WAITING: a late response is still delivered as its usual terminal
+  INTERRUPTED_RETIRED,  // cleared while INTERRUPTED: still distrusts late frames, ends in on_no_response
+  RETIRED,              // cleared, off the wire
+};
+
+// Per-command send options. Append-only; pass via designated initializers ({.continuous = true}).
+struct CommandOptions {
+  // A continuous poll lives in the queue until cancelled or failed; ignored for mutating codes.
+  bool continuous{false};
+};
+
 struct ModbusDeviceCommand {
   ModbusClientDevice *device;
   ModbusFrame frame;
-  bool interrupted{false};
+  FrameState state{FrameState::READY};
-  /// Marked by clear_tx_queue_for_address() before it starts notifying, so frames re-queued by an
+  // A continuous poll is a subscription: pending fixed at 1, removed only by cancellation or failure.
-  /// on_not_sent() callback (which are unmarked) are never swept by the clear that triggered them.
+  bool continuous{false};
-  bool marked_for_deletion{false};
+  // Accepted requests this entry stands for, capped at max_pending(); drains one terminal each.
+  uint8_t pending{1};
+  // Place-in-line stamp (hub's free-running counter); selection takes the oldest for round-robin
+  // fairness within a class. Meant to wrap.
+  uint16_t seq{0};
 
-  ModbusDeviceCommand(ModbusClientDevice *device, uint8_t address, const uint8_t *src, uint16_t len)
+  // Build a command from a PDU span (caller bounds it to MAX_PDU_SIZE); fully initialized here.
-      : device(device), frame(address, src, len) {}
+  ModbusDeviceCommand(ModbusClientDevice *device, uint8_t address, std::span<const uint8_t> pdu,
-  /// Build a command from a PDU span: a caller-supplied PDU, or an existing frame's own pdu() when re-queueing
+                      bool continuous = false, uint16_t seq = 0)
-  /// Callers must bound the PDU to MAX_PDU_SIZE
+      : device(device),
-  ModbusDeviceCommand(ModbusClientDevice *device, uint8_t address, std::span<const uint8_t> pdu)
+        frame(address, pdu.data(), static_cast<uint16_t>(pdu.size())),
-      : device(device), frame(address, pdu.data(), static_cast<uint16_t>(pdu.size())) {}
+        continuous(continuous),
+        seq(seq) {}
+
+  // Transmit ordering class, derived (never stored): a continuous poll ranks below every one-shot.
+  CommandPriority priority() const {
+    return this->continuous ? CommandPriority::CONTINUOUS : classify(this->frame.pdu()[0]);
+  }
+  // Wire-derived class: mutating codes rank WRITE; exception-flagged codes are excluded.
+  static CommandPriority classify(uint8_t function_code) {
+    if (helpers::is_function_code_exception(function_code))
+      return CommandPriority::READ;
+    const auto code = static_cast<FunctionCode>(function_code);
+    if (helpers::is_function_code_write(function_code) || code == FunctionCode::MASK_WRITE_REGISTER ||
+        code == FunctionCode::READ_WRITE_MULTIPLE_REGISTERS) {
+      return CommandPriority::WRITE;
+    }
+    return CommandPriority::READ;
+  }
+
+  // Requests this entry can serve: a standard read twice (run plus one re-run), everything else once.
+  uint8_t max_pending() const {
+    const uint8_t fc = this->frame.pdu()[0];
+    const bool requeueable = !helpers::is_function_code_exception(fc) && helpers::is_function_code_read(fc);
+    return (requeueable && !this->continuous) ? 2 : 1;
+  }
+  // Device-scoped clear: detach with no callback (device-less, pending 0). An entry still waiting for
+  // a response keeps its state as a reply-ignoring shell that resolves silently; any other goes RETIRED.
+  void silent_retire() {
+    if (!this->waiting_state())
+      this->state = FrameState::RETIRED;
+    this->pending = 0;
+    this->device = nullptr;
+  }
+  // Re-ready for another transmission, restamped to the tail of its class (hub passes next_seq_++).
+  void requeue(uint16_t seq) {
+    this->state = FrameState::READY;
+    this->seq = seq;
+  }
+  // Re-task a frame that lives on: upgrade a one-shot to a continuous poll, or downgrade a poll back to
+  // a one-shot. Either way the entry keeps running and owes a request, so this is not a plain setter -
+  // to tear an entry down instead, use retire()/silent_retire(), which leave pending as the count owed.
+  // On: the entry becomes a continuous poll, superseding any absorbed requests (pending resets to the
+  // single subscription). Off: a one-shot duplicate has cancelled the poll, but the entry must still run
+  // once to serve that request - so restore one first. While the flag is still set max_pending() is 1,
+  // so the restore lifts a terminated poll (pending 0, after an error/timeout) back to 1 and is a no-op
+  // on a live poll already at 1; the flag drops afterwards, when a read's cap can widen to 2 without
+  // retroactively inflating that no-op.
+  void make_continuous(bool continuous) {
+    if (continuous) {
+      this->continuous = true;
+      this->pending = 1;
+    } else {
+      this->increment_pending();
+      this->continuous = false;
+    }
+  }
+  // Address-scoped clear: keep pending and device so the sweep delivers one on_not_sent() per un-run
+  // request. An entry still waiting for a response keeps its in-flight request (whose usual terminal is
+  // still coming) and drains only its duplicates: WAITING -> WAITING_RETIRED, and INTERRUPTED ->
+  // INTERRUPTED_RETIRED which keeps distrusting late frames (they were already interrupted). Any other
+  // state -> RETIRED, draining everything. A cleared frame that then times out still honors a retry:
+  // the clear is address-scoped (any device may call it) while the retry is the owning device's call
+  // via on_no_response - the bus obeys the owner.
+  void retire() {
+    if (this->state == FrameState::WAITING) {
+      this->state = FrameState::WAITING_RETIRED;
+    } else if (this->state == FrameState::INTERRUPTED) {
+      this->state = FrameState::INTERRUPTED_RETIRED;
+    } else if (!this->waiting_state()) {  // an already-retired shell stays put; off the wire -> RETIRED
+      this->state = FrameState::RETIRED;
+    }
+    this->continuous = false;
+  }
+
+  // True while the entry is still waiting for a response; the erase pass exempts these even at pending 0.
+  bool waiting_state() const {
+    return this->state == FrameState::WAITING || this->state == FrameState::INTERRUPTED ||
+           this->state == FrameState::WAITING_RETIRED || this->state == FrameState::INTERRUPTED_RETIRED;
+  }
+
+  bool decrement_pending() {
+    if (this->pending > 0) {
+      this->pending--;
+      return true;
+    }
+    return false;
+  }
+  // Add one request, honouring the cap; false = already at cap (absorb a duplicate, restore a retry).
+  bool increment_pending() {
+    if (this->pending < this->max_pending()) {
+      this->pending++;
+      return true;
+    }
+    return false;
+  }
+
+  // Terminal/lifecycle methods: each owns its transition, callback, and pending accounting and
+  // returns whether a callback ran. Out-of-line: ModbusClientDevice is incomplete here.
+  bool sent();
+  bool response(std::span<const uint8_t> response_pdu);
+  bool error(ExceptionCode exception_code);
+  bool interrupt();
+  bool timed_out();
+  bool notify_retired();
+
+  /// True if this command carries the same wire frame (address + PDU) as the given one.
+  bool same_frame(uint8_t address, std::span<const uint8_t> pdu) const {
+    const auto own_pdu = this->frame.pdu();
+    return own_pdu.size() == pdu.size() && this->frame.address() == address &&
+           memcmp(own_pdu.data(), pdu.data(), pdu.size()) == 0;
+  }
 };
 
 class ModbusClientHub : public Modbus {
@@ -123,14 +270,15 @@ class ModbusClientHub : public Modbus {
                                               payload_len),
                    device);
   };
-  void send_pdu(uint8_t address, std::span<const uint8_t> pdu, ModbusClientDevice *device = nullptr);
+  // Queue a request; true once it is a live entry (resolving in one terminal), false if it never
+  // entered the machine (empty/oversize PDU, full queue, anonymous or over-cap duplicate) - no callback.
+  bool send_pdu(uint8_t address, std::span<const uint8_t> pdu, ModbusClientDevice *device = nullptr,
+                CommandOptions options = {});
   ESPDEPRECATED("Use send_pdu(payload[0], <pdu bytes>, device) instead. Removed in 2027.2.0", "2026.8.0")
   void send_raw(const std::vector<uint8_t> &payload, ModbusClientDevice *device = nullptr);
-  // Drop the queued commands for an address; every dropped frame resolves via its owner's on_not_sent(),
+  // Clear an address's commands; each un-run request resolves via on_not_sent(), but a frame on the
-  // so other devices sharing the address observe the drop. The in-flight frame is only detached (silently)
+  // wire still runs to its usual terminal. clear_tx_queue_for_device() instead discards silently.
-  // when clear_sent is set. clear_tx_queue_for_device() SILENTLY discards the caller's own frames
+  void clear_tx_queue_for_address(uint8_t address);
-  // (supersede/teardown semantics); see the lifecycle note on ModbusClientDevice.
-  void clear_tx_queue_for_address(uint8_t address, bool clear_sent = true);
   void clear_tx_queue_for_device(ModbusClientDevice *device);
 
  protected:
@@ -138,18 +286,29 @@ class ModbusClientHub : public Modbus {
   void parse_modbus_frames() override;
   void process_modbus_server_frame(uint8_t address, std::span<const uint8_t> pdu) override;
   void send_next_frame_();
-  // Notify the waiting device of no response; re-queues the frame if on_no_response() returns true.
+  // Deliver owed callbacks from a quiescent hub and apply lifecycle bookkeeping; see FrameState.
-  // wfr is the caller's checked reference to waiting_for_response_.
+  void sweep_();
-  void notify_no_response_(ModbusDeviceCommand &wfr);
+  // The selection function: best READY entry (WRITE class first, then one-shot reads, then the
-  void requeue_waiting_frame_(ModbusDeviceCommand &wfr);
+  // least-recently-served continuous; FIFO by seq within each group), or nullptr.
+  ModbusDeviceCommand *select_next_ready_();
+  // Locate the single entry waiting for a response (WAITING/INTERRUPTED/WAITING_RETIRED/INTERRUPTED_RETIRED).
+  ModbusDeviceCommand *find_waiting_();
+  // End the wait for a response on send-wait timeout (the loop() watchdog body); see FrameState.
+  void expire_waiting_();
 
   uint16_t send_wait_time_{2000};
   uint16_t turnaround_delay_ms_{0};
-  std::optional<ModbusDeviceCommand> waiting_for_response_;
 
-  // std::deque is appropriate here since we need a FIFO buffer, and we can't know ahead of time how many
+  // Set on transmit, cleared on the transaction-ending transition; send_next_frame_ won't select
-  // requests will be queued. Each modbus component may queue multiple requests, and the sequence of scheduling
+  // while it is set, so at most one frame is awaiting a response.
-  // may change at run time.
+  bool waiting_for_response_{false};
+
+  // Set whenever a transition leaves owed callbacks behind; quiet loop() passes skip the sweep.
+  bool sweep_needed_{false};
+  // Monotonic stamp source for ModbusDeviceCommand::seq.
+  uint16_t next_seq_{0};
+
+  // Plain append-order container; ordering lives in select_next_ready_(), lifecycle in FrameState.
   std::deque<ModbusDeviceCommand> tx_buffer_;
 };
 
@@ -176,7 +335,7 @@ class ModbusServerHub : public Modbus {
   std::vector<ModbusServerDevice *> devices_;
 
   // Holds the raw payload of a single reply deferred for sending when tx was blocked at send time.
-  // Only one server reply can be in flight at once, so a single fixed buffer avoids heap allocation.
+  // Only one server reply can be waiting at once, so a single fixed buffer avoids heap allocation.
   std::array<uint8_t, MAX_RAW_SIZE> deferred_payload_;
   uint16_t deferred_payload_len_{0};
 };
@@ -184,25 +343,24 @@ class ModbusServerHub : public Modbus {
 // Transaction status: std::nullopt on success, otherwise a Modbus exception code
 using ResponseStatus = std::optional<ExceptionCode>;
 
-/// Command lifecycle: each accepted command (a send_pdu()/typed-helper call, or a hub re-queue from
+/// Callback contract. Each accepted request ends in exactly ONE terminal: on_response() (data),
-/// a retry) ends in exactly ONE terminal callback: on_response() (valid response), on_error()
+/// on_error() (exception), on_no_response() (timeout/interruption), or on_not_sent() (dropped by
-/// (exception response), on_no_response() (timeout or interrupted transaction), or on_not_sent()
+/// clear_tx_queue_for_address before transmission). A request refused at send_pdu() (false return)
-/// (never transmitted: send failure or full queue). on_sent() is additional, not
+/// gets none. on_sent() is additional, once per transmission, never for an on_not_sent() request.
-/// terminal: it fires once per wire transmission, before whichever of data/error/no_response follows,
+/// on_response()/on_error() fire at parse time and on_no_response() at the send-wait watchdog, all
-/// and never for a command that ends in on_not_sent().
+/// from a quiescent hub; only on_not_sent() is delivered by the sweep. Sending or clearing from
-/// The exceptions to "exactly one terminal":
+/// inside a callback is safe (picked up by the next sweep). Exceptions to "exactly one terminal":
-///  - clear_tx_queue_for_device() drops the caller's OWN queued commands SILENTLY (supersede/teardown
+/// clear_tx_queue_for_device() drops the caller's own frames silently; a continuous poll's cycles are
-///    semantics), and both clear variants detach the in-flight frame silently.
+/// its own accounting (a one-shot duplicate downgrades the poll to a one-shot; a continuous duplicate
-///    clear_tx_queue_for_address() DOES resolve every queued frame it drops via the owner's
+/// merges into it).
-///    on_not_sent() (delivered one at a time, after that frame leaves the queue).
+///
-///  - while a device's own on_not_sent() is on the stack, further on_not_sent() deliveries to THAT
+/// Invariants:
-///    device are dropped (see trigger_not_sent()). In particular, a clear issued from inside your own
+/// - Public entry points (send_pdu/clear_tx_queue_*) only append to the queue or mutate an existing
-///    on_not_sent() resolves your remaining frames silently - treat it like
+///   entry through its callback-free transition methods.
-///    clear_tx_queue_for_device(): you cleared them, you know. Other owners are still notified.
+/// - Public entry points can never trigger a callback synchronously.
-/// Sending from inside on_not_sent() is hazardous: the notification may itself mean the queue is full
+/// - Callbacks are delivered only from within loop().
-/// or refusing, and this device's retry that is refused again is dropped WITHOUT a callback (the
+/// - At most one callback is ever issued between calls to sweep_():
-/// guard above, which bounds what would otherwise be unbounded re-entry) - prefer re-sending from a
+///     sweep_ -> parse (response OR error) OR timeout (no_response) -> sweep_ -> send (sent) -> sweep_ (next loop)
-/// later trigger or the component's update()/loop().
 class ModbusClientDevice {
  public:
   ModbusClientDevice() = default;
@@ -230,29 +388,14 @@ class ModbusClientDevice {
   virtual void on_error(std::span<const uint8_t> request_pdu, ExceptionCode exception_code) {
     this->dispatch_response_(request_pdu, {}, exception_code);
   }
-  /// Called when no request could be sent (e.g. queue full, transmission blocked).
+  /// Called when an accepted request was dropped before transmission by clear_tx_queue_for_address().
-  /// Do not attempt to queue a command in this callback.
+  /// (on_modbus_* below are deprecated pre-rename spellings; the defaults forward during deprecation.)
-  /// (The on_modbus_* names below are deprecated pre-rename spellings; the defaults forward so
-  /// external devices overriding them keep working through the deprecation window.)
   virtual void on_not_sent(std::span<const uint8_t> request_pdu) {
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
     this->on_modbus_not_sent();
 #pragma GCC diagnostic pop
   }
-  /// Non-virtual entry point the hub uses for EVERY on_not_sent() delivery (refusals and clear-queue
-  /// sweeps alike). While this device's on_not_sent() is on the stack, further deliveries to it are
-  /// dropped: this bounds every send->refuse and clear->sweep recursion, including cycles through
-  /// multiple devices (each device can appear on the stack at most once). The documented cost: a clear
-  /// issued from inside your own on_not_sent() resolves your remaining frames SILENTLY, while other
-  /// owners are still notified (their guards are not set) - see the lifecycle contract above.
-  void trigger_not_sent(std::span<const uint8_t> request_pdu) {
-    if (this->notifying_not_sent_)
-      return;
-    this->notifying_not_sent_ = true;
-    this->on_not_sent(request_pdu);
-    this->notifying_not_sent_ = false;
-  }
   /// Called when this device's frame is actually written to the wire
   virtual void on_sent(std::span<const uint8_t> request_pdu) {}
   /// Called when no matching, uninterrupted response arrived; return true to have the hub re-queue the frame for a
@@ -322,56 +465,60 @@ class ModbusClientDevice {
         helpers::create_client_pdu((FunctionCode) function, start_address, number_of_entities, payload, payload_len),
         this);
   }
-  void send_pdu(std::span<const uint8_t> pdu) { this->parent_->send_pdu(this->address_, pdu, this); }
+  /// See ModbusClientHub::send_pdu(): true = accepted (a terminal callback will follow),
+  /// false = refused at the door (no callback).
+  bool send_pdu(std::span<const uint8_t> pdu, CommandOptions options = {}) {
+    return this->parent_->send_pdu(this->address_, pdu, this, options);
+  }
   ESPDEPRECATED("Use send_pdu() instead (the device address is prepended for you). Removed in 2027.2.0", "2026.8.0")
-  void send_raw(const std::vector<uint8_t> &payload) {
+  bool send_raw(const std::vector<uint8_t> &payload) {
-    if (payload.empty()) {
+    if (payload.empty())
-      // Through the guard like every other delivery, so a handler calling send_raw({}) cannot recurse.
+      return false;  // too short to contain a PDU; refused at the door like any invalid send
-      this->trigger_not_sent({});
+    return this->parent_->send_pdu(payload[0], std::span<const uint8_t>(payload).subspan(1), this);
-      return;
-    }
-    this->parent_->send_pdu(payload[0], std::span<const uint8_t>(payload).subspan(1), this);
   }
   // Reads via the table-appropriate function code; an unreadable entity type maps to INVALID, which
-  // create_read_pdu() rejects into an empty PDU and send_pdu() signals via on_not_sent().
+  // create_read_pdu() rejects into an empty PDU and send_pdu() refuses with a false return.
-  void read_entities(EntityType entity_type, uint16_t start_address, uint16_t number_of_entities) {
+  bool read_entities(EntityType entity_type, uint16_t start_address, uint16_t number_of_entities,
-    this->send_pdu(helpers::create_read_pdu(helpers::modbus_register_read_function(entity_type), start_address,
+                     CommandOptions options = {}) {
-                                            number_of_entities));
+    return this->send_pdu(helpers::create_read_pdu(helpers::modbus_register_read_function(entity_type), start_address,
+                                                   number_of_entities),
+                          options);
   }
-  void read_input_registers(uint16_t start_address, uint16_t number_of_registers) {
+  bool read_input_registers(uint16_t start_address, uint16_t number_of_registers, CommandOptions options = {}) {
-    this->send_pdu(helpers::create_read_pdu(FunctionCode::READ_INPUT_REGISTERS, start_address, number_of_registers));
+    return this->send_pdu(
+        helpers::create_read_pdu(FunctionCode::READ_INPUT_REGISTERS, start_address, number_of_registers), options);
   }
-  void read_holding_registers(uint16_t start_address, uint16_t number_of_registers) {
+  bool read_holding_registers(uint16_t start_address, uint16_t number_of_registers, CommandOptions options = {}) {
-    this->send_pdu(helpers::create_read_pdu(FunctionCode::READ_HOLDING_REGISTERS, start_address, number_of_registers));
+    return this->send_pdu(
+        helpers::create_read_pdu(FunctionCode::READ_HOLDING_REGISTERS, start_address, number_of_registers), options);
   }
-  void read_coils(uint16_t start_address, uint16_t number_of_coils) {
+  bool read_coils(uint16_t start_address, uint16_t number_of_coils, CommandOptions options = {}) {
-    this->send_pdu(helpers::create_read_pdu(FunctionCode::READ_COILS, start_address, number_of_coils));
+    return this->send_pdu(helpers::create_read_pdu(FunctionCode::READ_COILS, start_address, number_of_coils), options);
   }
-  void read_discrete_inputs(uint16_t start_address, uint16_t number_of_inputs) {
+  bool read_discrete_inputs(uint16_t start_address, uint16_t number_of_inputs, CommandOptions options = {}) {
-    this->send_pdu(helpers::create_read_pdu(FunctionCode::READ_DISCRETE_INPUTS, start_address, number_of_inputs));
+    return this->send_pdu(helpers::create_read_pdu(FunctionCode::READ_DISCRETE_INPUTS, start_address, number_of_inputs),
+                          options);
   }
-  void write_single_register(uint16_t start_address, uint16_t value) {
+  bool write_single_register(uint16_t start_address, uint16_t value) {
-    this->send_pdu(helpers::create_write_single_register_pdu(start_address, value));
+    return this->send_pdu(helpers::create_write_single_register_pdu(start_address, value));
   }
-  void write_single_coil(uint16_t address, bool value) {
+  bool write_single_coil(uint16_t address, bool value) {
-    this->send_pdu(helpers::create_write_single_coil_pdu(address, value));
+    return this->send_pdu(helpers::create_write_single_coil_pdu(address, value));
   }
-  void write_multiple_registers(uint16_t start_address, std::span<const uint16_t> values) {
+  bool write_multiple_registers(uint16_t start_address, std::span<const uint16_t> values) {
-    this->send_pdu(helpers::create_write_registers_pdu(start_address, values));
+    return this->send_pdu(helpers::create_write_registers_pdu(start_address, values));
   }
   /// Note: std::vector<bool> cannot bind to std::span<const bool>; use a contiguous bool container or the packed
   /// overload.
-  void write_multiple_coils(uint16_t start_address, std::span<const bool> values) {
+  bool write_multiple_coils(uint16_t start_address, std::span<const bool> values) {
-    this->send_pdu(helpers::create_write_coils_pdu(start_address, values));
+    return this->send_pdu(helpers::create_write_coils_pdu(start_address, values));
   }
   /// Packed variant: a PackedBits view (the same layout on_read_coils() delivers), so
   /// read-modify-write needs no unpack/repack.
-  void write_multiple_coils(uint16_t start_address, PackedBits bits) {
+  bool write_multiple_coils(uint16_t start_address, PackedBits bits) {
-    this->send_pdu(helpers::create_write_coils_pdu(start_address, bits));
+    return this->send_pdu(helpers::create_write_coils_pdu(start_address, bits));
-  }
-  inline void clear_tx_queue_for_address(bool clear_sent = true) {
-    this->parent_->clear_tx_queue_for_address(this->address_, clear_sent);
   }
+  inline void clear_tx_queue_for_address() { this->parent_->clear_tx_queue_for_address(this->address_); }
   inline void clear_tx_queue_for_device() { this->parent_->clear_tx_queue_for_device(this); }
 
   // If more than one device is connected block sending a new command before a response is received
@@ -385,8 +532,6 @@ class ModbusClientDevice {
                           ResponseStatus status);
 
   ModbusClientHub *parent_{nullptr};
-  /// True while this device's on_not_sent() is on the stack (see trigger_not_sent()).
-  bool notifying_not_sent_{false};
   uint8_t address_{0};
   bool custom_response_warned_{false};  // first unhandled custom response warns; repeats log at VERBOSE
 };

esphome/components/modbus/modbus_definitions.h

@@ -116,6 +116,14 @@ static constexpr uint16_t READ_PDU_SIZE = 5;
 // A single-write PDU is always function code(1) + address(2) + value(2)
 static constexpr uint16_t WRITE_SINGLE_PDU_SIZE = 5;
 static constexpr uint16_t MAX_FRAME_SIZE = 256;
+// Both send paths bound their payload so the framed result lands exactly on the RTU limit: a client
+// PDU gains an address byte and a CRC, a raw server frame gains a CRC. send_frame_() therefore never
+// has to check the framed size - it cannot be exceeded.
+static_assert(MAX_PDU_SIZE + 3 == MAX_FRAME_SIZE, "a framed client PDU must fill the RTU frame limit");
+static_assert(MAX_RAW_SIZE + 2 == MAX_FRAME_SIZE, "a framed raw server payload must fill the RTU frame limit");
+/// Bits pack 8 per data byte, rounded up to whole bytes.
+constexpr size_t packed_bit_bytes(size_t bits) { return (bits + 7) / 8; }
+
 /** Read-only view of Modbus-packed bits: bit 0 of byte 0 is the first bit (LSB first), the layout
  * coil/discrete-input values use on the wire. Bundles the bit count with the packed bytes so the
  * two cannot desynchronize. The view does not own the bytes - it is only valid while they are.
@@ -123,9 +131,6 @@ static constexpr uint16_t MAX_FRAME_SIZE = 256;
  * with any subscript. Writes and forwarding are defensive: set() drops out-of-range bits and
  * bytes() clamps to the real span, because those paths touch buffers and the wire directly.
  */
-/// Bits pack 8 per data byte, rounded up to whole bytes.
-constexpr size_t packed_bit_bytes(size_t bits) { return (bits + 7) / 8; }
-
 class PackedBits {
  public:
   PackedBits(std::span<const uint8_t> data, uint16_t count) : data_(data), count_(count) {}

esphome/components/modbus/modbus_helpers.cpp

@@ -114,8 +114,10 @@ bool is_server_pdu_standard(const uint8_t *pdu, size_t size) {
     case FunctionCode::WRITE_MULTIPLE_REGISTERS: {
       // The response echoes start address and quantity: bound them like the request side does.
       const bool bits = function_code == FunctionCode::WRITE_MULTIPLE_COILS;
+      const uint16_t start_address = get_data<uint16_t>(pdu, 1);
+      const uint16_t quantity = get_data<uint16_t>(pdu, 3);
       const uint16_t max_quantity = bits ? MAX_NUM_OF_COILS_TO_WRITE : MAX_NUM_OF_REGISTERS_TO_WRITE;
-      return quantity_in_range(get_data<uint16_t>(pdu, 1), get_data<uint16_t>(pdu, 3), max_quantity);
+      return quantity_in_range(start_address, quantity, max_quantity);
     }
     case FunctionCode::WRITE_SINGLE_COIL:
       // The response echoes the request, so the same ON/OFF constraint applies.
@@ -137,25 +139,31 @@ bool is_client_pdu_standard(const uint8_t *pdu, size_t size) {
     case FunctionCode::READ_INPUT_REGISTERS: {
       const bool bits =
           function_code == FunctionCode::READ_COILS || function_code == FunctionCode::READ_DISCRETE_INPUTS;
+      const uint16_t start_address = get_data<uint16_t>(pdu, 1);
+      const uint16_t quantity = get_data<uint16_t>(pdu, 3);
       const uint16_t max_quantity = bits ? MAX_NUM_OF_COILS_TO_READ : MAX_NUM_OF_REGISTERS_TO_READ;
-      return quantity_in_range(get_data<uint16_t>(pdu, 1), get_data<uint16_t>(pdu, 3), max_quantity);
+      return quantity_in_range(start_address, quantity, max_quantity);
     }
     case FunctionCode::WRITE_MULTIPLE_COILS:
     case FunctionCode::WRITE_MULTIPLE_REGISTERS: {
       const bool bits = function_code == FunctionCode::WRITE_MULTIPLE_COILS;
+      const uint16_t start_address = get_data<uint16_t>(pdu, 1);
       const uint16_t quantity = get_data<uint16_t>(pdu, 3);
       const uint16_t max_quantity = bits ? MAX_NUM_OF_COILS_TO_WRITE : MAX_NUM_OF_REGISTERS_TO_WRITE;
       // Coils are packed 8 per data byte; registers are 2 bytes each.
       const size_t expected_data_bytes = bits ? packed_bit_bytes(quantity) : quantity * 2;
-      return quantity_in_range(get_data<uint16_t>(pdu, 1), quantity, max_quantity) && pdu[5] == expected_data_bytes;
+      return quantity_in_range(start_address, quantity, max_quantity) && pdu[5] == expected_data_bytes;
     }
     case FunctionCode::READ_FILE_RECORD:
     case FunctionCode::WRITE_FILE_RECORD:
       return pdu[1] <= MAX_PDU_SIZE - 2;
     case FunctionCode::READ_WRITE_MULTIPLE_REGISTERS: {
+      const uint16_t start_address_read = get_data<uint16_t>(pdu, 1);
+      const uint16_t quantity_read = get_data<uint16_t>(pdu, 3);
+      const uint16_t start_address_write = get_data<uint16_t>(pdu, 5);
       const uint16_t quantity_write = get_data<uint16_t>(pdu, 7);
-      return quantity_in_range(get_data<uint16_t>(pdu, 1), get_data<uint16_t>(pdu, 3), MAX_NUM_OF_REGISTERS_TO_READ) &&
+      return quantity_in_range(start_address_read, quantity_read, MAX_NUM_OF_REGISTERS_TO_READ) &&
-             quantity_in_range(get_data<uint16_t>(pdu, 5), quantity_write, MAX_NUM_OF_REGISTERS_TO_WRITE_RW) &&
+             quantity_in_range(start_address_write, quantity_write, MAX_NUM_OF_REGISTERS_TO_WRITE_RW) &&
              pdu[9] == quantity_write * 2;
     }
     case FunctionCode::WRITE_SINGLE_COIL:
@@ -299,6 +307,8 @@ static void append_pdu_header(StaticVector<uint8_t, CAP> &pdu, FunctionCode func
 
 // Zero the unused bits of a multi-coil write's final data byte, as the spec requires. Kept in one
 // place so the generic and typed coil builders produce identical wire bytes for the same write.
+// The caller must pass a span whose LAST byte is the final packed-bit byte - both builders pass the
+// whole PDU, which qualifies because the coil data is always the PDU's tail.
 static void mask_trailing_pad_bits(std::span<uint8_t> data, uint16_t bit_count) {
   if (data.empty() || bit_count % 8 == 0)
     return;

esphome/components/modbus_controller/modbus_controller.h

@@ -305,11 +305,8 @@ class ModbusController final : public PollingComponent, public modbus::ModbusCli
   /// Deliberately shadows the deprecated ModbusClientDevice::send_raw() with identical semantics:
   /// controller-level raw sends stay supported until the command machinery is replaced.
   void send_raw(const std::vector<uint8_t> &payload) {
-    if (payload.empty()) {
+    if (payload.empty())
-      // Through the guard like every other delivery, so a handler calling send_raw({}) cannot recurse.
+      return;  // refused at the door, like every invalid send; no callback follows
-      this->trigger_not_sent({});
-      return;
-    }
     this->parent_->send_pdu(payload[0], std::span<const uint8_t>(payload).subspan(1), this);
   }
   /// Registers a sensor with the controller. Called by esphomes code generator

tests/components/modbus/heap_probe_test.cpp

@@ -117,9 +117,10 @@ TEST(HeapProbe, TypicalFrameConstructionIsAllocationFree) {
   EXPECT_EQ(large.count, 1u);
 }
 
-// Queueing typical commands is fully allocation-free: the frame fits the inline buffer and the tx
+// Queueing typical commands is allocation-free within the deque's first block: the frame fits the
-// deque's first block is already allocated when the hub is constructed. (A queue deeper than one
+// inline buffer, every entry is a plain append (ordering lives in selection, not storage), and the
-// deque block - roughly a dozen commands - would allocate further blocks.)
+// first block is already allocated when the hub is constructed. A 512-byte deque block holds
+// 512 / sizeof(ModbusDeviceCommand) entries (16 on the 64-bit host); a deeper queue allocates more.
 TEST(HeapProbe, QueueingTypicalCommandsIsAllocationFree) {
   ModbusClientHub hub;
   ModbusClientDevice device(&hub, 0x02);
@@ -129,14 +130,36 @@ TEST(HeapProbe, QueueingTypicalCommandsIsAllocationFree) {
   req.assign(read_pdu, read_pdu + sizeof(read_pdu));
 
   constexpr int n = 12;
+  static_assert(n * sizeof(ModbusDeviceCommand) < 512, "keep n within one deque block so the probe stays meaningful");
   size_t total = 0;
   for (int i = 0; i != n; i++) {
+    req[2] = static_cast<uint8_t>(i);  // distinct start addresses: identical frames would dedup, not enqueue
     total += sample([&] { device.send_pdu(req); }).count;
   }
   printf("HEAPPROBE queue_%d_typical_commands total_allocs=%zu\n", n, total);
   EXPECT_EQ(total, 0u);
 }
 
+// A WRITE arriving behind queued reads is a plain append too - the old priority front-insert (and
+// its possible front-block allocation) is gone; the write wins transmit SELECTION instead.
+TEST(HeapProbe, WriteBehindQueuedReadsAppendsAllocationFree) {
+  ModbusClientHub hub;
+  ModbusClientDevice device(&hub, 0x02);
+
+  StaticVector<uint8_t, MAX_PDU_SIZE> req;
+  const uint8_t read_pdu[] = {0x03, 0x01, 0x00, 0x00, 0x02};
+  req.assign(read_pdu, read_pdu + sizeof(read_pdu));
+  for (int i = 0; i != 3; i++) {
+    req[2] = static_cast<uint8_t>(i);  // distinct start addresses: identical frames would dedup, not enqueue
+    device.send_pdu(req);
+  }
+
+  const uint8_t write_pdu[] = {0x06, 0x00, 0x10, 0xBE, 0xEF};
+  Sample append = sample([&] { device.send_pdu(write_pdu); });
+  printf("HEAPPROBE write_append count=%zu bytes=%zu\n", append.count, append.bytes);
+  EXPECT_EQ(append.count, 0u);
+}
+
 // End to end: bytes injected at the UART travel through receive, frame parsing, response matching and
 // device dispatch. The first response may grow the hub's rx buffer once; after that warm-up, handling a
 // response performs zero heap allocations all the way to the device callback.
@@ -189,6 +212,9 @@ TEST(HeapProbe, TypicalFrameConstructionIsAllocationFree) {
 TEST(HeapProbe, QueueingTypicalCommandsIsAllocationFree) {
   GTEST_SKIP() << "allocation counting requires an AddressSanitizer build";
 }
+TEST(HeapProbe, WriteBehindQueuedReadsAppendsAllocationFree) {
+  GTEST_SKIP() << "allocation counting requires an AddressSanitizer build";
+}
 TEST(HeapProbe, ResponseHandlingIsAllocationFreeAfterWarmup) {
   GTEST_SKIP() << "allocation counting requires an AddressSanitizer build";
 }