From 42be5d892aedf1ee2960c49b1fc0420255448c2f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 11 Jul 2025 10:58:05 -1000 Subject: [PATCH] cleanup --- esphome/components/api/api_connection.cpp | 11 ++++++--- esphome/components/api/api_connection.h | 6 ++--- esphome/components/api/api_frame_helper.cpp | 22 ++++++++++++++--- esphome/components/api/api_frame_helper.h | 27 ++++++++++++++++----- 4 files changed, 51 insertions(+), 15 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 49f14c171b..0a6bc7c9cd 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -201,7 +201,7 @@ void APIConnection::loop() { #ifdef USE_CAMERA if (this->image_reader_ && this->image_reader_->available() && this->helper_->can_write_without_blocking()) { - uint32_t to_send = std::min((size_t) MAX_PACKET_SIZE, this->image_reader_->available()); + uint32_t to_send = std::min((size_t) MAX_BATCH_PACKET_SIZE, this->image_reader_->available()); bool done = this->image_reader_->available() == to_send; uint32_t msg_size = 0; ProtoSize::add_fixed_field<4>(msg_size, 1, true); @@ -1614,6 +1614,11 @@ bool APIConnection::send_buffer(ProtoWriteBuffer buffer, uint8_t message_type) { if (err == APIError::WOULD_BLOCK) return false; if (err != APIError::OK) { + if (err == APIError::MESSAGE_TOO_LARGE) { + // Log error for oversized messages - safe here since we're not in the middle of encoding + ESP_LOGE(TAG, "%s: Message type %u is too large to send (exceeds %u byte limit)", + this->get_client_combined_info().c_str(), message_type, PacketInfo::MAX_PAYLOAD_SIZE); + } on_fatal_error(); if (err == APIError::SOCKET_WRITE_FAILED && errno == ECONNRESET) { ESP_LOGW(TAG, "%s: Connection reset", this->get_client_combined_info().c_str()); @@ -1771,9 +1776,9 @@ void APIConnection::process_batch_() { // Update tracking variables items_processed++; - // After first message, set remaining size to MAX_PACKET_SIZE to avoid fragmentation + // After first message, set remaining size to MAX_BATCH_PACKET_SIZE to avoid fragmentation if (items_processed == 1) { - remaining_size = MAX_PACKET_SIZE; + remaining_size = MAX_BATCH_PACKET_SIZE; } remaining_size -= payload_size; // Calculate where the next message's header padding will start diff --git a/esphome/components/api/api_connection.h b/esphome/components/api/api_connection.h index 83a8c10e43..fdc2fb3529 100644 --- a/esphome/components/api/api_connection.h +++ b/esphome/components/api/api_connection.h @@ -628,7 +628,7 @@ class APIConnection : public APIServerConnection { // to send in one go. This is the maximum size of a single packet // that can be sent over the network. // This is to avoid fragmentation of the packet. - static constexpr size_t MAX_PACKET_SIZE = 1390; // MTU + static constexpr size_t MAX_BATCH_PACKET_SIZE = 1390; // MTU bool schedule_batch_(); void process_batch_(); @@ -641,7 +641,7 @@ class APIConnection : public APIServerConnection { // Helper to log a proto message from a MessageCreator object void log_proto_message_(EntityBase *entity, const MessageCreator &creator, uint8_t message_type) { this->flags_.log_only_mode = true; - creator(entity, this, MAX_PACKET_SIZE, true, message_type); + creator(entity, this, MAX_BATCH_PACKET_SIZE, true, message_type); this->flags_.log_only_mode = false; } @@ -661,7 +661,7 @@ class APIConnection : public APIServerConnection { if (this->flags_.should_try_send_immediately && this->get_batch_delay_ms_() == 0 && this->helper_->can_write_without_blocking()) { // Now actually encode and send - if (creator(entity, this, MAX_PACKET_SIZE, true) && + if (creator(entity, this, MAX_BATCH_PACKET_SIZE, true) && this->send_buffer(ProtoWriteBuffer{&this->parent_->get_shared_buffer_ref()}, message_type)) { #ifdef HAS_PROTO_MESSAGE_DUMP // Log the message in verbose mode diff --git a/esphome/components/api/api_frame_helper.cpp b/esphome/components/api/api_frame_helper.cpp index 156fd42cb3..89193ed496 100644 --- a/esphome/components/api/api_frame_helper.cpp +++ b/esphome/components/api/api_frame_helper.cpp @@ -62,6 +62,8 @@ const char *api_error_to_str(APIError err) { return "BAD_HANDSHAKE_ERROR_BYTE"; } else if (err == APIError::CONNECTION_CLOSED) { return "CONNECTION_CLOSED"; + } else if (err == APIError::MESSAGE_TOO_LARGE) { + return "MESSAGE_TOO_LARGE"; } return "UNKNOWN"; } @@ -616,8 +618,15 @@ APIError APINoiseFrameHelper::read_packet(ReadPacketBuffer *buffer) { APIError APINoiseFrameHelper::write_protobuf_packet(uint8_t type, ProtoWriteBuffer buffer) { // Resize to include MAC space (required for Noise encryption) buffer.get_buffer()->resize(buffer.get_buffer()->size() + frame_footer_size_); - PacketInfo packet{type, 0, - static_cast(buffer.get_buffer()->size() - frame_header_padding_ - frame_footer_size_)}; + uint16_t payload_size = + static_cast(buffer.get_buffer()->size() - frame_header_padding_ - frame_footer_size_); + + // Check if message exceeds PacketInfo limits + if (payload_size > PacketInfo::MAX_PAYLOAD_SIZE) { + return APIError::MESSAGE_TOO_LARGE; + } + + PacketInfo packet{type, 0, payload_size}; return write_protobuf_packets(buffer, std::span(&packet, 1)); } @@ -1003,7 +1012,14 @@ APIError APIPlaintextFrameHelper::read_packet(ReadPacketBuffer *buffer) { return APIError::OK; } APIError APIPlaintextFrameHelper::write_protobuf_packet(uint8_t type, ProtoWriteBuffer buffer) { - PacketInfo packet{type, 0, static_cast(buffer.get_buffer()->size() - frame_header_padding_)}; + uint16_t payload_size = static_cast(buffer.get_buffer()->size() - frame_header_padding_); + + // Check if message exceeds PacketInfo limits + if (payload_size > PacketInfo::MAX_PAYLOAD_SIZE) { + return APIError::MESSAGE_TOO_LARGE; + } + + PacketInfo packet{type, 0, payload_size}; return write_protobuf_packets(buffer, std::span(&packet, 1)); } diff --git a/esphome/components/api/api_frame_helper.h b/esphome/components/api/api_frame_helper.h index 804768dff7..fedd24ed58 100644 --- a/esphome/components/api/api_frame_helper.h +++ b/esphome/components/api/api_frame_helper.h @@ -1,4 +1,5 @@ #pragma once +#include #include #include #include @@ -28,14 +29,27 @@ struct ReadPacketBuffer { uint16_t data_len; }; -// Packet info structure +// Packet info structure - packed into 4 bytes using bit fields +// Note: While the API protocol supports message sizes up to 65535 (uint16_t), +// we limit payload_size and offset to 4095 (12 bits) for practical reasons: +// 1. Messages larger than 4095 bytes cannot be sent immediately +// 2. They will be buffered, potentially filling up the tx buffer +// 3. Large messages risk network fragmentation issues +// 4. The typical MTU-based batch size (MAX_BATCH_PACKET_SIZE) is 1390 bytes +// This limitation provides a good balance between efficiency and practicality. struct PacketInfo { - uint16_t offset; // 2 bytes (sufficient for packet size ~1460 bytes) - uint16_t payload_size; // 2 bytes (up to 65535 bytes) - uint8_t message_type; // 1 byte (max 255 message types) - // Total: 5 bytes, compiler adds 3 bytes padding for alignment (8 bytes total) + static constexpr uint16_t MAX_OFFSET = 4095; // 12 bits max + static constexpr uint16_t MAX_PAYLOAD_SIZE = 4095; // 12 bits max - PacketInfo(uint8_t type, uint16_t off, uint16_t size) : offset(off), payload_size(size), message_type(type) {} + uint32_t offset : 12; // 12 bits: 0-4095 + uint32_t payload_size : 12; // 12 bits: 0-4095 + uint32_t message_type : 8; // 8 bits: 0-255 + // Total: 32 bits = 4 bytes exactly + + PacketInfo(uint8_t type, uint16_t off, uint16_t size) : offset(off), payload_size(size), message_type(type) { + assert(off <= MAX_OFFSET); + assert(size <= MAX_PAYLOAD_SIZE); + } }; enum class APIError : uint16_t { @@ -62,6 +76,7 @@ enum class APIError : uint16_t { HANDSHAKESTATE_SPLIT_FAILED = 1020, BAD_HANDSHAKE_ERROR_BYTE = 1021, CONNECTION_CLOSED = 1022, + MESSAGE_TOO_LARGE = 1023, }; const char *api_error_to_str(APIError err);