diff --git a/esphome/components/modbus/__init__.py b/esphome/components/modbus/__init__.py index 377dadad76..58bd0f65dc 100644 --- a/esphome/components/modbus/__init__.py +++ b/esphome/components/modbus/__init__.py @@ -28,6 +28,7 @@ MAX_NUM_OF_DISCRETE_INPUTS_TO_READ = 2000 MAX_NUM_OF_COILS_TO_WRITE = 1968 MAX_NUM_OF_REGISTERS_TO_READ = 125 MAX_NUM_OF_REGISTERS_TO_WRITE = 123 +MAX_NUM_OF_REGISTERS_TO_WRITE_RW = 121 modbus_ns = cg.esphome_ns.namespace("modbus") Modbus = modbus_ns.class_("Modbus", cg.Component, uart.UARTDevice) diff --git a/esphome/components/modbus/modbus.cpp b/esphome/components/modbus/modbus.cpp index 901bfcc52e..cff086aeea 100644 --- a/esphome/components/modbus/modbus.cpp +++ b/esphome/components/modbus/modbus.cpp @@ -382,7 +382,7 @@ ModbusServerDevice *ModbusServerHub::find_device_(uint8_t address) { } ResponseStatus ModbusServerHub::check_address_range_(uint16_t start_address, uint16_t count) { - if ((uint32_t) start_address + count > 0x10000u) { + if (!helpers::address_range_fits(start_address, count)) { ESP_LOGW(TAG, "Address out of range - start: %" PRIu16 " num: %" PRIu16, start_address, count); return ExceptionCode::ILLEGAL_DATA_ADDRESS; } @@ -1056,7 +1056,8 @@ bool ModbusClientHub::queue_pdu(uint8_t address, std::span pdu, M continue; if (device == nullptr) { // A dropped read is routine (DEBUG); a dropped write/custom warns (unobservable without a device). - const bool requeueable = !helpers::is_function_code_exception(pdu[0]) && helpers::is_function_code_read(pdu[0]); + const bool requeueable = + !helpers::is_function_code_exception(pdu[0]) && helpers::is_function_code_read_only(pdu[0]); if (requeueable) { ESP_LOGD(TAG, "Anonymous duplicate of active frame for %" PRIu8 " (function 0x%X), dropped", address, pdu[0]); } else { @@ -1236,7 +1237,14 @@ void ModbusClientDevice::dispatch_response_(std::span request_pdu switch (function_code) { case FunctionCode::READ_HOLDING_REGISTERS: - case FunctionCode::READ_INPUT_REGISTERS: { + case FunctionCode::READ_INPUT_REGISTERS: + // FC 0x17 lands here too: its read start address and read quantity sit at the same request offsets as a + // plain read's (bytes 1..2 and 3..4), so start_address and count_or_value already hold the read block; its + // response carries only that read data, and the write half is confirmed by the response arriving at all. + // An exception routes here as well (the gate only validates the request when status is set), delivering + // empty registers with the error in status - so a 0x17 subclass handles success and failure in the one + // on_read_holding_registers() callback and never needs to also override on_error(). + case FunctionCode::READ_WRITE_MULTIPLE_REGISTERS: { // Decode the big-endian register words into host byte order. The gate guarantees a success response // carries exactly count_or_value registers (and count_or_value <= MAX_NUM_OF_REGISTERS_TO_READ, the // capacity of RegisterValues); a mismatch was diverted to on_custom_response(), never clamped. On @@ -1248,10 +1256,15 @@ void ModbusClientDevice::dispatch_response_(std::span request_pdu } } std::span register_span(registers.data(), registers.size()); - if (function_code == FunctionCode::READ_HOLDING_REGISTERS) { + if (function_code == FunctionCode::READ_INPUT_REGISTERS) { + this->on_read_input_registers(start_address, register_span, status); + } else if (function_code == FunctionCode::READ_HOLDING_REGISTERS || + function_code == FunctionCode::READ_WRITE_MULTIPLE_REGISTERS) { this->on_read_holding_registers(start_address, register_span, status); } else { - this->on_read_input_registers(start_address, register_span, status); + // Unreachable for the current case labels; match explicitly so a function code added to this group + // later is diverted to on_custom_response() rather than silently delivered as a holding read. + this->on_custom_response(request_pdu, response_pdu, status); } break; } diff --git a/esphome/components/modbus/modbus.h b/esphome/components/modbus/modbus.h index 6331f23f99..6bd407a687 100644 --- a/esphome/components/modbus/modbus.h +++ b/esphome/components/modbus/modbus.h @@ -151,9 +151,7 @@ struct ModbusDeviceCommand { static CommandPriority classify(uint8_t function_code) { if (helpers::is_function_code_exception(function_code)) return CommandPriority::READ; - const auto code = static_cast(function_code); - if (helpers::is_function_code_write(function_code) || code == FunctionCode::MASK_WRITE_REGISTER || - code == FunctionCode::READ_WRITE_MULTIPLE_REGISTERS) { + if (helpers::is_function_code_write(function_code)) { return CommandPriority::WRITE; } return CommandPriority::READ; @@ -162,7 +160,7 @@ struct ModbusDeviceCommand { // Requests this entry can serve: a standard read twice (run plus one re-run), everything else once. uint8_t max_pending() const { const uint8_t fc = this->frame.pdu()[0]; - const bool requeueable = !helpers::is_function_code_exception(fc) && helpers::is_function_code_read(fc); + const bool requeueable = !helpers::is_function_code_exception(fc) && helpers::is_function_code_read_only(fc); return (requeueable && !this->continuous) ? 2 : 1; } // Device-scoped clear: detach with no callback (device-less, pending 0). An entry still waiting for @@ -594,6 +592,16 @@ class ModbusClientDevice { bool write_multiple_coils(uint16_t start_address, PackedBits bits) { return this->queue_pdu(helpers::create_write_coils_pdu(start_address, bits)); } + /// FC 0x17: the read-back is delivered through on_read_holding_registers() (the response carries only the + /// read registers, the same wire shape as a holding-register read). A device exception - typically a + /// rejected write half - arrives at that same on_read_holding_registers() with the error in its status, + /// exactly as success does, so a subclass overriding that one callback handles both outcomes and never + /// needs to also override on_error(). + bool read_write_multiple_registers(uint16_t read_start_address, uint16_t read_count, uint16_t write_start_address, + std::span write_values) { + return this->queue_pdu(helpers::create_read_write_multiple_registers_pdu(read_start_address, read_count, + write_start_address, write_values)); + } 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/modbus_helpers.cpp b/esphome/components/modbus/modbus_helpers.cpp index 4287256101..db21b6e6fd 100644 --- a/esphome/components/modbus/modbus_helpers.cpp +++ b/esphome/components/modbus/modbus_helpers.cpp @@ -8,10 +8,11 @@ namespace esphome::modbus::helpers { static const char *const TAG = "modbus_helpers"; // A quantity/address pair is standard when the quantity is non-zero, within the per-table maximum, -// and the range [start_address, start_address + quantity) stays inside the 16-bit address space -// (the 32-bit promotion is the overflow guard - a 16-bit sum could wrap and pass). +// and the range [start_address, start_address + quantity) stays inside the 16-bit address space. +// Non-logging twin of register_block_in_range(): the same three predicates for the parser side, taking a +// uint16_t quantity. register_block_in_range() is the builder-side variant that also logs which half failed. static bool quantity_in_range(uint16_t start_address, uint16_t quantity, uint16_t max_quantity) { - return quantity != 0 && quantity <= max_quantity && uint32_t(start_address) + quantity <= 0x10000u; + return quantity != 0 && quantity <= max_quantity && address_range_fits(start_address, quantity); } // The spec allows exactly ON (0xFF00) and OFF (0x0000) for a single-coil value, on the request and @@ -307,16 +308,20 @@ std::optional registers_to_number(const uint16_t *registers, size_t cou return payload_to_number(bytes, required_size, sensor_value_type, 0, 0xFFFFFFFF); } +// Append a 16-bit value to a PDU in big-endian (wire) byte order. +template static void append_pdu_word(StaticVector &pdu, uint16_t value) { + pdu.push_back(value >> 8); + pdu.push_back(value >> 0); +} + // 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); + append_pdu_word(pdu, first); + append_pdu_word(pdu, second); } // Zero the unused bits of a multi-coil write's final data byte, as the spec requires. Kept in one @@ -335,7 +340,7 @@ ReadPdu create_read_pdu(FunctionCode function_code, uint16_t start_address, uint ESP_LOGE(TAG, "Number of entities is zero for function code %02X", static_cast(function_code)); return pdu; } - if (uint32_t(start_address) + number_of_entities > 0x10000u) { + if (!address_range_fits(start_address, number_of_entities)) { 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; @@ -378,7 +383,7 @@ PduBuffer create_client_pdu(FunctionCode function_code, uint16_t start_address, 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 (is_function_code_read_only(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)); @@ -417,7 +422,7 @@ PduBuffer create_client_pdu(FunctionCode function_code, uint16_t start_address, static_cast(function_code)); return pdu; } - if (!is_single && uint32_t(start_address) + number_of_entities > 0x10000u) { + if (!is_single && !address_range_fits(start_address, number_of_entities)) { 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; @@ -460,29 +465,59 @@ PduBuffer create_client_pdu(FunctionCode function_code, uint16_t start_address, return pdu; } +// Validate one register block for a client builder: a non-zero quantity within max_quantity that does not +// run past the 16-bit address space (register count × 2 stays within MAX_PDU_SIZE as a result). On failure +// it logs the reason and returns false, on which the caller returns an empty PDU. `role` names the block in +// the log ("Read"/"Write"). Logging twin of quantity_in_range(): the same three predicates, split so each +// failure names its reason, and taking size_t so an oversize span is caught before any narrowing. +static bool register_block_in_range(const LogString *role, uint16_t start_address, size_t quantity, + uint16_t max_quantity) { + if (quantity == 0 || quantity > max_quantity) { + ESP_LOGE(TAG, "%s count %zu out of range [1, %u], dropping request", LOG_STR_ARG(role), quantity, max_quantity); + return false; + } + if (!address_range_fits(start_address, quantity)) { + ESP_LOGE(TAG, "%s of %zu registers at %u runs past the 16-bit address space, dropping request", LOG_STR_ARG(role), + quantity, start_address); + return false; + } + return true; +} + 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); + if (!register_block_in_range(LOG_STR("Write"), start_address, values.size(), MAX_NUM_OF_REGISTERS_TO_WRITE)) { 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]); + append_pdu_word(pdu, v); + } + return pdu; +} + +PduBuffer create_read_write_multiple_registers_pdu(uint16_t read_start_address, uint16_t read_count, + uint16_t write_start_address, + std::span write_values) { + PduBuffer pdu; + if (!register_block_in_range(LOG_STR("Read"), read_start_address, read_count, MAX_NUM_OF_REGISTERS_TO_READ)) { + return pdu; + } + if (!register_block_in_range(LOG_STR("Write"), write_start_address, write_values.size(), + MAX_NUM_OF_REGISTERS_TO_WRITE_RW)) { + return pdu; + } + // fc + read start(2) + read qty(2) + write start(2) + write qty(2) + write byte count(1) + write values. + const auto write_count = static_cast(write_values.size()); + pdu.push_back(static_cast(FunctionCode::READ_WRITE_MULTIPLE_REGISTERS)); + append_pdu_word(pdu, read_start_address); + append_pdu_word(pdu, read_count); + append_pdu_word(pdu, write_start_address); + append_pdu_word(pdu, write_count); + pdu.push_back(static_cast(write_count * 2)); // byte count + for (auto v : write_values) { + append_pdu_word(pdu, v); } return pdu; } @@ -512,7 +547,7 @@ static void build_write_coils_pdu(PduBuffer &pdu, uint16_t start_address, Packed 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) { + if (!address_range_fits(start_address, count)) { ESP_LOGE(TAG, "Write of %u coils at %u runs past the 16-bit address space, dropping request", count, start_address); return; } diff --git a/esphome/components/modbus/modbus_helpers.h b/esphome/components/modbus/modbus_helpers.h index e47a6835cd..c737e206c0 100644 --- a/esphome/components/modbus/modbus_helpers.h +++ b/esphome/components/modbus/modbus_helpers.h @@ -11,7 +11,8 @@ namespace esphome::modbus::helpers { -inline bool is_function_code_read(uint8_t function_code) { +// Pure read codes (0x01-0x04): they only read, so they are idempotent and safe to retry. +inline bool is_function_code_read_only(uint8_t function_code) { FunctionCode masked_function_code = static_cast(function_code & FUNCTION_CODE_MASK); return masked_function_code == FunctionCode::READ_COILS || masked_function_code == FunctionCode::READ_DISCRETE_INPUTS || @@ -19,12 +20,27 @@ inline bool is_function_code_read(uint8_t function_code) { masked_function_code == FunctionCode::READ_INPUT_REGISTERS; } +// Codes whose response carries read-back data: the pure reads plus 0x17, which reads and writes at once. +inline bool is_function_code_read(uint8_t function_code) { + return is_function_code_read_only(function_code) || + static_cast(function_code & FUNCTION_CODE_MASK) == FunctionCode::READ_WRITE_MULTIPLE_REGISTERS; +} + +// Codes that mutate registers or coils: the pure writes, 0x16 mask-write, and 0x17 read/write multiple. inline bool is_function_code_write(uint8_t function_code) { FunctionCode masked_function_code = static_cast(function_code & FUNCTION_CODE_MASK); return masked_function_code == FunctionCode::WRITE_SINGLE_COIL || masked_function_code == FunctionCode::WRITE_SINGLE_REGISTER || masked_function_code == FunctionCode::WRITE_MULTIPLE_COILS || - masked_function_code == FunctionCode::WRITE_MULTIPLE_REGISTERS; + masked_function_code == FunctionCode::WRITE_MULTIPLE_REGISTERS || + masked_function_code == FunctionCode::MASK_WRITE_REGISTER || + masked_function_code == FunctionCode::READ_WRITE_MULTIPLE_REGISTERS; +} + +// True if [start_address, start_address + count) fits within the 16-bit Modbus address space. The 32-bit +// promotion is the overflow guard - a 16-bit sum could wrap and pass. +inline bool address_range_fits(uint16_t start_address, size_t count) { + return uint32_t(start_address) + count <= 0x10000u; } inline bool is_function_code_exception(uint8_t function_code) { @@ -90,8 +106,8 @@ inline uint8_t server_frame_data_offset(const uint8_t *frame, size_t size) { } /** Returns the payload portion of a server response PDU: the bytes after the function code, and for the - * standard read responses (0x01-0x04) also after the byte-count byte. Responses to 0x14/0x17 also carry a - * byte-count byte, but those codes are not implemented and their count byte is left in the payload. For + * read responses (0x01-0x04 and 0x17) also after the byte-count byte. Response 0x14 also carries a + * byte-count byte, but that code is not implemented and its count byte is left in the payload. For * an exception PDU the payload is the exception code byte (the read check must not see the masked * function code, or an exception-of-read would classify as a read and return an empty span). Returns an * empty span if the PDU is too short. @@ -432,6 +448,21 @@ PduBuffer create_client_pdu(FunctionCode function_code, uint16_t start_address, */ PduBuffer create_write_registers_pdu(uint16_t start_address, std::span values); +/** Create modbus read/write multiple registers command + * Function 0x17 Read/Write Multiple Registers + * Writes write_values then reads read_count registers in one transaction (write first, per Modbus 6.17); + * the response carries only the read registers. + * @param read_start_address modbus address of the first register to read back + * @param read_count number of registers to read (at most MAX_NUM_OF_REGISTERS_TO_READ) + * @param write_start_address modbus address of the first register to write + * @param write_values register values to write; the register count is write_values.size() (at most + * MAX_NUM_OF_REGISTERS_TO_WRITE_RW). Any contiguous uint16_t container converts. + * @return PDU (function code + data, no address, no CRC); an empty PDU on any out-of-range input + */ +PduBuffer create_read_write_multiple_registers_pdu(uint16_t read_start_address, uint16_t read_count, + uint16_t write_start_address, + std::span write_values); + /** Create modbus write single register command * Function 0x06 Write Single Register * @param start_address modbus address of the register to write diff --git a/esphome/components/modbus_client/__init__.py b/esphome/components/modbus_client/__init__.py index 52a61cacad..bb113d649c 100644 --- a/esphome/components/modbus_client/__init__.py +++ b/esphome/components/modbus_client/__init__.py @@ -28,9 +28,12 @@ CONF_ON_NO_RESPONSE = "on_no_response" CONF_ON_NOT_SENT = "on_not_sent" CONF_ON_SENT = "on_sent" CONF_PDU = "pdu" +CONF_READ_ADDRESS = "read_address" +CONF_READ_COUNT = "read_count" CONF_RETRY = "retry" CONF_START_ADDRESS = "start_address" CONF_VALUES = "values" +CONF_WRITE_ADDRESS = "write_address" modbus_client_ns = cg.esphome_ns.namespace("modbus_client") ModbusClientSendAction = modbus_client_ns.class_( @@ -55,6 +58,9 @@ WriteMultipleRegistersAction = modbus_client_ns.class_( WriteMultipleCoilsAction = modbus_client_ns.class_( "WriteMultipleCoilsAction", automation.Action, modbus.ModbusClientDevice ) +ReadWriteMultipleRegistersAction = modbus_client_ns.class_( + "ReadWriteMultipleRegistersAction", automation.Action, modbus.ModbusClientDevice +) # Packed bit view delivered to read_coils / read_discrete_inputs on_response handlers. PackedBits = modbus.modbus_ns.class_("PackedBits") @@ -255,21 +261,30 @@ async def modbus_client_send_to_code(config, action_id, template_arg, args): _REGISTER_SPAN = cg.std_span.template(cg.uint16.operator("const")) -# Every typed action addresses a register or coil range and reports through the same two reply handlers. -_TYPED_ACTION_SCHEMA = _ACTION_BASE_SCHEMA.extend( +# The reply-handler pair every typed-dispatch action reports through. Kept in one place so the +# read/write-multiple schema (which cannot require start_address) shares it instead of drifting. +# Both use _handler_schema(): the decoded arguments (values span, bits view) point at buffers the hub +# reuses once the handler returns, so a deferring action would resume on freed memory. A reply the +# dispatch gate diverts (not a standard-conformant transaction) arrives at on_custom_response with the +# raw request/response PDUs; real device exceptions still arrive via on_error. +_REPLY_HANDLERS_SCHEMA = cv.Schema( { - cv.Required(CONF_START_ADDRESS): cv.templatable(cv.hex_uint16_t), - # Both use _handler_schema(): the decoded arguments (values span, bits view) point at buffers the - # hub reuses once the handler returns, so a deferring action would resume on freed memory. cv.Optional(CONF_ON_RESPONSE): _handler_schema(), - # A reply the dispatch gate diverts (not a standard-conformant transaction) arrives here with the - # raw request/response PDUs; real device exceptions still arrive via on_error. cv.Optional(CONF_ON_CUSTOM_RESPONSE): _handler_schema(), } ) +# Every typed action addresses a register or coil range and reports through the shared reply handlers. +_TYPED_ACTION_SCHEMA = _ACTION_BASE_SCHEMA.extend(_REPLY_HANDLERS_SCHEMA).extend( + { + cv.Required(CONF_START_ADDRESS): cv.templatable(cv.hex_uint16_t), + } +) -def _no_address_overflow(count_key: str) -> Callable[[ConfigType], ConfigType]: + +def _no_address_overflow( + count_key: str, address_key: str = CONF_START_ADDRESS +) -> Callable[[ConfigType], ConfigType]: """Reject a range that runs past the 16-bit address space, which the device could never answer. Only literal configurations can be checked: either operand may be a lambda, and its value is not known @@ -278,17 +293,17 @@ def _no_address_overflow(count_key: str) -> Callable[[ConfigType], ConfigType]: """ def validate(config: ConfigType) -> ConfigType: - start = config[CONF_START_ADDRESS] + start = config[address_key] count = config[count_key] if isinstance(start, Lambda) or isinstance(count, Lambda): return config - # CONF_COUNT is a number; CONF_VALUES is the list whose length is the count. + # A count key holds a number; a values key holds the list whose length is the count. length = count if isinstance(count, int) else len(count) if start + length > 0x10000: raise cv.Invalid( - f"{CONF_START_ADDRESS} 0x{start:04X} plus {length} entities runs past the end of the " + f"{address_key} 0x{start:04X} plus {length} entities runs past the end of the " f"16-bit address space (last addressable entity is 0xFFFF)", - path=[CONF_START_ADDRESS], + path=[address_key], ) return config @@ -468,3 +483,64 @@ async def write_multiple_coils_to_code(config, action_id, template_arg, args): arr = cg.static_const_array(arr_id, cg.ArrayInitializer(*packed)) cg.add(var.set_values_static(arr, len(values))) return await register_client_action(var, config, args, []) + + +# Read/write multiple registers (FC 0x17) writes one register block and reads another in a single +# transaction, so it has two address ranges and uses read_address/write_address instead of start_address. +# Note the two meanings of `values`: here it is the block being WRITTEN, while in on_response the lambda +# argument `values` is the block that was READ BACK (host-order words, the same shape as +# read_holding_registers, so a caller can feed it through the same handler). +_READ_WRITE_MULTIPLE_REGISTERS_SCHEMA = cv.All( + _ACTION_BASE_SCHEMA.extend(_REPLY_HANDLERS_SCHEMA).extend( + { + cv.Required(CONF_READ_ADDRESS): cv.templatable(cv.hex_uint16_t), + cv.Optional(CONF_READ_COUNT, default=1): cv.templatable( + cv.int_range(min=1, max=modbus.MAX_NUM_OF_REGISTERS_TO_READ) + ), + cv.Required(CONF_WRITE_ADDRESS): cv.templatable(cv.hex_uint16_t), + cv.Required(CONF_VALUES): cv.templatable( + cv.All( + cv.ensure_list(cv.hex_uint16_t), + cv.Length(min=1, max=modbus.MAX_NUM_OF_REGISTERS_TO_WRITE_RW), + ) + ), + } + ), + _no_address_overflow(CONF_READ_COUNT, CONF_READ_ADDRESS), + _no_address_overflow(CONF_VALUES, CONF_WRITE_ADDRESS), +) + + +@automation.register_action( + "modbus_client.read_write_multiple_registers", + ReadWriteMultipleRegistersAction, + _READ_WRITE_MULTIPLE_REGISTERS_SCHEMA, + synchronous=True, +) +async def read_write_multiple_registers_to_code(config, action_id, template_arg, args): + var = cg.new_Pvariable(action_id, template_arg) + cg.add( + var.set_read_address( + await cg.templatable(config[CONF_READ_ADDRESS], args, cg.uint16) + ) + ) + cg.add( + var.set_read_count( + await cg.templatable(config[CONF_READ_COUNT], args, cg.uint16) + ) + ) + cg.add( + var.set_write_address( + await cg.templatable(config[CONF_WRITE_ADDRESS], args, cg.uint16) + ) + ) + values = config[CONF_VALUES] + if cg.is_template(values): + templ = await cg.templatable(values, args, cg.std_vector.template(cg.uint16)) + cg.add(var.set_values_template(templ)) + else: + # A static list goes to flash, so play() sends straight from there without allocating. + arr_id = ID(f"{action_id}_values", is_declaration=True, type=cg.uint16) + arr = cg.static_const_array(arr_id, cg.ArrayInitializer(*values)) + cg.add(var.set_values_static(arr, len(values))) + return await register_client_action(var, config, args, [(_REGISTER_SPAN, "values")]) diff --git a/esphome/components/modbus_client/modbus_client.h b/esphome/components/modbus_client/modbus_client.h index f9a00d65f6..7e6d9d069f 100644 --- a/esphome/components/modbus_client/modbus_client.h +++ b/esphome/components/modbus_client/modbus_client.h @@ -332,4 +332,56 @@ template class WriteMultipleCoilsAction : public TypedClientActi } values_; }; +/// modbus_client.read_write_multiple_registers (FC 0x17): writes one register block and reads another back in +/// one transaction (write first, per Modbus 6.17). on_response delivers the read-back words as `values`. +template class ReadWriteMultipleRegistersAction : public TypedClientActionBase { + public: + TEMPLATABLE_VALUE(uint16_t, read_address) + TEMPLATABLE_VALUE(uint16_t, read_count) + TEMPLATABLE_VALUE(uint16_t, write_address) + + /// Static config: the write registers live in flash, so play() neither allocates nor copies. + void set_values_static(const uint16_t *values, size_t len) { + this->values_.data = values; + this->len_ = static_cast(len); + } + /// Lambda config: the write registers are only known at play() time. + void set_values_template(std::vector (*func)(Ts...)) { + this->values_.func = func; + this->len_ = -1; // sentinel: template mode + } + + Trigger> *get_response_trigger() { return &this->response_trigger_; } + + void play(const Ts &...x) override { + const uint16_t read_start = this->read_address_.value(x...); + const uint16_t read_count = this->read_count_.value(x...); + const uint16_t write_start = this->write_address_.value(x...); + // An out-of-range read/write count builds an empty PDU (the builder logs why), resolving via on_not_sent. + if (this->len_ >= 0) { + this->send_or_resolve_(modbus::helpers::create_read_write_multiple_registers_pdu( + read_start, read_count, write_start, + std::span(this->values_.data, static_cast(this->len_)))); + return; + } + const std::vector values = this->values_.func(x...); + this->send_or_resolve_(modbus::helpers::create_read_write_multiple_registers_pdu( + read_start, read_count, write_start, std::span(values))); + } + // The 0x17 response carries only the read block, so the hub dispatch delivers it as a holding-register read. + void on_read_registers(modbus::EntityType entity_type, uint16_t start_address, std::span registers, + modbus::ResponseStatus status) override { + if (modbus::succeeded(status)) + this->response_trigger_.trigger(registers); + } + + protected: + Trigger> response_trigger_; + ssize_t len_{-1}; // -1 = template mode, >= 0 = static mode with this many write registers + union Values { + std::vector (*func)(Ts...); + const uint16_t *data; + } values_; +}; + } // namespace esphome::modbus_client diff --git a/tests/components/modbus/modbus_client_device_test.cpp b/tests/components/modbus/modbus_client_device_test.cpp index 38c28ce2df..333da5b228 100644 --- a/tests/components/modbus/modbus_client_device_test.cpp +++ b/tests/components/modbus/modbus_client_device_test.cpp @@ -118,6 +118,35 @@ TEST(ModbusClientDeviceFanOut, ReadHoldingRegistersSuccess) { EXPECT_FALSE(call.status.has_value()); } +// FC 0x17: the response carries only the read block, so it decodes as a holding-register read of the read +// start/count. The write half has no client-side ack callback - it is confirmed by a successful response. +TEST(ModbusClientDeviceFanOut, ReadWriteMultipleRegistersDeliversReadBlockAsHolding) { + RecordingDevice device; + // read 2 regs at 0x0010, write 1 reg (0x00FF) at 0x0020 + const uint8_t request[] = {0x17, 0x00, 0x10, 0x00, 0x02, 0x00, 0x20, 0x00, 0x01, 0x02, 0x00, 0xFF}; + const uint8_t response[] = {0x17, 0x04, 0x00, 0x2A, 0x01, 0x00}; // read-back: 0x002A, 0x0100 + device.on_response(request, response); + + ASSERT_EQ(device.holding_calls.size(), 1u); + const auto &call = device.holding_calls.front(); + EXPECT_EQ(call.start_address, 0x0010); // the READ start address, not the write + EXPECT_EQ(call.registers, (std::vector{0x002A, 0x0100})); + EXPECT_FALSE(call.status.has_value()); + EXPECT_TRUE(device.write_multiple_registers_calls.empty()); // no separate write-ack on the client side +} + +// A 0x17 response shorter than the requested read count is self-consistent but wrong; it must be diverted +// to on_custom_response(), never clamped and delivered as if complete. +TEST(ModbusClientDeviceFanOut, ReadWriteMultipleRegistersShortResponseGoesToCustom) { + RecordingDevice device; + const uint8_t request[] = {0x17, 0x00, 0x10, 0x00, 0x02, 0x00, 0x20, 0x00, 0x01, 0x02, 0x00, 0xFF}; + const uint8_t response[] = {0x17, 0x02, 0x00, 0x2A}; // only 1 register, but 2 were requested + device.on_response(request, response); + + EXPECT_TRUE(device.holding_calls.empty()); + EXPECT_EQ(device.custom_requests.size(), 1u); +} + TEST(ModbusClientDeviceFanOut, ReadInputRegistersDelegateToGeneric) { GenericDevice device; const uint8_t request[] = {0x04, 0x00, 0x10, 0x00, 0x01}; diff --git a/tests/components/modbus/modbus_helpers_test.cpp b/tests/components/modbus/modbus_helpers_test.cpp index 6a65c3bf68..53f51b016b 100644 --- a/tests/components/modbus/modbus_helpers_test.cpp +++ b/tests/components/modbus/modbus_helpers_test.cpp @@ -483,6 +483,71 @@ TEST(ModbusTypedBuilders, WriteRegistersPduRejectsOverLimit) { EXPECT_FALSE(create_write_registers_pdu(0x0000, values).empty()); } +TEST(ModbusTypedBuilders, ReadWriteMultipleRegistersPduWireBytes) { + const uint16_t write_values[] = {0x000B, 0x0016}; + // Read 2 registers at 0x0010, write 2 registers at 0x0020. + auto pdu = create_read_write_multiple_registers_pdu(0x0010, 2, 0x0020, write_values); + const std::vector expected{0x17, 0x00, 0x10, 0x00, 0x02, 0x00, 0x20, + 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())); +} + +TEST(ModbusTypedBuilders, ReadWriteMultipleRegistersPduRejectsOutOfRange) { + const uint16_t one_value[] = {0x0001}; + const uint16_t two_values[] = {0x0001, 0x0002}; + // Read count out of range (zero and above the read ceiling). + EXPECT_TRUE(create_read_write_multiple_registers_pdu(0x0000, 0, 0x0020, one_value).empty()); + EXPECT_TRUE( + create_read_write_multiple_registers_pdu(0x0000, MAX_NUM_OF_REGISTERS_TO_READ + 1, 0x0020, one_value).empty()); + // Write count out of range (empty, and above the read/write ceiling which is lower than a plain write). + EXPECT_TRUE(create_read_write_multiple_registers_pdu(0x0000, 1, 0x0020, std::span()).empty()); + std::vector too_many(MAX_NUM_OF_REGISTERS_TO_WRITE_RW + 1, 0xAAAA); + EXPECT_TRUE(create_read_write_multiple_registers_pdu(0x0000, 1, 0x0020, too_many).empty()); + // Both blocks at their respective ceilings are accepted. + std::vector at_write_limit(MAX_NUM_OF_REGISTERS_TO_WRITE_RW, 0xAAAA); + EXPECT_FALSE( + create_read_write_multiple_registers_pdu(0x0000, MAX_NUM_OF_REGISTERS_TO_READ, 0x0020, at_write_limit).empty()); + // A block that runs past the 16-bit address space is refused (read block, then write block). + EXPECT_TRUE(create_read_write_multiple_registers_pdu(0xFFFF, 2, 0x0020, one_value).empty()); + EXPECT_TRUE(create_read_write_multiple_registers_pdu(0x0000, 2, 0xFFFF, two_values).empty()); + // Accept boundary: a block ending exactly at 0x10000 (last register 0xFFFF) still fits. + EXPECT_FALSE(create_read_write_multiple_registers_pdu(0xFFFE, 2, 0x0000, one_value).empty()); // read ends at 0x10000 + EXPECT_FALSE( + create_read_write_multiple_registers_pdu(0x0000, 1, 0xFFFF, one_value).empty()); // write ends at 0x10000 +} + +TEST(ModbusFunctionCodeClass, ReadWriteMultipleCountsAsBothReadAndWrite) { + const auto rw = static_cast(FC::READ_WRITE_MULTIPLE_REGISTERS); + // 0x17 both reads and writes, but it is not a pure (retry-safe) read. + EXPECT_TRUE(is_function_code_read(rw)); + EXPECT_TRUE(is_function_code_write(rw)); + EXPECT_FALSE(is_function_code_read_only(rw)); + // Pure reads are read and read-only, never write. + const auto rd = static_cast(FC::READ_HOLDING_REGISTERS); + EXPECT_TRUE(is_function_code_read(rd)); + EXPECT_TRUE(is_function_code_read_only(rd)); + EXPECT_FALSE(is_function_code_write(rd)); + // Plain writes are write only. + const auto wr = static_cast(FC::WRITE_MULTIPLE_REGISTERS); + EXPECT_TRUE(is_function_code_write(wr)); + EXPECT_FALSE(is_function_code_read(wr)); + EXPECT_FALSE(is_function_code_read_only(wr)); + // Mask-write register mutates via read-modify-write, so it classes as a write, never a read. + const auto mask = static_cast(FC::MASK_WRITE_REGISTER); + EXPECT_TRUE(is_function_code_write(mask)); + EXPECT_FALSE(is_function_code_read(mask)); + EXPECT_FALSE(is_function_code_read_only(mask)); +} + +TEST(ModbusCreateClientPdu, ReadWriteMultipleReturnsEmpty) { + // The generic builder cannot express 0x17's two blocks; callers use the dedicated builder instead. + const uint16_t values[] = {0x0001}; + EXPECT_TRUE(create_client_pdu(FC::READ_WRITE_MULTIPLE_REGISTERS, 0x0000, 1, reinterpret_cast(values), + sizeof(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. diff --git a/tests/components/modbus_client/common.yaml b/tests/components/modbus_client/common.yaml index cae2002342..bce2149fbf 100644 --- a/tests/components/modbus_client/common.yaml +++ b/tests/components/modbus_client/common.yaml @@ -35,6 +35,8 @@ button: id(bare_client).write_single_register(0x10, 42); id(bare_client).write_single_coil(0x01, true); id(bare_client_explicit_hub).read_holding_registers(0x20, 4); + const uint16_t rw_vals[] = {1, 2}; + id(bare_client).read_write_multiple_registers(0x0400, 2, 0x0300, rw_vals); - platform: template name: "Send Read" on_press: @@ -134,3 +136,13 @@ button: on_error: then: - lambda: 'ESP_LOGW("modbus_client.test", "fc 0x%X exception %d", request.empty() ? 0 : request[0], (int) exception_code);' + - modbus_client.read_write_multiple_registers: + address: 0x01 + write_address: 0x0300 + values: !lambda "return {1, 2};" + read_address: 0x0400 + read_count: 2 + on_response: + then: + # `values` here is the READ-BACK block, not the written block above + - lambda: 'ESP_LOGI("modbus_client.test", "rw read0=%u n=%u", values[0], (unsigned) values.size());' diff --git a/tests/integration/fixtures/uart_mock_modbus_client_read_write.yaml b/tests/integration/fixtures/uart_mock_modbus_client_read_write.yaml new file mode 100644 index 0000000000..1f89889c95 --- /dev/null +++ b/tests/integration/fixtures/uart_mock_modbus_client_read_write.yaml @@ -0,0 +1,111 @@ +esphome: + name: uart-mock-modbus-cli-rw + +host: +api: +logger: + level: VERBOSE + +external_components: + - source: + type: local + path: EXTERNAL_COMPONENT_PATH + +# Dummy uart entry to satisfy modbus's DEPENDENCIES = ["uart"] +# The actual UART bus used is the uart_mock component below +uart: + baud_rate: 115200 + port: /dev/null + +# Two virtual buses looped back to each other: the client's transmissions reach the server and the +# server's replies reach the client. auto_start so forwarding is active before the button fires. +uart_mock: + - id: virtual_uart_server + baud_rate: 9600 + auto_start: true + debug: + on_tx: + - then: + - uart_mock.inject_rx: + id: virtual_uart_client + data: !lambda return data; + - id: virtual_uart_client + baud_rate: 9600 + auto_start: true + debug: + on_tx: + - then: + - uart_mock.inject_rx: + id: virtual_uart_server + data: !lambda return data; + +globals: + - id: stored_1 + type: uint16_t + initial_value: "0" + +modbus: + - uart_id: virtual_uart_server + id: virtual_modbus_server + role: server + - uart_id: virtual_uart_client + id: virtual_modbus_client + role: client + turnaround_time: 10ms + +modbus_server: + - address: 1 + modbus_id: virtual_modbus_server + registers: + # Writable + readable register: the read publishes what it returns, so the test can confirm the + # write half of the 0x17 ran before the read half (Modbus 6.17). + - address: 0x01 + value_type: U_WORD + read_lambda: |- + id(srv_read_1).publish_state(id(stored_1)); + return id(stored_1); + write_lambda: |- + id(stored_1) = x; + id(srv_write_1).publish_state(x); + return true; + # Read-only register, returned together with 0x01 by the 2-register read half. + - address: 0x02 + value_type: U_WORD + read_lambda: return 0x00AA; + +sensor: + # Server-side observations. + - platform: template + name: "srv_write_1" + id: srv_write_1 + - platform: template + name: "srv_read_1" + id: srv_read_1 + # Client-side read-back: the values the client's on_response received. + - platform: template + name: "client_read_0" + id: client_read_0 + - platform: template + name: "client_read_1" + id: client_read_1 + +button: + - platform: template + name: "Start Scenario" + id: start_scenario_btn + on_press: + # FC 0x17: write reg 0x0001 = 0x1234, then read regs 0x0001..0x0002 back in the same transaction. + - modbus_client.read_write_multiple_registers: + address: 0x01 + read_address: 0x0001 + read_count: 2 + write_address: 0x0001 + values: [0x1234] + on_response: + then: + - lambda: |- + // values is the read-back block: reg 0x0001 (must be the just-written 0x1234) and reg 0x0002. + if (values.size() >= 2) { + id(client_read_0).publish_state(values[0]); + id(client_read_1).publish_state(values[1]); + } diff --git a/tests/integration/test_uart_mock_modbus.py b/tests/integration/test_uart_mock_modbus.py index ca0041cc5b..1994d02c34 100644 --- a/tests/integration/test_uart_mock_modbus.py +++ b/tests/integration/test_uart_mock_modbus.py @@ -756,3 +756,38 @@ async def test_uart_mock_modbus_fairness( f"controllers did not get a fair share of the bus: " f"controller 1 issued {count_1}, controller 2 issued {count_2}" ) + + +@pytest.mark.asyncio +async def test_uart_mock_modbus_client_read_write( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """A modbus_client.read_write_multiple_registers action (FC 0x17) drives a server end to end. + + The client writes reg 0x0001 = 0x1234 and reads regs 0x0001..0x0002 in one transaction; the server + applies the write first (Modbus 6.17). The test confirms both ends: the server's write_lambda ran + (srv_write_1) and the read half came back to the client's on_response (client_read_0 = the + just-written 0x1234, client_read_1 = the read-only 0x00AA). + """ + line_callback, error_log_lines, warning_log_lines = _make_modbus_line_callback() + + tracker = SensorTracker( + ["srv_write_1", "srv_read_1", "client_read_0", "client_read_1"] + ) + futures = tracker.expect_all( + { + "srv_write_1": 4660, # server wrote 0x1234 to reg 0x0001 + "client_read_0": 4660, # client read reg 0x0001 back as the just-written 0x1234 + "client_read_1": 170, # client read reg 0x0002 (0x00AA) in the same request + } + ) + + async with ( + run_compiled(yaml_config, line_callback=line_callback), + api_client_connected() as client, + ): + await tracker.setup_and_start_scenario(client) + await tracker.await_all(futures) + _assert_no_modbus_errors(error_log_lines, warning_log_lines)