From a2feff8f68c530f3d82ed9e799f0046d4b96e3bd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Tue, 11 Aug 2026 22:23:08 -0500 Subject: [PATCH] [api] Mark send_message nodiscard so refused frames are never silent (#18293) --- esphome/components/api/api_connection.cpp | 61 ++++++++++++++++--- esphome/components/api/api_connection.h | 27 ++++---- esphome/components/api/api_server.cpp | 15 +++-- .../bluetooth_proxy/bluetooth_proxy.cpp | 8 ++- .../bluetooth_proxy/bluetooth_proxy.h | 11 ++-- .../voice_assistant/voice_assistant.cpp | 16 +++-- .../components/zwave_proxy/zwave_proxy.cpp | 12 +++- 7 files changed, 113 insertions(+), 37 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index afd7e696afc..53fe40f682b 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -89,6 +89,13 @@ static_assert(ESPHOME_DEVICE_NAME_MAX_LEN <= 31, "Update max_data_length for nam static_assert(ESPHOME_FRIENDLY_NAME_MAX_LEN <= 120, "Update max_data_length for friendly_name in api.proto"); static const char *const TAG = "api.connection"; + +#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_WARN +void log_dropped_message(const char *tag, int line, const LogString *what) { + esp_log_printf_(ESPHOME_LOG_LEVEL_WARN, tag, line, ESPHOME_LOG_FORMAT("%s dropped, TCP buffer full"), + LOG_STR_ARG(what)); +} +#endif #ifdef USE_CAMERA static const int CAMERA_STOP_STREAM = 5000; #endif @@ -1536,7 +1543,13 @@ void APIConnection::on_infrared_rf_transmit_raw_timings_request(const InfraredRF #endif #if defined(USE_IR_RF) || defined(USE_RADIO_FREQUENCY) -void APIConnection::send_infrared_rf_receive_event(const InfraredRFReceiveEvent &msg) { this->send_message(msg); } +void APIConnection::send_infrared_rf_receive_event(const InfraredRFReceiveEvent &msg) { + if (!this->send_message(msg)) { + // V: fires per decoded frame with no subscription gate, so a warning + // would flood the congested link it reports on. + ESP_LOGV(TAG, "IR/RF event dropped, TCP buffer full"); + } +} #endif #ifdef USE_SERIAL_PROXY @@ -1578,7 +1591,9 @@ void APIConnection::on_serial_proxy_get_modem_pins_request(const SerialProxyGetM SerialProxyGetModemPinsResponse resp{}; resp.instance = msg.instance; resp.line_states = proxies[msg.instance]->get_modem_pins(); - this->send_message(resp); + if (!this->send_message(resp)) { + API_LOG_MSG_DROPPED(TAG, "Serial proxy response"); + } } void APIConnection::on_serial_proxy_request(const SerialProxyRequest &msg) { @@ -1610,7 +1625,9 @@ void APIConnection::on_serial_proxy_request(const SerialProxyRequest &msg) { resp.status = enums::SERIAL_PROXY_STATUS_ERROR; break; } - this->send_message(resp); + if (!this->send_message(resp)) { + API_LOG_MSG_DROPPED(TAG, "Serial proxy response"); + } break; } default: @@ -1619,7 +1636,11 @@ void APIConnection::on_serial_proxy_request(const SerialProxyRequest &msg) { } } -void APIConnection::send_serial_proxy_data(const SerialProxyDataReceived &msg) { this->send_message(msg); } +void APIConnection::send_serial_proxy_data(const SerialProxyDataReceived &msg) { + if (!this->send_message(msg)) { + ESP_LOGV(TAG, "Serial proxy data dropped, TCP buffer full"); + } +} #endif #ifdef USE_INFRARED @@ -1750,7 +1771,9 @@ bool APIConnection::send_hello_response_(const HelloRequest &msg) { // Acknowledge the hello so the client can read the server name, then request // disconnect with the reason. Authentication is intentionally not completed. this->log_client_(ESPHOME_LOG_LEVEL_WARN, LOG_STR("Provisioning closed; rejecting connection")); - this->send_message(resp); + if (!this->send_message(resp)) { + API_LOG_MSG_DROPPED(TAG, "Hello response"); + } DisconnectRequest req; req.reason = enums::DISCONNECT_REASON_PROVISIONING_CLOSED; return this->send_message(req); @@ -2039,7 +2062,9 @@ void APIConnection::send_execute_service_response(uint32_t call_id, bool success resp.call_id = call_id; resp.success = success; resp.error_message = error_message; - this->send_message(resp); + if (!this->send_message(resp)) { + API_LOG_MSG_DROPPED(TAG, "Action response"); + } } #ifdef USE_API_USER_DEFINED_ACTION_RESPONSES_JSON void APIConnection::send_execute_service_response(uint32_t call_id, bool success, StringRef error_message, @@ -2050,12 +2075,34 @@ void APIConnection::send_execute_service_response(uint32_t call_id, bool success resp.error_message = error_message; resp.response_data = response_data; resp.response_data_len = response_data_len; - this->send_message(resp); + if (!this->send_message(resp)) { + API_LOG_MSG_DROPPED(TAG, "Action response"); + } } #endif // USE_API_USER_DEFINED_ACTION_RESPONSES_JSON #endif // USE_API_USER_DEFINED_ACTION_RESPONSES #endif +#ifdef USE_API_HOMEASSISTANT_SERVICES +bool APIConnection::send_homeassistant_action(const HomeassistantActionRequest &call) { + if (!this->flags_.service_call_subscription) + return false; + if (!this->send_message(call)) { + API_LOG_MSG_DROPPED(TAG, "Action request"); + } + return true; +} +#endif // USE_API_HOMEASSISTANT_SERVICES + +#ifdef USE_HOMEASSISTANT_TIME +void APIConnection::send_time_request() { + GetTimeRequest req; + if (!this->send_message(req)) { + API_LOG_MSG_DROPPED(TAG, "Time request"); + } +} +#endif // USE_HOMEASSISTANT_TIME + #ifdef USE_API_HOMEASSISTANT_ACTION_RESPONSES void APIConnection::on_homeassistant_action_response(const HomeassistantActionResponse &msg) { #ifdef USE_API_HOMEASSISTANT_ACTION_RESPONSES_JSON diff --git a/esphome/components/api/api_connection.h b/esphome/components/api/api_connection.h index d548b921b39..bb51a130003 100644 --- a/esphome/components/api/api_connection.h +++ b/esphome/components/api/api_connection.h @@ -25,6 +25,7 @@ #include "esphome/components/esp8266/crash_handler.h" #endif #include "esphome/core/entity_base.h" +#include "esphome/core/log.h" #include "esphome/core/string_ref.h" #include @@ -40,6 +41,16 @@ namespace esphome::api { // Forward-declared to break the api_server.h cycle; full-type inlines are in api_connection_buffer.h. class APIServer; +// One shared flash string for every refused-frame warning: send_message() +// fails as soon as the TCP buffer is full, and each caller only pays for its +// short name. The guard drops the helper and its arguments below WARN. +#if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_WARN +void log_dropped_message(const char *tag, int line, const LogString *what); +#define API_LOG_MSG_DROPPED(tag, what) esphome::api::log_dropped_message(tag, __LINE__, LOG_STR(what)) +#else +#define API_LOG_MSG_DROPPED(tag, what) +#endif + // Keepalive timeout in milliseconds static constexpr uint32_t KEEPALIVE_TIMEOUT_MS = 60000; // Maximum number of entities to process in a single batch during initial state/info sending @@ -169,12 +180,7 @@ class APIConnection final : public APIServerConnectionBase { // Returns whether this client has subscribed to Home Assistant actions; the message // is only handed to the send path when subscribed. A true return does not guarantee // delivery - it lets the caller warn when no connected client has the subscription. - bool send_homeassistant_action(const HomeassistantActionRequest &call) { - if (!this->flags_.service_call_subscription) - return false; - this->send_message(call); - return true; - } + bool send_homeassistant_action(const HomeassistantActionRequest &call); #ifdef USE_API_HOMEASSISTANT_ACTION_RESPONSES void on_homeassistant_action_response(const HomeassistantActionResponse &msg); #endif // USE_API_HOMEASSISTANT_ACTION_RESPONSES @@ -198,10 +204,7 @@ class APIConnection final : public APIServerConnectionBase { #endif #ifdef USE_HOMEASSISTANT_TIME - void send_time_request() { - GetTimeRequest req; - this->send_message(req); - } + void send_time_request(); #endif #ifdef USE_VOICE_ASSISTANT @@ -337,7 +340,9 @@ class APIConnection final : public APIServerConnectionBase { // Function pointer type for type-erased size calculation using CalculateSizeFn = uint32_t (*)(const void *); - template bool send_message(const T &msg) { + /// Returns false as soon as the TCP buffer is full. Marked nodiscard so we + /// have no silent failures: every caller must handle (or log) a refusal. + template [[nodiscard]] bool send_message(const T &msg) { if constexpr (T::ESTIMATED_SIZE == 0) { return this->send_message_(0, T::MESSAGE_TYPE, &encode_msg_noop, &msg); } else { diff --git a/esphome/components/api/api_server.cpp b/esphome/components/api/api_server.cpp index 6e3448121c3..ef5b43d7b1b 100644 --- a/esphome/components/api/api_server.cpp +++ b/esphome/components/api/api_server.cpp @@ -123,7 +123,9 @@ void APIServer::setup() { // Best-effort: if the send buffer is full the reason is dropped, but the // client still learns the window is closed when it reconnects (rejected at // hello) or via the socket close. - c->send_message(req); + if (!c->send_message(req)) { + API_LOG_MSG_DROPPED(TAG, "Disconnect request"); + } } }); } @@ -394,8 +396,11 @@ void APIServer::on_update(update::UpdateEntity *obj) { void APIServer::on_zwave_proxy_request(const ZWaveProxyRequest &msg) { // We could add code to manage a second subscription type, but, since this message type is // very infrequent and small, we simply send it to all clients - for (auto &c : this->active_clients()) - c->send_message(msg); + for (auto &c : this->active_clients()) { + if (!c->send_message(msg)) { + API_LOG_MSG_DROPPED(TAG, "Home ID notification"); + } + } } #endif @@ -576,7 +581,9 @@ bool APIServer::update_noise_psk_(const SavedNoisePsk &new_psk, const LogString ESP_LOGW(TAG, "Disconnecting all clients to reset PSK"); for (auto &c : this->active_clients()) { DisconnectRequest req; - c->send_message(req); + if (!c->send_message(req)) { + API_LOG_MSG_DROPPED(TAG, "Disconnect request"); + } } }); } diff --git a/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp b/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp index 75830e83ef9..1489f9f4ba9 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp +++ b/esphome/components/bluetooth_proxy/bluetooth_proxy.cpp @@ -150,8 +150,12 @@ void BluetoothProxy::handle_gatt_not_connected_(uint64_t address, uint16_t handl } #endif -void BluetoothProxy::log_advertisement_flush_() { - ESP_LOGV(TAG, "Sent batch of %u BLE advertisements", this->response_.advertisements_len); +void BluetoothProxy::log_advertisement_flush_(bool sent) { + if (sent) { + ESP_LOGV(TAG, "Sent batch of %u BLE advertisements", this->response_.advertisements_len); + } else { + ESP_LOGV(TAG, "Batch of %u BLE advertisements dropped, TCP buffer full", this->response_.advertisements_len); + } } void BluetoothProxy::dump_config() { diff --git a/esphome/components/bluetooth_proxy/bluetooth_proxy.h b/esphome/components/bluetooth_proxy/bluetooth_proxy.h index e5f3a259a11..5d2605e7617 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_proxy.h +++ b/esphome/components/bluetooth_proxy/bluetooth_proxy.h @@ -234,16 +234,15 @@ class BluetoothProxy final : public Component { void flush_pending_advertisements_() { if (this->response_.advertisements_len == 0) return; - // The one deliberately ignored result: advertisements are perishable and - // this is the highest-frequency send here, so reporting each drop would be - // the flood the batch pacing exists to avoid. - this->api_connection_->send_message(this->response_); + // Perishable and the highest-frequency send here: a drop only reports at + // V, anything louder would be the flood the batch pacing exists to avoid. + [[maybe_unused]] bool sent = this->api_connection_->send_message(this->response_); #if ESPHOME_LOG_LEVEL >= ESPHOME_LOG_LEVEL_VERBOSE - this->log_advertisement_flush_(); + this->log_advertisement_flush_(sent); #endif this->response_.advertisements_len = 0; } - void log_advertisement_flush_(); + void log_advertisement_flush_(bool sent); #ifdef USE_BLUETOOTH_PROXY_CONNECTIONS BluetoothConnection *get_connection_(uint64_t address, bool reserve); diff --git a/esphome/components/voice_assistant/voice_assistant.cpp b/esphome/components/voice_assistant/voice_assistant.cpp index 76ae145b167..50add6b1d0c 100644 --- a/esphome/components/voice_assistant/voice_assistant.cpp +++ b/esphome/components/voice_assistant/voice_assistant.cpp @@ -248,7 +248,9 @@ void VoiceAssistant::stream_api_audio_() { msg.data2_len = available2; } - this->api_client_->send_message(msg); + if (!this->api_client_->send_message(msg)) { + ESP_LOGV(TAG, "Audio frame dropped, TCP buffer full"); + } this->audio_source_->consume(available); if (this->audio_source2_ != nullptr) { @@ -477,7 +479,9 @@ void VoiceAssistant::loop() { api::VoiceAssistantAnnounceFinished msg; msg.success = true; - this->api_client_->send_message(msg); + if (!this->api_client_->send_message(msg)) { + API_LOG_MSG_DROPPED(TAG, "Announce-finished"); + } break; } } @@ -741,7 +745,9 @@ void VoiceAssistant::signal_stop_() { ESP_LOGD(TAG, "Signaling stop"); api::VoiceAssistantRequest msg; msg.start = false; - this->api_client_->send_message(msg); + if (!this->api_client_->send_message(msg)) { + API_LOG_MSG_DROPPED(TAG, "Stop request"); + } } void VoiceAssistant::start_playback_timeout_() { @@ -753,7 +759,9 @@ void VoiceAssistant::start_playback_timeout_() { return; api::VoiceAssistantAnnounceFinished msg; msg.success = true; - this->api_client_->send_message(msg); + if (!this->api_client_->send_message(msg)) { + API_LOG_MSG_DROPPED(TAG, "Announce-finished"); + } }); } diff --git a/esphome/components/zwave_proxy/zwave_proxy.cpp b/esphome/components/zwave_proxy/zwave_proxy.cpp index 5f56861e6d8..6e3f109ca1a 100644 --- a/esphome/components/zwave_proxy/zwave_proxy.cpp +++ b/esphome/components/zwave_proxy/zwave_proxy.cpp @@ -166,7 +166,9 @@ void ZWaveProxy::process_uart_slow_() { // If this is a data frame, use frame length indicator + 2 (for SoF + checksum), else assume 1 for ACK/NAK/CAN this->outgoing_proto_msg_.data_len = this->buffer_[0] == ZWAVE_FRAME_TYPE_START ? this->buffer_[1] + 2 : 1; } - this->api_connection_->send_message(this->outgoing_proto_msg_); + if (!this->api_connection_->send_message(this->outgoing_proto_msg_)) { + ESP_LOGV(TAG, "Frame dropped, TCP buffer full"); + } } } } while (this->available()); @@ -328,7 +330,9 @@ void ZWaveProxy::send_homeid_changed_msg_(api::APIConnection *conn) { msg.data_len = this->home_id_.size(); if (conn != nullptr) { // Send to specific connection - conn->send_message(msg); + if (!conn->send_message(msg)) { + API_LOG_MSG_DROPPED(TAG, "Home ID notification"); + } } else if (api::global_api_server != nullptr) { // We could add code to manage a second subscription type, but, since this message is // very infrequent and small, we simply send it to all clients @@ -483,7 +487,9 @@ void ZWaveProxy::parse_start_(uint8_t byte) { this->buffer_[0] = byte; this->outgoing_proto_msg_.data = this->buffer_.data(); this->outgoing_proto_msg_.data_len = 1; - this->api_connection_->send_message(this->outgoing_proto_msg_); + if (!this->api_connection_->send_message(this->outgoing_proto_msg_)) { + ESP_LOGV(TAG, "Frame dropped, TCP buffer full"); + } } }