From 86c0fb48a3dc1454cefc8cc31338300e2b971e51 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sat, 28 Jun 2025 16:08:30 -0500 Subject: [PATCH] Replace ping retry timer with batch queue fallback (#9207) --- esphome/components/api/api_connection.cpp | 32 ++++++++++++----------- esphome/components/api/api_connection.h | 15 +++++++++-- esphome/components/api/api_server.cpp | 4 +-- 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 65588ad4d8..f339a4b26f 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -65,10 +65,6 @@ uint32_t APIConnection::get_batch_delay_ms_() const { return this->parent_->get_ void APIConnection::start() { this->last_traffic_ = App.get_loop_component_start_time(); - // Set next_ping_retry_ to prevent immediate ping - // This ensures the first ping happens after the keepalive period - this->next_ping_retry_ = this->last_traffic_ + KEEPALIVE_TIMEOUT_MS; - APIError err = this->helper_->init(); if (err != APIError::OK) { on_fatal_error(); @@ -176,20 +172,15 @@ void APIConnection::loop() { on_fatal_error(); ESP_LOGW(TAG, "%s is unresponsive; disconnecting", this->get_client_combined_info().c_str()); } - } else if (now - this->last_traffic_ > KEEPALIVE_TIMEOUT_MS && now > this->next_ping_retry_) { + } else if (now - this->last_traffic_ > KEEPALIVE_TIMEOUT_MS) { ESP_LOGVV(TAG, "Sending keepalive PING"); this->sent_ping_ = this->send_message(PingRequest()); if (!this->sent_ping_) { - this->next_ping_retry_ = now + PING_RETRY_INTERVAL; - this->ping_retries_++; - if (this->ping_retries_ >= MAX_PING_RETRIES) { - on_fatal_error(); - ESP_LOGE(TAG, "%s: Ping failed %u times", this->get_client_combined_info().c_str(), this->ping_retries_); - } else if (this->ping_retries_ >= 10) { - ESP_LOGW(TAG, "%s: Ping retry %u", this->get_client_combined_info().c_str(), this->ping_retries_); - } else { - ESP_LOGD(TAG, "%s: Ping retry %u", this->get_client_combined_info().c_str(), this->ping_retries_); - } + // If we can't send the ping request directly (tx_buffer full), + // schedule it at the front of the batch so it will be sent with priority + ESP_LOGW(TAG, "Buffer full, ping queued"); + this->schedule_message_front_(nullptr, &APIConnection::try_send_ping_request, PingRequest::MESSAGE_TYPE); + this->sent_ping_ = true; // Mark as sent to avoid scheduling multiple pings } } @@ -1773,6 +1764,11 @@ void APIConnection::DeferredBatch::add_item(EntityBase *entity, MessageCreator c items.emplace_back(entity, std::move(creator), message_type); } +void APIConnection::DeferredBatch::add_item_front(EntityBase *entity, MessageCreator creator, uint16_t message_type) { + // Insert at front for high priority messages (no deduplication check) + items.insert(items.begin(), BatchItem(entity, std::move(creator), message_type)); +} + bool APIConnection::schedule_batch_() { if (!this->deferred_batch_.batch_scheduled) { this->deferred_batch_.batch_scheduled = true; @@ -1963,6 +1959,12 @@ uint16_t APIConnection::try_send_disconnect_request(EntityBase *entity, APIConne return encode_message_to_buffer(req, DisconnectRequest::MESSAGE_TYPE, conn, remaining_size, is_single); } +uint16_t APIConnection::try_send_ping_request(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, + bool is_single) { + PingRequest req; + return encode_message_to_buffer(req, PingRequest::MESSAGE_TYPE, conn, remaining_size, is_single); +} + uint16_t APIConnection::get_estimated_message_size(uint16_t message_type) { // Use generated ESTIMATED_SIZE constants from each message type switch (message_type) { diff --git a/esphome/components/api/api_connection.h b/esphome/components/api/api_connection.h index 07e87ab39f..4397462d8e 100644 --- a/esphome/components/api/api_connection.h +++ b/esphome/components/api/api_connection.h @@ -185,7 +185,6 @@ class APIConnection : public APIServerConnection { void on_disconnect_response(const DisconnectResponse &value) override; void on_ping_response(const PingResponse &value) override { // we initiated ping - this->ping_retries_ = 0; this->sent_ping_ = false; } void on_home_assistant_state_response(const HomeAssistantStateResponse &msg) override; @@ -441,13 +440,16 @@ class APIConnection : public APIServerConnection { // Helper function to get estimated message size for buffer pre-allocation static uint16_t get_estimated_message_size(uint16_t message_type); + // Batch message method for ping requests + static uint16_t try_send_ping_request(EntityBase *entity, APIConnection *conn, uint32_t remaining_size, + bool is_single); + // Pointers first (4 bytes each, naturally aligned) std::unique_ptr helper_; APIServer *parent_; // 4-byte aligned types uint32_t last_traffic_; - uint32_t next_ping_retry_{0}; int state_subs_at_ = -1; // Strings (12 bytes each on 32-bit) @@ -470,6 +472,7 @@ class APIConnection : public APIServerConnection { bool sent_ping_{false}; bool service_call_subscription_{false}; bool next_close_ = false; + // 7 bytes used, 1 byte padding #ifdef HAS_PROTO_MESSAGE_DUMP // When true, encode_message_to_buffer will only log, not encode bool log_only_mode_{false}; @@ -602,6 +605,8 @@ class APIConnection : public APIServerConnection { // Add item to the batch void add_item(EntityBase *entity, MessageCreator creator, uint16_t message_type); + // Add item to the front of the batch (for high priority messages like ping) + void add_item_front(EntityBase *entity, MessageCreator creator, uint16_t message_type); void clear() { items.clear(); batch_scheduled = false; @@ -645,6 +650,12 @@ class APIConnection : public APIServerConnection { bool schedule_message_(EntityBase *entity, MessageCreatorPtr function_ptr, uint16_t message_type) { return schedule_message_(entity, MessageCreator(function_ptr), message_type); } + + // Helper function to schedule a high priority message at the front of the batch + bool schedule_message_front_(EntityBase *entity, MessageCreatorPtr function_ptr, uint16_t message_type) { + this->deferred_batch_.add_item_front(entity, MessageCreator(function_ptr), message_type); + return this->schedule_batch_(); + } }; } // namespace api diff --git a/esphome/components/api/api_server.cpp b/esphome/components/api/api_server.cpp index 583837af82..a33623b15a 100644 --- a/esphome/components/api/api_server.cpp +++ b/esphome/components/api/api_server.cpp @@ -526,8 +526,8 @@ void APIServer::on_shutdown() { for (auto &c : this->clients_) { if (!c->send_message(DisconnectRequest())) { // If we can't send the disconnect request directly (tx_buffer full), - // schedule it in the batch so it will be sent with the 5ms timer - c->schedule_message_(nullptr, &APIConnection::try_send_disconnect_request, DisconnectRequest::MESSAGE_TYPE); + // schedule it at the front of the batch so it will be sent with priority + c->schedule_message_front_(nullptr, &APIConnection::try_send_disconnect_request, DisconnectRequest::MESSAGE_TYPE); } } }