diff --git a/esphome/components/modbus/modbus.cpp b/esphome/components/modbus/modbus.cpp index 80065546cf..a79a7c3bd8 100644 --- a/esphome/components/modbus/modbus.cpp +++ b/esphome/components/modbus/modbus.cpp @@ -950,7 +950,7 @@ bool ModbusDeviceCommand::sent() { // on_sent() is not a terminal, so nothing is consumed. if (this->device == nullptr) return false; - this->device->on_sent(this->frame.pdu()); + this->device->on_sent(this->frame.address(), this->frame.pdu()); return true; } @@ -958,7 +958,7 @@ bool ModbusDeviceCommand::notify_retired() { if (!this->decrement_pending()) return false; // nothing owed - stop the sweep draining this entry if (this->device != nullptr) - this->device->on_not_sent(this->frame.pdu()); + this->device->on_not_sent(this->frame.address(), this->frame.pdu()); return true; // consumed one debt (delivered, or silent when device-less) - keep draining to zero } @@ -969,7 +969,7 @@ bool ModbusDeviceCommand::response(std::span response_pdu) { this->decrement_pending(); if (this->device == nullptr) return false; - this->device->on_response(this->frame.pdu(), response_pdu); + this->device->on_response(this->frame.address(), this->frame.pdu(), response_pdu); return true; } @@ -979,7 +979,7 @@ bool ModbusDeviceCommand::error(ExceptionCode exception_code) { this->decrement_pending(); if (this->device == nullptr) return false; - this->device->on_error(this->frame.pdu(), exception_code); + this->device->on_error(this->frame.address(), this->frame.pdu(), exception_code); return true; } @@ -1004,7 +1004,7 @@ bool ModbusDeviceCommand::timed_out() { return false; // resolved, no one to tell // A cleared frame that timed out still honors a retry: the clear is address-scoped (any device may // call it) while the retry is the owning device's call via on_no_response - the bus obeys the owner. - if (this->device->on_no_response(this->frame.pdu())) + if (this->device->on_no_response(this->frame.address(), this->frame.pdu())) this->increment_pending(); // granted retry = re-request (capped) return true; } diff --git a/esphome/components/modbus/modbus.h b/esphome/components/modbus/modbus.h index 298cd9f527..011813a20d 100644 --- a/esphome/components/modbus/modbus.h +++ b/esphome/components/modbus/modbus.h @@ -437,6 +437,21 @@ class ModbusClientDevice { return this->on_modbus_no_response(); #pragma GCC diagnostic pop } + /// Overloads of the hooks above with the device address of the request first, taken from the request frame, so it + /// stays correct after set_address(). The hub calls these; the defaults forward to the hooks above. + /// Override one form per hook: overriding an address form drops its forward, so the other form is no longer called. + virtual void on_response(uint8_t address, std::span request_pdu, + std::span response_pdu) { + this->on_response(request_pdu, response_pdu); + } + virtual void on_error(uint8_t address, std::span request_pdu, ExceptionCode exception_code) { + this->on_error(request_pdu, exception_code); + } + virtual void on_not_sent(uint8_t address, std::span request_pdu) { this->on_not_sent(request_pdu); } + virtual void on_sent(uint8_t address, std::span request_pdu) { this->on_sent(request_pdu); } + virtual bool on_no_response(uint8_t address, std::span request_pdu) { + return this->on_no_response(request_pdu); + } // Remove before 2027.2.0 ESPDEPRECATED("Override on_not_sent() instead. Removed in 2027.2.0", "2026.8.0") virtual void on_modbus_not_sent() {} diff --git a/tests/components/modbus/modbus_client_hub_test.cpp b/tests/components/modbus/modbus_client_hub_test.cpp index 3bdfa094e0..1e351b4971 100644 --- a/tests/components/modbus/modbus_client_hub_test.cpp +++ b/tests/components/modbus/modbus_client_hub_test.cpp @@ -2436,4 +2436,113 @@ TEST(ModbusClientHubPriority, ReadModifyWritesRankAsWrites) { EXPECT_EQ(next->priority(), CommandPriority::WRITE); // 0x16 wins selection over the queued read EXPECT_EQ(next->frame.pdu()[0], 0x16); } +namespace { +// Overrides only the address overloads and records the address each one is given. +class AddressRecordingDevice : public ModbusClientDevice { + public: + AddressRecordingDevice(ModbusClientHub *hub, uint8_t address) : ModbusClientDevice(hub, address) {} + void on_sent(uint8_t address, std::span request_pdu) override { this->sent_ = address; } + void on_response(uint8_t address, std::span request_pdu, + std::span response_pdu) override { + this->response_ = address; + } + void on_error(uint8_t address, std::span request_pdu, ExceptionCode exception_code) override { + this->error_ = address; + } + bool on_no_response(uint8_t address, std::span request_pdu) override { + this->no_response_ = address; + return this->retry_; + } + void on_not_sent(uint8_t address, std::span request_pdu) override { this->not_sent_ = address; } + int sent_{-1}; + int response_{-1}; + int error_{-1}; + int no_response_{-1}; + int not_sent_{-1}; + bool retry_{false}; +}; +} // namespace + +// Each callback gets the address its request was sent to, even when the device has changed its own +// address before the outcome arrives. +TEST(ModbusClientHubAddress, CallbacksReportTheRequestAddressAfterSetAddress) { + const uint8_t ok_response[] = {0x03, 0x04, 0x00, 0x2A, 0x01, 0x00}; + const uint8_t exception_response[] = {0x83, 0x02}; + { + NullUART uart; + NoResponseProbeHub hub; + hub.set_uart_parent(&uart); + hub.setup(); + AddressRecordingDevice device(&hub, 0x02); + ASSERT_TRUE(device.queue_pdu(read_pdu())); + device.set_address(0x05); + hub.send_next_for_test(); + EXPECT_EQ(device.sent_, 0x02); + hub.receive_frame_for_test(0x02, ok_response); + EXPECT_EQ(device.response_, 0x02); + } + { + NoResponseProbeHub hub; + AddressRecordingDevice device(&hub, 0x03); + ASSERT_TRUE(device.queue_pdu(read_pdu())); + device.set_address(0x05); + hub.force_send_next(); + hub.receive_frame_for_test(0x03, exception_response); + EXPECT_EQ(device.error_, 0x03); + } + { + NoResponseProbeHub hub; + AddressRecordingDevice device(&hub, 0x04); + ASSERT_TRUE(device.queue_pdu(read_pdu())); + device.set_address(0x05); + hub.force_send_next(); + hub.timeout_waiting(); + EXPECT_EQ(device.no_response_, 0x04); + } + { + NoResponseProbeHub hub; + AddressRecordingDevice device(&hub, 0x06); + ASSERT_TRUE(device.queue_pdu(read_pdu())); + device.set_address(0x05); + hub.clear_tx_queue_for_address(0x06); + hub.sweep_for_test(); + EXPECT_EQ(device.not_sent_, 0x06); + } +} + +// A granted retry resends the same frame, so the retry's callbacks keep the original address too. +TEST(ModbusClientHubAddress, GrantedRetryKeepsTheRequestAddress) { + NullUART uart; + NoResponseProbeHub hub; + hub.set_uart_parent(&uart); + hub.setup(); + AddressRecordingDevice device(&hub, 0x04); + device.retry_ = true; + ASSERT_TRUE(device.queue_pdu(read_pdu())); + device.set_address(0x05); + hub.send_next_for_test(); + hub.timeout_waiting(); + EXPECT_EQ(device.no_response_, 0x04); + + device.sent_ = -1; + hub.send_next_for_test(); + EXPECT_EQ(device.sent_, 0x04); + const uint8_t ok_response[] = {0x03, 0x04, 0x00, 0x2A, 0x01, 0x00}; + hub.receive_frame_for_test(0x04, ok_response); + EXPECT_EQ(device.response_, 0x04); +} + +// A broadcast reports address 0 from on_sent, its only callback. +TEST(ModbusClientHubAddress, BroadcastReportsAddressZero) { + NullUART uart; + NoResponseProbeHub hub; + hub.set_uart_parent(&uart); + hub.setup(); + AddressRecordingDevice device(&hub, BROADCAST_ADDRESS); + const uint8_t write[] = {0x06, 0x00, 0x10, 0x00, 0x01}; + ASSERT_TRUE(device.queue_pdu(write)); + device.set_address(0x05); + hub.send_next_for_test(); + EXPECT_EQ(device.sent_, BROADCAST_ADDRESS); +} } // namespace esphome::modbus::testing