From 82081fc56713f6a5b39f41b0674ed86d90c7f895 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 24 Apr 2026 02:58:45 -0500 Subject: [PATCH] [web_server_idf] Trim comments --- .../web_server_idf/web_server_idf.cpp | 23 ++++--------------- .../web_server_idf/web_server_idf.h | 17 ++++---------- 2 files changed, 9 insertions(+), 31 deletions(-) diff --git a/esphome/components/web_server_idf/web_server_idf.cpp b/esphome/components/web_server_idf/web_server_idf.cpp index 3610f4cdfaf..bb090bad408 100644 --- a/esphome/components/web_server_idf/web_server_idf.cpp +++ b/esphome/components/web_server_idf/web_server_idf.cpp @@ -482,27 +482,19 @@ AsyncEventSource::~AsyncEventSource() { } void AsyncEventSource::handleRequest(AsyncWebServerRequest *request) { - // Runs on the httpd task. Do only the HTTP-level setup that needs the live httpd_req_t, - // then hand the session off to the main loop for adoption and priming. This keeps - // sessions_, event_buffer_, and deferred_queue_ mutated exclusively from the main loop. + // Httpd task: set up the live httpd_req_t and park the session; main loop does the rest. // NOLINTNEXTLINE(cppcoreguidelines-owning-memory,clang-analyzer-cplusplus.NewDeleteLeaks) auto *rsp = new AsyncEventSourceResponse(request, this, this->web_server_); { LockGuard guard{this->pending_mutex_}; this->pending_sessions_.push_back(rsp); - // Release-store so the main loop's acquire-load sees the push_back above. this->has_pending_sessions_.store(true, std::memory_order_release); } - // Wake up WebServer::loop() to adopt and prime this client. - // Safe from httpd task context via the pending_enable_loop_ flag. this->web_server_->enable_loop_soon_any_context(); } bool AsyncEventSource::loop() { - // Adopt sessions handed off from the httpd task. Fast path: one atomic load per tick - // when nothing is pending. Only take the lock / touch the vector on a real connect. - // Swap under the lock and do the heavy work (on_connect_ callback, initial sends, - // entity iterator start) outside it so they cannot race with httpd handlers. + // Fast path: one atomic load per tick. Lock only on a real connect. if (this->has_pending_sessions_.load(std::memory_order_acquire)) { std::vector incoming; { @@ -515,8 +507,7 @@ bool AsyncEventSource::loop() { if (this->on_connect_) { this->on_connect_(rsp); } - // Skip priming if the client disconnected before we got here; the cleanup pass below - // will delete it. + // Already disconnected? Cleanup pass below will delete it. if (rsp->fd_.load() != 0) { rsp->prime_(); } @@ -567,9 +558,7 @@ AsyncEventSourceResponse::AsyncEventSourceResponse(const AsyncWebServerRequest * esphome::web_server_idf::AsyncEventSource *server, esphome::web_server::WebServer *ws) : server_(server), web_server_(ws), entities_iterator_(ws, server) { - // Runs on the httpd task. Only touch state tied to the live httpd_req_t here; the - // main loop will call prime_() later to do the initial sends and start the iterator. - // Writing to event_buffer_ / deferred_queue_ from this task would race with the main loop. + // Httpd task only. prime_() on the main loop handles event_buffer_ / iterator setup. httpd_req_t *req = *request; httpd_resp_set_status(req, HTTPD_200); @@ -594,11 +583,9 @@ AsyncEventSourceResponse::AsyncEventSourceResponse(const AsyncWebServerRequest * } void AsyncEventSourceResponse::prime_() { - // Runs on the main loop after AsyncEventSource::loop() adopts this session. auto *ws = this->web_server_; - // Configure reconnect timeout and send config - // this should always go through since the tcp send buffer is empty on connect + // tcp send buffer is empty on connect, so these should always go through auto message = ws->get_config_json(); this->try_send_nodefer(message.c_str(), "ping", millis(), 30000); diff --git a/esphome/components/web_server_idf/web_server_idf.h b/esphome/components/web_server_idf/web_server_idf.h index 60d07534b9b..dc9f5ea16b1 100644 --- a/esphome/components/web_server_idf/web_server_idf.h +++ b/esphome/components/web_server_idf/web_server_idf.h @@ -299,8 +299,7 @@ class AsyncEventSourceResponse { AsyncEventSourceResponse(const AsyncWebServerRequest *request, esphome::web_server_idf::AsyncEventSource *server, esphome::web_server::WebServer *ws); - // Sends the initial ping/config/sorting_groups and starts the entity iterator. - // Must be called from the main loop (writes event_buffer_ and touches entities_iterator_). + // Main-loop only: sends initial ping/config/sorting_groups, starts entity iterator. void prime_(); void deq_push_back_with_dedup_(void *source, message_generator_t *message_generator); @@ -351,19 +350,11 @@ class AsyncEventSource : public AsyncWebHandler { size_t count() const { return this->sessions_.size(); } protected: - // Members are ordered to minimize padding on 32-bit: all 4-byte-aligned members - // first, then the single-byte atomic at the end to consume what would otherwise - // be trailing padding. + // Ordered to minimize padding on 32-bit: atomic last consumes trailing pad. std::string url_; - // Use vector instead of set: SSE sessions are typically 1-5 connections (browsers, dashboards). - // Linear search is faster than red-black tree overhead for this small dataset. - // Only operations needed: add session, remove session, iterate sessions - no need for sorted order. - // Mutated only from the main loop. + // Main-loop only. Vector: SSE sessions are 1-5 connections, linear search beats set. std::vector sessions_; - // Sessions constructed on the httpd task wait here until the main loop adopts them. - // All mutations are guarded by pending_mutex_. has_pending_sessions_ (below) is a - // fast-path gate so the per-tick cost in loop() when no connect is pending is one - // atomic load. + // Httpd-task intake; guarded by pending_mutex_, gated by has_pending_sessions_. std::vector pending_sessions_; Mutex pending_mutex_; connect_handler_t on_connect_{};