From ab12e5490f680ab337236fd149ab59e96e1b373b Mon Sep 17 00:00:00 2001 From: Bonne Eggleston Date: Sun, 9 Aug 2026 15:00:44 -0700 Subject: [PATCH] [modbus] Rename send_pdu() to queue_pdu() (#18196) Co-authored-by: Claude Co-authored-by: J. Nick Koston --- esphome/components/modbus/modbus.cpp | 14 +- esphome/components/modbus/modbus.h | 99 +++--- .../components/modbus_client/modbus_client.h | 29 +- .../modbus_controller/modbus_controller.cpp | 15 +- esphome/components/pzemac/pzemac.cpp | 2 +- esphome/components/pzemdc/pzemdc.cpp | 2 +- tests/components/modbus/heap_probe_test.cpp | 8 +- .../modbus/modbus_client_hub_test.cpp | 286 ++++++++++-------- 8 files changed, 260 insertions(+), 195 deletions(-) diff --git a/esphome/components/modbus/modbus.cpp b/esphome/components/modbus/modbus.cpp index c9e443cd87..9f2527d9fb 100644 --- a/esphome/components/modbus/modbus.cpp +++ b/esphome/components/modbus/modbus.cpp @@ -883,8 +883,8 @@ void ModbusClientHub::sweep_() { } // Raw send for client: pushes to tx queue. Everything except the CRC must be contained in payload. -bool ModbusClientHub::send_pdu(uint8_t address, std::span pdu, ModbusClientDevice *device, - CommandOptions options) { +bool ModbusClientHub::queue_pdu(uint8_t address, std::span pdu, ModbusClientDevice *device, + CommandOptions options) { // Requests refused here never enter the machine and get no callback - the false return is it. if (pdu.empty()) { ESP_LOGW(TAG, "Empty PDU refused for address %" PRIu8, address); @@ -995,7 +995,7 @@ void ModbusClientHub::send_raw(const std::vector &payload, ModbusClient ESP_LOGW(TAG, "send_raw() payload too short to contain a PDU, refused"); return; } - this->send_pdu(payload[0], std::span(payload).subspan(1), device); + this->queue_pdu(payload[0], std::span(payload).subspan(1), device); } // Send raw command for server replies immediately. Except CRC everything must be contained in payload @@ -1077,7 +1077,7 @@ void ModbusClientDevice::dispatch_response_(std::span request_pdu // - On failure (status engaged) the response is empty by design (see on_error()), so only the request // is validated. bool custom = !helpers::is_client_pdu_standard(request_pdu.data(), request_pdu.size()); - if (!custom && !status.has_value()) { + if (!custom && succeeded(status)) { custom = !helpers::is_server_pdu_standard(response_pdu.data(), response_pdu.size()); if (!custom && helpers::is_function_code_read(static_cast(function_code))) { const bool bits = @@ -1104,7 +1104,7 @@ void ModbusClientDevice::dispatch_response_(std::span request_pdu // capacity of RegisterValues); a mismatch was diverted to on_custom_response(), never clamped. On // failure the registers span is empty. RegisterValues registers; - if (!status.has_value()) { + if (succeeded(status)) { for (size_t i = 0; i != count_or_value; i++) { registers.push_back(helpers::get_data(response_pdu.data(), 2 + 2 * i)); } @@ -1124,7 +1124,7 @@ void ModbusClientDevice::dispatch_response_(std::span request_pdu // PackedBits::operator[] is unchecked, so size() must never promise bits with no bytes behind them. std::span packed_bytes; uint16_t count = 0; - if (!status.has_value()) { + if (succeeded(status)) { packed_bytes = response_pdu.subspan(2); count = count_or_value; } @@ -1141,7 +1141,7 @@ void ModbusClientDevice::dispatch_response_(std::span request_pdu // copy. On an exception the response has no value and the request copy is the only one. case FunctionCode::WRITE_SINGLE_REGISTER: case FunctionCode::WRITE_SINGLE_COIL: { - const uint16_t value = (!status.has_value() && response_pdu.size() >= WRITE_SINGLE_PDU_SIZE) + const uint16_t value = (succeeded(status) && response_pdu.size() >= WRITE_SINGLE_PDU_SIZE) ? helpers::get_data(response_pdu.data(), 3) : count_or_value; if (function_code == FunctionCode::WRITE_SINGLE_REGISTER) { diff --git a/esphome/components/modbus/modbus.h b/esphome/components/modbus/modbus.h index 274b10f9b4..3b6028e90a 100644 --- a/esphome/components/modbus/modbus.h +++ b/esphome/components/modbus/modbus.h @@ -262,19 +262,31 @@ class ModbusClientHub : public Modbus { void set_turnaround_time(uint16_t time_in_ms) { this->turnaround_delay_ms_ = time_in_ms; } bool tx_buffer_empty(); bool tx_blocked() override; - ESPDEPRECATED("Use send_pdu() with create_client_pdu() instead. Removed in 2026.10.0", "2026.4.0") + ESPDEPRECATED("Use queue_pdu() with create_client_pdu() instead. Removed in 2026.10.0", "2026.4.0") void send(uint8_t address, uint8_t function_code, uint16_t start_address, uint16_t number_of_entities, uint8_t payload_len = 0, const uint8_t *payload = nullptr, ModbusClientDevice *device = nullptr) { - this->send_pdu(address, - helpers::create_client_pdu((FunctionCode) function_code, start_address, number_of_entities, payload, - payload_len), - device); + this->queue_pdu(address, + helpers::create_client_pdu((FunctionCode) function_code, start_address, number_of_entities, payload, + payload_len), + device); }; - // 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 pdu, ModbusClientDevice *device = nullptr, - CommandOptions options = {}); - ESPDEPRECATED("Use send_pdu(payload[0], , device) instead. Removed in 2027.2.0", "2026.8.0") + /// Queue a request. The name says queue, not send: the frame is appended to the transmit queue and + /// goes out later from loop(), so a true return means accepted into the machine (it will resolve in + /// exactly one terminal callback), NOT that anything reached the wire - that is on_sent(). False means + /// it never entered the machine at all (empty or oversize PDU, full queue, anonymous or over-cap + /// duplicate) and no callback of any kind will follow; the false return is the whole story. + bool queue_pdu(uint8_t address, std::span pdu, ModbusClientDevice *device = nullptr, + CommandOptions options = {}); + // Remove before 2027.2.0. Deliberately the signature 2026.7.4 shipped - void, and no CommandOptions: + // the bool return and the options argument arrived after that release, so nothing external can be + // relying on them under this name. Callers who want the queued/refused answer move to queue_pdu(). + ESPDEPRECATED("Use queue_pdu() instead - the call queues a request, it does not send one, and it " + "reports whether the request was accepted. Removed in 2027.2.0", + "2026.8.0") + void send_pdu(uint8_t address, std::span pdu, ModbusClientDevice *device = nullptr) { + this->queue_pdu(address, pdu, device); + } + ESPDEPRECATED("Use queue_pdu(payload[0], , device) instead. Removed in 2027.2.0", "2026.8.0") void send_raw(const std::vector &payload, ModbusClientDevice *device = nullptr); // Clear an address's commands; each un-run request resolves via on_not_sent(), but a frame on the // wire still runs to its usual terminal. clear_tx_queue_for_device() instead discards silently. @@ -315,6 +327,12 @@ class ModbusClientHub : public Modbus { // Transaction status: std::nullopt on success, otherwise a Modbus exception code using ResponseStatus = std::optional; +/// True when a transaction carried no exception. The optional holds the exception, so has_value() means +/// the request FAILED - the inverse of how "status" usually reads. Prefer this at the call site; the +/// bare !status.has_value() has already been mistaken for a failure check more than once. Where the code +/// is going to unwrap the exception anyway, status.has_value() followed by status.value() stays clearer. +inline bool succeeded(ResponseStatus status) { return !status.has_value(); } + // Register values exchanged with server handlers, in host byte order. Sized at the larger of the two protocol // maxima (read = 125 / 0x7D, write = 123 / 0x7B); the per-direction count limit is enforced by the hub, not by // the capacity of this type. @@ -373,7 +391,7 @@ class ModbusServerHub : public Modbus { /// Callback contract. Each accepted request ends in exactly ONE terminal: on_response() (data), /// on_error() (exception), on_no_response() (timeout/interruption), or on_not_sent() (dropped by -/// clear_tx_queue_for_address before transmission). A request refused at send_pdu() (false return) +/// clear_tx_queue_for_address before transmission). A request refused at queue_pdu() (false return) /// gets none. on_sent() is additional, once per transmission, never for an on_not_sent() request. /// on_response()/on_error() fire at parse time and on_no_response() at the send-wait watchdog, all /// from a quiescent hub; only on_not_sent() is delivered by the sweep. Sending or clearing from @@ -383,7 +401,7 @@ class ModbusServerHub : public Modbus { /// merges into it). /// /// Invariants: -/// - Public entry points (send_pdu/clear_tx_queue_*) only append to the queue or mutate an existing +/// - Public entry points (queue_pdu/clear_tx_queue_*) only append to the queue or mutate an existing /// entry through its callback-free transition methods. /// - Public entry points can never trigger a callback synchronously. /// - Callbacks are delivered only from within loop(). @@ -485,66 +503,75 @@ class ModbusClientDevice { /// to handle custom traffic (which also silences the warning). virtual void on_custom_response(std::span request_pdu, std::span response_pdu, ResponseStatus status); - ESPDEPRECATED("Use the typed read_*/write_* helpers or send_pdu() instead. Removed in 2027.2.0", "2026.8.0") + ESPDEPRECATED("Use the typed read_*/write_* helpers or queue_pdu() instead. Removed in 2027.2.0", "2026.8.0") void send(uint8_t function, uint16_t start_address, uint16_t number_of_entities, uint8_t payload_len = 0, const uint8_t *payload = nullptr) { - this->parent_->send_pdu( + this->parent_->queue_pdu( this->address_, helpers::create_client_pdu((FunctionCode) function, start_address, number_of_entities, payload, payload_len), this); } - /// See ModbusClientHub::send_pdu(): true = accepted (a terminal callback will follow), - /// false = refused at the door (no callback). - bool send_pdu(std::span pdu, CommandOptions options = {}) { - return this->parent_->send_pdu(this->address_, pdu, this, options); + /// See ModbusClientHub::queue_pdu(): true = accepted into the queue and a terminal callback will + /// follow, false = refused at the door and nothing further happens. Neither means the frame is on + /// the wire; on_sent() reports that. + bool queue_pdu(std::span pdu, CommandOptions options = {}) { + return this->parent_->queue_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") - bool send_raw(const std::vector &payload) { + // Remove before 2027.2.0. As on the hub, this is the signature 2026.7.4 shipped: void, no options. + ESPDEPRECATED("Use queue_pdu() instead - the call queues a request, it does not send one, and it " + "reports whether the request was accepted. Removed in 2027.2.0", + "2026.8.0") + void send_pdu(std::span pdu) { this->queue_pdu(pdu); } + ESPDEPRECATED("Use queue_pdu() instead (the device address is prepended for you). Removed in 2027.2.0", "2026.8.0") + void send_raw(const std::vector &payload) { if (payload.empty()) - return false; // too short to contain a PDU; refused at the door like any invalid send - return this->parent_->send_pdu(payload[0], std::span(payload).subspan(1), this); + return; // too short to contain a PDU; refused at the door like any invalid send + this->parent_->queue_pdu(payload[0], std::span(payload).subspan(1), this); } + // The typed request builders below all queue through queue_pdu(), so they share its contract: true + // means the request is queued and will resolve in exactly one terminal callback, false means it was + // refused outright with no callback. Neither says the frame has been transmitted - on_sent() does. // 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() refuses with a false return. + // create_read_pdu() rejects into an empty PDU and queue_pdu() refuses with a false return. bool read_entities(EntityType entity_type, uint16_t start_address, uint16_t number_of_entities, CommandOptions options = {}) { - return this->send_pdu(helpers::create_read_pdu(helpers::modbus_register_read_function(entity_type), start_address, - number_of_entities), - options); + return this->queue_pdu(helpers::create_read_pdu(helpers::modbus_register_read_function(entity_type), start_address, + number_of_entities), + options); } bool read_input_registers(uint16_t start_address, uint16_t number_of_registers, CommandOptions options = {}) { - return this->send_pdu( + return this->queue_pdu( helpers::create_read_pdu(FunctionCode::READ_INPUT_REGISTERS, start_address, number_of_registers), options); } bool read_holding_registers(uint16_t start_address, uint16_t number_of_registers, CommandOptions options = {}) { - return this->send_pdu( + return this->queue_pdu( helpers::create_read_pdu(FunctionCode::READ_HOLDING_REGISTERS, start_address, number_of_registers), options); } bool read_coils(uint16_t start_address, uint16_t number_of_coils, CommandOptions options = {}) { - return this->send_pdu(helpers::create_read_pdu(FunctionCode::READ_COILS, start_address, number_of_coils), options); + return this->queue_pdu(helpers::create_read_pdu(FunctionCode::READ_COILS, start_address, number_of_coils), options); } bool read_discrete_inputs(uint16_t start_address, uint16_t number_of_inputs, CommandOptions options = {}) { - return this->send_pdu(helpers::create_read_pdu(FunctionCode::READ_DISCRETE_INPUTS, start_address, number_of_inputs), - options); + return this->queue_pdu( + helpers::create_read_pdu(FunctionCode::READ_DISCRETE_INPUTS, start_address, number_of_inputs), options); } bool write_single_register(uint16_t start_address, uint16_t value) { - return this->send_pdu(helpers::create_write_single_register_pdu(start_address, value)); + return this->queue_pdu(helpers::create_write_single_register_pdu(start_address, value)); } bool write_single_coil(uint16_t address, bool value) { - return this->send_pdu(helpers::create_write_single_coil_pdu(address, value)); + return this->queue_pdu(helpers::create_write_single_coil_pdu(address, value)); } bool write_multiple_registers(uint16_t start_address, std::span values) { - return this->send_pdu(helpers::create_write_registers_pdu(start_address, values)); + return this->queue_pdu(helpers::create_write_registers_pdu(start_address, values)); } /// Note: std::vector cannot bind to std::span; use a contiguous bool container or the packed /// overload. bool write_multiple_coils(uint16_t start_address, std::span values) { - return this->send_pdu(helpers::create_write_coils_pdu(start_address, values)); + return this->queue_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. bool write_multiple_coils(uint16_t start_address, PackedBits bits) { - return this->send_pdu(helpers::create_write_coils_pdu(start_address, bits)); + return this->queue_pdu(helpers::create_write_coils_pdu(start_address, bits)); } 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); } diff --git a/esphome/components/modbus_client/modbus_client.h b/esphome/components/modbus_client/modbus_client.h index 599be85cb8..f9a00d65f6 100644 --- a/esphome/components/modbus_client/modbus_client.h +++ b/esphome/components/modbus_client/modbus_client.h @@ -33,7 +33,11 @@ template class ClientActionBase : public Action, public m /// The frame was written to the wire: fires once per transmission, before any reply, and never for a /// send that ended in on_not_sent. request_pdu is the PDU sent (function code + data). void on_sent(std::span request_pdu) override { this->sent_trigger_.trigger(request_pdu); } - /// Never reached the wire (tx queue full, cleared, or a duplicate write dropped by the hub's dedup). + /// Never reached the wire, from either of two sources. The hub calls this for a request it accepted + /// and then dropped, which happens only when clear_tx_queue_for_address() retires it - a modbus + /// device going offline, say. Everything the hub refuses at the door instead returns false from + /// queue_pdu() with no callback at all, so send_or_resolve_() below turns those into this same + /// callback: a full queue, a duplicate write, or an empty PDU from a rejecting builder. void on_not_sent(std::span request_pdu) override { this->not_sent_trigger_.trigger(request_pdu); } /// A Modbus exception reply. Lives here beside its trigger so every action subclass gets the pairing: /// register_client_action() wires on_error for all of them, so a derived class must not have to @@ -64,7 +68,7 @@ template class ClientActionBase : public Action, public m /// Takes a span, not a PduBuffer: the builders return right-sized buffers (a read PDU is 5 bytes), and /// a PduBuffer parameter would widen each one to the 253-byte maximum just to cross the call. void send_or_resolve_(std::span pdu) { - if (!this->send_pdu(pdu)) + if (!this->queue_pdu(pdu)) this->on_not_sent(pdu); } @@ -107,7 +111,9 @@ template class ModbusClientSendAction : public ClientActionBase< /// valid for the duration of the trigger. (For a typed-built request the gate can only divert on the /// response, never with an exception status - real device exceptions arrive via on_error, which /// ClientActionBase already routes straight to its trigger, so the typed callbacks below only ever see a -/// success status.) +/// success status.) Each typed callback still checks succeeded() before firing its trigger: that branch +/// is unreachable today, and is kept so a future change to that interception cannot silently deliver an +/// exception as a successful reply. template class TypedClientActionBase : public ClientActionBase { public: Trigger, std::span> *get_custom_response_trigger() { @@ -127,11 +133,6 @@ template class TypedClientActionBase : public ClientActionBase, std::span> custom_response_trigger_; bool custom_response_handled_{false}; }; @@ -154,7 +155,7 @@ template class ReadRegistersAction : public TypedClientActionBas } void on_read_registers(modbus::EntityType entity_type, uint16_t start_address, std::span registers, modbus::ResponseStatus status) override { - if (this->is_success_(status)) + if (modbus::succeeded(status)) this->response_trigger_.trigger(registers); } @@ -181,7 +182,7 @@ template class ReadBitsAction : public TypedClientActionBaseis_success_(status)) + if (modbus::succeeded(status)) this->response_trigger_.trigger(bits); } @@ -204,7 +205,7 @@ template class WriteSingleRegisterAction : public TypedClientAct modbus::helpers::create_write_single_register_pdu(this->start_address_.value(x...), this->value_.value(x...))); } void on_write_single_register(uint16_t address, uint16_t value, modbus::ResponseStatus status) override { - if (this->is_success_(status)) + if (modbus::succeeded(status)) this->response_trigger_.trigger(); } @@ -226,7 +227,7 @@ template class WriteSingleCoilAction : public TypedClientActionB modbus::helpers::create_write_single_coil_pdu(this->start_address_.value(x...), this->value_.value(x...))); } void on_write_single_coil(uint16_t address, bool value, modbus::ResponseStatus status) override { - if (this->is_success_(status)) + if (modbus::succeeded(status)) this->response_trigger_.trigger(); } @@ -270,7 +271,7 @@ template class WriteMultipleRegistersAction : public TypedClient } void on_write_multiple_registers(uint16_t start_address, std::span registers, modbus::ResponseStatus status) override { - if (this->is_success_(status)) + if (modbus::succeeded(status)) this->response_trigger_.trigger(); } @@ -318,7 +319,7 @@ template class WriteMultipleCoilsAction : public TypedClientActi } void on_write_multiple_coils(uint16_t start_address, modbus::PackedBits bits, modbus::ResponseStatus status) override { - if (this->is_success_(status)) + if (modbus::succeeded(status)) this->response_trigger_.trigger(); } diff --git a/esphome/components/modbus_controller/modbus_controller.cpp b/esphome/components/modbus_controller/modbus_controller.cpp index c4161d454f..da9d29887e 100644 --- a/esphome/components/modbus_controller/modbus_controller.cpp +++ b/esphome/components/modbus_controller/modbus_controller.cpp @@ -160,10 +160,11 @@ void ModbusController::queue_command(ModbusCommandItem command) { } void ModbusController::unqueue_command(const ModbusCommandItem *command) { - // Called as the last action of the command's own callback, and from send() after send_pdu (which may - // synchronously call on_not_sent). Destroying `command` here would leave send() and the hub touching a - // freed object, so we only FLAG it; sweep_completed_one_shots_() erases it later at a safe point. No-op - // for polling commands (they persist and are not in the one-shot list). + // Called as the last action of the command's own callback (on_response/on_error/on_not_sent/ + // on_no_response), which the hub runs from inside its sweep while this entry is still live. + // Destroying `command` here would leave the hub touching a freed object, so we only FLAG it; + // sweep_completed_one_shots_() erases it later at a safe point. No-op for polling commands + // (they persist and are not in the one-shot list). for (auto &item : this->one_shot_command_items_) { if (item.get() == command) { item->pending_removal = true; @@ -494,13 +495,13 @@ ModbusCommandItem ModbusCommandItem::create_custom_command( bool ModbusCommandItem::send() { bool accepted; if (this->function_code_ != FunctionCode::CUSTOM) { - accepted = this->send_pdu(modbus::helpers::create_client_pdu( + accepted = this->queue_pdu(modbus::helpers::create_client_pdu( this->function_code_, this->start_address_, this->register_count_, this->payload.empty() ? nullptr : this->payload.data(), this->payload.size())); } else { // Custom command: the bytes are a complete raw frame (address + PDU). Send the PDU to the frame's own // address (which may differ from this controller's); the hub appends the CRC and routes the response - // back to this item by pointer. (send_raw() is deprecated, so send_pdu() is called with the extracted + // back to this item by pointer. (send_raw() is deprecated, so queue_pdu() is called with the extracted // address. Raw-frame semantics are kept here; the custom_pdu migration is a later step.) std::span frame = this->custom_data_ != nullptr ? std::span(*this->custom_data_) : this->payload; @@ -508,7 +509,7 @@ bool ModbusCommandItem::send() { ESP_LOGW(TAG, "Empty custom command frame, not sent"); accepted = false; } else { - accepted = this->parent_->send_pdu(frame[0], frame.subspan(1), this); + accepted = this->parent_->queue_pdu(frame[0], frame.subspan(1), this); } } // The on_command_sent trigger fires from on_sent() when the frame actually reaches the wire. diff --git a/esphome/components/pzemac/pzemac.cpp b/esphome/components/pzemac/pzemac.cpp index 5651e07af0..d817888922 100644 --- a/esphome/components/pzemac/pzemac.cpp +++ b/esphome/components/pzemac/pzemac.cpp @@ -77,7 +77,7 @@ void PZEMAC::dump_config() { void PZEMAC::reset_energy_() { const uint8_t pdu[] = {PZEM_CMD_RESET_ENERGY}; - this->send_pdu(pdu); + this->queue_pdu(pdu); } } // namespace esphome::pzemac diff --git a/esphome/components/pzemdc/pzemdc.cpp b/esphome/components/pzemdc/pzemdc.cpp index 5e505cde0c..926ad83f09 100644 --- a/esphome/components/pzemdc/pzemdc.cpp +++ b/esphome/components/pzemdc/pzemdc.cpp @@ -65,7 +65,7 @@ void PZEMDC::dump_config() { void PZEMDC::reset_energy() { const uint8_t pdu[] = {PZEM_CMD_RESET_ENERGY}; - this->send_pdu(pdu); + this->queue_pdu(pdu); } } // namespace esphome::pzemdc diff --git a/tests/components/modbus/heap_probe_test.cpp b/tests/components/modbus/heap_probe_test.cpp index ddf905a8df..869b280b0d 100644 --- a/tests/components/modbus/heap_probe_test.cpp +++ b/tests/components/modbus/heap_probe_test.cpp @@ -134,7 +134,7 @@ TEST(HeapProbe, QueueingTypicalCommandsIsAllocationFree) { size_t total = 0; for (int i = 0; i != n; i++) { req[2] = static_cast(i); // distinct start addresses: identical frames would dedup, not enqueue - total += sample([&] { device.send_pdu(req); }).count; + total += sample([&] { device.queue_pdu(req); }).count; } printf("HEAPPROBE queue_%d_typical_commands total_allocs=%zu\n", n, total); EXPECT_EQ(total, 0u); @@ -151,11 +151,11 @@ TEST(HeapProbe, WriteBehindQueuedReadsAppendsAllocationFree) { req.assign(read_pdu, read_pdu + sizeof(read_pdu)); for (int i = 0; i != 3; i++) { req[2] = static_cast(i); // distinct start addresses: identical frames would dedup, not enqueue - device.send_pdu(req); + device.queue_pdu(req); } const uint8_t write_pdu[] = {0x06, 0x00, 0x10, 0xBE, 0xEF}; - Sample append = sample([&] { device.send_pdu(write_pdu); }); + Sample append = sample([&] { device.queue_pdu(write_pdu); }); printf("HEAPPROBE write_append count=%zu bytes=%zu\n", append.count, append.bytes); EXPECT_EQ(append.count, 0u); } @@ -180,7 +180,7 @@ TEST(HeapProbe, ResponseHandlingIsAllocationFreeAfterWarmup) { const uint8_t small_resp[] = {0x03, 0x04, 0x00, 0x2A, 0x01, 0x00}; auto round_trip = [&](std::span response_pdu) { - device.send_pdu(req); + device.queue_pdu(req); hub.loop(); // transmit; the tx queue is empty during the measured receive below uart.inject_frame(0x02, response_pdu); return sample([&] { hub.loop(); }); // receive + parse + match + dispatch diff --git a/tests/components/modbus/modbus_client_hub_test.cpp b/tests/components/modbus/modbus_client_hub_test.cpp index 4d5b4e7ee8..c2a36c0da7 100644 --- a/tests/components/modbus/modbus_client_hub_test.cpp +++ b/tests/components/modbus/modbus_client_hub_test.cpp @@ -116,7 +116,7 @@ TEST(ModbusClientHubNoResponse, RetryRequeuesWaitingFrame) { NoResponseProbeHub hub; RetryingDevice device(&hub, 0x02, /*retry=*/true); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); ASSERT_EQ(hub.queued_frames(), 1u); hub.force_send_next(); ASSERT_EQ(hub.queued_frames(), 0u); @@ -141,7 +141,7 @@ TEST(ModbusClientHubNoResponse, NoRetryDropsWaitingFrame) { NoResponseProbeHub hub; RetryingDevice device(&hub, 0x02, /*retry=*/false); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); hub.timeout_waiting(); @@ -157,7 +157,7 @@ TEST(ModbusClientHubNoResponse, DetachedDeviceIsNotNotified) { NoResponseProbeHub hub; { RetryingDevice device(&hub, 0x02, /*retry=*/true); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); // device destructor clears its queue entries, including the waiting frame's device pointer } @@ -177,7 +177,7 @@ TEST(ModbusClientHubNoResponse, RetryBehindInterruptedShell) { NoResponseProbeHub hub; RetryingDevice device(&hub, 0x02, /*retry=*/true); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); // A frame from the wrong address (0x07, expected 0x02) hits the unexpected-frame branch. @@ -204,7 +204,7 @@ TEST(ModbusClientHubNoResponse, InterruptedShellDeclinedRetryRetiresOnRelease) { NoResponseProbeHub hub; RetryingDevice device(&hub, 0x02, /*retry=*/false); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); const uint8_t stray_pdu[] = {0x03, 0x04, 0x00, 0x2A, 0x01, 0x00}; @@ -229,7 +229,7 @@ TEST(ModbusClientHubNoResponse, MidCallbackClearCancelsRetry) { NoResponseProbeHub hub; ClearingRetryDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); hub.timeout_waiting(); @@ -246,9 +246,9 @@ TEST(ModbusClientHubPriority, WritesSendBeforeQueuedReads) { const uint8_t read_a[] = {0x03, 0x01, 0x00, 0x00, 0x02}; const uint8_t read_b[] = {0x03, 0x02, 0x00, 0x00, 0x02}; const uint8_t write_pdu[] = {0x06, 0x00, 0x10, 0xBE, 0xEF}; - device.send_pdu(read_a); - device.send_pdu(read_b); - device.send_pdu(write_pdu); + device.queue_pdu(read_a); + device.queue_pdu(read_b); + device.queue_pdu(write_pdu); ASSERT_EQ(hub.queued_frames(), 3u); hub.force_send_next(); @@ -266,8 +266,8 @@ TEST(ModbusClientHubPriority, DuplicateQueuedFrameAbsorbedNotDuplicated) { NoResponseProbeHub hub; RetryingDevice device(&hub, 0x02, /*retry=*/false); - device.send_pdu(read_pdu()); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); + device.queue_pdu(read_pdu()); ASSERT_EQ(hub.queued_frames(), 1u); EXPECT_EQ(hub.queued(0).pending, 2u); // one entry standing for two accepted requests @@ -280,9 +280,9 @@ TEST(ModbusClientHubPriority, InFlightDuplicateRunsOnceMore) { NoResponseProbeHub hub; RetryingDevice device(&hub, 0x02, /*retry=*/false); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); - device.send_pdu(read_pdu()); // duplicate of the waiting frame + device.queue_pdu(read_pdu()); // duplicate of the waiting frame EXPECT_EQ(hub.queued_frames(), 0u); // not queued twice EXPECT_EQ(hub.waiting_command().pending, 2u); @@ -303,9 +303,9 @@ TEST(ModbusClientHubPriority, AbsorbedRequestSurvivesDeviceRetry) { NoResponseProbeHub hub; RetryingDevice device(&hub, 0x02, /*retry=*/true); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); - device.send_pdu(read_pdu()); // duplicate of the waiting frame -> absorbed + device.queue_pdu(read_pdu()); // duplicate of the waiting frame -> absorbed ASSERT_EQ(hub.waiting_command().pending, 2u); hub.timeout_waiting(); // no response; the device requests a retry @@ -459,8 +459,8 @@ TEST(ModbusClientHubPriority, WritesThenOneShotReadsThenContinuousPolls) { device.read_holding_registers(0x100, 2, {.continuous = true}); const uint8_t one_shot[] = {0x03, 0x02, 0x00, 0x00, 0x01}; const uint8_t write_pdu[] = {0x06, 0x00, 0x10, 0xBE, 0xEF}; - device.send_pdu(one_shot); - device.send_pdu(write_pdu); + device.queue_pdu(one_shot); + device.queue_pdu(write_pdu); ASSERT_EQ(hub.queued_frames(), 3u); EXPECT_EQ(hub.queued(0).priority(), CommandPriority::CONTINUOUS); EXPECT_EQ(hub.queued(1).priority(), CommandPriority::READ); @@ -482,7 +482,7 @@ TEST(ModbusClientHubPriority, ContinuousIgnoredForWrites) { RetryingDevice device(&hub, 0x02, /*retry=*/false); const uint8_t write_pdu[] = {0x06, 0x00, 0x10, 0xBE, 0xEF}; - device.send_pdu(write_pdu, {.continuous = true}); + device.queue_pdu(write_pdu, {.continuous = true}); ASSERT_EQ(hub.queued_frames(), 1u); EXPECT_EQ(hub.queued(0).priority(), CommandPriority::WRITE); EXPECT_FALSE(hub.queued(0).continuous); @@ -530,8 +530,8 @@ TEST(ModbusClientHubPriority, DuplicateQueuedWriteRefused) { SentCountingDevice device(&hub, 0x02); const uint8_t write_pdu[] = {0x06, 0x00, 0x10, 0xBE, 0xEF}; - EXPECT_TRUE(device.send_pdu(write_pdu)); - EXPECT_FALSE(device.send_pdu(write_pdu)); // duplicate write: refused + EXPECT_TRUE(device.queue_pdu(write_pdu)); + EXPECT_FALSE(device.queue_pdu(write_pdu)); // duplicate write: refused hub.sweep_for_test(); ASSERT_EQ(hub.queued_frames(), 1u); @@ -547,8 +547,8 @@ TEST(ModbusClientHubPriority, DuplicateCustomFunctionCodeRefused) { SentCountingDevice device(&hub, 0x02); const uint8_t custom_pdu[] = {0x41, 0x01, 0x02}; // user-defined function code - EXPECT_TRUE(device.send_pdu(custom_pdu)); - EXPECT_FALSE(device.send_pdu(custom_pdu)); // duplicate custom command: refused + EXPECT_TRUE(device.queue_pdu(custom_pdu)); + EXPECT_FALSE(device.queue_pdu(custom_pdu)); // duplicate custom command: refused hub.sweep_for_test(); ASSERT_EQ(hub.queued_frames(), 1u); @@ -562,8 +562,8 @@ TEST(ModbusClientHubPriority, AnonymousDuplicateDroppedNotPromoted) { NoResponseProbeHub hub; const uint8_t read[] = {0x03, 0x01, 0x00, 0x00, 0x02}; - hub.send_pdu(0x02, read); - hub.send_pdu(0x02, read); // anonymous duplicate: dropped + hub.queue_pdu(0x02, read); + hub.queue_pdu(0x02, read); // anonymous duplicate: dropped ASSERT_EQ(hub.queued_frames(), 1u); EXPECT_EQ(hub.queued(0).pending, 1u); // never absorbed for a null owner @@ -575,13 +575,13 @@ TEST(ModbusClientHubPriority, RetriedReadGoesBehindFreshReads) { NoResponseProbeHub hub; RetryingDevice device(&hub, 0x02, /*retry=*/true); - device.send_pdu(read_pdu()); - hub.force_send_next(); // the frame that will time out and retry - device.send_pdu(read_pdu()); // waiting duplicate: absorbed into the waiting entry + device.queue_pdu(read_pdu()); + hub.force_send_next(); // the frame that will time out and retry + device.queue_pdu(read_pdu()); // waiting duplicate: absorbed into the waiting entry const uint8_t fresh_a[] = {0x03, 0x00, 0x10, 0x00, 0x01}; const uint8_t fresh_b[] = {0x03, 0x00, 0x20, 0x00, 0x01}; - device.send_pdu(fresh_a); - device.send_pdu(fresh_b); + device.queue_pdu(fresh_a); + device.queue_pdu(fresh_b); ASSERT_EQ(hub.queued_frames(), 2u); hub.timeout_waiting(); // device retries; the entry returns to READY behind the fresh reads @@ -608,9 +608,9 @@ TEST(ModbusClientHubPriority, AbsorbedDuplicateKeepsPlaceInLine) { const uint8_t read_a[] = {0x03, 0x00, 0x10, 0x00, 0x01}; const uint8_t read_b[] = {0x03, 0x00, 0x20, 0x00, 0x01}; - device.send_pdu(read_a); - device.send_pdu(read_b); - device.send_pdu(read_a); // duplicate of the older entry: absorbed, place unchanged + device.queue_pdu(read_a); + device.queue_pdu(read_b); + device.queue_pdu(read_a); // duplicate of the older entry: absorbed, place unchanged ASSERT_EQ(hub.queued_frames(), 2u); const ModbusDeviceCommand *next = hub.next_ready(); @@ -626,14 +626,14 @@ TEST(ModbusClientHubPriority, RetriedWriteKeepsWritePriorityAndStaysNonRequeueab RetryingDevice device(&hub, 0x02, /*retry=*/true); const uint8_t write_pdu[] = {0x06, 0x00, 0x10, 0xBE, 0xEF}; - device.send_pdu(write_pdu); + device.queue_pdu(write_pdu); hub.force_send_next(); hub.timeout_waiting(); // no response -> device requests retry -> back to READY ASSERT_EQ(hub.queued_frames(), 1u); EXPECT_EQ(hub.queued(0).priority(), CommandPriority::WRITE); // retry preserves the WRITE class - device.send_pdu(write_pdu); // duplicate of the retried write + device.queue_pdu(write_pdu); // duplicate of the retried write ASSERT_EQ(hub.queued_frames(), 1u); // still not queued twice... EXPECT_EQ(hub.queued(0).priority(), CommandPriority::WRITE); hub.sweep_for_test(); @@ -655,7 +655,7 @@ TEST(ModbusClientHubSent, BlockedHubDefersInsteadOfFailing) { AlwaysBlockedHub hub; SentCountingDevice device(&hub, 0x02); - EXPECT_TRUE(device.send_pdu(read_pdu())); + EXPECT_TRUE(device.queue_pdu(read_pdu())); hub.send_next_for_test(); EXPECT_EQ(device.sent_count_, 0); @@ -673,7 +673,7 @@ TEST(ModbusClientHubSent, FiresOnWireNotOnQueue) { hub.setup(); // frame timing derives from the baud rate SentCountingDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); EXPECT_EQ(device.sent_count_, 0); // queued only - nothing sent yet hub.send_next_for_test(); @@ -703,7 +703,7 @@ TEST(ModbusClientHubSent, SendRejectedAfterDelayLeavesFrameReady) { hub.setup(); SentCountingDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.send_next_for_test(); // gate passes, send_frame_ rejects on the post-delay re-check EXPECT_EQ(device.sent_count_, 0); // nothing transmitted @@ -766,7 +766,7 @@ TEST(ModbusClientHubCallbackCount, SingleReadSingleCallback) { NoResponseProbeHub hub; DataCountingDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); drain_with_responses(hub, OK_RESPONSE); EXPECT_EQ(device.data_count_, 1); @@ -781,8 +781,8 @@ TEST(ModbusClientHubCallbackCount, DuplicateReadExactlyTwoCallbacks) { NoResponseProbeHub hub; DataCountingDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); + device.queue_pdu(read_pdu()); int cycles = drain_with_responses(hub, OK_RESPONSE); EXPECT_EQ(cycles, 2); @@ -812,8 +812,8 @@ TEST(ModbusClientHubCallbackCount, ClearFromResponseResolvesDuplicateWithNotSent NoResponseProbeHub hub; ClearOnFirstResponseDevice device(&hub, 0x02); - EXPECT_TRUE(device.send_pdu(read_pdu())); - EXPECT_TRUE(device.send_pdu(read_pdu())); // absorbed: one entry, pending 2 + EXPECT_TRUE(device.queue_pdu(read_pdu())); + EXPECT_TRUE(device.queue_pdu(read_pdu())); // absorbed: one entry, pending 2 hub.force_send_next(); hub.receive_frame_for_test(0x02, OK_RESPONSE); // response -> on_response -> clear, then sweep @@ -829,9 +829,9 @@ TEST(ModbusClientHubCallbackCount, TripleReadRefusesTheThird) { NoResponseProbeHub hub; DataCountingDevice device(&hub, 0x02); - EXPECT_TRUE(device.send_pdu(read_pdu())); - EXPECT_TRUE(device.send_pdu(read_pdu())); - EXPECT_FALSE(device.send_pdu(read_pdu())); // the entry is already at its cap + EXPECT_TRUE(device.queue_pdu(read_pdu())); + EXPECT_TRUE(device.queue_pdu(read_pdu())); + EXPECT_FALSE(device.queue_pdu(read_pdu())); // the entry is already at its cap hub.sweep_for_test(); EXPECT_EQ(device.not_sent_count_, 0); // refused synchronously, nothing owed int cycles = drain_with_responses(hub, OK_RESPONSE); @@ -849,8 +849,8 @@ TEST(ModbusClientHubCallbackCount, DuplicateWriteRefusedWithoutLifecycle) { DataCountingDevice device(&hub, 0x02); const uint8_t write_pdu[] = {0x06, 0x00, 0x10, 0xBE, 0xEF}; - EXPECT_TRUE(device.send_pdu(write_pdu)); - EXPECT_FALSE(device.send_pdu(write_pdu)); + EXPECT_TRUE(device.queue_pdu(write_pdu)); + EXPECT_FALSE(device.queue_pdu(write_pdu)); hub.sweep_for_test(); EXPECT_EQ(device.terminals(), 0); // the accepted write has not resolved; the other never existed @@ -867,7 +867,7 @@ TEST(ModbusClientHubCallbackCount, ErrorResponseIsSoleTerminal) { hub.setup(); DataCountingDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.send_next_for_test(); const uint8_t exception_response[] = {0x83, 0x02}; hub.receive_frame_for_test(0x02, exception_response); @@ -886,7 +886,7 @@ TEST(ModbusClientHubCallbackCount, NoResponseIsSoleTerminalAndNotSentHasNoSent) hub.setup(); DataCountingDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.send_next_for_test(); hub.timeout_waiting(); EXPECT_EQ(device.no_response_count_, 1); @@ -896,8 +896,8 @@ TEST(ModbusClientHubCallbackCount, NoResponseIsSoleTerminalAndNotSentHasNoSent) // Unabsorbable duplicate: the second identical write is refused at the door - no lifecycle, no // terminal, nothing sent. const uint8_t write_pdu[] = {0x06, 0x00, 0x10, 0xBE, 0xEF}; - EXPECT_TRUE(device.send_pdu(write_pdu)); - EXPECT_FALSE(device.send_pdu(write_pdu)); + EXPECT_TRUE(device.queue_pdu(write_pdu)); + EXPECT_FALSE(device.queue_pdu(write_pdu)); hub.sweep_for_test(); EXPECT_EQ(device.not_sent_count_, 0); EXPECT_EQ(device.terminals(), 1); // still just the read's timeout @@ -920,7 +920,7 @@ TEST(ModbusClientHubCallbackCount, RetryLifecyclesEachGetSentAndTerminal) { DataCountingDevice device(&hub, 0x02); device.retries_ = 1; // ask for exactly one retry - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.send_next_for_test(); hub.timeout_waiting(); // lifecycle 1: sent + no_response (retry requested -> re-queued) ASSERT_EQ(hub.queued_frames(), 1u); @@ -946,13 +946,13 @@ TEST(ModbusClientHubCallbackCount, RetryIsNeverRefusedByFullQueue) { device.retries_ = 1; SentCountingDevice filler(&hub, 0x05); - device.send_pdu(read_pdu()); - hub.force_send_next(); // waiting - device.send_pdu(read_pdu()); // absorbed: two requests pending + device.queue_pdu(read_pdu()); + hub.force_send_next(); // waiting + device.queue_pdu(read_pdu()); // absorbed: two requests pending // Fill the remaining live capacity with distinct frames. for (uint16_t i = 0; hub.entries() < MODBUS_TX_BUFFER_SIZE; i++) { const uint8_t fill[] = {0x03, static_cast(i >> 8), static_cast(i & 0xFF), 0x00, 0x01}; - filler.send_pdu(fill); + filler.queue_pdu(fill); } hub.timeout_waiting(); // retry requested; the entry flips back to READY regardless of capacity @@ -991,10 +991,10 @@ TEST(ModbusClientHubQueue, SendRawTooShortIsRefusedAtTheDoor) { NotSentCountingRawDevice device(&hub, 0x02); #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeprecated-declarations" - EXPECT_FALSE(device.send_raw({})); // too short to contain a PDU + device.send_raw({}); // too short to contain a PDU; the deprecated void spelling cannot report it #pragma GCC diagnostic pop - EXPECT_EQ(device.not_sent_count_, 0); // refusals are returned, never delivered - EXPECT_TRUE(hub.tx_buffer_empty()); + EXPECT_EQ(device.not_sent_count_, 0); // refused at the door: no callback delivered + EXPECT_TRUE(hub.tx_buffer_empty()); // the only evidence of the refusal is that nothing queued } // A continuous read: every wire transmission pairs one sent with one terminal, ending on the error. @@ -1040,7 +1040,7 @@ class ChainOnSentDevice : public ModbusClientDevice { if (!this->chained_) { this->chained_ = true; const uint8_t follow[] = {0x03, 0x00, 0x09, 0x00, 0x01}; // read holding 0x0009 x1 - this->send_pdu(follow); + this->queue_pdu(follow); } } bool chained_{false}; @@ -1059,9 +1059,9 @@ TEST(ModbusClientHubQueue, ClearAddressQueueNotifiesEveryOwner) { const uint8_t read_a[] = {0x03, 0x01, 0x00, 0x00, 0x02}; const uint8_t read_b[] = {0x03, 0x02, 0x00, 0x00, 0x02}; const uint8_t read_c[] = {0x03, 0x03, 0x00, 0x00, 0x02}; - controller_like.send_pdu(read_a); - bystander_same.send_pdu(read_b); - bystander_other.send_pdu(read_c); + controller_like.queue_pdu(read_a); + bystander_same.queue_pdu(read_b); + bystander_other.queue_pdu(read_c); ASSERT_EQ(hub.queued_frames(), 3u); controller_like.clear_tx_queue_for_address(); @@ -1083,8 +1083,8 @@ TEST(ModbusClientHubQueue, ClearAddressDeliversOneTerminalPerAcceptedRequest) { SentCountingDevice device(&hub, 0x02); const uint8_t read[] = {0x03, 0x01, 0x00, 0x00, 0x02}; - device.send_pdu(read); - device.send_pdu(read); // duplicate: absorbed into the queued entry + device.queue_pdu(read); + device.queue_pdu(read); // duplicate: absorbed into the queued entry ASSERT_EQ(hub.queued_frames(), 1u); ASSERT_EQ(hub.queued(0).pending, 2u); @@ -1103,8 +1103,8 @@ TEST(ModbusClientHubQueue, ClearSentOnInFlightDuplicateStillNotifiesTheDuplicate NoResponseProbeHub hub; DataCountingDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); - device.send_pdu(read_pdu()); // absorbed: one entry, pending 2 + device.queue_pdu(read_pdu()); + device.queue_pdu(read_pdu()); // absorbed: one entry, pending 2 ASSERT_EQ(hub.queued(0).pending, 2u); hub.force_send_next(); // the frame is sent (WAITING); pending still 2 ASSERT_TRUE(hub.waiting()); @@ -1122,7 +1122,7 @@ TEST(ModbusClientHubQueue, ClearWhileInFlightStillDeliversTheResponse) { NoResponseProbeHub hub; DataCountingDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); // sent, now WAITING ASSERT_TRUE(hub.waiting()); @@ -1151,7 +1151,7 @@ class ResendOnNotSentDevice : public ModbusClientDevice { this->not_sent_count_++; if (this->not_sent_count_ == 1) { const uint8_t again[] = {0x06, 0x00, 0x40, 0x00, 0x01}; // a write: ranked first at selection, not by position - this->send_pdu(again); + this->queue_pdu(again); } } int not_sent_count_{0}; @@ -1165,7 +1165,7 @@ TEST(ModbusClientHubQueue, ClearAddressReentrantResendSurvives) { ResendOnNotSentDevice device(&hub, 0x02); const uint8_t read[] = {0x03, 0x00, 0x10, 0x00, 0x01}; - device.send_pdu(read); + device.queue_pdu(read); ASSERT_EQ(hub.queued_frames(), 1u); hub.clear_tx_queue_for_address(0x02); @@ -1187,8 +1187,8 @@ TEST(ModbusClientHubQueue, ClearAddressReentrantResendNotSwept) { const uint8_t read_victim[] = {0x03, 0x00, 0x10, 0x00, 0x01}; const uint8_t read_other[] = {0x03, 0x00, 0x20, 0x00, 0x01}; - resender.send_pdu(read_victim); - bystander_other.send_pdu(read_other); + resender.queue_pdu(read_victim); + bystander_other.queue_pdu(read_other); ASSERT_EQ(hub.queued_frames(), 2u); hub.clear_tx_queue_for_address(0x02); @@ -1212,13 +1212,14 @@ class AlwaysResendDevice : public ModbusClientDevice { void on_not_sent(std::span request_pdu) override { this->not_sent_count_++; const uint8_t again[] = {0x03, 0x00, 0x50, 0x00, 0x01}; - this->send_pdu(again); + this->queue_pdu(again); } int not_sent_count_{0}; }; -// From inside on_not_sent, clears ANOTHER address - those victims must still be notified (the per-device -// guard suppresses deliveries only to a device already inside its own on_not_sent()). +// From inside on_not_sent, clears ANOTHER address - those victims must still be notified. Nothing +// suppresses that: a re-entrant clear only flips states, retire() is a no-op on an already-retired +// entry, and each entry still owes one notification per un-run request until pending reaches zero. class ClearOtherOnNotSentDevice : public ModbusClientDevice { public: ClearOtherOnNotSentDevice(ModbusClientHub *hub, uint8_t address) : ModbusClientDevice(hub, address) {} @@ -1238,9 +1239,9 @@ TEST(ModbusClientHubQueue, PendingNeverExceedsTheServableCap) { AlwaysResendDevice device(&hub, 0x02); const uint8_t read[] = {0x03, 0x00, 0x50, 0x00, 0x01}; - EXPECT_TRUE(device.send_pdu(read)); - EXPECT_TRUE(device.send_pdu(read)); - EXPECT_FALSE(device.send_pdu(read)); // at the cap: refused + EXPECT_TRUE(device.queue_pdu(read)); + EXPECT_TRUE(device.queue_pdu(read)); + EXPECT_FALSE(device.queue_pdu(read)); // at the cap: refused ASSERT_EQ(hub.queued_frames(), 1u); EXPECT_EQ(hub.queued(0).pending, 2u); @@ -1260,12 +1261,12 @@ TEST(ModbusClientHubQueue, FullQueueRefusesWithoutCallbacks) { // Fill the queue with distinct frames (distinct start addresses keep the dedup from absorbing them). for (uint16_t i = 0; i < MODBUS_TX_BUFFER_SIZE; i++) { const uint8_t fill[] = {0x03, static_cast(i >> 8), static_cast(i & 0xFF), 0x00, 0x01}; - filler.send_pdu(fill); + filler.queue_pdu(fill); } ASSERT_EQ(hub.queued_frames(), MODBUS_TX_BUFFER_SIZE); const uint8_t read[] = {0x03, 0x00, 0x10, 0x00, 0x01}; - EXPECT_FALSE(device.send_pdu(read)); // refused synchronously + EXPECT_FALSE(device.queue_pdu(read)); // refused synchronously hub.sweep_for_test(); EXPECT_EQ(device.not_sent_count_, 0); // nothing was accepted, so nothing is owed @@ -1298,13 +1299,13 @@ TEST(ModbusClientHubQueue, SelfClearFromNotSentResolvesEveryRequest) { const uint8_t read_a[] = {0x03, 0x00, 0x10, 0x00, 0x01}; const uint8_t read_b[] = {0x03, 0x00, 0x20, 0x00, 0x01}; const uint8_t read_c[] = {0x03, 0x00, 0x30, 0x00, 0x01}; - clearer.send_pdu(read_a); - clearer.send_pdu(read_b); - bystander.send_pdu(read_c); + clearer.queue_pdu(read_a); + clearer.queue_pdu(read_b); + bystander.queue_pdu(read_c); ASSERT_EQ(hub.queued_frames(), 3u); - EXPECT_FALSE(clearer.send_pdu(std::span{})); // empty: refused, no callback - clearer.clear_tx_queue_for_address(); // the clear the handler used to make + EXPECT_FALSE(clearer.queue_pdu(std::span{})); // empty: refused, no callback + clearer.clear_tx_queue_for_address(); // the clear the handler used to make hub.sweep_for_test(); @@ -1323,8 +1324,8 @@ TEST(ModbusClientHubQueue, NestedClearFromNotSentStillNotifiesVictims) { const uint8_t read_a[] = {0x03, 0x00, 0x10, 0x00, 0x01}; const uint8_t read_b[] = {0x03, 0x00, 0x20, 0x00, 0x01}; - clearer.send_pdu(read_a); - victim.send_pdu(read_b); + clearer.queue_pdu(read_a); + victim.queue_pdu(read_b); ASSERT_EQ(hub.queued_frames(), 2u); hub.clear_tx_queue_for_address(0x02); // clearer's on_not_sent clears address 0x03 in turn @@ -1345,7 +1346,7 @@ class ResendSecondFrameDevice : public ModbusClientDevice { this->not_sent_count_++; if (this->not_sent_count_ == 1) { const uint8_t same_as_r2[] = {0x03, 0x00, 0x22, 0x00, 0x01}; - this->send_pdu(same_as_r2); + this->queue_pdu(same_as_r2); } } int not_sent_count_{0}; @@ -1360,8 +1361,8 @@ TEST(ModbusClientHubQueue, SweepDedupSkipsDeletedFrames) { const uint8_t r1[] = {0x03, 0x00, 0x21, 0x00, 0x01}; const uint8_t r2[] = {0x03, 0x00, 0x22, 0x00, 0x01}; - device.send_pdu(r1); - device.send_pdu(r2); + device.queue_pdu(r1); + device.queue_pdu(r2); ASSERT_EQ(hub.queued_frames(), 2u); hub.clear_tx_queue_for_address(0x02); @@ -1383,7 +1384,7 @@ class ResendAndClearOnNotSentDevice : public ModbusClientDevice { void on_not_sent(std::span request_pdu) override { this->not_sent_count_++; const uint8_t again[] = {0x03, 0x00, 0x70, 0x00, 0x01}; - this->send_pdu(again); + this->queue_pdu(again); this->clear_tx_queue_for_address(); } int not_sent_count_{0}; @@ -1397,7 +1398,7 @@ TEST(ModbusClientHubQueue, ResendAndClearFromNotSentCannotExtendTheSweep) { ResendAndClearOnNotSentDevice device(&hub, 0x02); const uint8_t read[] = {0x03, 0x00, 0x70, 0x00, 0x01}; - device.send_pdu(read); + device.queue_pdu(read); hub.clear_tx_queue_for_address(0x02); hub.sweep_for_test(); @@ -1421,8 +1422,8 @@ TEST(ModbusClientHubQueue, ClearDeviceQueueDropsSilently) { const uint8_t read_a[] = {0x03, 0x01, 0x00, 0x00, 0x02}; const uint8_t read_b[] = {0x03, 0x02, 0x00, 0x00, 0x02}; - device.send_pdu(read_a); - device.send_pdu(read_b); + device.queue_pdu(read_a); + device.queue_pdu(read_b); ASSERT_EQ(hub.queued_frames(), 2u); device.clear_tx_queue_for_device(); @@ -1431,7 +1432,7 @@ TEST(ModbusClientHubQueue, ClearDeviceQueueDropsSilently) { EXPECT_EQ(device.not_sent_count_, 0); // silent drop: no terminal callback } -// A send_pdu() from inside on_sent() enqueues behind the waiting frame rather than sending +// A queue_pdu() from inside on_sent() enqueues behind the waiting frame rather than sending // immediately or corrupting the waiting transaction. TEST(ModbusClientHubSent, ReentrantSendFromOnSentQueues) { NullUART uart; @@ -1440,7 +1441,7 @@ TEST(ModbusClientHubSent, ReentrantSendFromOnSentQueues) { hub.setup(); ChainOnSentDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.send_next_for_test(); // first frame is sent -> on_sent chains a follow-up EXPECT_TRUE(hub.waiting()); // first frame is waiting @@ -1507,34 +1508,69 @@ class LegacyNameDevice : public ModbusClientDevice { #pragma GCC diagnostic pop } // namespace +// send_pdu() was renamed queue_pdu() because the call queues a request rather than transmitting one. +// The old spelling stays for the deprecation window with the signature 2026.7.4 shipped - void, no +// CommandOptions - so a component built against a real release still compiles and still queues. +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +TEST(ModbusClientHubCompat, DeprecatedSendPduStillQueues) { + NoResponseProbeHub hub; + RetryingDevice device(&hub, 0x02, /*retry=*/false); + + const uint8_t read[] = {0x03, 0x00, 0x10, 0x00, 0x01}; + device.send_pdu(read); // deprecated device spelling: void, as 2026.7.4 shipped it + EXPECT_EQ(hub.queued_frames(), 1u); + + // A refusal is invisible to this spelling - no return value and no callback - so the only evidence + // is that nothing was queued. Reporting the refusal is exactly what moving to queue_pdu() buys. + device.send_pdu(std::span()); + EXPECT_EQ(hub.queued_frames(), 1u); + + // The deprecated hub spelling queues the same way, addressed explicitly. + const uint8_t other[] = {0x03, 0x00, 0x20, 0x00, 0x01}; + hub.send_pdu(0x03, other, &device); + EXPECT_EQ(hub.queued_frames(), 2u); + + // Both frames resolve to the same owner. Drain them in turn: the device-spelling frame first (FIFO), + // then the hub-spelling frame - addressed to 0x03 yet owned by &device, so reaching device's + // on_no_response proves the request routes by owner pointer, not by address. + hub.force_send_next(); + hub.timeout_waiting(); + EXPECT_EQ(device.no_response_count_, 1); // device-spelling frame (address 0x02) + hub.force_send_next(); + hub.timeout_waiting(); + EXPECT_EQ(device.no_response_count_, 2); // hub-spelling frame (address 0x03, &device routing) +} +#pragma GCC diagnostic pop + TEST(ModbusClientHubCompat, LegacyCallbackNamesStillForward) { NoResponseProbeHub hub; LegacyNameDevice device(&hub, 0x02); const uint8_t read[] = {0x03, 0x00, 0x10, 0x00, 0x01}; - device.send_pdu(read); + device.queue_pdu(read); hub.force_send_next(); hub.timeout_waiting(); // no reply -> on_no_response -> forwards to on_modbus_no_response EXPECT_EQ(device.legacy_no_response_, 1); // A refused send returns false with no callback, so exercise the forward through an accepted // request instead: a cleared queue entry delivers on_not_sent(), which forwards to the old name. - EXPECT_FALSE(device.send_pdu(std::span())); // empty PDU: refused at the door + EXPECT_FALSE(device.queue_pdu(std::span())); // empty PDU: refused at the door EXPECT_EQ(device.legacy_not_sent_, 0); const uint8_t queued[] = {0x03, 0x00, 0x11, 0x00, 0x01}; - EXPECT_TRUE(device.send_pdu(queued)); + EXPECT_TRUE(device.queue_pdu(queued)); hub.clear_tx_queue_for_address(0x02); hub.sweep_for_test(); EXPECT_EQ(device.legacy_not_sent_, 1); } -// The send_pdu() capacity bound: a PDU larger than MAX_PDU_SIZE would build a frame past the RTU +// The queue_pdu() capacity bound: a PDU larger than MAX_PDU_SIZE would build a frame past the RTU // 256-byte limit, so it is refused up front - false at the call site, no entry, no callback. TEST(ModbusClientHub, OversizedPduIsRefusedAtTheDoor) { NoResponseProbeHub hub; LegacyNameDevice device(&hub, 0x02); std::vector big(MAX_PDU_SIZE + 1, 0x41); - EXPECT_FALSE(device.send_pdu(big)); + EXPECT_FALSE(device.queue_pdu(big)); EXPECT_EQ(device.legacy_not_sent_, 0); // refusals are returned, never delivered EXPECT_TRUE(hub.tx_buffer_empty()); EXPECT_EQ(hub.entries(), 0u); @@ -1568,7 +1604,7 @@ TEST(ModbusDeviceShim, LegacyCallbacksReceiveTheOldShapes) { // Read response: on_modbus_data() historically received the payload after the function code and // the byte-count byte, as an owning vector. const uint8_t read_req[] = {0x03, 0x00, 0x10, 0x00, 0x02}; - device.send_pdu(read_req); + device.queue_pdu(read_req); hub.force_send_next(); const uint8_t response[] = {0x03, 0x04, 0x00, 0x2A, 0x01, 0x00}; hub.receive_frame_for_test(0x02, response); @@ -1577,14 +1613,14 @@ TEST(ModbusDeviceShim, LegacyCallbacksReceiveTheOldShapes) { // Write echo: no byte-count byte, so the payload is everything after the function code. const uint8_t write_req[] = {0x06, 0x00, 0x10, 0x00, 0x2A}; - device.send_pdu(write_req); + device.queue_pdu(write_req); hub.force_send_next(); hub.receive_frame_for_test(0x02, write_req); // single-write responses echo the request const std::vector expected_echo{0x00, 0x10, 0x00, 0x2A}; EXPECT_EQ(device.last_data_, expected_echo); // Exception response: on_modbus_error() received the masked function code and the exception code. - device.send_pdu(read_req); + device.queue_pdu(read_req); hub.force_send_next(); const uint8_t error[] = {0x83, 0x02}; hub.receive_frame_for_test(0x02, error); @@ -1675,9 +1711,9 @@ class ResendOnDataDevice : public ModbusClientDevice { public: ResendOnDataDevice(ModbusClientHub *hub, uint8_t address) : ModbusClientDevice(hub, address) {} void on_response(std::span request_pdu, std::span response_pdu) override { - this->send_pdu(std::vector(request_pdu.begin(), request_pdu.end())); + this->queue_pdu(std::vector(request_pdu.begin(), request_pdu.end())); } - void send_pdu(const std::vector &pdu) { ModbusClientDevice::send_pdu(pdu); } + void queue_pdu(const std::vector &pdu) { ModbusClientDevice::queue_pdu(pdu); } }; } // namespace @@ -1704,8 +1740,8 @@ TEST(ModbusClientHubPriority, ExceptionFlaggedDuplicateDroppedNotPromoted) { SentCountingDevice device(&hub, 0x02); const uint8_t weird[] = {0x83, 0x01, 0x00, 0x00, 0x02}; // read-shaped but exception-flagged - EXPECT_TRUE(device.send_pdu(weird)); - EXPECT_FALSE(device.send_pdu(weird)); // non-requeueable: cap of one, so the duplicate is refused + EXPECT_TRUE(device.queue_pdu(weird)); + EXPECT_FALSE(device.queue_pdu(weird)); // non-requeueable: cap of one, so the duplicate is refused hub.sweep_for_test(); ASSERT_EQ(hub.queued_frames(), 1u); @@ -1715,7 +1751,7 @@ TEST(ModbusClientHubPriority, ExceptionFlaggedDuplicateDroppedNotPromoted) { // The write-shaped twin (0x86 masks to WRITE_SINGLE_REGISTER) must not take WRITE-class // ordering either: exception-flagged codes are excluded from the mutates classification. const uint8_t weird_write[] = {0x86, 0x00, 0x10, 0xBE, 0xEF}; - device.send_pdu(weird_write); + device.queue_pdu(weird_write); ASSERT_EQ(hub.queued_frames(), 2u); EXPECT_EQ(hub.queued(1).priority(), CommandPriority::READ); // not WRITE const ModbusDeviceCommand *next = hub.next_ready(); @@ -1732,7 +1768,7 @@ class ResendInFlightOnNotSentDevice : public ModbusClientDevice { this->not_sent_count_++; if (this->not_sent_count_ == 1) { const uint8_t same_as_waiting[] = {0x03, 0x01, 0x00, 0x00, 0x02}; // == READ_PDU - this->send_pdu(same_as_waiting); + this->queue_pdu(same_as_waiting); } } int not_sent_count_{0}; @@ -1746,10 +1782,10 @@ TEST(ModbusClientHubQueue, SweepResendAfterClearQueuesFreshNotAbsorbedIntoShell) NoResponseProbeHub hub; ResendInFlightOnNotSentDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); // READ_PDU now waiting const uint8_t queued_read[] = {0x03, 0x00, 0x10, 0x00, 0x01}; - device.send_pdu(queued_read); // a queued frame for the sweep to notify + device.queue_pdu(queued_read); // a queued frame for the sweep to notify ASSERT_EQ(hub.queued_frames(), 1u); hub.clear_tx_queue_for_address(0x02); @@ -1787,7 +1823,7 @@ TEST(ModbusClientHubNoResponse, SelfClearFromNoResponseDoesNotDoubleResolve) { NoResponseProbeHub hub; ClearAddressOnNoResponseDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); hub.timeout_waiting(); @@ -1802,8 +1838,8 @@ TEST(ModbusClientHubNoResponse, SelfClearFromNoResponseResolvesTheAbsorbedReques NoResponseProbeHub hub; ClearAddressOnNoResponseDevice device(&hub, 0x02); - EXPECT_TRUE(device.send_pdu(read_pdu())); - EXPECT_TRUE(device.send_pdu(read_pdu())); // absorbed: one entry, two requests + EXPECT_TRUE(device.queue_pdu(read_pdu())); + EXPECT_TRUE(device.queue_pdu(read_pdu())); // absorbed: one entry, two requests hub.force_send_next(); hub.timeout_waiting(); @@ -1818,7 +1854,7 @@ TEST(ModbusClientHubQueue, ClearedShellReleasesTheBusOnLateResponse) { NoResponseProbeHub hub; DataCountingDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); hub.clear_tx_queue_for_address(0x02); ASSERT_EQ(hub.waiting_command().state, FrameState::WAITING_RETIRED); @@ -1836,7 +1872,7 @@ TEST(ModbusClientHubQueue, ClearedShellReleasesTheBusOnTimeout) { NoResponseProbeHub hub; DataCountingDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); hub.clear_tx_queue_for_address(0x02); ASSERT_EQ(hub.waiting_command().state, FrameState::WAITING_RETIRED); @@ -1856,7 +1892,7 @@ TEST(ModbusClientHubQueue, ClearInterruptedFrameGetsNoResponseAtTimeout) { NoResponseProbeHub hub; DataCountingDevice device(&hub, 0x02); // declines the retry (retries_ == 0) - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); const uint8_t stray_pdu[] = {0x03, 0x04, 0x00, 0x2A, 0x01, 0x00}; hub.receive_frame_for_test(0x07, stray_pdu); // wrong address: interrupts the transaction @@ -1887,7 +1923,7 @@ TEST(ModbusClientHubQueue, InterruptAfterClearStillDistrusts) { NoResponseProbeHub hub; DataCountingDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); hub.clear_tx_queue_for_address(0x02); ASSERT_EQ(hub.waiting_command().state, FrameState::WAITING_RETIRED); @@ -1912,8 +1948,8 @@ TEST(ModbusClientHubQueue, ClearedInFlightDuplicateTimesOutWithoutRerunning) { NoResponseProbeHub hub; DataCountingDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); - device.send_pdu(read_pdu()); // absorbed: one entry, pending 2 + device.queue_pdu(read_pdu()); + device.queue_pdu(read_pdu()); // absorbed: one entry, pending 2 ASSERT_EQ(hub.queued(0).pending, 2u); hub.force_send_next(); // sent, pending still 2 hub.clear_tx_queue_for_address(0x02); @@ -1936,9 +1972,9 @@ TEST(ModbusClientHubCallbackCount, AbsorbedRequestRunsAfterErrorResponse) { hub.setup(); DataCountingDevice device(&hub, 0x02); - device.send_pdu(read_pdu()); + device.queue_pdu(read_pdu()); hub.force_send_next(); - device.send_pdu(read_pdu()); // waiting duplicate: absorbed + device.queue_pdu(read_pdu()); // waiting duplicate: absorbed const uint8_t exception_response[] = {0x83, 0x02}; hub.receive_frame_for_test(0x02, exception_response); // error terminal for request 1 @@ -1954,8 +1990,8 @@ TEST(ModbusClientHubPriority, ReadModifyWritesRankAsWrites) { const uint8_t read[] = {0x03, 0x00, 0x10, 0x00, 0x01}; const uint8_t mask_write[] = {0x16, 0x00, 0x10, 0x00, 0xFF, 0x00, 0x01}; - device.send_pdu(read); - device.send_pdu(mask_write); + device.queue_pdu(read); + device.queue_pdu(mask_write); ASSERT_EQ(hub.queued_frames(), 2u); const ModbusDeviceCommand *next = hub.next_ready();