From 574e9f4687dfffdc8236063660180ae7187a555b Mon Sep 17 00:00:00 2001 From: clydebarrow <2366188+clydebarrow@users.noreply.github.com> Date: Sun, 18 Oct 2026 18:06:54 +1100 Subject: [PATCH] [packet_interface] Keep the UART receive buffer in static memory, fix clang-tidy - Code generation declares the UART link's receive buffer as a static array of max_packet_size bytes, so it is not allocated at run time - Move the increment out of the condition in note_received_() - Initialise the UART transmit copy buffer Co-Authored-By: Claude Opus 5.5 --- .../packet_interface/packet_interface.cpp | 9 ++++++--- .../components/uart/packet_interface/__init__.py | 16 ++++++++++++++-- .../packet_interface/uart_packet_interface.cpp | 6 ++---- .../packet_interface/uart_packet_interface.h | 9 ++++----- tests/components/uart/uart_packet_interface.cpp | 4 +++- 5 files changed, 29 insertions(+), 15 deletions(-) diff --git a/esphome/components/packet_interface/packet_interface.cpp b/esphome/components/packet_interface/packet_interface.cpp index e73540e1f15..d9a9c9bfdad 100644 --- a/esphome/components/packet_interface/packet_interface.cpp +++ b/esphome/components/packet_interface/packet_interface.cpp @@ -28,9 +28,12 @@ void PacketInterface::on_receive_from_interface_(const PacketBuffer &data, const void PacketInterface::note_received_(bool understood) { if (understood) { this->not_understood_count_ = 0; - } else if (this->not_understood_count_ < MISMATCH_COUNT && ++this->not_understood_count_ == MISMATCH_COUNT) { - ESP_LOGW(TAG, "%s: %u packets in a row could not be understood; check that both ends have the same layers", - this->name_, MISMATCH_COUNT); + } else if (this->not_understood_count_ < MISMATCH_COUNT) { + this->not_understood_count_++; + if (this->not_understood_count_ == MISMATCH_COUNT) { + ESP_LOGW(TAG, "%s: %u packets in a row could not be understood; check that both ends have the same layers", + this->name_, MISMATCH_COUNT); + } } } } // namespace esphome::packet_interface diff --git a/esphome/components/uart/packet_interface/__init__.py b/esphome/components/uart/packet_interface/__init__.py index 1a5991125cd..0564725c2b1 100644 --- a/esphome/components/uart/packet_interface/__init__.py +++ b/esphome/components/uart/packet_interface/__init__.py @@ -1,5 +1,10 @@ +import esphome.codegen as cg from esphome.components import packet_interface, uart -from esphome.components.packet_interface import packet_interface_schema +from esphome.components.packet_interface import ( + CONF_MAX_PACKET_SIZE, + packet_interface_schema, +) +from esphome.const import CONF_ID from esphome.types import ConfigType from .. import uart_ns @@ -20,5 +25,12 @@ CONFIG_SCHEMA = packet_interface_schema(UartPacketInterface, 508).extend( async def to_code(config: ConfigType) -> None: - var = await packet_interface.new_packet_interface(config) + # The receive buffer's size is known here, so it is a static array rather than allocated at run time + rx_buffer = f"{config[CONF_ID].id}__rx_buffer" + cg.add_global( + cg.RawStatement(f"static uint8_t {rx_buffer}[{config[CONF_MAX_PACKET_SIZE]}];") + ) + var = await packet_interface.new_packet_interface( + config, cg.RawExpression(rx_buffer) + ) await uart.register_uart_device(var, config) diff --git a/esphome/components/uart/packet_interface/uart_packet_interface.cpp b/esphome/components/uart/packet_interface/uart_packet_interface.cpp index a6978e79e0d..9669df530b1 100644 --- a/esphome/components/uart/packet_interface/uart_packet_interface.cpp +++ b/esphome/components/uart/packet_interface/uart_packet_interface.cpp @@ -14,8 +14,6 @@ static constexpr uint8_t ESCAPE_XOR = 0x20; // Raw bytes escaped and written per UART write static constexpr size_t TX_CHUNK_SIZE = 32; -void UartPacketInterface::setup() { this->rx_buffer_ = std::make_unique(this->max_packet_size_); } - void UartPacketInterface::loop() { while (this->available()) { uint8_t byte; @@ -26,7 +24,7 @@ void UartPacketInterface::loop() { if (byte == FLAG_BYTE) { // A flag that ends a frame also starts the next, so an unfinished frame cannot hide the one after it if (this->rx_started_ && this->rx_length_ != 0) - this->on_receive_from_interface_(PacketBuffer(this->rx_buffer_.get(), this->rx_length_), {}); + this->on_receive_from_interface_(PacketBuffer(this->rx_buffer_, this->rx_length_), {}); this->rx_started_ = true; this->rx_length_ = 0; this->rx_control_ = false; @@ -60,7 +58,7 @@ bool UartPacketInterface::send_to_interface(const PacketBuffer &data, const Pack ESP_LOGW(TAG, "Packet of %zu bytes is larger than %zu", size, this->max_packet_size_); return false; } - uint8_t raw[TX_CHUNK_SIZE]; + uint8_t raw[TX_CHUNK_SIZE]{}; // Every byte may be escaped, plus a flag byte at each end uint8_t out[TX_CHUNK_SIZE * 2 + 2]; size_t out_len = 0; diff --git a/esphome/components/uart/packet_interface/uart_packet_interface.h b/esphome/components/uart/packet_interface/uart_packet_interface.h index a4145164fa6..ce089dbcdfe 100644 --- a/esphome/components/uart/packet_interface/uart_packet_interface.h +++ b/esphome/components/uart/packet_interface/uart_packet_interface.h @@ -1,7 +1,5 @@ #pragma once -#include - #include "esphome/components/packet_interface/packet_interface.h" #include "esphome/components/uart/uart.h" @@ -16,14 +14,15 @@ namespace esphome::uart { */ class UartPacketInterface : public packet_interface::PacketInterface, public UARTDevice { public: - void setup() override; + /// @param rx_buffer Holds max_packet_size bytes of unescaped received data; must outlive the link + explicit UartPacketInterface(uint8_t *rx_buffer) : rx_buffer_(rx_buffer) {} + void loop() override; bool send_to_interface(const packet_interface::PacketBuffer &data, const packet_interface::PacketMetaData &meta_data) override; protected: - // Holds max_packet_size_ bytes of unescaped received data - std::unique_ptr rx_buffer_; + uint8_t *const rx_buffer_; size_t rx_length_{0}; bool rx_started_{}; bool rx_control_{}; diff --git a/tests/components/uart/uart_packet_interface.cpp b/tests/components/uart/uart_packet_interface.cpp index 3b65dce3dc4..0ccce65e591 100644 --- a/tests/components/uart/uart_packet_interface.cpp +++ b/tests/components/uart/uart_packet_interface.cpp @@ -16,7 +16,9 @@ using packet_interface::PacketMetaData; class UartPacketInterfaceTest : public ::testing::Test { protected: MockUARTComponent uart_{}; - UartPacketInterface interface_{}; + // As large as the default max_packet_size; tests only make it smaller + uint8_t rx_buffer_[512]{}; + UartPacketInterface interface_{this->rx_buffer_}; std::vector> received_packets_; void SetUp() override {