[modbus] Store ModbusFrame inline to cut per-frame heap churn (#17282)

Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Bonne Eggleston
2026-07-06 16:12:25 -07:00
committed by GitHub
parent 0b311962b5
commit ebff49072e
4 changed files with 146 additions and 26 deletions
+13 -12
View File
@@ -51,7 +51,7 @@ void ModbusClientHub::loop() {
// If we're past the send_wait_time timeout and response buffer doesn't have the start of the expected response
if (this->waiting_for_response_.has_value()) {
ModbusDeviceCommand &wfr = this->waiting_for_response_.value();
uint8_t expected_address = wfr.frame.data.get()[0];
uint8_t expected_address = wfr.frame.data.data()[0];
if (this->last_receive_check_ - this->last_send_ > this->last_send_tx_offset_ + this->send_wait_time_ &&
(this->rx_buffer_.empty() || this->rx_buffer_[0] != expected_address)) {
ESP_LOGW(TAG, "Stop waiting for response from %" PRIu8 " %" PRIu32 "ms after last send", expected_address,
@@ -270,8 +270,8 @@ void ModbusClientHub::process_modbus_server_frame(uint8_t address, uint8_t funct
// Check if the response matches the expected address and function code
ModbusDeviceCommand &wfr = this->waiting_for_response_.value();
uint8_t expected_address = wfr.frame.data.get()[0];
uint8_t expected_function_code = wfr.frame.data.get()[1];
uint8_t expected_address = wfr.frame.data.data()[0];
uint8_t expected_function_code = wfr.frame.data.data()[1];
if (expected_address != address || expected_function_code != (function_code & FUNCTION_CODE_MASK)) {
ESP_LOGW(TAG,
"Received incorrect frame address %" PRIu8 " <> %" PRIu8 " or function code 0x%X <> 0x%X, %" PRIu32
@@ -458,7 +458,7 @@ bool Modbus::send_frame_(const ModbusFrame &frame) {
ESP_LOGE(TAG, "Attempted to send while transmission blocked");
return false;
}
if (frame.size > MAX_FRAME_SIZE) {
if (frame.size() > MAX_FRAME_SIZE) {
ESP_LOGE(TAG, "Attempted to send frame larger than max frame size of %" PRIu16 " bytes", MAX_FRAME_SIZE);
return false;
}
@@ -470,13 +470,13 @@ bool Modbus::send_frame_(const ModbusFrame &frame) {
if (this->flow_control_pin_ != nullptr) {
this->flow_control_pin_->digital_write(true);
this->write_array(frame.data.get(), frame.size);
this->write_array(frame.data.data(), frame.size());
this->flush();
this->flow_control_pin_->digital_write(false);
this->last_send_tx_offset_ = 0;
} else {
this->write_array(frame.data.get(), frame.size);
this->last_send_tx_offset_ = frame.size * MODBUS_BITS_PER_CHAR * MS_PER_SEC / this->parent_->get_baud_rate() + 1;
this->write_array(frame.data.data(), frame.size());
this->last_send_tx_offset_ = frame.size() * MODBUS_BITS_PER_CHAR * MS_PER_SEC / this->parent_->get_baud_rate() + 1;
}
uint32_t now = millis();
@@ -484,7 +484,7 @@ bool Modbus::send_frame_(const ModbusFrame &frame) {
char hex_buf[format_hex_pretty_size(MODBUS_MAX_LOG_BYTES)];
#endif
ESP_LOGV(TAG, "Write: %s %" PRIu32 "ms after last send, %" PRIu32 "ms after last receive",
format_hex_pretty_to(hex_buf, frame.data.get(), frame.size), now - this->last_send_,
format_hex_pretty_to(hex_buf, frame.data.data(), frame.size()), now - this->last_send_,
now - this->last_modbus_byte_);
this->last_send_ = now;
return true;
@@ -590,12 +590,13 @@ void ModbusClientHub::queue_raw_(uint8_t address, const uint8_t *pdu, uint16_t p
void ModbusClientHub::clear_tx_queue_for_address(uint8_t address, bool clear_sent) {
// Remove any pending commands for this address from the tx buffer
auto &tx_buffer = this->tx_buffer_;
tx_buffer.erase(std::remove_if(tx_buffer.begin(), tx_buffer.end(),
[address](const ModbusDeviceCommand &cmd) { return cmd.frame.data[0] == address; }),
tx_buffer.end());
tx_buffer.erase(
std::remove_if(tx_buffer.begin(), tx_buffer.end(),
[address](const ModbusDeviceCommand &cmd) { return cmd.frame.data.data()[0] == address; }),
tx_buffer.end());
if (clear_sent && this->waiting_for_response_.has_value() && this->waiting_for_response_.value().device) {
if (this->waiting_for_response_.value().frame.data[0] == address) {
if (this->waiting_for_response_.value().frame.data.data()[0] == address) {
ESP_LOGV(TAG, "Clearing waiting for response for address %" PRIu8, address);
// Invalidate the waiting device so it won't process a response.
this->waiting_for_response_.value().device = nullptr;
+19 -11
View File
@@ -18,19 +18,27 @@ namespace esphome::modbus {
static constexpr uint16_t MODBUS_TX_BUFFER_SIZE = 15;
static constexpr uint16_t MODBUS_TX_MAX_DELAY_MS = 5;
struct ModbusFrame {
// Frame with exact-size allocation to avoid std::vector overhead
std::unique_ptr<uint8_t[]> data;
uint16_t size; // Modbus RTU max is 256 bytes
// Typical frames -- reads and single-register/coil writes -- are exactly 8 bytes
// (address + 5-byte PDU + 2-byte CRC) and fit inline with no heap allocation.
static constexpr uint16_t MODBUS_FRAME_INLINE_SIZE = 8;
ModbusFrame(uint8_t address, const uint8_t *pdu, uint16_t pdu_len)
: data(std::make_unique<uint8_t[]>(pdu_len + 3)), size(pdu_len + 3) {
data[0] = address;
memcpy(data.get() + 1, pdu, pdu_len);
auto crc = crc16(data.get(), pdu_len + 1);
data[pdu_len + 1] = crc >> 0;
data[pdu_len + 2] = crc >> 8;
struct ModbusFrame {
// Frame held in a small-buffer-optimized buffer. Typical frames fit inline; only larger
// multi-register or custom frames spill to a single heap allocation. This keeps the common,
// high-frequency tx traffic off the heap entirely, avoiding per-frame alloc/free churn.
// The buffer tracks its own length, so no separate size field is needed.
SmallInlineBuffer<MODBUS_FRAME_INLINE_SIZE> data; // Modbus RTU max is 256 bytes
ModbusFrame(uint8_t address, const uint8_t *pdu, uint16_t pdu_len) {
uint8_t *buf = this->data.init(pdu_len + 3);
buf[0] = address;
memcpy(buf + 1, pdu, pdu_len);
auto crc = crc16(buf, pdu_len + 1);
buf[pdu_len + 1] = crc >> 0;
buf[pdu_len + 2] = crc >> 8;
}
uint16_t size() const { return static_cast<uint16_t>(this->data.size()); }
};
class Modbus : public uart::UARTDevice, public Component {
+8 -3
View File
@@ -184,8 +184,10 @@ template<size_t InlineSize = 8> class SmallInlineBuffer {
SmallInlineBuffer(const SmallInlineBuffer &) = delete;
SmallInlineBuffer &operator=(const SmallInlineBuffer &) = delete;
/// Set buffer contents, allocating heap if needed
void set(const uint8_t *src, size_t size) {
/// Resize to `size` bytes of (uninitialized) storage and return a writable pointer to fill.
/// Allocates heap only when `size` exceeds the inline capacity. Use this when the contents are
/// built in place (e.g. assembling a frame and appending a checksum) to avoid a staging copy.
uint8_t *init(size_t size) {
// Free existing heap allocation if switching from heap to inline or different heap size
if (!this->is_inline_() && (size <= InlineSize || size != this->len_)) {
delete[] this->heap_;
@@ -196,9 +198,12 @@ template<size_t InlineSize = 8> class SmallInlineBuffer {
this->heap_ = new uint8_t[size]; // NOLINT(cppcoreguidelines-owning-memory)
}
this->len_ = size;
memcpy(this->data(), src, size);
return this->data();
}
/// Set buffer contents, allocating heap if needed
void set(const uint8_t *src, size_t size) { memcpy(this->init(size), src, size); }
uint8_t *data() { return this->is_inline_() ? this->inline_ : this->heap_; }
const uint8_t *data() const { return this->is_inline_() ? this->inline_ : this->heap_; }
size_t size() const { return this->len_; }
+106
View File
@@ -0,0 +1,106 @@
#include <gtest/gtest.h>
#include <atomic>
#include <cstdint>
#include <cstdlib>
#include <cstring>
#include <span>
#include <vector>
#include "esphome/components/modbus/modbus.h"
// The allocation counters rely on AddressSanitizer's malloc hooks. The cpp_unit_test harness always
// builds with ASan, so this is exercised in CI; the fallback only applies to out-of-harness builds.
#ifndef __has_feature
#define __has_feature(x) 0
#endif
#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)
#define HEAP_PROBE_HAS_ASAN
#endif
#ifdef HEAP_PROBE_HAS_ASAN
// Allocation counters fed by ASan's malloc hooks; sampled tightly around the calls under test.
static std::atomic<size_t> g_alloc_count{0};
static std::atomic<size_t> g_alloc_bytes{0};
static void malloc_hook(const volatile void *, size_t size) {
g_alloc_count++;
g_alloc_bytes += size;
}
static void free_hook(const volatile void *) {}
extern "C" int __sanitizer_install_malloc_and_free_hooks(void (*malloc_hook)(const volatile void *, size_t),
void (*free_hook)(const volatile void *));
[[maybe_unused]] static const int g_hooks_installed = __sanitizer_install_malloc_and_free_hooks(malloc_hook, free_hook);
namespace esphome::modbus::testing {
namespace {
struct Sample {
size_t count;
size_t bytes;
};
template<typename F> Sample sample(F &&f) {
size_t c0 = g_alloc_count.load(), b0 = g_alloc_bytes.load();
f();
return {g_alloc_count.load() - c0, g_alloc_bytes.load() - b0};
}
} // namespace
// Typical frames (reads and single-register/coil writes are exactly address + 5-byte PDU + CRC = 8
// bytes) fit the SmallInlineBuffer and are built with zero heap allocations; only larger frames spill
// to a single allocation.
TEST(HeapProbe, TypicalFrameConstructionIsAllocationFree) {
const uint8_t read_pdu[] = {0x03, 0x01, 0x00, 0x00, 0x02}; // 5 bytes -> 8-byte frame, inline
Sample typical = sample([&] {
ModbusFrame frame(0x02, read_pdu, sizeof(read_pdu));
(void) frame;
});
printf("HEAPPROBE frame_typical count=%zu bytes=%zu\n", typical.count, typical.bytes);
EXPECT_EQ(typical.count, 0u);
uint8_t large_pdu[250] = {0x10}; // multi-register write -> 253-byte frame, spills once
Sample large = sample([&] {
ModbusFrame frame(0x02, large_pdu, sizeof(large_pdu));
(void) frame;
});
printf("HEAPPROBE frame_large count=%zu bytes=%zu\n", large.count, large.bytes);
EXPECT_EQ(large.count, 1u);
}
// Queueing typical commands is fully allocation-free: the frame fits the inline buffer and the tx
// deque's first block is already allocated when the hub is constructed. (A queue deeper than one
// deque block - roughly a dozen commands - would allocate further blocks.)
TEST(HeapProbe, QueueingTypicalCommandsIsAllocationFree) {
ModbusClientHub hub;
ModbusClientDevice device(&hub, 0x02);
StaticVector<uint8_t, MAX_PDU_SIZE> req;
const uint8_t read_pdu[] = {0x03, 0x01, 0x00, 0x00, 0x02};
req.assign(read_pdu, read_pdu + sizeof(read_pdu));
constexpr int n = 12;
size_t total = 0;
for (int i = 0; i != n; i++) {
total += sample([&] { device.send_pdu(req); }).count;
}
printf("HEAPPROBE queue_%d_typical_commands total_allocs=%zu\n", n, total);
EXPECT_EQ(total, 0u);
}
} // namespace esphome::modbus::testing
#else // !HEAP_PROBE_HAS_ASAN
namespace esphome::modbus::testing {
TEST(HeapProbe, TypicalFrameConstructionIsAllocationFree) {
GTEST_SKIP() << "allocation counting requires an AddressSanitizer build";
}
} // namespace esphome::modbus::testing
#endif // HEAP_PROBE_HAS_ASAN