From 2371ec1f9e750aeacf093ea21dab0dd055748ca0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 26 Jun 2025 02:11:17 +0200 Subject: [PATCH] Replace ping retry timer with batch queue fallback --- esphome/components/api/api_connection.cpp | 36 +++++++++++------------ esphome/components/api/api_connection.h | 17 ++++++++--- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 634174ce0a..29eac240c0 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -60,10 +60,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(); @@ -161,30 +157,21 @@ void APIConnection::loop() { if (!this->initial_state_iterator_.completed() && this->list_entities_iterator_.completed()) this->initial_state_iterator_.advance(); - static uint8_t max_ping_retries = 60; - static uint16_t ping_retry_interval = 1000; if (this->sent_ping_) { // Disconnect if not responded within 2.5*keepalive if (now - this->last_traffic_ > (KEEPALIVE_TIMEOUT_MS * 5) / 2) { 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_++; - std::string warn_str = str_sprintf("%s: Sending keepalive failed %u time(s);", - this->get_client_combined_info().c_str(), this->ping_retries_); - if (this->ping_retries_ >= max_ping_retries) { - on_fatal_error(); - ESP_LOGE(TAG, "%s disconnecting", warn_str.c_str()); - } else if (this->ping_retries_ >= 10) { - ESP_LOGW(TAG, "%s retrying in %u ms", warn_str.c_str(), ping_retry_interval); - } else { - ESP_LOGD(TAG, "%s retrying in %u ms", warn_str.c_str(), ping_retry_interval); - } + // 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_LOGVV(TAG, "Failed to send ping directly, scheduling at front of batch"); + 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 } } @@ -1760,6 +1747,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; @@ -1938,6 +1930,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 da12a3e449..5bfe421d6b 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,8 +472,7 @@ class APIConnection : public APIServerConnection { bool sent_ping_{false}; bool service_call_subscription_{false}; bool next_close_ = false; - uint8_t ping_retries_{0}; - // 8 bytes used, no padding needed + // 7 bytes used, 1 byte padding // Larger objects at the end InitialStateIterator initial_state_iterator_; @@ -591,6 +592,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; @@ -630,6 +633,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