diff --git a/esphome/components/modbus/modbus.cpp b/esphome/components/modbus/modbus.cpp index eaca168ed8..3f8aef433f 100644 --- a/esphome/components/modbus/modbus.cpp +++ b/esphome/components/modbus/modbus.cpp @@ -51,7 +51,7 @@ void ModbusClientHub::loop() { // If we're past the send_wait_time timeout and response buffer doesn't have the start of the expected response if (this->waiting_for_response_.has_value()) { ModbusDeviceCommand &wfr = this->waiting_for_response_.value(); - uint8_t expected_address = wfr.frame.data.data()[0]; + uint8_t expected_address = wfr.frame.address(); if (this->last_receive_check_ - this->last_send_ > this->last_send_tx_offset_ + this->send_wait_time_ && (this->rx_buffer_.empty() || this->rx_buffer_[0] != expected_address)) { ESP_LOGW(TAG, "Stop waiting for response from %" PRIu8 " %" PRIu32 "ms after last send", expected_address, @@ -258,13 +258,13 @@ bool ModbusServerHub::parse_modbus_client_frame_() { } // Bounds contract, enforced by the parser (parse_modbus_server_frame_) rather than locally: -// - pdu is never empty: helpers::server_frame_length() returns at least MIN_FRAME_SIZE (4) on every -// branch, and find_custom_frame_end_() only ever lengthens that, so the PDU (frame minus address -// and CRC) always holds at least the function code. -// - When the exception bit is set, pdu has at least 2 bytes: server_frame_length() checks the -// exception bit before anything else and pins those frames to 5 bytes, so the exception code +// - pdu is never empty: helpers::server_pdu_length() returns at least MIN_PDU_SIZE (1) on every +// 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_frame_length() or adding callers. +// Keep those guarantees in mind when changing server_pdu_length() or adding callers. void ModbusClientHub::process_modbus_server_frame(uint8_t address, std::span pdu) { const uint8_t function_code = pdu[0]; if (!this->waiting_for_response_.has_value()) { @@ -276,8 +276,8 @@ void ModbusClientHub::process_modbus_server_frame(uint8_t address, std::spanwaiting_for_response_.value(); - uint8_t expected_address = wfr.frame.data.data()[0]; - uint8_t expected_function_code = wfr.frame.data.data()[1]; + uint8_t expected_address = wfr.frame.address(); + uint8_t expected_function_code = wfr.frame.pdu()[0]; if (expected_address != address || expected_function_code != (function_code & FUNCTION_CODE_MASK)) { ESP_LOGW(TAG, "Received incorrect frame address %" PRIu8 " <> %" PRIu8 " or function code 0x%X <> 0x%X, %" PRIu32 @@ -304,7 +304,7 @@ void ModbusClientHub::process_modbus_server_frame(uint8_t address, std::spanwaiting_for_response_.reset(); ModbusClientDevice *device = command.device; // The request PDU is the sent frame without the leading address and the trailing CRC. - std::span request_pdu(command.frame.data.data() + 1, command.frame.size() - 3); + std::span request_pdu = command.frame.pdu(); // Is it an error response? if (helpers::is_function_code_exception(function_code)) { uint8_t exception = pdu[1]; // exception frames are fixed-length, so the code is always present @@ -586,18 +586,26 @@ void ModbusClientHub::notify_no_response_(ModbusDeviceCommand &wfr) { void ModbusClientHub::requeue_waiting_frame_(ModbusDeviceCommand &wfr) { const ModbusFrame &frame = wfr.frame; if (this->tx_buffer_.size() >= MODBUS_TX_BUFFER_SIZE) { - ESP_LOGE(TAG, "Write buffer full, dropped retry for address %" PRIu8, frame.data.data()[0]); + ESP_LOGE(TAG, "Write buffer full, dropped retry for address %" PRIu8, frame.address()); if (wfr.device != nullptr) wfr.device->on_not_sent(); return; } // 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.data.data()[0], frame.data.data() + 1, frame.size() - 3); + 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::queue_raw_(uint8_t address, const uint8_t *pdu, uint16_t pdu_len, ModbusClientDevice *device) { - if (pdu_len == 0) { +void ModbusClientHub::send_pdu(uint8_t address, std::span pdu, ModbusClientDevice *device) { + if (pdu.empty()) { + if (device) + device->on_not_sent(); + 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()); if (device) device->on_not_sent(); return; @@ -607,13 +615,15 @@ void ModbusClientHub::queue_raw_(uint8_t address, const uint8_t *pdu, uint16_t p #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, pdu_len)); - this->tx_buffer_.emplace_back(device, address, pdu, pdu_len); + 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); } else { #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, format_hex_pretty_to(hex_buf, pdu, pdu_len)); + ESP_LOGE(TAG, "Write buffer full, dropped: %" PRIu8 ":%s", address, + format_hex_pretty_to(hex_buf, pdu.data(), pdu.size())); if (device) device->on_not_sent(); } @@ -622,13 +632,12 @@ void ModbusClientHub::queue_raw_(uint8_t address, const uint8_t *pdu, uint16_t p void ModbusClientHub::clear_tx_queue_for_address(uint8_t address, bool clear_sent) { // 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(), - [address](const ModbusDeviceCommand &cmd) { return cmd.frame.data.data()[0] == address; }), - tx_buffer.end()); + tx_buffer.erase(std::remove_if(tx_buffer.begin(), tx_buffer.end(), + [address](const ModbusDeviceCommand &cmd) { return cmd.frame.address() == address; }), + tx_buffer.end()); if (clear_sent && this->waiting_for_response_.has_value() && this->waiting_for_response_.value().device) { - if (this->waiting_for_response_.value().frame.data.data()[0] == address) { + 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; @@ -657,7 +666,7 @@ void ModbusClientHub::send_raw(const std::vector &payload, ModbusClient device->on_not_sent(); return; } - this->queue_raw_(payload[0], payload.data() + 1, static_cast(payload.size() - 1), device); + this->send_pdu(payload[0], std::span(payload).subspan(1), device); } // Send raw command for server replies immediately. Except CRC everything must be contained in payload diff --git a/esphome/components/modbus/modbus.h b/esphome/components/modbus/modbus.h index 61d5f552c1..a607e75523 100644 --- a/esphome/components/modbus/modbus.h +++ b/esphome/components/modbus/modbus.h @@ -40,6 +40,13 @@ struct ModbusFrame { } uint16_t size() const { return static_cast(this->data.size()); } + + // A frame is [address][PDU...][CRC lo][CRC hi]. These are the only places that need to know that layout + uint8_t address() const { return this->data.data()[0]; } + /// The PDU: function code + data, without address or CRC. Only valid while the frame is alive. + /// Requires a complete frame (size() >= MIN_FRAME_SIZE, guaranteed by the constructors) - the + /// subtraction would wrap on anything shorter. + std::span pdu() const { return std::span(this->data.data() + 1, this->size() - 3u); } }; class Modbus : public uart::UARTDevice, public Component { @@ -90,6 +97,10 @@ struct ModbusDeviceCommand { ModbusDeviceCommand(ModbusClientDevice *device, uint8_t address, const uint8_t *src, uint16_t len) : device(device), frame(address, src, len) {} + /// Build a command from a PDU span: a caller-supplied PDU, or an existing frame's own pdu() when re-queueing + /// Callers must bound the PDU to MAX_PDU_SIZE + ModbusDeviceCommand(ModbusClientDevice *device, uint8_t address, std::span pdu) + : device(device), frame(address, pdu.data(), static_cast(pdu.size())) {} }; class ModbusClientHub : public Modbus { @@ -109,9 +120,8 @@ class ModbusClientHub : public Modbus { payload_len), device); }; - void send_pdu(uint8_t address, std::span pdu, ModbusClientDevice *device = nullptr) { - this->queue_raw_(address, pdu.data(), pdu.size(), device); - } + void send_pdu(uint8_t address, std::span pdu, ModbusClientDevice *device = nullptr); + ESPDEPRECATED("Use send_pdu(payload[0], , device) instead. Removed in 2027.2.0", "2026.8.0") void send_raw(const std::vector &payload, ModbusClientDevice *device = nullptr); void clear_tx_queue_for_address(uint8_t address, bool clear_sent = true); void clear_tx_queue_for_device(ModbusClientDevice *device); @@ -125,7 +135,6 @@ class ModbusClientHub : public Modbus { // wfr is the caller's checked reference to waiting_for_response_. void notify_no_response_(ModbusDeviceCommand &wfr); void requeue_waiting_frame_(ModbusDeviceCommand &wfr); - void queue_raw_(uint8_t address, const uint8_t *pdu, uint16_t pdu_len, ModbusClientDevice *device = nullptr); uint16_t send_wait_time_{2000}; uint16_t turnaround_delay_ms_{0}; @@ -165,6 +174,9 @@ class ModbusServerHub : public Modbus { uint16_t deferred_payload_len_{0}; }; +// Transaction status: std::nullopt on success, otherwise a Modbus exception code +using ResponseStatus = std::optional; + class ModbusClientDevice { public: ModbusClientDevice() = default; @@ -217,7 +229,14 @@ class ModbusClientDevice { this); } void send_pdu(std::span pdu) { this->parent_->send_pdu(this->address_, pdu, this); } - void send_raw(const std::vector &payload) { this->parent_->send_raw(payload, this); } + 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 &payload) { + if (payload.empty()) { + this->on_not_sent(); // match the hub-level send_raw(): a refused send is always signalled + return; + } + this->parent_->send_pdu(payload[0], std::span(payload).subspan(1), this); + } inline void clear_tx_queue_for_address(bool clear_sent = true) { this->parent_->clear_tx_queue_for_address(this->address_, clear_sent); } @@ -238,9 +257,6 @@ class ModbusClientDevice { using ModbusDevice ESPDEPRECATED("Use ModbusClientDevice instead. Removed in 2026.12.0", "2026.6.0") = ModbusClientDevice; -// Transaction status: std::nullopt on success, otherwise the Modbus exception code. Server handlers return it; -// (future) client response callbacks receive it. Named without a side prefix so both directions share it. -using ResponseStatus = std::optional; // 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. diff --git a/esphome/components/modbus/modbus_definitions.h b/esphome/components/modbus/modbus_definitions.h index 841222ff6a..f1e6a1f57e 100644 --- a/esphome/components/modbus/modbus_definitions.h +++ b/esphome/components/modbus/modbus_definitions.h @@ -84,9 +84,15 @@ enum class ExceptionCode : uint8_t { using ModbusExceptionCode ESPDEPRECATED("Use modbus::ExceptionCode instead. Removed in 2027.2.0", "2026.8.0") = ExceptionCode; +// 6.11 15 (0x0F) Write Multiple Coils +static constexpr uint16_t MAX_NUM_OF_COILS_TO_WRITE = 1968; // 0x7B0 + // 6.12 16 (0x10) Write Multiple registers: static constexpr uint16_t MAX_NUM_OF_REGISTERS_TO_WRITE = 123; // 0x7B +// 6.17 23 (0x17) Read/Write Multiple Registers: +static constexpr uint16_t MAX_NUM_OF_REGISTERS_TO_WRITE_RW = 121; // 0x79 + // 6.1 01 (0x01) Read Coils // 6.2 02 (0x02) Read Discrete Inputs static constexpr uint16_t MAX_NUM_OF_COILS_TO_READ = 2000; // 0x7D0 @@ -98,6 +104,7 @@ static constexpr uint16_t MAX_NUM_OF_REGISTERS_TO_READ = 125; // 0x7D // Smallest possible frame is 4 bytes (custom function with no data): address(1) + function(1) + CRC(2) static constexpr uint16_t MIN_FRAME_SIZE = 4; +static constexpr uint16_t MIN_PDU_SIZE = 1; static constexpr uint16_t MAX_PDU_SIZE = 253; // Max PDU size is 256 - address(1) - CRC(2) = 253 static constexpr uint16_t MAX_RAW_SIZE = 254; // Max RAW size is 256 - CRC(2) = 254 static constexpr uint16_t MAX_FRAME_SIZE = 256; diff --git a/esphome/components/modbus/modbus_helpers.cpp b/esphome/components/modbus/modbus_helpers.cpp index 1ed7912b40..900c9e3093 100644 --- a/esphome/components/modbus/modbus_helpers.cpp +++ b/esphome/components/modbus/modbus_helpers.cpp @@ -7,74 +7,157 @@ namespace esphome::modbus::helpers { static const char *const TAG = "modbus_helpers"; -uint16_t server_frame_length(const uint8_t *frame, size_t size) { - if (size < 2) - return MIN_FRAME_SIZE; - if (is_function_code_exception(frame[1])) { - return 5; // address(1) + function(1) + exception(1) + CRC(2) +uint16_t server_pdu_length(const uint8_t *frame, size_t size) { + if (size < MIN_PDU_SIZE) + return MIN_PDU_SIZE; + if (is_function_code_exception(frame[0])) { + return 2; // function(1) + exception(1) } - switch (static_cast(frame[1])) { + switch (static_cast(frame[0])) { case FunctionCode::READ_COILS: case FunctionCode::READ_DISCRETE_INPUTS: case FunctionCode::READ_HOLDING_REGISTERS: case FunctionCode::READ_INPUT_REGISTERS: - // address(1) + function(1) + byte count(1) + data + CRC(2) - return 5 + (size > 2 ? std::min(frame[2], uint8_t(MAX_NUM_OF_REGISTERS_TO_READ * 2)) : 0); + // function(1) + byte count(1) + data + return 2 + (size > 1 ? std::min(frame[1], uint8_t(MAX_NUM_OF_REGISTERS_TO_READ * 2)) : 0); case FunctionCode::WRITE_SINGLE_COIL: case FunctionCode::WRITE_SINGLE_REGISTER: case FunctionCode::WRITE_MULTIPLE_COILS: case FunctionCode::WRITE_MULTIPLE_REGISTERS: - return 8; // address(1) + function(1) + output/register address(2) + value(2) + CRC(2) + return 5; // function(1) + output/register address(2) + value(2) // Unsupported function codes. Included here to prevent parser failures. Excluding Serial Line specific functions. case FunctionCode::READ_FILE_RECORD: case FunctionCode::WRITE_FILE_RECORD: - // address(1) + function(1) + byte count(1) + data + CRC(2) - return 5 + (size > 2 ? std::min(frame[2], uint8_t(MAX_FRAME_SIZE - 5)) : 0); + // function(1) + byte count(1) + data + return 2 + (size > 1 ? std::min(frame[1], uint8_t(MAX_PDU_SIZE - 2)) : 0); case FunctionCode::MASK_WRITE_REGISTER: - return 10; // address(1) + function(1) + reference address(2) + AND mask(2) + OR mask(2) + CRC(2) + return 7; // function(1) + reference address(2) + AND mask(2) + OR mask(2) case FunctionCode::READ_WRITE_MULTIPLE_REGISTERS: - // address(1) + function(1) + byte count(1) + data + CRC(2) - return 5 + (size > 2 ? std::min(frame[2], uint8_t(MAX_NUM_OF_REGISTERS_TO_READ * 2)) : 0); + // function(1) + byte count(1) + data + return 2 + (size > 1 ? std::min(frame[1], uint8_t(MAX_NUM_OF_REGISTERS_TO_READ * 2)) : 0); case FunctionCode::READ_FIFO_QUEUE: - // address(1) + function(1) + fifo address(2) CRC(2) - return 6; + // function(1) + fifo address(2) + return 3; default: - return MIN_FRAME_SIZE; // unknown length + return MIN_PDU_SIZE; // unknown length } } -uint16_t client_frame_length(const uint8_t *frame, size_t size) { - if (size < 2) - return MIN_FRAME_SIZE; - switch (static_cast(frame[1])) { +uint16_t client_pdu_length(const uint8_t *frame, size_t size) { + if (size < MIN_PDU_SIZE) + return MIN_PDU_SIZE; + switch (static_cast(frame[0])) { case FunctionCode::READ_COILS: case FunctionCode::READ_DISCRETE_INPUTS: case FunctionCode::READ_HOLDING_REGISTERS: case FunctionCode::READ_INPUT_REGISTERS: - // address(1) + function(1) + start address(2) + quantity(2) + CRC(2) + // function(1) + start address(2) + quantity(2) case FunctionCode::WRITE_SINGLE_COIL: case FunctionCode::WRITE_SINGLE_REGISTER: - return 8; // address(1) + function(1) + output/register address(2) + value(2) + CRC(2) + return 5; // function(1) + output/register address(2) + value(2) case FunctionCode::WRITE_MULTIPLE_COILS: case FunctionCode::WRITE_MULTIPLE_REGISTERS: - // address(1) + function(1) + start address(2) + quantity(2) + byte count(1) + data + CRC(2) - return 9 + (size > 6 ? std::min(frame[6], uint8_t(MAX_NUM_OF_REGISTERS_TO_WRITE * 2)) : 0); + // function(1) + start address(2) + quantity(2) + byte count(1) + data + return 6 + (size > 5 ? std::min(frame[5], uint8_t(MAX_NUM_OF_REGISTERS_TO_WRITE * 2)) : 0); // Unsupported function codes. Included here to prevent parser failures. Excluding Serial Line specific functions. case FunctionCode::READ_FILE_RECORD: case FunctionCode::WRITE_FILE_RECORD: - // address(1) + function(1) + byte count(1) + data + CRC(2) - return 5 + (size > 2 ? std::min(frame[2], uint8_t(MAX_FRAME_SIZE - 5)) : 0); + // function(1) + byte count(1) + data + return 2 + (size > 1 ? std::min(frame[1], uint8_t(MAX_PDU_SIZE - 2)) : 0); case FunctionCode::MASK_WRITE_REGISTER: - return 10; // address(1) + function(1) + reference address(2) + AND mask(2) + OR mask(2) + CRC(2) + return 7; // function(1) + reference address(2) + AND mask(2) + OR mask(2) case FunctionCode::READ_WRITE_MULTIPLE_REGISTERS: - // address(1) + function(1) + read start address(2) + read quantity(2) + write start address(2) + - // write quantity(2) + byte count(1) + data + CRC(2) - return 13 + (size > 10 ? std::min(frame[10], uint8_t(MAX_NUM_OF_REGISTERS_TO_WRITE * 2)) : 0); + // function(1) + read start address(2) + read quantity(2) + write start address(2) + + // write quantity(2) + byte count(1) + data + return 10 + (size > 9 ? std::min(frame[9], uint8_t(MAX_NUM_OF_REGISTERS_TO_WRITE_RW * 2)) : 0); case FunctionCode::READ_FIFO_QUEUE: - // address(1) + function(1) + fifo address(2) CRC(2) - return 6; + // function(1) + fifo address(2) + return 3; default: - return MIN_FRAME_SIZE; // unknown length + return MIN_PDU_SIZE; // unknown length + } +} + +bool is_server_pdu_standard(const uint8_t *pdu, size_t size) { + if (server_pdu_length(pdu, size) != size) + return false; + + switch (static_cast(pdu[0])) { + case FunctionCode::READ_COILS: + case FunctionCode::READ_DISCRETE_INPUTS: + // A conformant bit-read response carries at least one packed byte (up to 2000 bits = 250 bytes). + return pdu[1] != 0 && pdu[1] <= uint8_t((MAX_NUM_OF_COILS_TO_READ + 7) / 8); + case FunctionCode::READ_HOLDING_REGISTERS: + case FunctionCode::READ_INPUT_REGISTERS: + // Registers are 2 bytes each: the byte count must be a non-zero even count within the read maximum. + return pdu[1] != 0 && pdu[1] % 2 == 0 && pdu[1] <= uint8_t(MAX_NUM_OF_REGISTERS_TO_READ * 2); + case FunctionCode::READ_FILE_RECORD: + case FunctionCode::WRITE_FILE_RECORD: + return pdu[1] <= uint8_t(MAX_PDU_SIZE - 2); + case FunctionCode::READ_WRITE_MULTIPLE_REGISTERS: + return pdu[1] != 0 && pdu[1] % 2 == 0 && pdu[1] <= uint8_t(MAX_NUM_OF_REGISTERS_TO_READ * 2); + case FunctionCode::WRITE_MULTIPLE_COILS: + case FunctionCode::WRITE_MULTIPLE_REGISTERS: { + // The response echoes start address and quantity: bound them like the request side does. + const bool bits = static_cast(pdu[0]) == FunctionCode::WRITE_MULTIPLE_COILS; + const uint16_t start_address = get_data(pdu, 1); + const uint16_t quantity = get_data(pdu, 3); + const uint16_t max_quantity = bits ? MAX_NUM_OF_COILS_TO_WRITE : MAX_NUM_OF_REGISTERS_TO_WRITE; + return quantity != 0 && quantity <= max_quantity && (uint32_t) start_address + quantity <= 0x10000u; + } + case FunctionCode::WRITE_SINGLE_COIL: + // The response echoes the request, so the same ON/OFF constraint applies. + return (pdu[3] == 0xFF || pdu[3] == 0x00) && pdu[4] == 0x00; + default: + return true; // All other function codes validated by length alone + } +} + +bool is_client_pdu_standard(const uint8_t *pdu, size_t size) { + if (client_pdu_length(pdu, size) != size) + return false; + + switch (static_cast(pdu[0])) { + case FunctionCode::READ_COILS: + case FunctionCode::READ_DISCRETE_INPUTS: + case FunctionCode::READ_HOLDING_REGISTERS: + case FunctionCode::READ_INPUT_REGISTERS: { + const bool bits = static_cast(pdu[0]) == FunctionCode::READ_COILS || + static_cast(pdu[0]) == FunctionCode::READ_DISCRETE_INPUTS; + const uint16_t start_address = get_data(pdu, 1); + const uint16_t quantity = get_data(pdu, 3); + const uint16_t max_quantity = bits ? MAX_NUM_OF_COILS_TO_READ : MAX_NUM_OF_REGISTERS_TO_READ; + return quantity != 0 && quantity <= max_quantity && (uint32_t) start_address + quantity <= 0x10000u; + } + case FunctionCode::WRITE_MULTIPLE_COILS: + case FunctionCode::WRITE_MULTIPLE_REGISTERS: { + const bool bits = static_cast(pdu[0]) == FunctionCode::WRITE_MULTIPLE_COILS; + const uint16_t start_address = get_data(pdu, 1); + const uint16_t quantity = get_data(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 ? (static_cast(quantity) + 7) / 8 : quantity * 2; + return quantity != 0 && quantity <= max_quantity && (uint32_t) start_address + quantity <= 0x10000u && + 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(pdu, 1); + const uint16_t quantity_read = get_data(pdu, 3); + const uint16_t start_address_write = get_data(pdu, 5); + const uint16_t quantity_write = get_data(pdu, 7); + return quantity_read != 0 && quantity_read <= MAX_NUM_OF_REGISTERS_TO_READ && quantity_write != 0 && + quantity_write <= MAX_NUM_OF_REGISTERS_TO_WRITE_RW && + (uint32_t) start_address_read + quantity_read <= 0x10000u && + (uint32_t) start_address_write + quantity_write <= 0x10000u && pdu[9] == quantity_write * 2; + } + case FunctionCode::WRITE_SINGLE_COIL: + // The one variable field in an otherwise fixed-shape PDU: the spec allows exactly ON/OFF. + return (pdu[3] == 0xFF || pdu[3] == 0x00) && pdu[4] == 0x00; + default: + return true; // All other function codes validated by length alone } } diff --git a/esphome/components/modbus/modbus_helpers.h b/esphome/components/modbus/modbus_helpers.h index 0fece3be5d..44c1a01211 100644 --- a/esphome/components/modbus/modbus_helpers.h +++ b/esphome/components/modbus/modbus_helpers.h @@ -39,13 +39,39 @@ inline bool is_function_code_custom(uint8_t function_code) { masked_function_code <= FUNCTION_CODE_USER_DEFINED_SPACE_2_END); } -// Returns the expected length of a server response frame based on the function code -// If the frame is too short to determine the length, returns the minimum length -uint16_t server_frame_length(const uint8_t *frame, size_t size); +// Returns the expected length of a server response PDU based on the function code. +// If too few bytes have arrived to determine the length, returns the minimum length. `size` is the +// number of bytes available so far, which may exceed the eventual PDU (e.g. include the frame's CRC +// bytes): only fixed header positions are interpreted, so surplus bytes are never misread. +uint16_t server_pdu_length(const uint8_t *frame, size_t size); +// Frame counterpart: address(1) + PDU + CRC(2). Passes every received byte after the address through, +// so header fields (e.g. a byte count) are interpreted as soon as they arrive. +inline uint16_t server_frame_length(const uint8_t *frame, size_t size) { + if (size < 2) + return MIN_FRAME_SIZE; // function code not received yet + return server_pdu_length(frame + 1, size - 1) + 3; +} -// Returns the expected length of a client request frame based on the function code -// If the frame is too short to determine the length, returns the minimum length -uint16_t client_frame_length(const uint8_t *frame, size_t size); +// Returns the expected length of a client request PDU based on the function code. +// Same contract as server_pdu_length(): `size` is bytes available so far, may exceed the PDU. +uint16_t client_pdu_length(const uint8_t *frame, size_t size); +inline uint16_t client_frame_length(const uint8_t *frame, size_t size) { + if (size < 2) + return MIN_FRAME_SIZE; // function code not received yet + return client_pdu_length(frame + 1, size - 1) + 3; +} + +// Returns true if pdu is a complete transaction whose shape is consistent with its function code. +// Unlike *_pdu_length(), `size` here is the exact PDU length: a size mismatch is non-conformant. +// Function codes with nothing variable to cross-check are validated by their fixed length alone: the +// single writes (except 0x05's value field, which must be 0x0000 or 0xFF00), mask-write and FIFO, and +// - deliberately - custom/unknown codes and exception responses, so a dispatcher can still route +// them by function code rather than reject them outright. Tests pin this contract. +bool is_server_pdu_standard(const uint8_t *pdu, size_t size); + +// Client counterpart: additionally checks quantity bounds and address-range arithmetic per function code. +// The same acceptance rule applies to custom/unknown function codes. +bool is_client_pdu_standard(const uint8_t *pdu, size_t size); // Remove before 2027.2.0 ESPDEPRECATED("Use server_pdu_payload() on the response PDU instead. Removed in 2027.2.0", "2026.8.0") diff --git a/esphome/components/modbus_controller/modbus_controller.h b/esphome/components/modbus_controller/modbus_controller.h index 5315a4f325..8be2e333ac 100644 --- a/esphome/components/modbus_controller/modbus_controller.h +++ b/esphome/components/modbus_controller/modbus_controller.h @@ -298,6 +298,17 @@ class ModbusController final : public PollingComponent, public modbus::ModbusCli /// queues a modbus command in the send queue void queue_command(const ModbusCommandItem &command); + /// Sends a raw payload (address byte + PDU, no CRC) with responses routed back to this controller. + /// The payload carries its own address byte, which may differ from this controller's address. + /// 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 &payload) { + if (payload.empty()) { + this->on_not_sent(); // match the hub-level send_raw(): a refused send is always signalled + return; + } + this->parent_->send_pdu(payload[0], std::span(payload).subspan(1), this); + } /// Registers a sensor with the controller. Called by esphomes code generator void add_sensor_item(SensorItem *item) { sensorset_.insert(item); } /// called when a modbus response was parsed without errors diff --git a/esphome/components/pzemac/pzemac.cpp b/esphome/components/pzemac/pzemac.cpp index 0f22092f34..233ad6fc53 100644 --- a/esphome/components/pzemac/pzemac.cpp +++ b/esphome/components/pzemac/pzemac.cpp @@ -77,10 +77,8 @@ void PZEMAC::dump_config() { } void PZEMAC::reset_energy_() { - std::vector cmd; - cmd.push_back(this->address_); - cmd.push_back(PZEM_CMD_RESET_ENERGY); - this->send_raw(cmd); + const uint8_t pdu[] = {PZEM_CMD_RESET_ENERGY}; + this->send_pdu(pdu); } } // namespace esphome::pzemac diff --git a/esphome/components/pzemdc/pzemdc.cpp b/esphome/components/pzemdc/pzemdc.cpp index 31d1a7dac1..9de72d51f9 100644 --- a/esphome/components/pzemdc/pzemdc.cpp +++ b/esphome/components/pzemdc/pzemdc.cpp @@ -65,10 +65,8 @@ void PZEMDC::dump_config() { } void PZEMDC::reset_energy() { - std::vector cmd; - cmd.push_back(this->address_); - cmd.push_back(PZEM_CMD_RESET_ENERGY); - this->send_raw(cmd); + const uint8_t pdu[] = {PZEM_CMD_RESET_ENERGY}; + this->send_pdu(pdu); } } // namespace esphome::pzemdc diff --git a/tests/components/modbus/modbus_client_hub_test.cpp b/tests/components/modbus/modbus_client_hub_test.cpp index 4447ca9344..f372af4ab5 100644 --- a/tests/components/modbus/modbus_client_hub_test.cpp +++ b/tests/components/modbus/modbus_client_hub_test.cpp @@ -94,8 +94,9 @@ TEST(ModbusClientHubNoResponse, RetryRequeuesWaitingFrame) { EXPECT_EQ(requeued.device, &device); // address + PDU + CRC ASSERT_EQ(requeued.frame.size(), sizeof(READ_PDU) + 3); - EXPECT_EQ(requeued.frame.data.data()[0], 0x02); - EXPECT_EQ(0, memcmp(requeued.frame.data.data() + 1, READ_PDU, sizeof(READ_PDU))); + EXPECT_EQ(requeued.frame.address(), 0x02); + ASSERT_EQ(requeued.frame.pdu().size(), sizeof(READ_PDU)); + EXPECT_EQ(0, memcmp(requeued.frame.pdu().data(), READ_PDU, sizeof(READ_PDU))); } // A device that declines the retry has the frame dropped. @@ -208,4 +209,15 @@ TEST(ModbusClientHubCompat, LegacyCallbackNamesStillForward) { 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 +// 256-byte limit, so it is refused up front and signalled like any other failed send. +TEST(ModbusClientHub, OversizedPduIsRefusedWithNotSent) { + NoResponseProbeHub hub; + LegacyNameDevice device(&hub, 0x02); + std::vector big(MAX_PDU_SIZE + 1, 0x41); + device.send_pdu(big); + EXPECT_EQ(device.legacy_not_sent_, 1); // on_not_sent, observed via the legacy forward + EXPECT_TRUE(hub.tx_buffer_empty()); +} + } // namespace esphome::modbus::testing diff --git a/tests/components/modbus/modbus_helpers_test.cpp b/tests/components/modbus/modbus_helpers_test.cpp index 0cd6ac6693..71ea3556b8 100644 --- a/tests/components/modbus/modbus_helpers_test.cpp +++ b/tests/components/modbus/modbus_helpers_test.cpp @@ -83,6 +83,13 @@ TEST(ModbusClientFrameLength, WriteMultipleByteCountCapped) { EXPECT_EQ(client_frame_length(frame, sizeof(frame)), 9 + MAX_NUM_OF_REGISTERS_TO_WRITE * 2); } +TEST(ModbusClientFrameLength, ReadWriteMultipleByteCountCappedAtSpecLimit) { + // FC 0x17's write byte count caps at the spec 6.17 limit of 121 registers (242 bytes), deliberately + // tighter than FC 0x10's 123, so a corrupt byte count cannot make the parser wait past the real frame. + const uint8_t pdu[] = {0x17, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0xFF}; // claims 255 bytes + EXPECT_EQ(client_pdu_length(pdu, sizeof(pdu)), 10 + MAX_NUM_OF_REGISTERS_TO_WRITE_RW * 2); +} + TEST(ModbusClientFrameLength, WriteMultipleMissingByteCount) { const uint8_t frame[] = {0x01, 0x10, 0x00, 0x00, 0x00, 0x02}; EXPECT_EQ(client_frame_length(frame, sizeof(frame)), 9); @@ -97,6 +104,124 @@ TEST(ModbusClientFrameLength, MiscFixedAndUnknown) { EXPECT_EQ(client_frame_length(unknown, sizeof(unknown)), MIN_FRAME_SIZE); } +// --- file-record length cap -------------------------------------------------- +// FC 0x14/0x15 are parsed only to keep the frame parser in sync; the byte count caps at 251 +// (MAX_PDU_SIZE - 2), reproducing the released frame-relative bound of MAX_FRAME_SIZE - 5. + +TEST(ModbusFileRecordCap, PduLengthCapsByteCountAt251) { + const uint8_t pdu[] = {static_cast(FC::READ_FILE_RECORD), 0xFF}; // claims 255 bytes + EXPECT_EQ(server_pdu_length(pdu, sizeof(pdu)), 2 + (MAX_PDU_SIZE - 2)); + EXPECT_EQ(client_pdu_length(pdu, sizeof(pdu)), 2 + (MAX_PDU_SIZE - 2)); + // Frame wrappers: address(1) + PDU + CRC(2) stays within the RTU 256-byte frame limit. + const uint8_t frame[] = {0x01, static_cast(FC::WRITE_FILE_RECORD), 0xFF}; + EXPECT_EQ(server_frame_length(frame, sizeof(frame)), MAX_FRAME_SIZE); + EXPECT_EQ(client_frame_length(frame, sizeof(frame)), MAX_FRAME_SIZE); +} + +TEST(ModbusFileRecordCap, StandardChecksAcceptUpTo251) { + // A full-length PDU at the cap: function(1) + byte count(1) + 251 data bytes = MAX_PDU_SIZE. + std::vector at_cap(MAX_PDU_SIZE, 0x00); + at_cap[0] = static_cast(FC::READ_FILE_RECORD); + at_cap[1] = MAX_PDU_SIZE - 2; + EXPECT_TRUE(is_server_pdu_standard(at_cap.data(), at_cap.size())); + EXPECT_TRUE(is_client_pdu_standard(at_cap.data(), at_cap.size())); + // Byte count 252 in the same 253-byte buffer: the parsed length still matches (capped), so this + // exercises the byte-count bound itself rather than the length identity. + at_cap[1] = MAX_PDU_SIZE - 1; + EXPECT_FALSE(is_server_pdu_standard(at_cap.data(), at_cap.size())); + EXPECT_FALSE(is_client_pdu_standard(at_cap.data(), at_cap.size())); +} + +// --- is_client_pdu_standard / is_server_pdu_standard ------------------------- +// The gatekeepers for the typed client dispatch: a PDU must be exactly its function code's standard +// shape, with byte count, quantity, and address range all consistent. + +TEST(ModbusPduStandard, ClientReadAndWriteConformant) { + const uint8_t read_regs[] = {0x03, 0x01, 0x00, 0x00, 0x02}; + EXPECT_TRUE(is_client_pdu_standard(read_regs, sizeof(read_regs))); + const uint8_t write_regs[] = {0x10, 0x00, 0x20, 0x00, 0x02, 0x04, 0x00, 0x01, 0x00, 0x02}; + EXPECT_TRUE(is_client_pdu_standard(write_regs, sizeof(write_regs))); + // 10 coils pack into 2 data bytes - the coil formula, not the register one. + const uint8_t write_coils[] = {0x0F, 0x00, 0x30, 0x00, 0x0A, 0x02, 0xFF, 0x03}; + EXPECT_TRUE(is_client_pdu_standard(write_coils, sizeof(write_coils))); +} + +TEST(ModbusPduStandard, ClientRejectsNonConformant) { + // Truncated: header claims 4 data bytes, only 2 present. + const uint8_t truncated[] = {0x10, 0x00, 0x20, 0x00, 0x02, 0x04, 0x00, 0x01}; + EXPECT_FALSE(is_client_pdu_standard(truncated, sizeof(truncated))); + // Byte count disagrees with quantity (2 registers need 4 bytes, header says 2). + const uint8_t inconsistent[] = {0x10, 0x00, 0x20, 0x00, 0x02, 0x02, 0x00, 0x01}; + EXPECT_FALSE(is_client_pdu_standard(inconsistent, sizeof(inconsistent))); + // Coil write using the register byte-count formula (10 coils with 20 data bytes). + const uint8_t coil_as_regs[] = {0x0F, 0x00, 0x30, 0x00, 0x0A, 0x14, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; + EXPECT_FALSE(is_client_pdu_standard(coil_as_regs, sizeof(coil_as_regs))); + // Quantity zero and quantity beyond the per-function-code maximum. + const uint8_t zero_qty[] = {0x03, 0x01, 0x00, 0x00, 0x00}; + EXPECT_FALSE(is_client_pdu_standard(zero_qty, sizeof(zero_qty))); + const uint8_t too_many[] = {0x03, 0x01, 0x00, 0x00, 0x7E}; // 126 > 125 + EXPECT_FALSE(is_client_pdu_standard(too_many, sizeof(too_many))); + // Address range overflow: 0xFFFF + 2 registers exceeds the 16-bit register space. + const uint8_t wraps[] = {0x03, 0xFF, 0xFF, 0x00, 0x02}; + EXPECT_FALSE(is_client_pdu_standard(wraps, sizeof(wraps))); +} + +TEST(ModbusPduStandard, ServerReadResponses) { + const uint8_t ok[] = {0x03, 0x04, 0x00, 0x2A, 0x01, 0x00}; + EXPECT_TRUE(is_server_pdu_standard(ok, sizeof(ok))); + // Byte-count header disagrees with the actual length. + const uint8_t lying[] = {0x03, 0x06, 0x00, 0x2A, 0x01, 0x00}; + EXPECT_FALSE(is_server_pdu_standard(lying, sizeof(lying))); + // An empty PDU (the on_error path) is not a standard response. + EXPECT_FALSE(is_server_pdu_standard(ok, 0)); +} + +TEST(ModbusPduStandard, ServerResponsesRejectDegenerateShapes) { + // A read response always carries data: byte count zero is non-conformant. + const uint8_t zero_bc[] = {0x03, 0x00}; + EXPECT_FALSE(is_server_pdu_standard(zero_bc, sizeof(zero_bc))); + // Registers are 2 bytes each: an odd byte count would silently truncate a register. + const uint8_t odd_bc[] = {0x03, 0x03, 0x00, 0x01, 0x02}; + EXPECT_FALSE(is_server_pdu_standard(odd_bc, sizeof(odd_bc))); + // Bit reads have no parity requirement: one packed byte is a fine coil response. + const uint8_t coil_one_byte[] = {0x01, 0x01, 0x05}; + EXPECT_TRUE(is_server_pdu_standard(coil_one_byte, sizeof(coil_one_byte))); + // A write-multiple echo claiming 65535 registers written is bounded like the request side. + const uint8_t wild_echo[] = {0x10, 0x00, 0x00, 0xFF, 0xFF}; + EXPECT_FALSE(is_server_pdu_standard(wild_echo, sizeof(wild_echo))); + const uint8_t ok_echo[] = {0x10, 0x00, 0x00, 0x00, 0x02}; + EXPECT_TRUE(is_server_pdu_standard(ok_echo, sizeof(ok_echo))); +} + +TEST(ModbusPduStandard, SingleCoilValueMustBeCanonical) { + // FC 0x05's value field allows exactly 0xFF00 (ON) and 0x0000 (OFF); anything else is non-standard. + const uint8_t on[] = {0x05, 0x00, 0x10, 0xFF, 0x00}; + const uint8_t off[] = {0x05, 0x00, 0x10, 0x00, 0x00}; + const uint8_t junk[] = {0x05, 0x00, 0x10, 0x12, 0x34}; + EXPECT_TRUE(is_client_pdu_standard(on, sizeof(on))); + EXPECT_TRUE(is_client_pdu_standard(off, sizeof(off))); + EXPECT_FALSE(is_client_pdu_standard(junk, sizeof(junk))); + EXPECT_TRUE(is_server_pdu_standard(on, sizeof(on))); // the response echoes the request + EXPECT_FALSE(is_server_pdu_standard(junk, sizeof(junk))); +} + +TEST(ModbusPduStandard, NonStandardFunctionCodesAcceptedOnLengthAlone) { + // Custom, unimplemented, and exception function codes have no standard shape to check: they are + // accepted whenever the parsed length matches, so a dispatcher can still route them by function + // code instead of having them rejected outright. This is the documented contract - see the header. + const uint8_t custom[] = {0x42}; // user-defined space; 1 byte matches the MIN_PDU_SIZE fallback + EXPECT_TRUE(is_client_pdu_standard(custom, sizeof(custom))); + EXPECT_TRUE(is_server_pdu_standard(custom, sizeof(custom))); + const uint8_t unimplemented[] = {0x07}; // READ_EXCEPTION_STATUS + EXPECT_TRUE(is_server_pdu_standard(unimplemented, sizeof(unimplemented))); + const uint8_t exception[] = {0x83, 0x02}; // exception response; length pinned to 2 bytes + EXPECT_TRUE(is_server_pdu_standard(exception, sizeof(exception))); + // The length identity still gates: extra bytes beyond the parsed fallback are non-conformant. + const uint8_t custom_long[] = {0x42, 0x01}; + EXPECT_FALSE(is_client_pdu_standard(custom_long, sizeof(custom_long))); +} + // --- create_client_pdu ----------------------------------------------------- // PDU = function code + data (no address, no CRC).