diff --git a/esphome/components/modbus/modbus_definitions.h b/esphome/components/modbus/modbus_definitions.h index f1e6a1f57e..09f430f703 100644 --- a/esphome/components/modbus/modbus_definitions.h +++ b/esphome/components/modbus/modbus_definitions.h @@ -1,5 +1,9 @@ #pragma once +#include +#include +#include + #include "esphome/core/component.h" #include "esphome/core/helpers.h" @@ -107,6 +111,62 @@ 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 +// A read request PDU is always function code(1) + start address(2) + quantity(2) +static constexpr uint16_t READ_PDU_SIZE = 5; +// A single-write PDU is always function code(1) + address(2) + value(2) +static constexpr uint16_t WRITE_SINGLE_PDU_SIZE = 5; static constexpr uint16_t MAX_FRAME_SIZE = 256; +/** Read-only view of Modbus-packed bits: bit 0 of byte 0 is the first bit (LSB first), the layout + * coil/discrete-input values use on the wire. Bundles the bit count with the packed bytes so the + * two cannot desynchronize. The view does not own the bytes - it is only valid while they are. + * Reads (operator[]) are unchecked by design - the caller owns the bit < size() precondition, as + * with any subscript. Writes and forwarding are defensive: set() drops out-of-range bits and + * bytes() clamps to the real span, because those paths touch buffers and the wire directly. + */ +class PackedBits { + public: + PackedBits(std::span data, uint16_t count) : data_(data), count_(count) {} + /// Value of the given bit; bit must be < size(). + bool operator[](size_t bit) const { return (this->data_[bit / 8] & (1 << (bit % 8))) != 0; } + /// Number of bits in the view. + uint16_t size() const { return this->count_; } + /// The underlying packed bytes: exactly ceil(size() / 8) bytes, even when the view was constructed + /// over a larger buffer - forwarding this span onto the wire can never leak trailing buffer content. + /// Clamped to the actual span so a view over a too-short buffer stays detectable instead of UB. + std::span bytes() const { + return this->data_.first(std::min((this->count_ + 7) / 8, this->data_.size())); + } + + private: + std::span data_; // must cover ceil(count_ / 8) bytes + uint16_t count_; +}; + +/** Mutable counterpart of PackedBits: set() writes bits in place (deliberately no proxy operator[]=). + * Converts implicitly to PackedBits for read access. + */ +class MutablePackedBits { + public: + MutablePackedBits(std::span data, uint16_t count) : data_(data), count_(count) {} + bool operator[](size_t bit) const { return (this->data_[bit / 8] & (1 << (bit % 8))) != 0; } + /// Set or clear the given bit. Out-of-range bits are dropped: on the server read path the span wraps a + /// stack response buffer, so a handler looping past size() must not be able to smash the frame. + void set(size_t bit, bool value) { + if (bit >= this->count_ || bit / 8 >= this->data_.size()) + return; + if (value) { + this->data_[bit / 8] |= (1 << (bit % 8)); + } else { + this->data_[bit / 8] &= ~(1 << (bit % 8)); + } + } + uint16_t size() const { return this->count_; } + operator PackedBits() const { return PackedBits(this->data_, this->count_); } + + private: + std::span data_; // must cover ceil(count_ / 8) bytes + uint16_t count_; +}; + /// End of Modbus definitions } // namespace esphome::modbus diff --git a/esphome/components/modbus/modbus_helpers.cpp b/esphome/components/modbus/modbus_helpers.cpp index 900c9e3093..96561cf56f 100644 --- a/esphome/components/modbus/modbus_helpers.cpp +++ b/esphome/components/modbus/modbus_helpers.cpp @@ -280,27 +280,28 @@ std::optional registers_to_number(const uint16_t *registers, size_t cou return payload_to_number(bytes, required_size, sensor_value_type, 0, 0xFFFFFFFF); } -StaticVector create_client_pdu(FunctionCode function_code, uint16_t start_address, - uint16_t number_of_entities, const uint8_t *values, - size_t values_len) { - if (is_function_code_read(static_cast(function_code))) { - if (values != nullptr || values_len > 0) { - ESP_LOGW(TAG, "Values provided for read function code %02X, but will be ignored", - static_cast(function_code)); - } - } else if (is_function_code_write(static_cast(function_code))) { - if (values == nullptr || values_len == 0) { - ESP_LOGE(TAG, "No values provided for write function code %02X", static_cast(function_code)); - return {}; - } - } else { - ESP_LOGE(TAG, "Unsupported function code %02X for client PDU creation", static_cast(function_code)); - return {}; - } +// Every request PDU opens with the same 5-byte layout: function code, then two big-endian 16-bit +// fields (start address + quantity for reads and multi-writes, address + value for single writes). +template +static void append_pdu_header(StaticVector &pdu, FunctionCode function_code, uint16_t first, + uint16_t second) { + pdu.push_back(static_cast(function_code)); + pdu.push_back(first >> 8); + pdu.push_back(first >> 0); + pdu.push_back(second >> 8); + pdu.push_back(second >> 0); +} +ReadPdu create_read_pdu(FunctionCode function_code, uint16_t start_address, uint16_t number_of_entities) { + ReadPdu pdu; // declared before every return so NRVO fires (all paths return the same object) if (number_of_entities == 0) { ESP_LOGE(TAG, "Number of entities is zero for function code %02X", static_cast(function_code)); - return {}; + return pdu; + } + if (uint32_t(start_address) + number_of_entities > 0x10000u) { + ESP_LOGE(TAG, "Read of %u entities at %u runs past the 16-bit address space, dropping request", number_of_entities, + start_address); + return pdu; } switch (function_code) { @@ -308,14 +309,14 @@ StaticVector create_client_pdu(FunctionCode function_code if (number_of_entities > MAX_NUM_OF_COILS_TO_READ) { ESP_LOGE(TAG, "number_of_entities %u exceeds maximum coils to read %u for function code %02X", number_of_entities, MAX_NUM_OF_COILS_TO_READ, static_cast(function_code)); - return {}; + return pdu; } break; case FunctionCode::READ_DISCRETE_INPUTS: if (number_of_entities > MAX_NUM_OF_DISCRETE_INPUTS_TO_READ) { ESP_LOGE(TAG, "number_of_entities %u exceeds maximum discrete inputs to read %u for function code %02X", number_of_entities, MAX_NUM_OF_DISCRETE_INPUTS_TO_READ, static_cast(function_code)); - return {}; + return pdu; } break; case FunctionCode::READ_HOLDING_REGISTERS: @@ -323,57 +324,201 @@ StaticVector create_client_pdu(FunctionCode function_code if (number_of_entities > MAX_NUM_OF_REGISTERS_TO_READ) { ESP_LOGE(TAG, "number_of_entities %u exceeds maximum registers to read %u for function code %02X", number_of_entities, MAX_NUM_OF_REGISTERS_TO_READ, static_cast(function_code)); - return {}; - } - break; - case FunctionCode::WRITE_SINGLE_COIL: - case FunctionCode::WRITE_SINGLE_REGISTER: - break; // number_of_entities is ignored for single write, so no need to validate - case FunctionCode::WRITE_MULTIPLE_COILS: - case FunctionCode::WRITE_MULTIPLE_REGISTERS: - if (number_of_entities > MAX_NUM_OF_REGISTERS_TO_WRITE) { - ESP_LOGE(TAG, "number_of_entities %u exceeds maximum registers to write %u for function code %02X", - number_of_entities, MAX_NUM_OF_REGISTERS_TO_WRITE, static_cast(function_code)); - return {}; + return pdu; } break; default: - ESP_LOGE(TAG, "Unsupported function code %u for client PDU creation", static_cast(function_code)); - return {}; + ESP_LOGE(TAG, "Unsupported function code %02X for read PDU creation", static_cast(function_code)); + return pdu; } - StaticVector pdu; - pdu.push_back(static_cast(function_code)); - pdu.push_back(start_address >> 8); - pdu.push_back(start_address >> 0); - if (function_code != FunctionCode::WRITE_SINGLE_COIL && function_code != FunctionCode::WRITE_SINGLE_REGISTER) { - pdu.push_back(number_of_entities >> 8); - pdu.push_back(number_of_entities >> 0); - } + append_pdu_header(pdu, function_code, start_address, number_of_entities); + return pdu; +} - if (is_function_code_write(static_cast(function_code))) { - if (function_code == FunctionCode::WRITE_MULTIPLE_COILS || - function_code == FunctionCode::WRITE_MULTIPLE_REGISTERS) { - // 6 bytes of overhead (fc + start_addr×2 + qty×2 + byte_count) leave MAX_PDU_SIZE-6 bytes for values - static constexpr size_t MAX_WRITE_MULTIPLE_VALUES_LEN = MAX_PDU_SIZE - 6; - if (values_len > MAX_WRITE_MULTIPLE_VALUES_LEN) { - ESP_LOGE(TAG, "values_len %zu exceeds PDU capacity %zu, dropping request", values_len, - MAX_WRITE_MULTIPLE_VALUES_LEN); - return {}; - } - pdu.push_back(values_len); // Byte count is required for write multiple - for (size_t i = 0; i < values_len; i++) - pdu.push_back(values[i]); - } else { - // Write single register or coil (2 bytes) - if (values_len < 2) { - ESP_LOGE(TAG, "values_len %zu too small for write-single command (need 2), dropping request", values_len); - return {}; - } - pdu.push_back(values[0]); - pdu.push_back(values[1]); +PduBuffer create_client_pdu(FunctionCode function_code, uint16_t start_address, uint16_t number_of_entities, + const uint8_t *values, size_t values_len) { + PduBuffer pdu; // declared before every return so NRVO fires (all paths return the same object) + // Generic entry point; prefer the direction- and type-specific builders (create_read_pdu(), + // create_write_registers_pdu(), etc.) which bound their inputs per spec. + if (is_function_code_read(static_cast(function_code))) { + if (values != nullptr || values_len > 0) { + ESP_LOGW(TAG, "Values provided for read function code %02X, but will be ignored", + static_cast(function_code)); } + auto read_pdu = create_read_pdu(function_code, start_address, number_of_entities); + pdu.assign(read_pdu.begin(), read_pdu.end()); + return pdu; + } + // Exact codes only: is_function_code_write() masks the exception bit, which would let the + // exception-flagged forms (0x85/0x86/0x8F/0x90) build a request announcing itself as an exception. + const bool is_single = + function_code == FunctionCode::WRITE_SINGLE_COIL || function_code == FunctionCode::WRITE_SINGLE_REGISTER; + const bool is_multi = + function_code == FunctionCode::WRITE_MULTIPLE_COILS || function_code == FunctionCode::WRITE_MULTIPLE_REGISTERS; + if (!is_single && !is_multi) { + ESP_LOGE(TAG, "Unsupported function code %02X for client PDU creation", static_cast(function_code)); + return pdu; + } + + // Generic write builder: raw caller-supplied bytes, so we can only guard against the PDU byte capacity here. + if (values == nullptr || values_len == 0) { + ESP_LOGE(TAG, "No values provided for write function code %02X", static_cast(function_code)); + return pdu; + } + if (number_of_entities == 0) { + ESP_LOGE(TAG, "Number of entities is zero for function code %02X", static_cast(function_code)); + return pdu; + } + // number_of_entities is ignored for single write, so only validate it for the multiple variants. + // The bound is per function code (coils pack 8 per byte, so their quantity limit is far higher) - + // the same limits is_client_pdu_standard() accepts, so builder and validator agree. + const uint16_t max_entities = + function_code == FunctionCode::WRITE_MULTIPLE_COILS ? MAX_NUM_OF_COILS_TO_WRITE : MAX_NUM_OF_REGISTERS_TO_WRITE; + if (!is_single && number_of_entities > max_entities) { + ESP_LOGE(TAG, "number_of_entities %u exceeds maximum %u for function code %02X", number_of_entities, max_entities, + static_cast(function_code)); + return pdu; + } + if (!is_single && uint32_t(start_address) + number_of_entities > 0x10000u) { + ESP_LOGE(TAG, "Write of %u entities at %u runs past the 16-bit address space, dropping request", number_of_entities, + start_address); + return pdu; + } + + if (is_single) { + // Write single register or coil: the two value bytes are the header's second field. + if (values_len < 2) { + ESP_LOGE(TAG, "values_len %zu too small for write-single command (need 2), dropping request", values_len); + return pdu; + } + // The spec allows exactly ON (0xFF00) and OFF (0x0000) for a single-coil write - the same rule + // is_client_pdu_standard() enforces, so a built frame cannot be misclassified on reply. + if (function_code == FunctionCode::WRITE_SINGLE_COIL && + ((values[0] != 0xFF && values[0] != 0x00) || values[1] != 0x00)) { + ESP_LOGE(TAG, "Invalid single-coil value %02X%02X (must be FF00 or 0000), dropping request", values[0], + values[1]); + return pdu; + } + append_pdu_header(pdu, function_code, start_address, uint16_t((values[0] << 8) | values[1])); + return pdu; + } + // The quantity is spec-bounded above, so the data length just has to agree with it exactly + // (registers are 2 bytes each, coils pack 8 per byte). This is the same consistency the response + // dispatch enforces via is_client_pdu_standard(), so a frame built here can never be classified + // non-standard on reply, and the spec bound keeps the PDU within capacity by construction. + // Checked before the header append: a failed check must return an empty PDU, not a 5-byte partial one. + const bool bits = function_code == FunctionCode::WRITE_MULTIPLE_COILS; + const size_t expected_len = + bits ? (static_cast(number_of_entities) + 7) / 8 : static_cast(number_of_entities) * 2; + if (values_len != expected_len) { + ESP_LOGE(TAG, "values_len %zu does not match %u entities (expected %zu) for function code %02X, dropping request", + values_len, number_of_entities, expected_len, static_cast(function_code)); + return pdu; + } + append_pdu_header(pdu, function_code, start_address, number_of_entities); + pdu.push_back(values_len); // Byte count is required for write multiple + for (size_t i = 0; i < values_len; i++) + pdu.push_back(values[i]); + return pdu; +} + +PduBuffer create_write_registers_pdu(uint16_t start_address, std::span values) { + PduBuffer pdu; // declared before every return so NRVO fires (all paths return the same object) + if (values.empty()) { + ESP_LOGE(TAG, "No values provided for write multiple registers, dropping request"); + return pdu; + } + // Byte count is registers × 2 (per spec); bounding the register count keeps the PDU within MAX_PDU_SIZE. + if (values.size() > MAX_NUM_OF_REGISTERS_TO_WRITE) { + ESP_LOGE(TAG, "values.size() %zu exceeds maximum registers to write %u, dropping request", values.size(), + MAX_NUM_OF_REGISTERS_TO_WRITE); + return pdu; + } + if (uint32_t(start_address) + values.size() > 0x10000u) { + ESP_LOGE(TAG, "Write of %zu registers at %u runs past the 16-bit address space, dropping request", values.size(), + start_address); + return pdu; + } + append_pdu_header(pdu, FunctionCode::WRITE_MULTIPLE_REGISTERS, start_address, values.size()); + pdu.push_back(static_cast(values.size() * 2)); // byte count + for (auto v : values) { + auto decoded_value = decode_value(v); + pdu.push_back(decoded_value[0]); + pdu.push_back(decoded_value[1]); } return pdu; } + +WriteSinglePdu create_write_single_register_pdu(uint16_t start_address, uint16_t value) { + WriteSinglePdu pdu; + append_pdu_header(pdu, FunctionCode::WRITE_SINGLE_REGISTER, start_address, value); + return pdu; +} + +WriteSinglePdu create_write_single_coil_pdu(uint16_t address, bool value) { + WriteSinglePdu pdu; + append_pdu_header(pdu, FunctionCode::WRITE_SINGLE_COIL, address, value ? 0xFF00 : 0x0000); + return pdu; +} + +// Shared core for the two coil-write overloads: validates, then builds into the caller's named +// pdu (left empty on failure). Each overload's returns all name one local, so NRVO fires. +static void build_write_coils_pdu(PduBuffer &pdu, uint16_t start_address, PackedBits bits) { + const uint16_t count = bits.size(); + const std::span packed_bits = bits.bytes(); + if (count == 0) { + ESP_LOGE(TAG, "No coils requested for write multiple coils, dropping request"); + return; + } + if (count > MAX_NUM_OF_COILS_TO_WRITE) { + ESP_LOGE(TAG, "count %u exceeds maximum coils to write %u, dropping request", count, MAX_NUM_OF_COILS_TO_WRITE); + return; + } + if (uint32_t(start_address) + count > 0x10000u) { + ESP_LOGE(TAG, "Write of %u coils at %u runs past the 16-bit address space, dropping request", count, start_address); + return; + } + const size_t byte_count = (count + 7) / 8; + if (packed_bits.size() < byte_count) { + ESP_LOGE(TAG, "packed_bits (%zu bytes) does not cover %u coils (%zu bytes), dropping request", packed_bits.size(), + count, byte_count); + return; + } + append_pdu_header(pdu, FunctionCode::WRITE_MULTIPLE_COILS, start_address, count); + pdu.push_back(static_cast(byte_count)); + for (size_t i = 0; i != byte_count; i++) { + pdu.push_back(packed_bits[i]); + } + // Zero the unused bits of the final byte, as the spec requires + if (count % 8 != 0) { + pdu[pdu.size() - 1] &= static_cast((1 << (count % 8)) - 1); + } +} + +PduBuffer create_write_coils_pdu(uint16_t start_address, PackedBits bits) { + PduBuffer pdu; + build_write_coils_pdu(pdu, start_address, bits); + return pdu; +} + +PduBuffer create_write_coils_pdu(uint16_t start_address, std::span values) { + PduBuffer pdu; // declared before every return so NRVO fires (all paths return the same object) + // Bound before packing so the transient buffer below cannot overflow; the shared core validates the rest. + if (values.size() > MAX_NUM_OF_COILS_TO_WRITE) { + ESP_LOGE(TAG, "values.size() %zu exceeds maximum coils to write %u, dropping request", values.size(), + MAX_NUM_OF_COILS_TO_WRITE); + return pdu; + } + StaticVector packed; + for (size_t i = 0; i != values.size(); i++) { + if (i % 8 == 0) + packed.push_back(0); + if (values[i]) + packed[i / 8] |= (1 << (i % 8)); + } + build_write_coils_pdu(pdu, start_address, + PackedBits(std::span(packed.data(), packed.size()), values.size())); + return pdu; +} } // namespace esphome::modbus::helpers diff --git a/esphome/components/modbus/modbus_helpers.h b/esphome/components/modbus/modbus_helpers.h index 44c1a01211..3dd933c4d7 100644 --- a/esphome/components/modbus/modbus_helpers.h +++ b/esphome/components/modbus/modbus_helpers.h @@ -350,7 +350,24 @@ inline int64_t payload_to_number(const std::vector &data, SensorValueTy */ std::optional registers_to_number(const uint16_t *registers, size_t count, SensorValueType sensor_value_type); -/** Create a modbus clinet pdu for reading/writing single/multiple coils/register/inputs. +// Named PDU buffer types: the builders' storage strategy (currently stack-allocated StaticVector, +// right-sized per shape) can be swapped in one place without touching every signature. +using PduBuffer = StaticVector; +using ReadPdu = StaticVector; +using WriteSinglePdu = StaticVector; + +/** Create a modbus read request PDU. + * @param function_code one of READ_COILS, READ_DISCRETE_INPUTS, READ_HOLDING_REGISTERS, READ_INPUT_REGISTERS + * @param start_address coil/register/input starting address + * @param number_of_entities number of coils/registers/inputs to read + * @return PDU (function code + data, no address, no CRC); empty on invalid input + */ +ReadPdu create_read_pdu(FunctionCode function_code, uint16_t start_address, uint16_t number_of_entities); + +/** Create a modbus client pdu for reading/writing single/multiple coils/register/inputs. + * Generic entry point; prefer the direction- and type-specific builders (create_read_pdu(), + * create_write_registers_pdu(), create_write_single_register_pdu(), create_write_coils_pdu(), + * create_write_single_coil_pdu()) which bound their inputs per spec. * @param function_code the modbus function code to use. One of: * READ_COILS * READ_DISCRETE_INPUTS @@ -366,11 +383,59 @@ std::optional registers_to_number(const uint16_t *registers, size_t cou * @param values_len length of values array * @return PDU (function code + data, no address, no CRC) */ -StaticVector create_client_pdu(FunctionCode function_code, uint16_t start_address, - uint16_t number_of_entities, const uint8_t *values = nullptr, - size_t values_len = 0); +PduBuffer create_client_pdu(FunctionCode function_code, uint16_t start_address, uint16_t number_of_entities, + const uint8_t *values = nullptr, size_t values_len = 0); -inline std::vector float_to_payload(float value, SensorValueType value_type) { +/** Create modbus write multiple registers command + * Function 0x10 Write Multiple Registers + * @param start_address modbus address of the first register to write + * @param values register values to write; the register count is values.size() (at most + * MAX_NUM_OF_REGISTERS_TO_WRITE, an over-long set is rejected and an empty PDU is returned). + * Any contiguous uint16_t container converts (std::vector, std::array). + * @return PDU (function code + data, no address, no CRC) + */ +PduBuffer create_write_registers_pdu(uint16_t start_address, std::span values); + +/** Create modbus write single register command + * Function 0x06 Write Single Register + * @param start_address modbus address of the register to write + * @param value uint16_t value to write + * @return PDU (function code + data, no address, no CRC) + */ +WriteSinglePdu create_write_single_register_pdu(uint16_t start_address, uint16_t value); + +/** Create modbus write single coil command + * Function 0x05 Write Single Coil + * @param address modbus address of the coil to write + * @param value coil value to write + * @return PDU (function code + data, no address, no CRC) + */ +WriteSinglePdu create_write_single_coil_pdu(uint16_t address, bool value); + +/** Create modbus write multiple coils command + * Function 0x0F Write Multiple Coils + * @param start_address modbus address of the first coil to write + * @param values coil values to write; the coil count is values.size() (at most MAX_NUM_OF_COILS_TO_WRITE, an + * over-long set is rejected and an empty PDU is returned). Note std::vector is bit-packed and + * does not convert to a span; pass a std::array or other contiguous bool container. + * @return PDU (function code + data, no address, no CRC) + */ +PduBuffer create_write_coils_pdu(uint16_t start_address, std::span values); + +/** Create modbus write multiple coils command (function 0x0F) from bits packed as on the wire. + * @param start_address modbus address of the first coil to write + * @param bits PackedBits view of the coils to write (at most MAX_NUM_OF_COILS_TO_WRITE); invalid + * input returns an empty PDU + * @return PDU (function code + data, no address, no CRC) + */ +PduBuffer create_write_coils_pdu(uint16_t start_address, PackedBits bits); + +/** Append a float converted to register words to any push_back container (heap-free with StaticVector). + * @param data container the register words are appended to + * @param value value to convert + * @param value_type defines if 16/32/64 bits or FP32 is used + */ +template void float_to_payload(Container &data, float value, SensorValueType value_type) { int64_t val; if (value_type_is_float(value_type)) { @@ -379,8 +444,14 @@ inline std::vector float_to_payload(float value, SensorValueType value val = llroundf(value); } - std::vector data; number_to_payload(data, val, value_type); +} + +// Remove before 2027.2.0 +ESPDEPRECATED("Use the container overload of float_to_payload() instead. Removed in 2027.2.0", "2026.8.0") +inline std::vector float_to_payload(float value, SensorValueType value_type) { + std::vector data; + float_to_payload(data, value, value_type); return data; } diff --git a/esphome/components/modbus_controller/modbus_controller.h b/esphome/components/modbus_controller/modbus_controller.h index 8be2e333ac..9616c1d935 100644 --- a/esphome/components/modbus_controller/modbus_controller.h +++ b/esphome/components/modbus_controller/modbus_controller.h @@ -98,7 +98,9 @@ inline int64_t payload_to_number(const std::vector &data, SensorValueTy ESPDEPRECATED("Use modbus::helpers::float_to_payload() instead. Removed in 2026.10.0", "2026.4.0") inline std::vector float_to_payload(float value, SensorValueType value_type) { - return modbus::helpers::float_to_payload(value, value_type); + std::vector data; + modbus::helpers::float_to_payload(data, value, value_type); + return data; } class ModbusController; diff --git a/esphome/components/modbus_controller/number/modbus_number.cpp b/esphome/components/modbus_controller/number/modbus_number.cpp index fdb770fd96..7b18b9e9fc 100644 --- a/esphome/components/modbus_controller/number/modbus_number.cpp +++ b/esphome/components/modbus_controller/number/modbus_number.cpp @@ -61,7 +61,8 @@ void ModbusNumber::control(float value) { this->parent_->on_write_register_response(write_cmd.register_type, this->start_address, data); }); } else { - data = modbus::helpers::float_to_payload(write_value, this->sensor_value_type); + std::vector payload; + modbus::helpers::float_to_payload(payload, write_value, this->sensor_value_type); ESP_LOGD(TAG, "Updating register: connected Sensor=%s start address=0x%X register count=%d new value=%.02f (val=%.02f)", @@ -71,10 +72,10 @@ void ModbusNumber::control(float value) { if (this->register_count == 1 && !this->use_write_multiple_) { // since offset is in bytes and a register is 16 bits we get the start by adding offset/2 write_cmd = ModbusCommandItem::create_write_single_command(this->parent_, this->start_address + this->offset / 2, - data[0]); + payload[0]); } else { write_cmd = ModbusCommandItem::create_write_multiple_command( - this->parent_, this->start_address + this->offset / 2, this->register_count, data); + this->parent_, this->start_address + this->offset / 2, this->register_count, payload); } // publish new value write_cmd.on_data_func = [this, write_cmd, value](modbus::EntityType register_type, uint16_t start_address, diff --git a/esphome/components/modbus_controller/output/modbus_output.cpp b/esphome/components/modbus_controller/output/modbus_output.cpp index ffe6f3bdfa..95618a7505 100644 --- a/esphome/components/modbus_controller/output/modbus_output.cpp +++ b/esphome/components/modbus_controller/output/modbus_output.cpp @@ -33,12 +33,30 @@ void ModbusFloatOutput::write_state(float value) { } // lambda didn't set payload if (data.empty()) { - data = modbus::helpers::float_to_payload(value, this->sensor_value_type); + modbus::helpers::float_to_payload(data, value, this->sensor_value_type); } ESP_LOGD(TAG, "Updating register: start address=0x%X register count=%d new value=%.02f (val=%.02f)", this->start_address, this->register_count, value, original_value); + // The command declares register_count registers, so the payload must be exactly that many words; + // anything else would put a byte count on the wire that disagrees with the quantity field. + // number_to_payload() appends nothing for RAW, so an empty payload must be caught before data[0]. + if (data.empty()) { + ESP_LOGW(TAG, "No payload was created for updating output"); + return; + } + + // register_count declares the READ range width - it may pull neighboring registers into one poll - + // so a write covers exactly the registers the value occupies: the quantity comes from the payload, + // never from register_count (padding to it would zero registers the user only declared for reading). + // A payload wider than the declared range means the config and the lambda disagree - drop it. + if (data.size() > this->register_count) { + ESP_LOGE(TAG, "Payload has %zu registers but register_count is %u; dropping write", data.size(), + this->register_count); + return; + } + // Create and send the write command ModbusCommandItem write_cmd; if (this->register_count == 1 && !this->use_write_multiple_) { @@ -46,7 +64,7 @@ void ModbusFloatOutput::write_state(float value) { ModbusCommandItem::create_write_single_command(this->parent_, this->start_address + this->offset, data[0]); } else { write_cmd = ModbusCommandItem::create_write_multiple_command(this->parent_, this->start_address + this->offset, - this->register_count, data); + data.size(), data); } this->parent_->queue_command(write_cmd); } diff --git a/esphome/components/modbus_controller/select/modbus_select.cpp b/esphome/components/modbus_controller/select/modbus_select.cpp index c650ca7641..daa6b10da4 100644 --- a/esphome/components/modbus_controller/select/modbus_select.cpp +++ b/esphome/components/modbus_controller/select/modbus_select.cpp @@ -72,13 +72,26 @@ void ModbusSelect::control(size_t index) { return; } + // The command declares register_count registers, so the payload must be exactly that many words: + // a value type narrower than the declared width is zero-padded (the config deliberately allows + // register_count larger than the value type). Anything else would put a byte count on the wire + // that disagrees with the quantity field, which conformant devices reject. + // register_count declares the READ range width - it may pull neighboring registers into one poll - + // so a write covers exactly the registers the value occupies: the quantity comes from the payload, + // never from register_count (padding to it would zero registers the user only declared for reading). + // A payload wider than the declared range means the config and the lambda disagree - drop it. + if (data.size() > this->register_count) { + ESP_LOGE(TAG, "Payload has %zu registers but register_count is %u; dropping write", data.size(), + this->register_count); + return; + } + const uint16_t write_address = this->start_address + this->offset / 2; ModbusCommandItem write_cmd; if ((this->register_count == 1) && (!this->use_write_multiple_)) { write_cmd = ModbusCommandItem::create_write_single_command(this->parent_, write_address, data[0]); } else { - write_cmd = - ModbusCommandItem::create_write_multiple_command(this->parent_, write_address, this->register_count, data); + write_cmd = ModbusCommandItem::create_write_multiple_command(this->parent_, write_address, data.size(), data); } this->parent_->queue_command(write_cmd); diff --git a/tests/components/modbus/modbus_helpers_test.cpp b/tests/components/modbus/modbus_helpers_test.cpp index 71ea3556b8..b471644226 100644 --- a/tests/components/modbus/modbus_helpers_test.cpp +++ b/tests/components/modbus/modbus_helpers_test.cpp @@ -1,5 +1,7 @@ #include +#include + #include "esphome/components/modbus/modbus_helpers.h" namespace esphome::modbus::helpers { @@ -304,6 +306,31 @@ TEST(ModbusCreateClientPdu, WriteMultipleOverEntityLimitReturnsEmpty) { EXPECT_TRUE(pdu.empty()); } +// The generic write path requires the data length to agree exactly with the entity count +// (registers: 2 bytes each; coils: 8 packed per byte) - the same rule the response dispatch +// enforces via is_client_pdu_standard(), so a frame built here always passes that gate. +TEST(ModbusCreateClientPdu, WriteMultipleRejectsMismatchedDataLength) { + const uint8_t values[] = {0x00, 0x0B, 0x00, 0x16}; + // 2 registers need exactly 4 data bytes. + EXPECT_TRUE(create_client_pdu(FC::WRITE_MULTIPLE_REGISTERS, 0x0000, 2, values, 3).empty()); + EXPECT_FALSE(create_client_pdu(FC::WRITE_MULTIPLE_REGISTERS, 0x0000, 2, values, 4).empty()); + // 10 coils pack into exactly 2 data bytes - the coil formula, not the register one. + EXPECT_FALSE(create_client_pdu(FC::WRITE_MULTIPLE_COILS, 0x0000, 10, values, 2).empty()); + EXPECT_TRUE(create_client_pdu(FC::WRITE_MULTIPLE_COILS, 0x0000, 10, values, 4).empty()); +} + +TEST(ModbusCreateClientPdu, WriteCoilsUseTheCoilLimitNotTheRegisterLimit) { + // 200 coils: above the 123-register write limit but well within the 1968-coil limit; 25 data bytes. + std::vector values(25, 0xAA); + auto pdu = create_client_pdu(FC::WRITE_MULTIPLE_COILS, 0x0000, 200, values.data(), values.size()); + ASSERT_FALSE(pdu.empty()); + EXPECT_EQ(pdu[5], 25); // byte count uses the coil formula + EXPECT_TRUE(is_client_pdu_standard(pdu.data(), pdu.size())); // builder output passes the validator + // Builder and validator agree at the top of the range too: 1969 coils rejected. + std::vector big((1969 + 7) / 8, 0x00); + EXPECT_TRUE(create_client_pdu(FC::WRITE_MULTIPLE_COILS, 0x0000, 1969, big.data(), big.size()).empty()); +} + TEST(ModbusHelpersTest, PayloadToNumberRejectsOffsetAtEndOfBuffer) { const std::vector data{0x12, 0x34}; EXPECT_FALSE(payload_to_number(std::span(data), SensorValueType::U_WORD, 2, 0xFFFFFFFF).has_value()); @@ -354,6 +381,159 @@ TEST(ModbusHelpersTest, RegistersToNumberRejectsTruncatedMultiRegisterValue) { EXPECT_FALSE(registers_to_number(registers, 1, SensorValueType::U_DWORD).has_value()); } +// --- typed builders ---------------------------------------------------------- + +TEST(ModbusTypedBuilders, ReadPduWireBytes) { + auto pdu = create_read_pdu(FC::READ_HOLDING_REGISTERS, 0x0102, 3); + const std::vector expected{0x03, 0x01, 0x02, 0x00, 0x03}; + EXPECT_EQ(std::vector(pdu.begin(), pdu.end()), expected); + EXPECT_TRUE(is_client_pdu_standard(pdu.data(), pdu.size())); + // Reads that run past the 16-bit address space are refused. + EXPECT_TRUE(create_read_pdu(FC::READ_HOLDING_REGISTERS, 0xFFFF, 2).empty()); +} + +TEST(ModbusTypedBuilders, WriteSinglePduWireBytes) { + auto reg = create_write_single_register_pdu(0x0010, 0xABCD); + const std::vector expected_reg{0x06, 0x00, 0x10, 0xAB, 0xCD}; + EXPECT_EQ(std::vector(reg.begin(), reg.end()), expected_reg); + EXPECT_TRUE(is_client_pdu_standard(reg.data(), reg.size())); + auto coil_on = create_write_single_coil_pdu(0x0011, true); + auto coil_off = create_write_single_coil_pdu(0x0011, false); + const std::vector expected_on{0x05, 0x00, 0x11, 0xFF, 0x00}; + const std::vector expected_off{0x05, 0x00, 0x11, 0x00, 0x00}; + EXPECT_EQ(std::vector(coil_on.begin(), coil_on.end()), expected_on); + EXPECT_EQ(std::vector(coil_off.begin(), coil_off.end()), expected_off); + EXPECT_TRUE(is_client_pdu_standard(coil_on.data(), coil_on.size())); + EXPECT_TRUE(is_client_pdu_standard(coil_off.data(), coil_off.size())); +} + +TEST(ModbusTypedBuilders, WriteRegistersPduWireBytes) { + const uint16_t values[] = {0x000B, 0x0016}; + auto pdu = create_write_registers_pdu(0x0000, values); + const std::vector expected{0x10, 0x00, 0x00, 0x00, 0x02, 0x04, 0x00, 0x0B, 0x00, 0x16}; + EXPECT_EQ(std::vector(pdu.begin(), pdu.end()), expected); + EXPECT_TRUE(is_client_pdu_standard(pdu.data(), pdu.size())); + // Writes that run past the 16-bit address space are refused. + EXPECT_TRUE(create_write_registers_pdu(0xFFFF, values).empty()); +} + +TEST(ModbusTypedBuilders, WriteRegistersPduRejectsOverLimit) { + std::vector values(MAX_NUM_OF_REGISTERS_TO_WRITE + 1, 0xAAAA); + EXPECT_TRUE(create_write_registers_pdu(0x0000, values).empty()); + values.pop_back(); + EXPECT_FALSE(create_write_registers_pdu(0x0000, values).empty()); +} + +TEST(ModbusTypedBuilders, FloatToPayloadAppendsToExistingContent) { + // The container overload appends - the semantic every migrated caller relies on when a lambda + // has already put words into the buffer. + std::vector data{0x1234}; + float_to_payload(data, 1.0f, SensorValueType::U_WORD); + ASSERT_EQ(data.size(), 2u); + EXPECT_EQ(data[0], 0x1234); + EXPECT_EQ(data[1], 0x0001); +} + +TEST(ModbusCreateClientPdu, ExceptionFlaggedWriteCodesRejected) { + // is_function_code_write() masks the exception bit; the builder must not. + const uint8_t values[] = {0x00, 0x0B, 0x00, 0x16}; + EXPECT_TRUE(create_client_pdu(FunctionCode(0x90), 0x0000, 2, values, 4).empty()); + EXPECT_TRUE(create_client_pdu(FunctionCode(0x85), 0x0000, 1, values, 2).empty()); +} + +TEST(ModbusTypedBuilders, BoolSpanCoilBuilderRejectsOverLimit) { + // This early guard is what keeps the 246-byte packing buffer from overflowing - the shared core's + // identical check runs after packing, so it cannot protect it. + auto big = std::make_unique(MAX_NUM_OF_COILS_TO_WRITE + 1); + EXPECT_TRUE(create_write_coils_pdu(0, std::span(big.get(), MAX_NUM_OF_COILS_TO_WRITE + 1)).empty()); +} + +TEST(ModbusCreateClientPdu, SingleCoilValueValidated) { + const uint8_t on[] = {0xFF, 0x00}; + const uint8_t junk[] = {0x01, 0x00}; + EXPECT_FALSE(create_client_pdu(FC::WRITE_SINGLE_COIL, 0x0003, 1, on, 2).empty()); + EXPECT_TRUE(create_client_pdu(FC::WRITE_SINGLE_COIL, 0x0003, 1, junk, 2).empty()); +} + +// --- create_write_coils_pdu (packed) --------------------------------------- + +TEST(ModbusWriteCoilsPacked, MatchesBoolBuilder) { + const bool coils[] = {true, false, true, true, false, false, true, false, true, true}; + uint8_t packed[] = {0b01001101, 0b00000011}; + auto from_bools = create_write_coils_pdu(0x13, coils); + auto from_packed = create_write_coils_pdu(0x13, PackedBits(packed, 10)); + ASSERT_EQ(from_packed.size(), from_bools.size()); + EXPECT_EQ(0, memcmp(from_packed.data(), from_bools.data(), from_bools.size())); +} + +TEST(ModbusWriteCoilsPacked, MasksUnusedTrailingBits) { + uint8_t packed[] = {0xFF}; + auto pdu = create_write_coils_pdu(0, PackedBits(packed, 3)); + ASSERT_EQ(pdu.size(), 7u); + EXPECT_EQ(pdu[6], 0x07); +} + +TEST(ModbusWriteCoilsPacked, RejectsShortBufferAndZeroCount) { + uint8_t packed[] = {0xFF}; + EXPECT_TRUE(create_write_coils_pdu(0, PackedBits(packed, 9)).empty()); // needs 2 bytes + EXPECT_TRUE(create_write_coils_pdu(0, PackedBits(packed, 0)).empty()); +} + +TEST(ModbusHelpersTest, PackedBitsReadsLsbFirst) { + const uint8_t packed[] = {0x0D, 0x03}; // bits 0,2,3 and 8,9 + PackedBits bits(packed, 11); + EXPECT_EQ(bits.size(), 11u); + EXPECT_TRUE(bits[0]); + EXPECT_FALSE(bits[1]); + EXPECT_TRUE(bits[2]); + EXPECT_TRUE(bits[3]); + EXPECT_FALSE(bits[7]); + EXPECT_TRUE(bits[8]); + EXPECT_TRUE(bits[9]); + EXPECT_FALSE(bits[10]); + EXPECT_EQ(bits.bytes().size(), 2u); +} + +TEST(ModbusHelpersTest, MutablePackedBitsSetsAndClears) { + uint8_t packed[2] = {0x00, 0xFF}; + MutablePackedBits bits(packed, 16); + bits.set(0, true); + bits.set(3, true); + bits.set(9, false); + EXPECT_EQ(packed[0], 0x09); // bits 0 and 3 + EXPECT_EQ(packed[1], 0xFD); // bit 9 (bit 1 of byte 1) cleared +} + +TEST(ModbusHelpersTest, MutablePackedBitsRoundTripAndConversion) { + const bool original[] = {true, true, false, true, false, false, false, false, true, false, true}; + constexpr uint16_t count = sizeof(original); + uint8_t packed[(count + 7) / 8] = {}; + MutablePackedBits out(packed, count); + for (uint16_t i = 0; i != count; i++) + out.set(i, original[i]); + PackedBits view = out; // implicit conversion to the read-only view + ASSERT_EQ(view.size(), count); + for (uint16_t i = 0; i != count; i++) + EXPECT_EQ(view[i], original[i]) << "bit " << i; +} + +TEST(ModbusHelpersTest, PackedBitsViewContractsEnforced) { + uint8_t buf[8] = {}; + PackedBits view(buf, 10); // 10 bits -> 2 bytes, over an 8-byte buffer + EXPECT_EQ(view.bytes().size(), 2u); + + MutablePackedBits bits(std::span(buf, 2), 10); + bits.set(9, true); // in range: lands in byte 1 + bits.set(10, true); // out of range: dropped + bits.set(300, true); // far out of range: dropped, no write past the span + + MutablePackedBits short_bits(std::span(buf, 1), 10); // contract-violating: 10 bits over 1 byte + short_bits.set(9, false); // within count_ but past the span: dropped (would clear bit 9 set above) + EXPECT_EQ(buf[1], 0x02); + for (size_t i = 2; i < sizeof(buf); i++) + EXPECT_EQ(buf[i], 0) << "byte " << i; +} + // server_pdu_payload() must never classify an exception PDU as a read: [fc|0x80, code] is 2 bytes, and a // read-offset of 2 would return an empty span, losing the exception code. The payload of an exception PDU // is the exception code byte, for reads and writes alike.