From fcc2c26367dd2f877bd443ebf4d1f99606442ab2 Mon Sep 17 00:00:00 2001 From: Bonne Eggleston Date: Sun, 2 Aug 2026 09:40:34 -0700 Subject: [PATCH] [modbus] Restore function code byte for custom codes in deprecated on_modbus_data (#18006) --- esphome/components/modbus/modbus.h | 7 +++- .../modbus/modbus_client_device_test.cpp | 41 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/esphome/components/modbus/modbus.h b/esphome/components/modbus/modbus.h index c49f52df55..c73aa6878d 100644 --- a/esphome/components/modbus/modbus.h +++ b/esphome/components/modbus/modbus.h @@ -550,7 +550,12 @@ class ESPDEPRECATED("Subclass ModbusClientDevice and override on_response()/on_e virtual void on_modbus_error(uint8_t function_code, uint8_t exception_code) {} void on_response(std::span request_pdu, std::span response_pdu) override { - auto payload = helpers::server_pdu_payload(response_pdu); + // Custom (user-defined) function codes historically delivered the payload starting AT the function + // code byte (frame data_offset 1). server_pdu_payload() drops that byte, so pass the whole PDU for + // them - external components match the first byte against the code they sent (issue #17994). + auto payload = !response_pdu.empty() && helpers::is_function_code_custom(response_pdu[0]) + ? response_pdu + : helpers::server_pdu_payload(response_pdu); this->on_modbus_data(std::vector(payload.begin(), payload.end())); } void on_error(std::span request_pdu, ExceptionCode exception_code) override { diff --git a/tests/components/modbus/modbus_client_device_test.cpp b/tests/components/modbus/modbus_client_device_test.cpp index 8638c37688..38c28ce2df 100644 --- a/tests/components/modbus/modbus_client_device_test.cpp +++ b/tests/components/modbus/modbus_client_device_test.cpp @@ -341,4 +341,45 @@ TEST(ModbusTypedDispatch, SingleWriteAckPrefersTheResponseEcho) { EXPECT_EQ(device.write_single_register_calls.back().value, 0x002A); // exception: request copy } +// Deprecated on_modbus_data() compatibility shim (pre-2026.8 API). Records the vectors delivered to +// the old callback so we can pin its payload framing against the pre-2026.7 behavior. +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +namespace { +class LegacyDevice : public ModbusDevice { + public: + void on_modbus_data(const std::vector &data) override { this->data_calls.push_back(data); } + void on_modbus_error(uint8_t function_code, uint8_t exception_code) override { + this->error_calls.emplace_back(function_code, exception_code); + } + std::vector> data_calls; + std::vector> error_calls; +}; +} // namespace + +// Custom (user-defined) function codes historically delivered the payload INCLUDING the function code +// byte (frame data_offset 1). External components such as the Century VS pump match that first byte +// against the code they sent, so dropping it (issue #17994) makes every response get ignored. +TEST(ModbusLegacyShim, CustomFunctionCodeKeepsFunctionCodeByte) { + LegacyDevice device; + const uint8_t request[] = {0x45, 0x01, 0x02}; // custom function 0x45 + const uint8_t response[] = {0x45, 0xAA, 0xBB, 0xCC}; // echo of the custom code + data + device.on_response(request, response); + + ASSERT_EQ(device.data_calls.size(), 1u); + EXPECT_EQ(device.data_calls.front(), (std::vector{0x45, 0xAA, 0xBB, 0xCC})); +} + +// Standard reads still strip the function code and byte-count header, matching the pre-2026.7 shim. +TEST(ModbusLegacyShim, StandardReadStripsHeader) { + LegacyDevice device; + const uint8_t request[] = {0x03, 0x01, 0x00, 0x00, 0x02}; + const uint8_t response[] = {0x03, 0x04, 0x00, 0x2A, 0x01, 0x00}; + device.on_response(request, response); + + ASSERT_EQ(device.data_calls.size(), 1u); + EXPECT_EQ(device.data_calls.front(), (std::vector{0x00, 0x2A, 0x01, 0x00})); +} +#pragma GCC diagnostic pop + } // namespace esphome::modbus::testing