diff --git a/esphome/components/modbus/modbus.cpp b/esphome/components/modbus/modbus.cpp index a507e7e513..92b4dc25ac 100644 --- a/esphome/components/modbus/modbus.cpp +++ b/esphome/components/modbus/modbus.cpp @@ -718,24 +718,149 @@ void Modbus::clear_rx_buffer_(const LogString *reason, bool warn, size_t bytes_t } } -void ModbusClientDevice::read_entities(EntityType entity_type, uint16_t start_address, uint16_t number_of_entities) { - switch (entity_type) { - case EntityType::HOLDING: - this->read_holding_registers(start_address, number_of_entities); - return; - case EntityType::INPUT_REGISTER: - this->read_input_registers(start_address, number_of_entities); - return; - case EntityType::COIL: - this->read_coils(start_address, number_of_entities); - return; - case EntityType::DISCRETE_INPUT: - this->read_discrete_inputs(start_address, number_of_entities); - return; +void ModbusClientDevice::dispatch_response_(std::span request_pdu, std::span response_pdu, + ResponseStatus status) { + if (request_pdu.empty()) + return; + auto function_code = static_cast(request_pdu[0]); + // All standard requests handled below are function code + start address + count/value (5 bytes); + // anything shorter cannot be parsed and is handed to the catch-all. + if (request_pdu.size() < READ_PDU_SIZE) { + this->on_custom_response(request_pdu, response_pdu, status); + return; + } + const uint16_t start_address = helpers::get_data(request_pdu.data(), 1); + // count for reads/multi-writes, value for single writes + const uint16_t count_or_value = helpers::get_data(request_pdu.data(), 3); + + // Gatekeeper for the typed dispatch below: anything that is not a standard-conformant transaction is + // handed to on_custom_response() with the raw PDUs, so the decode cases can trust every length, byte + // count, and quantity field without re-clamping. + // - The REQUEST must be standard: nothing upstream validates a caller-built request PDU, so its + // internal byte count, quantity, and address range are checked here (is_client_pdu_standard()). + // - On success, the RESPONSE must be standard (self-consistent; the frame parser already guarantees + // most of this, but the check keeps the safety proof local), and a read response's length must also + // match the REQUESTED count - the per-PDU checks cannot see that relationship, and a short but + // self-consistent response must be diverted, never silently clamped and delivered as complete. + // - On failure (status engaged) the response is empty by design (see on_error()), so only the request + // is validated. + bool custom = !helpers::is_client_pdu_standard(request_pdu.data(), request_pdu.size()); + if (!custom && !status.has_value()) { + custom = !helpers::is_server_pdu_standard(response_pdu.data(), response_pdu.size()); + if (!custom && helpers::is_function_code_read(static_cast(function_code))) { + const bool bits = + function_code == FunctionCode::READ_COILS || function_code == FunctionCode::READ_DISCRETE_INPUTS; + const size_t expected_data_size = + bits ? (static_cast(count_or_value) + 7) / 8 : static_cast(count_or_value) * 2; + if (response_pdu.size() != expected_data_size + 2) { + ESP_LOGD(TAG, "Response length %zu does not match request (expected %zu) for function code 0x%X", + response_pdu.size(), expected_data_size + 2, static_cast(function_code)); + custom = true; + } + } + } + if (custom) { + this->on_custom_response(request_pdu, response_pdu, status); + return; + } + + switch (function_code) { + case FunctionCode::READ_HOLDING_REGISTERS: + case FunctionCode::READ_INPUT_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 + // failure the registers span is empty. + RegisterValues registers; + if (!status.has_value()) { + for (size_t i = 0; i != count_or_value; i++) { + registers.push_back(helpers::get_data(response_pdu.data(), 2 + 2 * i)); + } + } + std::span register_span(registers.data(), registers.size()); + if (function_code == FunctionCode::READ_HOLDING_REGISTERS) { + this->on_read_holding_registers(start_address, register_span, status); + } else { + this->on_read_input_registers(start_address, register_span, status); + } + break; + } + case FunctionCode::READ_COILS: + case FunctionCode::READ_DISCRETE_INPUTS: { + // Deliver the bits packed as on the wire; the gate guarantees a success response carries exactly + // (count_or_value + 7) / 8 data bytes. On failure the view is empty AND the count is zero - + // PackedBits::operator[] is unchecked, so size() must never promise bits with no bytes behind them. + std::span packed_bytes; + uint16_t count = 0; + if (!status.has_value()) { + packed_bytes = response_pdu.subspan(2); + count = count_or_value; + } + PackedBits bits(packed_bytes, count); + if (function_code == FunctionCode::READ_COILS) { + this->on_read_coils(start_address, bits, status); + } else { + this->on_read_discrete_inputs(start_address, bits, status); + } + break; + } + // Single-write acks echo the value: on success that echo is device-confirmed state - the one + // write whose acknowledgement carries a real read-back - so it is preferred over the request + // copy. On an exception the response has no value and the request copy is the only one. + case FunctionCode::WRITE_SINGLE_REGISTER: + case FunctionCode::WRITE_SINGLE_COIL: { + const uint16_t value = (!status.has_value() && response_pdu.size() >= WRITE_SINGLE_PDU_SIZE) + ? helpers::get_data(response_pdu.data(), 3) + : count_or_value; + if (function_code == FunctionCode::WRITE_SINGLE_REGISTER) { + this->on_write_single_register(start_address, value, status); + } else { + this->on_write_single_coil(start_address, value == 0xFF00, status); + } + break; + } + case FunctionCode::WRITE_MULTIPLE_REGISTERS: { + // Request layout: [0] function code, [1..2] start address, [3..4] register count, [5] byte count, + // [6..] register data. The gate guarantees the request carries exactly count_or_value registers + // (<= MAX_NUM_OF_REGISTERS_TO_WRITE, within RegisterValues capacity). Decoded from the request and + // delivered regardless of status - see the write-acknowledgement note in modbus.h. + RegisterValues registers; + for (size_t i = 0; i != count_or_value; i++) { + registers.push_back(helpers::get_data(request_pdu.data(), 6 + 2 * i)); + } + std::span register_span(registers.data(), registers.size()); + this->on_write_multiple_registers(start_address, register_span, status); + break; + } + case FunctionCode::WRITE_MULTIPLE_COILS: { + // Request layout: [0] function code, [1..2] start address, [3..4] coil count, [5] byte count, + // [6..] packed bits. The gate guarantees the request carries exactly (count_or_value + 7) / 8 packed + // bytes. Decoded from the request and delivered regardless of status - see the write-acknowledgement + // note in modbus.h. + std::span packed_bytes = request_pdu.subspan(6); + PackedBits bits(packed_bytes, count_or_value); + this->on_write_multiple_coils(start_address, bits, status); + break; + } default: - ESP_LOGW(TAG, "Invalid entity type for read_entities: %d", (int) entity_type); - this->on_not_sent(); // every rejected send is signalled, like send_pdu()'s own refusals - return; + this->on_custom_response(request_pdu, response_pdu, status); + break; + } +} + +// Default on_custom_response handler to warn when responses unexpectedly trigger on_custom_response +void ModbusClientDevice::on_custom_response(std::span request_pdu, std::span response_pdu, + ResponseStatus status) { + // The dispatcher never calls this with an empty request, but this is a public virtual - stay safe. + const uint8_t function_code = request_pdu.empty() ? 0 : request_pdu[0]; + // Warn once per device, then drop to VERBOSE: a mildly non-conformant peer answers every poll, + // and an unhandled-response warning per transaction would flood the log permanently. + if (!this->custom_response_warned_) { + this->custom_response_warned_ = true; + ESP_LOGW(TAG, "Non-standard request or response for function code 0x%X. No on_custom_response handler declared", + function_code); + } else { + ESP_LOGV(TAG, "Non-standard request or response for function code 0x%X (unhandled)", function_code); } } diff --git a/esphome/components/modbus/modbus.h b/esphome/components/modbus/modbus.h index 6afb8584aa..d83075eda1 100644 --- a/esphome/components/modbus/modbus.h +++ b/esphome/components/modbus/modbus.h @@ -191,15 +191,25 @@ class ModbusClientDevice { ModbusClientDevice &operator=(ModbusClientDevice &&) = delete; void set_parent(ModbusClientHub *parent) { this->parent_ = parent; } void set_address(uint8_t address) { this->address_ = address; } - /// Called with the request PDU this device sent and the response PDU received (both: function code + - /// data, no address, no CRC). The spans are only valid for the duration of the call - copy the bytes - /// if they must outlive it. Slice the payload out of the response with helpers::server_pdu_payload(). - virtual void on_response(std::span request_pdu, std::span response_pdu) {} - /// Called with the request PDU and the modbus exception code decoded from the error response. - virtual void on_error(std::span request_pdu, ExceptionCode exception_code) {} - // The on_modbus_* names are signature-identical renames, so the new defaults forward to the old - // virtuals: external devices overriding the old names keep working through the deprecation window. - // Remove the forwards together with the deprecated names. + /// Low-level response hook: called with the request PDU this device sent and the response PDU received + /// The spans are only valid for the duration of the call - copy the bytes if they must outlive it. + /// The default implementation decodes standard responses and dispatches to on_read_* / on_write_* callbacks below. + /// Override it to handle raw PDUs directly. + virtual void on_response(std::span request_pdu, std::span response_pdu) { + this->dispatch_response_(request_pdu, response_pdu, std::nullopt); + } + /// Low-level error hook: called with the request PDU and the modbus exception code from the error response. + /// The default implementation dispatches to the same typed callbacks with the exception code as status. + /// Devices implementing the High-level typed callbacks see success and failure through one interface. + virtual void on_error(std::span request_pdu, ExceptionCode exception_code) { + this->dispatch_response_(request_pdu, {}, exception_code); + } + + /// Called when no request could be sent (e.g. queue full, transmission blocked) + /// Do not attempt to queue a command in this callback. + /// (The on_modbus_* names are signature-identical renames, so the new defaults forward to the old + /// virtuals: external devices overriding the old names keep working through the deprecation window. + /// Remove the forwards together with the deprecated names.) virtual void on_not_sent() { #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeprecated-declarations" @@ -220,6 +230,51 @@ class ModbusClientDevice { // Remove before 2027.2.0 ESPDEPRECATED("Override on_no_response() instead. Removed in 2027.2.0", "2026.8.0") virtual bool on_modbus_no_response() { return false; } + + /// High-level typed response callbacks, fired by the default on_response()/on_error() with arguments + /// parsed from the request and response PDUs. + /// Status is std::nullopt on success; holds the exception code on failure. + /// Register values are in host byte order; spans are only valid for the duration of the call. + virtual void on_read_registers(EntityType entity_type, uint16_t start_address, std::span registers, + ResponseStatus status) {} + virtual void on_read_holding_registers(uint16_t start_address, std::span registers, + ResponseStatus status) { + this->on_read_registers(EntityType::HOLDING, start_address, registers, status); + } + virtual void on_read_input_registers(uint16_t start_address, std::span registers, + ResponseStatus status) { + this->on_read_registers(EntityType::INPUT_REGISTER, start_address, registers, status); + } + /// Coil/discrete-input reads are delivered as a PackedBits view (bit 0 = the bit at start_address, + /// bits.size() = the count requested). The view points into the hub's receive buffer and is only + /// valid during the call. + virtual void on_read_bits(EntityType entity_type, uint16_t start_address, PackedBits bits, ResponseStatus status) {} + virtual void on_read_coils(uint16_t start_address, PackedBits bits, ResponseStatus status) { + this->on_read_bits(EntityType::COIL, start_address, bits, status); + } + virtual void on_read_discrete_inputs(uint16_t start_address, PackedBits bits, ResponseStatus status) { + this->on_read_bits(EntityType::DISCRETE_INPUT, start_address, bits, status); + } + /// Write acknowledgements. These deliberately mirror the read callbacks' shapes, so a write ack can be fed + /// through the same handler as a read (registers.size() / bits.size() gives the count) + /// + /// IMPORTANT - for the multi-writes these are the values that were REQUESTED, not device-confirmed + /// state: a multi-write ack only echoes the start address and count, so the values are decoded from + /// the request PDU, and they are delivered even when status holds an exception code. Always check + /// status, and treat publishing them as an optimistic update rather than a read-back. The single + /// writes are the exception: their successful ack echoes the value, so on success the delivered + /// value is the device's echo (on an exception it falls back to the request copy). + virtual void on_write_single_register(uint16_t address, uint16_t value, ResponseStatus status) {} + virtual void on_write_single_coil(uint16_t address, bool value, ResponseStatus status) {} + virtual void on_write_multiple_registers(uint16_t start_address, std::span registers, + ResponseStatus status) {} + virtual void on_write_multiple_coils(uint16_t start_address, PackedBits bits, ResponseStatus status) {} + /// Catch-all for custom function codes and anything that is not a standard-conformant transaction + /// (see dispatch_response_()); on failure the response is empty and the exception code is in status. + /// The default implementation only logs a warning that the response is going unhandled - override it + /// to handle custom traffic (which also silences the warning). + virtual void on_custom_response(std::span request_pdu, std::span response_pdu, + ResponseStatus status); ESPDEPRECATED("Use the typed read_*/write_* helpers or send_pdu() instead. Removed in 2027.2.0", "2026.8.0") void send(uint8_t function, uint16_t start_address, uint16_t number_of_entities, uint8_t payload_len = 0, const uint8_t *payload = nullptr) { @@ -237,8 +292,12 @@ class ModbusClientDevice { } this->parent_->send_pdu(payload[0], std::span(payload).subspan(1), this); } - // Dispatches to the matching read_* method; defined in modbus.cpp because it logs on an invalid type. - void read_entities(EntityType entity_type, uint16_t start_address, uint16_t number_of_entities); + // Reads via the table-appropriate function code; an unreadable entity type maps to INVALID, which + // create_read_pdu() rejects into an empty PDU and send_pdu() signals via on_not_sent(). + void read_entities(EntityType entity_type, uint16_t start_address, uint16_t number_of_entities) { + this->send_pdu(helpers::create_read_pdu(helpers::modbus_register_read_function(entity_type), start_address, + number_of_entities)); + } void read_input_registers(uint16_t start_address, uint16_t number_of_registers) { this->send_pdu(helpers::create_read_pdu(FunctionCode::READ_INPUT_REGISTERS, start_address, number_of_registers)); } @@ -281,8 +340,13 @@ class ModbusClientDevice { bool ready_for_immediate_send() { return this->parent_->tx_buffer_empty() && !this->parent_->tx_blocked(); } protected: + /// Parses the request/response PDU pair and dispatches to the matching high-level typed callback + void dispatch_response_(std::span request_pdu, std::span response_pdu, + ResponseStatus status); + ModbusClientHub *parent_{nullptr}; uint8_t address_{0}; + bool custom_response_warned_{false}; // first unhandled custom response warns; repeats log at VERBOSE }; // Compatibility shim for external components written against the pre-2026.8 API, which subclassed diff --git a/esphome/components/modbus/modbus_helpers.cpp b/esphome/components/modbus/modbus_helpers.cpp index 96561cf56f..88e10287a2 100644 --- a/esphome/components/modbus/modbus_helpers.cpp +++ b/esphome/components/modbus/modbus_helpers.cpp @@ -420,6 +420,11 @@ PduBuffer create_client_pdu(FunctionCode function_code, uint16_t start_address, 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]); + // Zero the unused bits of the final byte as the spec requires, matching the typed coil builder + // so both produce identical wire bytes for the same write. + if (bits && number_of_entities % 8 != 0) { + pdu[pdu.size() - 1] &= static_cast((1 << (number_of_entities % 8)) - 1); + } return pdu; } diff --git a/tests/components/modbus/modbus_client_device_test.cpp b/tests/components/modbus/modbus_client_device_test.cpp new file mode 100644 index 0000000000..8638c37688 --- /dev/null +++ b/tests/components/modbus/modbus_client_device_test.cpp @@ -0,0 +1,344 @@ +#include + +#include +#include +#include +#include + +#include "esphome/components/modbus/modbus.h" + +namespace esphome::modbus::testing { + +namespace { + +// Records every typed callback so tests can assert on the dispatch performed by the default +// on_response()/on_error() implementations. +class RecordingDevice : public ModbusClientDevice { + public: + struct ReadRegistersCall { + uint16_t start_address; + std::vector registers; + ResponseStatus status; + }; + struct ReadBitsCall { + uint16_t start_address; + uint16_t count; + std::vector packed; + ResponseStatus status; + }; + struct WriteCall { + uint16_t address; + uint16_t value; + ResponseStatus status; + }; + + void on_read_holding_registers(uint16_t start_address, std::span registers, + ResponseStatus status) override { + this->holding_calls.push_back({start_address, {registers.begin(), registers.end()}, status}); + } + void on_read_input_registers(uint16_t start_address, std::span registers, + ResponseStatus status) override { + this->input_calls.push_back({start_address, {registers.begin(), registers.end()}, status}); + } + void on_read_coils(uint16_t start_address, PackedBits bits, ResponseStatus status) override { + this->coil_calls.push_back({start_address, bits.size(), {bits.bytes().begin(), bits.bytes().end()}, status}); + } + void on_read_discrete_inputs(uint16_t start_address, PackedBits bits, ResponseStatus status) override { + this->discrete_calls.push_back({start_address, bits.size(), {bits.bytes().begin(), bits.bytes().end()}, status}); + } + void on_write_single_register(uint16_t address, uint16_t value, ResponseStatus status) override { + this->write_single_register_calls.push_back({address, value, status}); + } + void on_write_single_coil(uint16_t address, bool value, ResponseStatus status) override { + this->write_single_coil_calls.push_back({address, static_cast(value), status}); + } + void on_write_multiple_registers(uint16_t start_address, std::span registers, + ResponseStatus status) override { + this->write_multiple_registers_calls.push_back({start_address, {registers.begin(), registers.end()}, status}); + } + void on_write_multiple_coils(uint16_t start_address, PackedBits bits, ResponseStatus status) override { + this->write_multiple_coils_calls.push_back( + {start_address, bits.size(), {bits.bytes().begin(), bits.bytes().end()}, status}); + } + void on_custom_response(std::span request_pdu, std::span response_pdu, + ResponseStatus status) override { + this->custom_requests.emplace_back(request_pdu.begin(), request_pdu.end()); + this->custom_responses.emplace_back(response_pdu.begin(), response_pdu.end()); + this->custom_statuses.push_back(status); + } + + std::vector holding_calls; + std::vector input_calls; + std::vector coil_calls; + std::vector discrete_calls; + std::vector write_single_register_calls; + std::vector write_single_coil_calls; + std::vector write_multiple_registers_calls; + std::vector write_multiple_coils_calls; + std::vector> custom_requests; + std::vector> custom_responses; + std::vector custom_statuses; +}; + +// Overrides only the generic callbacks to verify the typed defaults delegate to them. +class GenericDevice : public ModbusClientDevice { + public: + void on_read_registers(EntityType register_type, uint16_t start_address, std::span registers, + ResponseStatus status) override { + this->register_type = register_type; + this->start_address = start_address; + this->registers.assign(registers.begin(), registers.end()); + this->calls++; + } + void on_read_bits(EntityType register_type, uint16_t start_address, PackedBits bits, ResponseStatus status) override { + this->register_type = register_type; + this->start_address = start_address; + this->bit_count = bits.size(); + this->calls++; + } + EntityType register_type{EntityType::CUSTOM}; + uint16_t start_address{0}; + uint16_t bit_count{0}; + std::vector registers; + int calls{0}; +}; + +} // namespace + +TEST(ModbusClientDeviceFanOut, ReadHoldingRegistersSuccess) { + RecordingDevice device; + const uint8_t request[] = {0x03, 0x01, 0x00, 0x00, 0x02}; // read 2 regs at 0x100 + const uint8_t response[] = {0x03, 0x04, 0x00, 0x2A, 0x01, 0x00}; // 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, 0x100); + EXPECT_EQ(call.registers, (std::vector{0x002A, 0x0100})); + EXPECT_FALSE(call.status.has_value()); +} + +TEST(ModbusClientDeviceFanOut, ReadInputRegistersDelegateToGeneric) { + GenericDevice device; + const uint8_t request[] = {0x04, 0x00, 0x10, 0x00, 0x01}; + const uint8_t response[] = {0x04, 0x02, 0x12, 0x34}; + device.on_response(request, response); + + EXPECT_EQ(device.calls, 1); + EXPECT_EQ(device.register_type, EntityType::INPUT_REGISTER); + EXPECT_EQ(device.start_address, 0x10); + EXPECT_EQ(device.registers, (std::vector{0x1234})); +} + +TEST(ModbusClientDeviceFanOut, ReadDiscreteInputsDelegateToGenericBits) { + GenericDevice device; + const uint8_t request[] = {0x02, 0x00, 0x20, 0x00, 0x05}; // 5 inputs at 0x20 + const uint8_t response[] = {0x02, 0x01, 0x15}; + device.on_response(request, response); + + EXPECT_EQ(device.calls, 1); + EXPECT_EQ(device.register_type, EntityType::DISCRETE_INPUT); + EXPECT_EQ(device.start_address, 0x20); + EXPECT_EQ(device.bit_count, 5); +} + +// A CRC-valid response whose length does not match its request cannot be decoded per the +// function-code contract: it goes to the catch-all with the raw PDUs, not to the typed callback. +TEST(ModbusClientDeviceFanOut, ReadRegistersMismatchedLengthGoesToCatchAll) { + RecordingDevice device; + // Request asks for 4 registers but the response only carries 1. + const uint8_t request[] = {0x03, 0x00, 0x00, 0x00, 0x04}; + const uint8_t response[] = {0x03, 0x02, 0xBE, 0xEF}; + device.on_response(request, response); + + EXPECT_TRUE(device.holding_calls.empty()); + ASSERT_EQ(device.custom_responses.size(), 1u); + EXPECT_EQ(device.custom_responses.front(), (std::vector(response, response + sizeof(response)))); +} + +// Coil responses are validated the same way: byte count must be ceil(count / 8). +TEST(ModbusClientDeviceFanOut, ReadCoilsMismatchedLengthGoesToCatchAll) { + RecordingDevice device; + const uint8_t request[] = {0x01, 0x00, 0x13, 0x00, 0x13}; // 19 coils -> 3 packed bytes + const uint8_t response[] = {0x01, 0x02, 0xCD, 0x6B}; // only 2 + device.on_response(request, response); + + EXPECT_TRUE(device.coil_calls.empty()); + EXPECT_EQ(device.custom_responses.size(), 1u); +} + +TEST(ModbusClientDeviceFanOut, ReadCoilsSuccess) { + RecordingDevice device; + const uint8_t request[] = {0x01, 0x00, 0x13, 0x00, 0x13}; // 19 coils at 0x13 + const uint8_t response[] = {0x01, 0x03, 0xCD, 0x6B, 0x05}; + device.on_response(request, response); + + ASSERT_EQ(device.coil_calls.size(), 1u); + const auto &call = device.coil_calls.front(); + EXPECT_EQ(call.start_address, 0x13); + EXPECT_EQ(call.count, 19); + EXPECT_EQ(call.packed, (std::vector{0xCD, 0x6B, 0x05})); + EXPECT_FALSE(call.status.has_value()); + // first coil = bit 0 of byte 0 + EXPECT_TRUE(helpers::bit_from_packed(0, call.packed)); + EXPECT_FALSE(helpers::bit_from_packed(1, call.packed)); +} + +TEST(ModbusClientDeviceFanOut, WriteSingleRegisterSuccess) { + RecordingDevice device; + const uint8_t request[] = {0x06, 0x00, 0x01, 0x00, 0x03}; + device.on_response(request, request); // echo + + ASSERT_EQ(device.write_single_register_calls.size(), 1u); + const auto &call = device.write_single_register_calls.front(); + EXPECT_EQ(call.address, 1); + EXPECT_EQ(call.value, 3); + EXPECT_FALSE(call.status.has_value()); +} + +TEST(ModbusClientDeviceFanOut, WriteSingleCoilSuccess) { + RecordingDevice device; + const uint8_t request[] = {0x05, 0x00, 0xAC, 0xFF, 0x00}; + device.on_response(request, request); + + ASSERT_EQ(device.write_single_coil_calls.size(), 1u); + EXPECT_EQ(device.write_single_coil_calls.front().address, 0xAC); + EXPECT_EQ(device.write_single_coil_calls.front().value, 1u); +} + +TEST(ModbusClientDeviceFanOut, WriteErrorReportsRequestArgumentsAndStatus) { + RecordingDevice device; + const uint8_t request[] = {0x06, 0x00, 0x01, 0x00, 0x03}; + const uint8_t exception[] = {0x86, 0x02}; // ILLEGAL_DATA_ADDRESS + device.on_error(request, static_cast(exception[1])); + + ASSERT_EQ(device.write_single_register_calls.size(), 1u); + const auto &call = device.write_single_register_calls.front(); + EXPECT_EQ(call.address, 1); + EXPECT_EQ(call.value, 3); + EXPECT_EQ(call.status, ExceptionCode::ILLEGAL_DATA_ADDRESS); +} + +TEST(ModbusClientDeviceFanOut, ReadErrorReportsEmptyDataAndStatus) { + RecordingDevice device; + const uint8_t request[] = {0x03, 0x01, 0x00, 0x00, 0x02}; + const uint8_t exception[] = {0x83, 0x02}; + device.on_error(request, static_cast(exception[1])); + + ASSERT_EQ(device.holding_calls.size(), 1u); + const auto &call = device.holding_calls.front(); + EXPECT_EQ(call.start_address, 0x100); + EXPECT_TRUE(call.registers.empty()); + EXPECT_EQ(call.status, ExceptionCode::ILLEGAL_DATA_ADDRESS); +} + +TEST(ModbusClientDeviceFanOut, CustomFunctionCodeGoesToCatchAll) { + RecordingDevice device; + const uint8_t request[] = {0x47, 0x01, 0x02, 0x03, 0x04}; + const uint8_t response[] = {0x47, 0xAA, 0xBB}; + device.on_response(request, response); + + ASSERT_EQ(device.custom_requests.size(), 1u); + EXPECT_EQ(device.custom_requests.front(), (std::vector{0x47, 0x01, 0x02, 0x03, 0x04})); + EXPECT_EQ(device.custom_responses.front(), (std::vector{0x47, 0xAA, 0xBB})); + EXPECT_FALSE(device.custom_statuses.front().has_value()); + EXPECT_TRUE(device.holding_calls.empty()); + + // On failure the catch-all receives an empty response and the status (the exception code). + const uint8_t exception[] = {0xC7, 0x02}; + device.on_error(request, static_cast(exception[1])); + ASSERT_EQ(device.custom_statuses.size(), 2u); + EXPECT_EQ(device.custom_statuses.back(), ExceptionCode::ILLEGAL_DATA_ADDRESS); + EXPECT_TRUE(device.custom_responses.back().empty()); +} + +// A write ack only echoes the start address and count, so the data that was written is decoded from the +// request PDU: [0] function code, [1..2] start address, [3..4] count, [5] byte count, [6..] data. +TEST(ModbusClientDeviceFanOut, WriteMultipleAcksReportStartAndData) { + RecordingDevice device; + // Write 2 registers (0x0001, 0x0002) at 0x0020: byte count 4, data from offset 6. + const uint8_t reg_request[] = {0x10, 0x00, 0x20, 0x00, 0x02, 0x04, 0x00, 0x01, 0x00, 0x02}; + const uint8_t reg_ack[] = {0x10, 0x00, 0x20, 0x00, 0x02}; + device.on_response(reg_request, reg_ack); + // Write 10 coils at 0x0030: byte count 2, packed bits 0xFF 0x03 from offset 6. + const uint8_t coil_request[] = {0x0F, 0x00, 0x30, 0x00, 0x0A, 0x02, 0xFF, 0x03}; + const uint8_t coil_ack[] = {0x0F, 0x00, 0x30, 0x00, 0x0A}; + device.on_response(coil_request, coil_ack); + + ASSERT_EQ(device.write_multiple_registers_calls.size(), 1u); + EXPECT_EQ(device.write_multiple_registers_calls.front().start_address, 0x20); + EXPECT_EQ(device.write_multiple_registers_calls.front().registers, (std::vector{0x0001, 0x0002})); + ASSERT_EQ(device.write_multiple_coils_calls.size(), 1u); + EXPECT_EQ(device.write_multiple_coils_calls.front().start_address, 0x30); + EXPECT_EQ(device.write_multiple_coils_calls.front().count, 10); + EXPECT_EQ(device.write_multiple_coils_calls.front().packed, (std::vector{0xFF, 0x03})); +} + +// A truncated request (byte-count header promises more data than the PDU carries) is not a standard +// write-multiple, so it is diverted to on_custom_response() - never clamped and delivered as if complete. +TEST(ModbusClientDeviceFanOut, WriteMultipleTruncatedRequestDispatchesAsCustom) { + RecordingDevice device; + // Header claims 2 registers / 4 data bytes, but only one register's worth is present. + const uint8_t reg_request[] = {0x10, 0x00, 0x20, 0x00, 0x02, 0x04, 0x00, 0x01}; + const uint8_t reg_ack[] = {0x10, 0x00, 0x20, 0x00, 0x02}; + device.on_response(reg_request, reg_ack); + + EXPECT_TRUE(device.write_multiple_registers_calls.empty()); + ASSERT_EQ(device.custom_requests.size(), 1u); + EXPECT_EQ(device.custom_requests.front(), (std::vector{0x10, 0x00, 0x20, 0x00, 0x02, 0x04, 0x00, 0x01})); +} + +// A request whose byte-count header disagrees with its own quantity field (here: 2 registers but a +// byte count of 2 instead of 4, with matching data) is non-standard and diverted to the catch-all. +TEST(ModbusClientDeviceFanOut, WriteMultipleInconsistentByteCountDispatchesAsCustom) { + RecordingDevice device; + const uint8_t reg_request[] = {0x10, 0x00, 0x20, 0x00, 0x02, 0x02, 0x00, 0x01}; + const uint8_t reg_ack[] = {0x10, 0x00, 0x20, 0x00, 0x02}; + device.on_response(reg_request, reg_ack); + + EXPECT_TRUE(device.write_multiple_registers_calls.empty()); + EXPECT_EQ(device.custom_requests.size(), 1u); +} + +// An exception on a read still dispatches to the typed callback (empty data, status set): the gate must +// not require a standard response on the failure path, because on_error() delivers an empty response by +// design. +TEST(ModbusClientDeviceFanOut, ReadErrorWithEmptyResponseStillDispatchesTyped) { + RecordingDevice device; + const uint8_t read_request[] = {0x03, 0x01, 0x00, 0x00, 0x02}; + device.on_error(read_request, ExceptionCode::ILLEGAL_DATA_ADDRESS); + + ASSERT_EQ(device.holding_calls.size(), 1u); + EXPECT_TRUE(device.holding_calls.front().registers.empty()); + EXPECT_EQ(device.holding_calls.front().status, ExceptionCode::ILLEGAL_DATA_ADDRESS); + EXPECT_TRUE(device.custom_requests.empty()); +} + +// An error on a coil read must deliver a PackedBits view whose size() is zero - the count must never +// promise bits that have no bytes behind them (operator[] is unchecked). +TEST(ModbusClientDeviceFanOut, ReadCoilsErrorDeliversZeroCountBits) { + RecordingDevice device; + const uint8_t read_request[] = {0x01, 0x01, 0x00, 0x00, 0x0A}; + device.on_error(read_request, ExceptionCode::SERVICE_DEVICE_FAILURE); + + ASSERT_EQ(device.coil_calls.size(), 1u); + EXPECT_EQ(device.coil_calls.front().count, 0); + EXPECT_TRUE(device.coil_calls.front().packed.empty()); +} + +// Single-write acks: on success the delivered value is the device's echo (real read-back); +// on an exception it falls back to the request copy. +TEST(ModbusTypedDispatch, SingleWriteAckPrefersTheResponseEcho) { + RecordingDevice device; + const uint8_t request[] = {0x06, 0x00, 0x10, 0x00, 0x2A}; + const uint8_t echo_clamped[] = {0x06, 0x00, 0x10, 0x00, 0x28}; // device clamped 42 -> 40 + device.on_response(request, echo_clamped); + ASSERT_EQ(device.write_single_register_calls.size(), 1u); + EXPECT_EQ(device.write_single_register_calls.front().value, 0x0028); // the echo, not the request + + device.on_error(request, ExceptionCode::ILLEGAL_DATA_VALUE); + ASSERT_EQ(device.write_single_register_calls.size(), 2u); + EXPECT_EQ(device.write_single_register_calls.back().value, 0x002A); // exception: request copy +} + +} // namespace esphome::modbus::testing diff --git a/tests/components/modbus/modbus_helpers_test.cpp b/tests/components/modbus/modbus_helpers_test.cpp index b471644226..49de4f9d14 100644 --- a/tests/components/modbus/modbus_helpers_test.cpp +++ b/tests/components/modbus/modbus_helpers_test.cpp @@ -448,6 +448,15 @@ TEST(ModbusTypedBuilders, BoolSpanCoilBuilderRejectsOverLimit) { EXPECT_TRUE(create_write_coils_pdu(0, std::span(big.get(), MAX_NUM_OF_COILS_TO_WRITE + 1)).empty()); } +TEST(ModbusCreateClientPdu, GenericCoilWriteMasksTrailingPadBits) { + // 10 coils with junk in the pad bits of the last data byte: the generic path masks them like the + // typed builder, so both produce identical wire bytes. + const uint8_t values[] = {0xFF, 0xFF}; + auto pdu = create_client_pdu(FC::WRITE_MULTIPLE_COILS, 0x0000, 10, values, 2); + ASSERT_FALSE(pdu.empty()); + EXPECT_EQ(pdu[pdu.size() - 1], 0x03); // bits 8-9 kept, pad bits 10-15 zeroed +} + TEST(ModbusCreateClientPdu, SingleCoilValueValidated) { const uint8_t on[] = {0xFF, 0x00}; const uint8_t junk[] = {0x01, 0x00};