From b7b1d17ecbf30762c9768e5e38fdfd81b8642b64 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 19 Jun 2025 02:06:39 +0200 Subject: [PATCH 1/9] Remove empty generated protobuf methods (#9098) --- esphome/components/api/api_pb2.cpp | 28 -------------------------- esphome/components/api/api_pb2.h | 28 -------------------------- esphome/components/api/proto.h | 6 ++++-- script/api_protobuf/api_protobuf.py | 31 ++++++++++++++--------------- 4 files changed, 19 insertions(+), 74 deletions(-) diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index bde1824d71..517b4d41b4 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -797,28 +797,18 @@ void ConnectResponse::dump_to(std::string &out) const { out.append("}"); } #endif -void DisconnectRequest::encode(ProtoWriteBuffer buffer) const {} -void DisconnectRequest::calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP void DisconnectRequest::dump_to(std::string &out) const { out.append("DisconnectRequest {}"); } #endif -void DisconnectResponse::encode(ProtoWriteBuffer buffer) const {} -void DisconnectResponse::calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP void DisconnectResponse::dump_to(std::string &out) const { out.append("DisconnectResponse {}"); } #endif -void PingRequest::encode(ProtoWriteBuffer buffer) const {} -void PingRequest::calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP void PingRequest::dump_to(std::string &out) const { out.append("PingRequest {}"); } #endif -void PingResponse::encode(ProtoWriteBuffer buffer) const {} -void PingResponse::calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP void PingResponse::dump_to(std::string &out) const { out.append("PingResponse {}"); } #endif -void DeviceInfoRequest::encode(ProtoWriteBuffer buffer) const {} -void DeviceInfoRequest::calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP void DeviceInfoRequest::dump_to(std::string &out) const { out.append("DeviceInfoRequest {}"); } #endif @@ -1039,18 +1029,12 @@ void DeviceInfoResponse::dump_to(std::string &out) const { out.append("}"); } #endif -void ListEntitiesRequest::encode(ProtoWriteBuffer buffer) const {} -void ListEntitiesRequest::calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP void ListEntitiesRequest::dump_to(std::string &out) const { out.append("ListEntitiesRequest {}"); } #endif -void ListEntitiesDoneResponse::encode(ProtoWriteBuffer buffer) const {} -void ListEntitiesDoneResponse::calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP void ListEntitiesDoneResponse::dump_to(std::string &out) const { out.append("ListEntitiesDoneResponse {}"); } #endif -void SubscribeStatesRequest::encode(ProtoWriteBuffer buffer) const {} -void SubscribeStatesRequest::calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP void SubscribeStatesRequest::dump_to(std::string &out) const { out.append("SubscribeStatesRequest {}"); } #endif @@ -3371,8 +3355,6 @@ void NoiseEncryptionSetKeyResponse::dump_to(std::string &out) const { out.append("}"); } #endif -void SubscribeHomeassistantServicesRequest::encode(ProtoWriteBuffer buffer) const {} -void SubscribeHomeassistantServicesRequest::calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP void SubscribeHomeassistantServicesRequest::dump_to(std::string &out) const { out.append("SubscribeHomeassistantServicesRequest {}"); @@ -3498,8 +3480,6 @@ void HomeassistantServiceResponse::dump_to(std::string &out) const { out.append("}"); } #endif -void SubscribeHomeAssistantStatesRequest::encode(ProtoWriteBuffer buffer) const {} -void SubscribeHomeAssistantStatesRequest::calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP void SubscribeHomeAssistantStatesRequest::dump_to(std::string &out) const { out.append("SubscribeHomeAssistantStatesRequest {}"); @@ -3603,8 +3583,6 @@ void HomeAssistantStateResponse::dump_to(std::string &out) const { out.append("}"); } #endif -void GetTimeRequest::encode(ProtoWriteBuffer buffer) const {} -void GetTimeRequest::calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP void GetTimeRequest::dump_to(std::string &out) const { out.append("GetTimeRequest {}"); } #endif @@ -7499,8 +7477,6 @@ void BluetoothGATTNotifyDataResponse::dump_to(std::string &out) const { out.append("}"); } #endif -void SubscribeBluetoothConnectionsFreeRequest::encode(ProtoWriteBuffer buffer) const {} -void SubscribeBluetoothConnectionsFreeRequest::calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP void SubscribeBluetoothConnectionsFreeRequest::dump_to(std::string &out) const { out.append("SubscribeBluetoothConnectionsFreeRequest {}"); @@ -7784,8 +7760,6 @@ void BluetoothDeviceUnpairingResponse::dump_to(std::string &out) const { out.append("}"); } #endif -void UnsubscribeBluetoothLEAdvertisementsRequest::encode(ProtoWriteBuffer buffer) const {} -void UnsubscribeBluetoothLEAdvertisementsRequest::calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP void UnsubscribeBluetoothLEAdvertisementsRequest::dump_to(std::string &out) const { out.append("UnsubscribeBluetoothLEAdvertisementsRequest {}"); @@ -8451,8 +8425,6 @@ void VoiceAssistantWakeWord::dump_to(std::string &out) const { out.append("}"); } #endif -void VoiceAssistantConfigurationRequest::encode(ProtoWriteBuffer buffer) const {} -void VoiceAssistantConfigurationRequest::calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP void VoiceAssistantConfigurationRequest::dump_to(std::string &out) const { out.append("VoiceAssistantConfigurationRequest {}"); diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index 9d270bcdc1..7d92125290 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -357,8 +357,6 @@ class DisconnectRequest : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP static constexpr const char *message_name() { return "disconnect_request"; } #endif - void encode(ProtoWriteBuffer buffer) const override; - void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP void dump_to(std::string &out) const override; #endif @@ -372,8 +370,6 @@ class DisconnectResponse : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP static constexpr const char *message_name() { return "disconnect_response"; } #endif - void encode(ProtoWriteBuffer buffer) const override; - void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP void dump_to(std::string &out) const override; #endif @@ -387,8 +383,6 @@ class PingRequest : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP static constexpr const char *message_name() { return "ping_request"; } #endif - void encode(ProtoWriteBuffer buffer) const override; - void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP void dump_to(std::string &out) const override; #endif @@ -402,8 +396,6 @@ class PingResponse : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP static constexpr const char *message_name() { return "ping_response"; } #endif - void encode(ProtoWriteBuffer buffer) const override; - void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP void dump_to(std::string &out) const override; #endif @@ -417,8 +409,6 @@ class DeviceInfoRequest : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP static constexpr const char *message_name() { return "device_info_request"; } #endif - void encode(ProtoWriteBuffer buffer) const override; - void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP void dump_to(std::string &out) const override; #endif @@ -468,8 +458,6 @@ class ListEntitiesRequest : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP static constexpr const char *message_name() { return "list_entities_request"; } #endif - void encode(ProtoWriteBuffer buffer) const override; - void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP void dump_to(std::string &out) const override; #endif @@ -483,8 +471,6 @@ class ListEntitiesDoneResponse : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP static constexpr const char *message_name() { return "list_entities_done_response"; } #endif - void encode(ProtoWriteBuffer buffer) const override; - void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP void dump_to(std::string &out) const override; #endif @@ -498,8 +484,6 @@ class SubscribeStatesRequest : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP static constexpr const char *message_name() { return "subscribe_states_request"; } #endif - void encode(ProtoWriteBuffer buffer) const override; - void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP void dump_to(std::string &out) const override; #endif @@ -1011,8 +995,6 @@ class SubscribeHomeassistantServicesRequest : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP static constexpr const char *message_name() { return "subscribe_homeassistant_services_request"; } #endif - void encode(ProtoWriteBuffer buffer) const override; - void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP void dump_to(std::string &out) const override; #endif @@ -1061,8 +1043,6 @@ class SubscribeHomeAssistantStatesRequest : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP static constexpr const char *message_name() { return "subscribe_home_assistant_states_request"; } #endif - void encode(ProtoWriteBuffer buffer) const override; - void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP void dump_to(std::string &out) const override; #endif @@ -1115,8 +1095,6 @@ class GetTimeRequest : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP static constexpr const char *message_name() { return "get_time_request"; } #endif - void encode(ProtoWriteBuffer buffer) const override; - void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP void dump_to(std::string &out) const override; #endif @@ -2117,8 +2095,6 @@ class SubscribeBluetoothConnectionsFreeRequest : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP static constexpr const char *message_name() { return "subscribe_bluetooth_connections_free_request"; } #endif - void encode(ProtoWriteBuffer buffer) const override; - void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP void dump_to(std::string &out) const override; #endif @@ -2244,8 +2220,6 @@ class UnsubscribeBluetoothLEAdvertisementsRequest : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP static constexpr const char *message_name() { return "unsubscribe_bluetooth_le_advertisements_request"; } #endif - void encode(ProtoWriteBuffer buffer) const override; - void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP void dump_to(std::string &out) const override; #endif @@ -2512,8 +2486,6 @@ class VoiceAssistantConfigurationRequest : public ProtoMessage { #ifdef HAS_PROTO_MESSAGE_DUMP static constexpr const char *message_name() { return "voice_assistant_configuration_request"; } #endif - void encode(ProtoWriteBuffer buffer) const override; - void calculate_size(uint32_t &total_size) const override; #ifdef HAS_PROTO_MESSAGE_DUMP void dump_to(std::string &out) const override; #endif diff --git a/esphome/components/api/proto.h b/esphome/components/api/proto.h index eb0dbc151b..6ece509c8d 100644 --- a/esphome/components/api/proto.h +++ b/esphome/components/api/proto.h @@ -327,9 +327,11 @@ class ProtoWriteBuffer { class ProtoMessage { public: virtual ~ProtoMessage() = default; - virtual void encode(ProtoWriteBuffer buffer) const = 0; + // Default implementation for messages with no fields + virtual void encode(ProtoWriteBuffer buffer) const {} void decode(const uint8_t *buffer, size_t length); - virtual void calculate_size(uint32_t &total_size) const = 0; + // Default implementation for messages with no fields + virtual void calculate_size(uint32_t &total_size) const {} #ifdef HAS_PROTO_MESSAGE_DUMP std::string dump() const; virtual void dump_to(std::string &out) const = 0; diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 24b6bef843..5ac101c673 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -959,36 +959,35 @@ def build_message_type( prot = "bool decode_64bit(uint32_t field_id, Proto64Bit value) override;" protected_content.insert(0, prot) - o = f"void {desc.name}::encode(ProtoWriteBuffer buffer) const {{" + # Only generate encode method if there are fields to encode if encode: + o = f"void {desc.name}::encode(ProtoWriteBuffer buffer) const {{" if len(encode) == 1 and len(encode[0]) + len(o) + 3 < 120: o += f" {encode[0]} " else: o += "\n" o += indent("\n".join(encode)) + "\n" - o += "}\n" - cpp += o - prot = "void encode(ProtoWriteBuffer buffer) const override;" - public_content.append(prot) + o += "}\n" + cpp += o + prot = "void encode(ProtoWriteBuffer buffer) const override;" + public_content.append(prot) + # If no fields to encode, the default implementation in ProtoMessage will be used - # Add calculate_size method - o = f"void {desc.name}::calculate_size(uint32_t &total_size) const {{" - - # Add a check for empty/default objects to short-circuit the calculation - # Only add this optimization if we have fields to check + # Add calculate_size method only if there are fields if size_calc: + o = f"void {desc.name}::calculate_size(uint32_t &total_size) const {{" # For a single field, just inline it for simplicity if len(size_calc) == 1 and len(size_calc[0]) + len(o) + 3 < 120: o += f" {size_calc[0]} " else: - # For multiple fields, add a short-circuit check + # For multiple fields o += "\n" - # Performance optimization: add all the size calculations o += indent("\n".join(size_calc)) + "\n" - o += "}\n" - cpp += o - prot = "void calculate_size(uint32_t &total_size) const override;" - public_content.append(prot) + o += "}\n" + cpp += o + prot = "void calculate_size(uint32_t &total_size) const override;" + public_content.append(prot) + # If no fields to calculate size for, the default implementation in ProtoMessage will be used o = f"void {desc.name}::dump_to(std::string &out) const {{" if dump: From a08d021f77a9d076eebb24d28ae67c067bb44612 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 19 Jun 2025 02:10:01 +0200 Subject: [PATCH 2/9] Reduce code duplication in auto-generated API protocol code (#9097) --- esphome/components/api/api_pb2_service.cpp | 442 +++++---------------- esphome/components/api/proto.h | 20 + script/api_protobuf/api_protobuf.py | 49 ++- 3 files changed, 151 insertions(+), 360 deletions(-) diff --git a/esphome/components/api/api_pb2_service.cpp b/esphome/components/api/api_pb2_service.cpp index dacb23c12b..03017fdfff 100644 --- a/esphome/components/api/api_pb2_service.cpp +++ b/esphome/components/api/api_pb2_service.cpp @@ -620,544 +620,300 @@ void APIServerConnection::on_ping_request(const PingRequest &msg) { } } void APIServerConnection::on_device_info_request(const DeviceInfoRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; - } - DeviceInfoResponse ret = this->device_info(msg); - if (!this->send_message(ret)) { - this->on_fatal_error(); + if (this->check_connection_setup_()) { + DeviceInfoResponse ret = this->device_info(msg); + if (!this->send_message(ret)) { + this->on_fatal_error(); + } } } void APIServerConnection::on_list_entities_request(const ListEntitiesRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->list_entities(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->list_entities(msg); } void APIServerConnection::on_subscribe_states_request(const SubscribeStatesRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->subscribe_states(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->subscribe_states(msg); } void APIServerConnection::on_subscribe_logs_request(const SubscribeLogsRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->subscribe_logs(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->subscribe_logs(msg); } void APIServerConnection::on_subscribe_homeassistant_services_request( const SubscribeHomeassistantServicesRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->subscribe_homeassistant_services(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->subscribe_homeassistant_services(msg); } void APIServerConnection::on_subscribe_home_assistant_states_request(const SubscribeHomeAssistantStatesRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->subscribe_home_assistant_states(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->subscribe_home_assistant_states(msg); } void APIServerConnection::on_get_time_request(const GetTimeRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; - } - GetTimeResponse ret = this->get_time(msg); - if (!this->send_message(ret)) { - this->on_fatal_error(); + if (this->check_connection_setup_()) { + GetTimeResponse ret = this->get_time(msg); + if (!this->send_message(ret)) { + this->on_fatal_error(); + } } } void APIServerConnection::on_execute_service_request(const ExecuteServiceRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->execute_service(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->execute_service(msg); } #ifdef USE_API_NOISE void APIServerConnection::on_noise_encryption_set_key_request(const NoiseEncryptionSetKeyRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; - } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - NoiseEncryptionSetKeyResponse ret = this->noise_encryption_set_key(msg); - if (!this->send_message(ret)) { - this->on_fatal_error(); + if (this->check_authenticated_()) { + NoiseEncryptionSetKeyResponse ret = this->noise_encryption_set_key(msg); + if (!this->send_message(ret)) { + this->on_fatal_error(); + } } } #endif #ifdef USE_BUTTON void APIServerConnection::on_button_command_request(const ButtonCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->button_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->button_command(msg); } #endif #ifdef USE_ESP32_CAMERA void APIServerConnection::on_camera_image_request(const CameraImageRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->camera_image(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->camera_image(msg); } #endif #ifdef USE_CLIMATE void APIServerConnection::on_climate_command_request(const ClimateCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->climate_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->climate_command(msg); } #endif #ifdef USE_COVER void APIServerConnection::on_cover_command_request(const CoverCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->cover_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->cover_command(msg); } #endif #ifdef USE_DATETIME_DATE void APIServerConnection::on_date_command_request(const DateCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->date_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->date_command(msg); } #endif #ifdef USE_DATETIME_DATETIME void APIServerConnection::on_date_time_command_request(const DateTimeCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->datetime_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->datetime_command(msg); } #endif #ifdef USE_FAN void APIServerConnection::on_fan_command_request(const FanCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->fan_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->fan_command(msg); } #endif #ifdef USE_LIGHT void APIServerConnection::on_light_command_request(const LightCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->light_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->light_command(msg); } #endif #ifdef USE_LOCK void APIServerConnection::on_lock_command_request(const LockCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->lock_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->lock_command(msg); } #endif #ifdef USE_MEDIA_PLAYER void APIServerConnection::on_media_player_command_request(const MediaPlayerCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->media_player_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->media_player_command(msg); } #endif #ifdef USE_NUMBER void APIServerConnection::on_number_command_request(const NumberCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->number_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->number_command(msg); } #endif #ifdef USE_SELECT void APIServerConnection::on_select_command_request(const SelectCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->select_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->select_command(msg); } #endif #ifdef USE_SIREN void APIServerConnection::on_siren_command_request(const SirenCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->siren_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->siren_command(msg); } #endif #ifdef USE_SWITCH void APIServerConnection::on_switch_command_request(const SwitchCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->switch_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->switch_command(msg); } #endif #ifdef USE_TEXT void APIServerConnection::on_text_command_request(const TextCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->text_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->text_command(msg); } #endif #ifdef USE_DATETIME_TIME void APIServerConnection::on_time_command_request(const TimeCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->time_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->time_command(msg); } #endif #ifdef USE_UPDATE void APIServerConnection::on_update_command_request(const UpdateCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->update_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->update_command(msg); } #endif #ifdef USE_VALVE void APIServerConnection::on_valve_command_request(const ValveCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->valve_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->valve_command(msg); } #endif #ifdef USE_BLUETOOTH_PROXY void APIServerConnection::on_subscribe_bluetooth_le_advertisements_request( const SubscribeBluetoothLEAdvertisementsRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->subscribe_bluetooth_le_advertisements(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->subscribe_bluetooth_le_advertisements(msg); } #endif #ifdef USE_BLUETOOTH_PROXY void APIServerConnection::on_bluetooth_device_request(const BluetoothDeviceRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->bluetooth_device_request(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->bluetooth_device_request(msg); } #endif #ifdef USE_BLUETOOTH_PROXY void APIServerConnection::on_bluetooth_gatt_get_services_request(const BluetoothGATTGetServicesRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->bluetooth_gatt_get_services(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->bluetooth_gatt_get_services(msg); } #endif #ifdef USE_BLUETOOTH_PROXY void APIServerConnection::on_bluetooth_gatt_read_request(const BluetoothGATTReadRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->bluetooth_gatt_read(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->bluetooth_gatt_read(msg); } #endif #ifdef USE_BLUETOOTH_PROXY void APIServerConnection::on_bluetooth_gatt_write_request(const BluetoothGATTWriteRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->bluetooth_gatt_write(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->bluetooth_gatt_write(msg); } #endif #ifdef USE_BLUETOOTH_PROXY void APIServerConnection::on_bluetooth_gatt_read_descriptor_request(const BluetoothGATTReadDescriptorRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->bluetooth_gatt_read_descriptor(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->bluetooth_gatt_read_descriptor(msg); } #endif #ifdef USE_BLUETOOTH_PROXY void APIServerConnection::on_bluetooth_gatt_write_descriptor_request(const BluetoothGATTWriteDescriptorRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->bluetooth_gatt_write_descriptor(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->bluetooth_gatt_write_descriptor(msg); } #endif #ifdef USE_BLUETOOTH_PROXY void APIServerConnection::on_bluetooth_gatt_notify_request(const BluetoothGATTNotifyRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->bluetooth_gatt_notify(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->bluetooth_gatt_notify(msg); } #endif #ifdef USE_BLUETOOTH_PROXY void APIServerConnection::on_subscribe_bluetooth_connections_free_request( const SubscribeBluetoothConnectionsFreeRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; - } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - BluetoothConnectionsFreeResponse ret = this->subscribe_bluetooth_connections_free(msg); - if (!this->send_message(ret)) { - this->on_fatal_error(); + if (this->check_authenticated_()) { + BluetoothConnectionsFreeResponse ret = this->subscribe_bluetooth_connections_free(msg); + if (!this->send_message(ret)) { + this->on_fatal_error(); + } } } #endif #ifdef USE_BLUETOOTH_PROXY void APIServerConnection::on_unsubscribe_bluetooth_le_advertisements_request( const UnsubscribeBluetoothLEAdvertisementsRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->unsubscribe_bluetooth_le_advertisements(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->unsubscribe_bluetooth_le_advertisements(msg); } #endif #ifdef USE_BLUETOOTH_PROXY void APIServerConnection::on_bluetooth_scanner_set_mode_request(const BluetoothScannerSetModeRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->bluetooth_scanner_set_mode(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->bluetooth_scanner_set_mode(msg); } #endif #ifdef USE_VOICE_ASSISTANT void APIServerConnection::on_subscribe_voice_assistant_request(const SubscribeVoiceAssistantRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->subscribe_voice_assistant(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->subscribe_voice_assistant(msg); } #endif #ifdef USE_VOICE_ASSISTANT void APIServerConnection::on_voice_assistant_configuration_request(const VoiceAssistantConfigurationRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; - } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - VoiceAssistantConfigurationResponse ret = this->voice_assistant_get_configuration(msg); - if (!this->send_message(ret)) { - this->on_fatal_error(); + if (this->check_authenticated_()) { + VoiceAssistantConfigurationResponse ret = this->voice_assistant_get_configuration(msg); + if (!this->send_message(ret)) { + this->on_fatal_error(); + } } } #endif #ifdef USE_VOICE_ASSISTANT void APIServerConnection::on_voice_assistant_set_configuration(const VoiceAssistantSetConfiguration &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->voice_assistant_set_configuration(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->voice_assistant_set_configuration(msg); } #endif #ifdef USE_ALARM_CONTROL_PANEL void APIServerConnection::on_alarm_control_panel_command_request(const AlarmControlPanelCommandRequest &msg) { - if (!this->is_connection_setup()) { - this->on_no_setup_connection(); - return; + if (this->check_authenticated_()) { + this->alarm_control_panel_command(msg); } - if (!this->is_authenticated()) { - this->on_unauthenticated_access(); - return; - } - this->alarm_control_panel_command(msg); } #endif diff --git a/esphome/components/api/proto.h b/esphome/components/api/proto.h index 6ece509c8d..e850236db6 100644 --- a/esphome/components/api/proto.h +++ b/esphome/components/api/proto.h @@ -379,6 +379,26 @@ class ProtoService { // Send the buffer return this->send_buffer(buffer, message_type); } + + // Authentication helper methods + bool check_connection_setup_() { + if (!this->is_connection_setup()) { + this->on_no_setup_connection(); + return false; + } + return true; + } + + bool check_authenticated_() { + if (!this->check_connection_setup_()) { + return false; + } + if (!this->is_authenticated()) { + this->on_unauthenticated_access(); + return false; + } + return true; + } }; } // namespace api diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 5ac101c673..bd1be66649 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -1424,25 +1424,40 @@ def main() -> None: hpp_protected += f" void {on_func}(const {inp} &msg) override;\n" hpp += f" virtual {ret} {func}(const {inp} &msg) = 0;\n" cpp += f"void {class_name}::{on_func}(const {inp} &msg) {{\n" - body = "" - if needs_conn: - body += "if (!this->is_connection_setup()) {\n" - body += " this->on_no_setup_connection();\n" - body += " return;\n" - body += "}\n" - if needs_auth: - body += "if (!this->is_authenticated()) {\n" - body += " this->on_unauthenticated_access();\n" - body += " return;\n" - body += "}\n" - if is_void: - body += f"this->{func}(msg);\n" - else: - body += f"{ret} ret = this->{func}(msg);\n" - body += "if (!this->send_message(ret)) {\n" - body += " this->on_fatal_error();\n" + # Start with authentication/connection check if needed + if needs_auth or needs_conn: + # Determine which check to use + if needs_auth: + check_func = "this->check_authenticated_()" + else: + check_func = "this->check_connection_setup_()" + + body = f"if ({check_func}) {{\n" + + # Add the actual handler code, indented + handler_body = "" + if is_void: + handler_body = f"this->{func}(msg);\n" + else: + handler_body = f"{ret} ret = this->{func}(msg);\n" + handler_body += "if (!this->send_message(ret)) {\n" + handler_body += " this->on_fatal_error();\n" + handler_body += "}\n" + + body += indent(handler_body) + "\n" body += "}\n" + else: + # No auth check needed, just call the handler + body = "" + if is_void: + body += f"this->{func}(msg);\n" + else: + body += f"{ret} ret = this->{func}(msg);\n" + body += "if (!this->send_message(ret)) {\n" + body += " this->on_fatal_error();\n" + body += "}\n" + cpp += indent(body) + "\n" + "}\n" if ifdef is not None: From 95544e489dc59b1bb751b4c2d417d7ab41dfa35e Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 19 Jun 2025 02:10:50 +0200 Subject: [PATCH 3/9] Use smaller atomic types for ESP32 BLE Tracker ring buffer indices (#9106) --- .../components/esp32_ble_tracker/esp32_ble_tracker.cpp | 10 +++++----- .../components/esp32_ble_tracker/esp32_ble_tracker.h | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp index c5906779f1..4785c29230 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.cpp @@ -122,10 +122,10 @@ void ESP32BLETracker::loop() { // Consumer side: This runs in the main loop thread if (this->scanner_state_ == ScannerState::RUNNING) { // Load our own index with relaxed ordering (we're the only writer) - size_t read_idx = this->ring_read_index_.load(std::memory_order_relaxed); + uint8_t read_idx = this->ring_read_index_.load(std::memory_order_relaxed); // Load producer's index with acquire to see their latest writes - size_t write_idx = this->ring_write_index_.load(std::memory_order_acquire); + uint8_t write_idx = this->ring_write_index_.load(std::memory_order_acquire); while (read_idx != write_idx) { // Process one result at a time directly from ring buffer @@ -409,11 +409,11 @@ void ESP32BLETracker::gap_scan_event_handler(const BLEScanResult &scan_result) { // IMPORTANT: Only this thread writes to ring_write_index_ // Load our own index with relaxed ordering (we're the only writer) - size_t write_idx = this->ring_write_index_.load(std::memory_order_relaxed); - size_t next_write_idx = (write_idx + 1) % SCAN_RESULT_BUFFER_SIZE; + uint8_t write_idx = this->ring_write_index_.load(std::memory_order_relaxed); + uint8_t next_write_idx = (write_idx + 1) % SCAN_RESULT_BUFFER_SIZE; // Load consumer's index with acquire to see their latest updates - size_t read_idx = this->ring_read_index_.load(std::memory_order_acquire); + uint8_t read_idx = this->ring_read_index_.load(std::memory_order_acquire); // Check if buffer is full if (next_write_idx != read_idx) { diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h index 16a100fb47..490ed19645 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h @@ -289,9 +289,9 @@ class ESP32BLETracker : public Component, // Consumer: ESPHome main loop (loop() method) // This design ensures zero blocking in the BT callback and prevents scan result loss BLEScanResult *scan_ring_buffer_; - std::atomic ring_write_index_{0}; // Written only by BT callback (producer) - std::atomic ring_read_index_{0}; // Written only by main loop (consumer) - std::atomic scan_results_dropped_{0}; // Tracks buffer overflow events + std::atomic ring_write_index_{0}; // Written only by BT callback (producer) + std::atomic ring_read_index_{0}; // Written only by main loop (consumer) + std::atomic scan_results_dropped_{0}; // Tracks buffer overflow events esp_bt_status_t scan_start_failed_{ESP_BT_STATUS_SUCCESS}; esp_bt_status_t scan_set_param_failed_{ESP_BT_STATUS_SUCCESS}; From 43c677ef37f84cc826191f16f148387979118497 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 19 Jun 2025 02:12:14 +0200 Subject: [PATCH 4/9] Optimize API server performance by using cached loop time (#9104) --- esphome/components/api/api_server.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esphome/components/api/api_server.cpp b/esphome/components/api/api_server.cpp index 6852afe937..740e4259b1 100644 --- a/esphome/components/api/api_server.cpp +++ b/esphome/components/api/api_server.cpp @@ -106,7 +106,7 @@ void APIServer::setup() { } #endif - this->last_connected_ = millis(); + this->last_connected_ = App.get_loop_component_start_time(); #ifdef USE_ESP32_CAMERA if (esp32_camera::global_esp32_camera != nullptr && !esp32_camera::global_esp32_camera->is_internal()) { @@ -164,7 +164,7 @@ void APIServer::loop() { } if (this->reboot_timeout_ != 0) { - const uint32_t now = millis(); + const uint32_t now = App.get_loop_component_start_time(); if (!this->is_connected()) { if (now - this->last_connected_ > this->reboot_timeout_) { ESP_LOGE(TAG, "No client connected; rebooting"); From eeb0710ad461036521568eb3cc005ad5efa1c1b0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 19 Jun 2025 03:08:25 +0200 Subject: [PATCH 5/9] Optimize API component memory usage by reordering class members to reduce padding (#9111) --- esphome/components/api/api_connection.cpp | 45 +++++---- esphome/components/api/api_connection.h | 55 ++++++----- esphome/components/api/api_frame_helper.h | 106 ++++++++++++---------- esphome/components/api/api_server.h | 16 +++- 4 files changed, 125 insertions(+), 97 deletions(-) diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 3e2b7c0154..ca5689bdf6 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -61,8 +61,8 @@ void APIConnection::start() { APIError err = this->helper_->init(); if (err != APIError::OK) { on_fatal_error(); - ESP_LOGW(TAG, "%s: Helper init failed: %s errno=%d", this->client_combined_info_.c_str(), api_error_to_str(err), - errno); + ESP_LOGW(TAG, "%s: Helper init failed: %s errno=%d", this->get_client_combined_info().c_str(), + api_error_to_str(err), errno); return; } this->client_info_ = helper_->getpeername(); @@ -91,7 +91,7 @@ void APIConnection::loop() { // when network is disconnected force disconnect immediately // don't wait for timeout this->on_fatal_error(); - ESP_LOGW(TAG, "%s: Network unavailable; disconnecting", this->client_combined_info_.c_str()); + ESP_LOGW(TAG, "%s: Network unavailable; disconnecting", this->get_client_combined_info().c_str()); return; } if (this->next_close_) { @@ -104,7 +104,7 @@ void APIConnection::loop() { APIError err = this->helper_->loop(); if (err != APIError::OK) { on_fatal_error(); - ESP_LOGW(TAG, "%s: Socket operation failed: %s errno=%d", this->client_combined_info_.c_str(), + ESP_LOGW(TAG, "%s: Socket operation failed: %s errno=%d", this->get_client_combined_info().c_str(), api_error_to_str(err), errno); return; } @@ -118,12 +118,12 @@ void APIConnection::loop() { } else if (err != APIError::OK) { on_fatal_error(); if (err == APIError::SOCKET_READ_FAILED && errno == ECONNRESET) { - ESP_LOGW(TAG, "%s: Connection reset", this->client_combined_info_.c_str()); + ESP_LOGW(TAG, "%s: Connection reset", this->get_client_combined_info().c_str()); } else if (err == APIError::CONNECTION_CLOSED) { - ESP_LOGW(TAG, "%s: Connection closed", this->client_combined_info_.c_str()); + ESP_LOGW(TAG, "%s: Connection closed", this->get_client_combined_info().c_str()); } else { - ESP_LOGW(TAG, "%s: Reading failed: %s errno=%d", this->client_combined_info_.c_str(), api_error_to_str(err), - errno); + ESP_LOGW(TAG, "%s: Reading failed: %s errno=%d", this->get_client_combined_info().c_str(), + api_error_to_str(err), errno); } return; } else { @@ -157,7 +157,7 @@ void APIConnection::loop() { // 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->client_combined_info_.c_str()); + 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_) { ESP_LOGVV(TAG, "Sending keepalive PING"); @@ -166,7 +166,7 @@ void APIConnection::loop() { 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->client_combined_info_.c_str(), this->ping_retries_); + 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()); @@ -233,7 +233,7 @@ DisconnectResponse APIConnection::disconnect(const DisconnectRequest &msg) { // remote initiated disconnect_client // don't close yet, we still need to send the disconnect response // close will happen on next loop - ESP_LOGD(TAG, "%s disconnected", this->client_combined_info_.c_str()); + ESP_LOGD(TAG, "%s disconnected", this->get_client_combined_info().c_str()); this->next_close_ = true; DisconnectResponse resp; return resp; @@ -1544,8 +1544,7 @@ bool APIConnection::try_send_log_message(int level, const char *tag, const char HelloResponse APIConnection::hello(const HelloRequest &msg) { this->client_info_ = msg.client_info; this->client_peername_ = this->helper_->getpeername(); - this->client_combined_info_ = this->client_info_ + " (" + this->client_peername_ + ")"; - this->helper_->set_log_info(this->client_combined_info_); + this->helper_->set_log_info(this->get_client_combined_info()); this->client_api_version_major_ = msg.api_version_major; this->client_api_version_minor_ = msg.api_version_minor; ESP_LOGV(TAG, "Hello from client: '%s' | %s | API Version %" PRIu32 ".%" PRIu32, this->client_info_.c_str(), @@ -1567,7 +1566,7 @@ ConnectResponse APIConnection::connect(const ConnectRequest &msg) { // bool invalid_password = 1; resp.invalid_password = !correct; if (correct) { - ESP_LOGD(TAG, "%s connected", this->client_combined_info_.c_str()); + ESP_LOGD(TAG, "%s connected", this->get_client_combined_info().c_str()); this->connection_state_ = ConnectionState::AUTHENTICATED; this->parent_->get_client_connected_trigger()->trigger(this->client_info_, this->client_peername_); #ifdef USE_HOMEASSISTANT_TIME @@ -1673,7 +1672,7 @@ bool APIConnection::try_to_clear_buffer(bool log_out_of_space) { APIError err = this->helper_->loop(); if (err != APIError::OK) { on_fatal_error(); - ESP_LOGW(TAG, "%s: Socket operation failed: %s errno=%d", this->client_combined_info_.c_str(), + ESP_LOGW(TAG, "%s: Socket operation failed: %s errno=%d", this->get_client_combined_info().c_str(), api_error_to_str(err), errno); return false; } @@ -1695,10 +1694,10 @@ bool APIConnection::send_buffer(ProtoWriteBuffer buffer, uint16_t message_type) if (err != APIError::OK) { on_fatal_error(); if (err == APIError::SOCKET_WRITE_FAILED && errno == ECONNRESET) { - ESP_LOGW(TAG, "%s: Connection reset", this->client_combined_info_.c_str()); + ESP_LOGW(TAG, "%s: Connection reset", this->get_client_combined_info().c_str()); } else { - ESP_LOGW(TAG, "%s: Packet write failed %s errno=%d", this->client_combined_info_.c_str(), api_error_to_str(err), - errno); + ESP_LOGW(TAG, "%s: Packet write failed %s errno=%d", this->get_client_combined_info().c_str(), + api_error_to_str(err), errno); } return false; } @@ -1707,11 +1706,11 @@ bool APIConnection::send_buffer(ProtoWriteBuffer buffer, uint16_t message_type) } void APIConnection::on_unauthenticated_access() { this->on_fatal_error(); - ESP_LOGD(TAG, "%s requested access without authentication", this->client_combined_info_.c_str()); + ESP_LOGD(TAG, "%s requested access without authentication", this->get_client_combined_info().c_str()); } void APIConnection::on_no_setup_connection() { this->on_fatal_error(); - ESP_LOGD(TAG, "%s requested access without full connection", this->client_combined_info_.c_str()); + ESP_LOGD(TAG, "%s requested access without full connection", this->get_client_combined_info().c_str()); } void APIConnection::on_fatal_error() { this->helper_->close(); @@ -1860,10 +1859,10 @@ void APIConnection::process_batch_() { if (err != APIError::OK && err != APIError::WOULD_BLOCK) { on_fatal_error(); if (err == APIError::SOCKET_WRITE_FAILED && errno == ECONNRESET) { - ESP_LOGW(TAG, "%s: Connection reset during batch write", this->client_combined_info_.c_str()); + ESP_LOGW(TAG, "%s: Connection reset during batch write", this->get_client_combined_info().c_str()); } else { - ESP_LOGW(TAG, "%s: Batch write failed %s errno=%d", this->client_combined_info_.c_str(), api_error_to_str(err), - errno); + ESP_LOGW(TAG, "%s: Batch write failed %s errno=%d", this->get_client_combined_info().c_str(), + api_error_to_str(err), errno); } } diff --git a/esphome/components/api/api_connection.h b/esphome/components/api/api_connection.h index 7cd41561d4..66b7ce38a7 100644 --- a/esphome/components/api/api_connection.h +++ b/esphome/components/api/api_connection.h @@ -275,7 +275,13 @@ class APIConnection : public APIServerConnection { bool try_to_clear_buffer(bool log_out_of_space); bool send_buffer(ProtoWriteBuffer buffer, uint16_t message_type) override; - std::string get_client_combined_info() const { return this->client_combined_info_; } + std::string get_client_combined_info() const { + if (this->client_info_ == this->client_peername_) { + // Before Hello message, both are the same (just IP:port) + return this->client_info_; + } + return this->client_info_ + " (" + this->client_peername_ + ")"; + } // Buffer allocator methods for batch processing ProtoWriteBuffer allocate_single_message_buffer(uint16_t size); @@ -432,37 +438,44 @@ 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); - enum class ConnectionState { + // 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) + std::string client_info_; + std::string client_peername_; + + // 2-byte aligned types + uint16_t client_api_version_major_{0}; + uint16_t client_api_version_minor_{0}; + + // Group all 1-byte types together to minimize padding + enum class ConnectionState : uint8_t { WAITING_FOR_HELLO, CONNECTED, AUTHENTICATED, } connection_state_{ConnectionState::WAITING_FOR_HELLO}; - + uint8_t log_subscription_{ESPHOME_LOG_LEVEL_NONE}; bool remove_{false}; - - std::unique_ptr helper_; - - std::string client_info_; - std::string client_peername_; - std::string client_combined_info_; - uint32_t client_api_version_major_{0}; - uint32_t client_api_version_minor_{0}; -#ifdef USE_ESP32_CAMERA - esp32_camera::CameraImageReader image_reader_; -#endif - bool state_subscription_{false}; - int log_subscription_{ESPHOME_LOG_LEVEL_NONE}; - uint32_t last_traffic_; - uint32_t next_ping_retry_{0}; - uint8_t ping_retries_{0}; bool sent_ping_{false}; bool service_call_subscription_{false}; bool next_close_ = false; - APIServer *parent_; + uint8_t ping_retries_{0}; + // 8 bytes used, no padding needed + + // Larger objects at the end InitialStateIterator initial_state_iterator_; ListEntitiesIterator list_entities_iterator_; - int state_subs_at_ = -1; +#ifdef USE_ESP32_CAMERA + esp32_camera::CameraImageReader image_reader_; +#endif // Function pointer type for message encoding using MessageCreatorPtr = uint16_t (*)(EntityBase *, APIConnection *, uint32_t remaining_size, bool is_single); diff --git a/esphome/components/api/api_frame_helper.h b/esphome/components/api/api_frame_helper.h index dc71a7ca17..7e90153091 100644 --- a/esphome/components/api/api_frame_helper.h +++ b/esphome/components/api/api_frame_helper.h @@ -125,38 +125,6 @@ class APIFrameHelper { const uint8_t *current_data() const { return data.data() + offset; } }; - // Queue of data buffers to be sent - std::deque tx_buf_; - - // Common state enum for all frame helpers - // Note: Not all states are used by all implementations - // - INITIALIZE: Used by both Noise and Plaintext - // - CLIENT_HELLO, SERVER_HELLO, HANDSHAKE: Only used by Noise protocol - // - DATA: Used by both Noise and Plaintext - // - CLOSED: Used by both Noise and Plaintext - // - FAILED: Used by both Noise and Plaintext - // - EXPLICIT_REJECT: Only used by Noise protocol - enum class State { - INITIALIZE = 1, - CLIENT_HELLO = 2, // Noise only - SERVER_HELLO = 3, // Noise only - HANDSHAKE = 4, // Noise only - DATA = 5, - CLOSED = 6, - FAILED = 7, - EXPLICIT_REJECT = 8, // Noise only - }; - - // Current state of the frame helper - State state_{State::INITIALIZE}; - - // Helper name for logging - std::string info_; - - // Socket for communication - socket::Socket *socket_{nullptr}; - std::unique_ptr socket_owned_; - // Common implementation for writing raw data to socket APIError write_raw_(const struct iovec *iov, int iovcnt); @@ -169,15 +137,41 @@ class APIFrameHelper { APIError write_raw_(const struct iovec *iov, int iovcnt, socket::Socket *socket, std::vector &tx_buf, const std::string &info, StateEnum &state, StateEnum failed_state); + // Pointers first (4 bytes each) + socket::Socket *socket_{nullptr}; + std::unique_ptr socket_owned_; + + // Common state enum for all frame helpers + // Note: Not all states are used by all implementations + // - INITIALIZE: Used by both Noise and Plaintext + // - CLIENT_HELLO, SERVER_HELLO, HANDSHAKE: Only used by Noise protocol + // - DATA: Used by both Noise and Plaintext + // - CLOSED: Used by both Noise and Plaintext + // - FAILED: Used by both Noise and Plaintext + // - EXPLICIT_REJECT: Only used by Noise protocol + enum class State : uint8_t { + INITIALIZE = 1, + CLIENT_HELLO = 2, // Noise only + SERVER_HELLO = 3, // Noise only + HANDSHAKE = 4, // Noise only + DATA = 5, + CLOSED = 6, + FAILED = 7, + EXPLICIT_REJECT = 8, // Noise only + }; + + // Containers (size varies, but typically 12+ bytes on 32-bit) + std::deque tx_buf_; + std::string info_; + std::vector reusable_iovs_; + std::vector rx_buf_; + + // Group smaller types together + uint16_t rx_buf_len_ = 0; + State state_{State::INITIALIZE}; uint8_t frame_header_padding_{0}; uint8_t frame_footer_size_{0}; - - // Reusable IOV array for write_protobuf_packets to avoid repeated allocations - std::vector reusable_iovs_; - - // Receive buffer for reading frame data - std::vector rx_buf_; - uint16_t rx_buf_len_ = 0; + // 5 bytes total, 3 bytes padding // Common initialization for both plaintext and noise protocols APIError init_common_(); @@ -213,19 +207,28 @@ class APINoiseFrameHelper : public APIFrameHelper { APIError init_handshake_(); APIError check_handshake_finished_(); void send_explicit_handshake_reject_(const std::string &reason); + + // Pointers first (4 bytes each) + NoiseHandshakeState *handshake_{nullptr}; + NoiseCipherState *send_cipher_{nullptr}; + NoiseCipherState *recv_cipher_{nullptr}; + + // Shared pointer (8 bytes on 32-bit = 4 bytes control block pointer + 4 bytes object pointer) + std::shared_ptr ctx_; + + // Vector (12 bytes on 32-bit) + std::vector prologue_; + + // NoiseProtocolId (size depends on implementation) + NoiseProtocolId nid_; + + // Group small types together // Fixed-size header buffer for noise protocol: // 1 byte for indicator + 2 bytes for message size (16-bit value, not varint) // Note: Maximum message size is UINT16_MAX (65535), with a limit of 128 bytes during handshake phase uint8_t rx_header_buf_[3]; uint8_t rx_header_buf_len_ = 0; - - std::vector prologue_; - - std::shared_ptr ctx_; - NoiseHandshakeState *handshake_{nullptr}; - NoiseCipherState *send_cipher_{nullptr}; - NoiseCipherState *recv_cipher_{nullptr}; - NoiseProtocolId nid_; + // 4 bytes total, no padding }; #endif // USE_API_NOISE @@ -252,6 +255,12 @@ class APIPlaintextFrameHelper : public APIFrameHelper { protected: APIError try_read_frame_(ParsedFrame *frame); + + // Group 2-byte aligned types + uint16_t rx_header_parsed_type_ = 0; + uint16_t rx_header_parsed_len_ = 0; + + // Group 1-byte types together // Fixed-size header buffer for plaintext protocol: // We now store the indicator byte + the two varints. // To match noise protocol's maximum message size (UINT16_MAX = 65535), we need: @@ -263,8 +272,7 @@ class APIPlaintextFrameHelper : public APIFrameHelper { uint8_t rx_header_buf_[6]; // 1 byte indicator + 5 bytes for varints (3 for size + 2 for type) uint8_t rx_header_buf_pos_ = 0; bool rx_header_parsed_ = false; - uint16_t rx_header_parsed_type_ = 0; - uint16_t rx_header_parsed_len_ = 0; + // 8 bytes total, no padding needed }; #endif diff --git a/esphome/components/api/api_server.h b/esphome/components/api/api_server.h index 971c192e4b..33412d8a68 100644 --- a/esphome/components/api/api_server.h +++ b/esphome/components/api/api_server.h @@ -142,19 +142,27 @@ class APIServer : public Component, public Controller { } protected: - bool shutting_down_ = false; + // Pointers and pointer-like types first (4 bytes each) std::unique_ptr socket_ = nullptr; - uint16_t port_{6053}; + Trigger *client_connected_trigger_ = new Trigger(); + Trigger *client_disconnected_trigger_ = new Trigger(); + + // 4-byte aligned types uint32_t reboot_timeout_{300000}; uint32_t batch_delay_{100}; uint32_t last_connected_{0}; + + // Vectors and strings (12 bytes each on 32-bit) std::vector> clients_; std::string password_; std::vector shared_write_buffer_; // Shared proto write buffer for all connections std::vector state_subs_; std::vector user_services_; - Trigger *client_connected_trigger_ = new Trigger(); - Trigger *client_disconnected_trigger_ = new Trigger(); + + // Group smaller types together + uint16_t port_{6053}; + bool shutting_down_ = false; + // 3 bytes used, 1 byte padding #ifdef USE_API_NOISE std::shared_ptr noise_ctx_ = std::make_shared(); From 2e11e66db473cf454b93f6dbd115d53e5e4f3b28 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 19 Jun 2025 03:11:13 +0200 Subject: [PATCH 6/9] Optimize bluetooth_proxy memory usage on ESP32 (#9114) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../bluetooth_proxy/bluetooth_connection.h | 11 +++++-- .../bluetooth_proxy/bluetooth_proxy.h | 12 +++++-- .../esp32_ble_client/ble_client_base.h | 33 +++++++++++++------ .../esp32_ble_tracker/esp32_ble_tracker.h | 12 ++++--- 4 files changed, 49 insertions(+), 19 deletions(-) diff --git a/esphome/components/bluetooth_proxy/bluetooth_connection.h b/esphome/components/bluetooth_proxy/bluetooth_connection.h index fd83f8dd00..73c034d93b 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_connection.h +++ b/esphome/components/bluetooth_proxy/bluetooth_connection.h @@ -26,10 +26,17 @@ class BluetoothConnection : public esp32_ble_client::BLEClientBase { protected: friend class BluetoothProxy; - bool seen_mtu_or_services_{false}; - int16_t send_service_{-2}; + // Memory optimized layout for 32-bit systems + // Group 1: Pointers (4 bytes each, naturally aligned) BluetoothProxy *proxy_; + + // Group 2: 2-byte types + int16_t send_service_{-2}; // Needs to handle negative values and service count + + // Group 3: 1-byte types + bool seen_mtu_or_services_{false}; + // 1 byte used, 1 byte padding }; } // namespace bluetooth_proxy diff --git a/esphome/components/bluetooth_proxy/bluetooth_proxy.h b/esphome/components/bluetooth_proxy/bluetooth_proxy.h index 16db0a0a11..f0632350e0 100644 --- a/esphome/components/bluetooth_proxy/bluetooth_proxy.h +++ b/esphome/components/bluetooth_proxy/bluetooth_proxy.h @@ -134,11 +134,17 @@ class BluetoothProxy : public esp32_ble_tracker::ESPBTDeviceListener, public Com BluetoothConnection *get_connection_(uint64_t address, bool reserve); - bool active_; - - std::vector connections_{}; + // Memory optimized layout for 32-bit systems + // Group 1: Pointers (4 bytes each, naturally aligned) api::APIConnection *api_connection_{nullptr}; + + // Group 2: Container types (typically 12 bytes on 32-bit) + std::vector connections_{}; + + // Group 3: 1-byte types grouped together + bool active_; bool raw_advertisements_{false}; + // 2 bytes used, 2 bytes padding }; extern BluetoothProxy *global_bluetooth_proxy; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) diff --git a/esphome/components/esp32_ble_client/ble_client_base.h b/esphome/components/esp32_ble_client/ble_client_base.h index 814a9664d9..bf3b589b1b 100644 --- a/esphome/components/esp32_ble_client/ble_client_base.h +++ b/esphome/components/esp32_ble_client/ble_client_base.h @@ -96,21 +96,34 @@ class BLEClientBase : public espbt::ESPBTClient, public Component { void set_state(espbt::ClientState st) override; protected: - int gattc_if_; - esp_bd_addr_t remote_bda_; - esp_ble_addr_type_t remote_addr_type_{BLE_ADDR_TYPE_PUBLIC}; - uint16_t conn_id_{UNSET_CONN_ID}; + // Memory optimized layout for 32-bit systems + // Group 1: 8-byte types uint64_t address_{0}; - bool auto_connect_{false}; + + // Group 2: Container types (grouped for memory optimization) std::string address_str_{}; - uint8_t connection_index_; - int16_t service_count_{0}; - uint16_t mtu_{23}; - bool paired_{false}; - espbt::ConnectionType connection_type_{espbt::ConnectionType::V1}; std::vector services_; + + // Group 3: 4-byte types + int gattc_if_; esp_gatt_status_t status_{ESP_GATT_OK}; + // Group 4: Arrays (6 bytes) + esp_bd_addr_t remote_bda_; + + // Group 5: 2-byte types + uint16_t conn_id_{UNSET_CONN_ID}; + uint16_t mtu_{23}; + + // Group 6: 1-byte types and small enums + esp_ble_addr_type_t remote_addr_type_{BLE_ADDR_TYPE_PUBLIC}; + espbt::ConnectionType connection_type_{espbt::ConnectionType::V1}; + uint8_t connection_index_; + uint8_t service_count_{0}; // ESP32 has max handles < 255, typical devices have < 50 services + bool auto_connect_{false}; + bool paired_{false}; + // 6 bytes used, 2 bytes padding + void log_event_(const char *name); }; diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h index 490ed19645..414c9f4b48 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h @@ -129,7 +129,7 @@ class ESPBTDeviceListener { ESP32BLETracker *parent_{nullptr}; }; -enum class ClientState { +enum class ClientState : uint8_t { // Connection is allocated INIT, // Client is disconnecting @@ -165,7 +165,7 @@ enum class ScannerState { STOPPED, }; -enum class ConnectionType { +enum class ConnectionType : uint8_t { // The default connection type, we hold all the services in ram // for the duration of the connection. V1, @@ -193,15 +193,19 @@ class ESPBTClient : public ESPBTDeviceListener { } } ClientState state() const { return state_; } - int app_id; + + // Memory optimized layout + uint8_t app_id; // App IDs are small integers assigned sequentially protected: + // Group 1: 1-byte types ClientState state_{ClientState::INIT}; // want_disconnect_ is set to true when a disconnect is requested // while the client is connecting. This is used to disconnect the // client as soon as we get the connection id (conn_id_) from the // ESP_GATTC_OPEN_EVT event. bool want_disconnect_{false}; + // 2 bytes used, 2 bytes padding }; class ESP32BLETracker : public Component, @@ -262,7 +266,7 @@ class ESP32BLETracker : public Component, /// Called to set the scanner state. Will also call callbacks to let listeners know when state is changed. void set_scanner_state_(ScannerState state); - int app_id_{0}; + uint8_t app_id_{0}; /// Vector of addresses that have already been printed in print_bt_device_info std::vector already_discovered_; From 8ba22183b962db2664112c772c3b5fbd88f0a7ee Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 19 Jun 2025 03:30:41 +0200 Subject: [PATCH 7/9] Add enable_loop_soon_any_context() for thread and ISR-safe loop enabling (#9127) --- esphome/core/application.cpp | 94 ++++++++++++++++--- esphome/core/application.h | 3 + esphome/core/component.cpp | 23 ++++- esphome/core/component.h | 31 +++++- .../loop_test_component/__init__.py | 18 ++++ .../loop_test_isr_component.cpp | 80 ++++++++++++++++ .../loop_test_isr_component.h | 32 +++++++ .../fixtures/loop_disable_enable.yaml | 5 + tests/integration/test_loop_disable_enable.py | 59 +++++++++++- 9 files changed, 325 insertions(+), 20 deletions(-) create mode 100644 tests/integration/fixtures/external_components/loop_test_component/loop_test_isr_component.cpp create mode 100644 tests/integration/fixtures/external_components/loop_test_component/loop_test_isr_component.h diff --git a/esphome/core/application.cpp b/esphome/core/application.cpp index 58df49f0f2..49c1e5fd61 100644 --- a/esphome/core/application.cpp +++ b/esphome/core/application.cpp @@ -97,6 +97,20 @@ void Application::loop() { // Feed WDT with time this->feed_wdt(last_op_end_time); + // Process any pending enable_loop requests from ISRs + // This must be done before marking in_loop_ = true to avoid race conditions + if (this->has_pending_enable_loop_requests_) { + // Clear flag BEFORE processing to avoid race condition + // If ISR sets it during processing, we'll catch it next loop iteration + // This is safe because: + // 1. Each component has its own pending_enable_loop_ flag that we check + // 2. If we can't process a component (wrong state), enable_pending_loops_() + // will set this flag back to true + // 3. Any new ISR requests during processing will set the flag again + this->has_pending_enable_loop_requests_ = false; + this->enable_pending_loops_(); + } + // Mark that we're in the loop for safe reentrant modifications this->in_loop_ = true; @@ -286,24 +300,82 @@ void Application::disable_component_loop_(Component *component) { } } +void Application::activate_looping_component_(uint16_t index) { + // Helper to move component from inactive to active section + if (index != this->looping_components_active_end_) { + std::swap(this->looping_components_[index], this->looping_components_[this->looping_components_active_end_]); + } + this->looping_components_active_end_++; +} + void Application::enable_component_loop_(Component *component) { - // This method must be reentrant - components can re-enable themselves during their own loop() call - // Single pass through all components to find and move if needed - // With typical 10-30 components, O(n) is faster than maintaining a map + // This method is only called when component state is LOOP_DONE, so we know + // the component must be in the inactive section (if it exists in looping_components_) + // Only search the inactive portion for better performance + // With typical 0-5 inactive components, O(k) is much faster than O(n) const uint16_t size = this->looping_components_.size(); - for (uint16_t i = 0; i < size; i++) { + for (uint16_t i = this->looping_components_active_end_; i < size; i++) { if (this->looping_components_[i] == component) { - if (i < this->looping_components_active_end_) { - return; // Already active - } // Found in inactive section - move to active - if (i != this->looping_components_active_end_) { - std::swap(this->looping_components_[i], this->looping_components_[this->looping_components_active_end_]); - } - this->looping_components_active_end_++; + this->activate_looping_component_(i); return; } } + // Component not found in looping_components_ - this is normal for components + // that don't have loop() or were not included in the partitioned vector +} + +void Application::enable_pending_loops_() { + // Process components that requested enable_loop from ISR context + // Only iterate through inactive looping_components_ (typically 0-5) instead of all components + // + // Race condition handling: + // 1. We check if component is already in LOOP state first - if so, just clear the flag + // This handles reentrancy where enable_loop() was called between ISR and processing + // 2. We only clear pending_enable_loop_ after checking state, preventing lost requests + // 3. If any components aren't in LOOP_DONE state, we set has_pending_enable_loop_requests_ + // back to true to ensure we check again next iteration + // 4. ISRs can safely set flags at any time - worst case is we process them next iteration + // 5. The global flag (has_pending_enable_loop_requests_) is cleared before this method, + // so any ISR that fires during processing will be caught in the next loop + const uint16_t size = this->looping_components_.size(); + bool has_pending = false; + + for (uint16_t i = this->looping_components_active_end_; i < size; i++) { + Component *component = this->looping_components_[i]; + if (!component->pending_enable_loop_) { + continue; // Skip components without pending requests + } + + // Check current state + uint8_t state = component->component_state_ & COMPONENT_STATE_MASK; + + // If already in LOOP state, nothing to do - clear flag and continue + if (state == COMPONENT_STATE_LOOP) { + component->pending_enable_loop_ = false; + continue; + } + + // If not in LOOP_DONE state, can't enable yet - keep flag set + if (state != COMPONENT_STATE_LOOP_DONE) { + has_pending = true; // Keep tracking this component + continue; // Keep the flag set - try again next iteration + } + + // Clear the pending flag and enable the loop + component->pending_enable_loop_ = false; + ESP_LOGD(TAG, "%s loop enabled from ISR", component->get_component_source()); + component->component_state_ &= ~COMPONENT_STATE_MASK; + component->component_state_ |= COMPONENT_STATE_LOOP; + + // Move to active section + this->activate_looping_component_(i); + } + + // If we couldn't process some requests, ensure we check again next iteration + if (has_pending) { + this->has_pending_enable_loop_requests_ = true; + } } #ifdef USE_SOCKET_SELECT_SUPPORT diff --git a/esphome/core/application.h b/esphome/core/application.h index ea298638d2..93d5a78958 100644 --- a/esphome/core/application.h +++ b/esphome/core/application.h @@ -577,6 +577,8 @@ class Application { // to ensure component state is properly updated along with the loop partition void disable_component_loop_(Component *component); void enable_component_loop_(Component *component); + void enable_pending_loops_(); + void activate_looping_component_(uint16_t index); void feed_wdt_arch_(); @@ -682,6 +684,7 @@ class Application { uint32_t loop_interval_{16}; size_t dump_config_at_{SIZE_MAX}; uint8_t app_state_{0}; + volatile bool has_pending_enable_loop_requests_{false}; Component *current_component_{nullptr}; uint32_t loop_component_start_time_{0}; diff --git a/esphome/core/component.cpp b/esphome/core/component.cpp index 3117f49ac1..625a7b2125 100644 --- a/esphome/core/component.cpp +++ b/esphome/core/component.cpp @@ -148,10 +148,12 @@ void Component::mark_failed() { App.disable_component_loop_(this); } void Component::disable_loop() { - ESP_LOGD(TAG, "%s loop disabled", this->get_component_source()); - this->component_state_ &= ~COMPONENT_STATE_MASK; - this->component_state_ |= COMPONENT_STATE_LOOP_DONE; - App.disable_component_loop_(this); + if ((this->component_state_ & COMPONENT_STATE_MASK) != COMPONENT_STATE_LOOP_DONE) { + ESP_LOGD(TAG, "%s loop disabled", this->get_component_source()); + this->component_state_ &= ~COMPONENT_STATE_MASK; + this->component_state_ |= COMPONENT_STATE_LOOP_DONE; + App.disable_component_loop_(this); + } } void Component::enable_loop() { if ((this->component_state_ & COMPONENT_STATE_MASK) == COMPONENT_STATE_LOOP_DONE) { @@ -161,6 +163,19 @@ void Component::enable_loop() { App.enable_component_loop_(this); } } +void IRAM_ATTR HOT Component::enable_loop_soon_any_context() { + // This method is thread and ISR-safe because: + // 1. Only performs simple assignments to volatile variables (atomic on all platforms) + // 2. No read-modify-write operations that could be interrupted + // 3. No memory allocation, object construction, or function calls + // 4. IRAM_ATTR ensures code is in IRAM, not flash (required for ISR execution) + // 5. Components are never destroyed, so no use-after-free concerns + // 6. App is guaranteed to be initialized before any ISR could fire + // 7. Multiple ISR/thread calls are safe - just sets the same flags to true + // 8. Race condition with main loop is handled by clearing flag before processing + this->pending_enable_loop_ = true; + App.has_pending_enable_loop_requests_ = true; +} void Component::reset_to_construction_state() { if ((this->component_state_ & COMPONENT_STATE_MASK) == COMPONENT_STATE_FAILED) { ESP_LOGI(TAG, "Component %s is being reset to construction state", this->get_component_source()); diff --git a/esphome/core/component.h b/esphome/core/component.h index a37d64086a..7f2bdd8414 100644 --- a/esphome/core/component.h +++ b/esphome/core/component.h @@ -171,6 +171,27 @@ class Component { */ void enable_loop(); + /** Thread and ISR-safe version of enable_loop() that can be called from any context. + * + * This method defers the actual enable via enable_pending_loops_ to the main loop, + * making it safe to call from ISR handlers, timer callbacks, other threads, + * or any interrupt context. + * + * @note The actual loop enabling will happen on the next main loop iteration. + * @note Only one pending enable request is tracked per component. + * @note There is no disable_loop_soon_any_context() on purpose - it would race + * against enable calls and synchronization would get too complex + * to provide a safe version that would work for each component. + * + * Use disable_loop() from the main thread only. + * + * If you need to disable the loop from ISR, carefully implement + * it in the component itself, with an ISR safe approach, and call + * disable_loop() in its next ::loop() iteration. Implementations + * will need to carefully consider all possible race conditions. + */ + void enable_loop_soon_any_context(); + bool is_failed() const; bool is_ready() const; @@ -331,16 +352,18 @@ class Component { /// Cancel a defer callback using the specified name, name must not be empty. bool cancel_defer(const std::string &name); // NOLINT + // Ordered for optimal packing on 32-bit systems + float setup_priority_override_{NAN}; + const char *component_source_{nullptr}; + const char *error_message_{nullptr}; + uint16_t warn_if_blocking_over_{WARN_IF_BLOCKING_OVER_MS}; ///< Warn if blocked for this many ms (max 65.5s) /// State of this component - each bit has a purpose: /// Bits 0-1: Component state (0x00=CONSTRUCTION, 0x01=SETUP, 0x02=LOOP, 0x03=FAILED) /// Bit 2: STATUS_LED_WARNING /// Bit 3: STATUS_LED_ERROR /// Bits 4-7: Unused - reserved for future expansion (50% of the bits are free) uint8_t component_state_{0x00}; - float setup_priority_override_{NAN}; - const char *component_source_{nullptr}; - uint16_t warn_if_blocking_over_{WARN_IF_BLOCKING_OVER_MS}; ///< Warn if blocked for this many ms (max 65.5s) - const char *error_message_{nullptr}; + volatile bool pending_enable_loop_{false}; ///< ISR-safe flag for enable_loop_soon_any_context }; /** This class simplifies creating components that periodically check a state. diff --git a/tests/integration/fixtures/external_components/loop_test_component/__init__.py b/tests/integration/fixtures/external_components/loop_test_component/__init__.py index c5eda67d1e..b66d4598f4 100644 --- a/tests/integration/fixtures/external_components/loop_test_component/__init__.py +++ b/tests/integration/fixtures/external_components/loop_test_component/__init__.py @@ -7,9 +7,13 @@ CODEOWNERS = ["@esphome/tests"] loop_test_component_ns = cg.esphome_ns.namespace("loop_test_component") LoopTestComponent = loop_test_component_ns.class_("LoopTestComponent", cg.Component) +LoopTestISRComponent = loop_test_component_ns.class_( + "LoopTestISRComponent", cg.Component +) CONF_DISABLE_AFTER = "disable_after" CONF_TEST_REDUNDANT_OPERATIONS = "test_redundant_operations" +CONF_ISR_COMPONENTS = "isr_components" COMPONENT_CONFIG_SCHEMA = cv.Schema( { @@ -20,10 +24,18 @@ COMPONENT_CONFIG_SCHEMA = cv.Schema( } ) +ISR_COMPONENT_CONFIG_SCHEMA = cv.Schema( + { + cv.GenerateID(): cv.declare_id(LoopTestISRComponent), + cv.Required(CONF_NAME): cv.string, + } +) + CONFIG_SCHEMA = cv.Schema( { cv.GenerateID(): cv.declare_id(LoopTestComponent), cv.Required(CONF_COMPONENTS): cv.ensure_list(COMPONENT_CONFIG_SCHEMA), + cv.Optional(CONF_ISR_COMPONENTS): cv.ensure_list(ISR_COMPONENT_CONFIG_SCHEMA), } ).extend(cv.COMPONENT_SCHEMA) @@ -76,3 +88,9 @@ async def to_code(config): comp_config[CONF_TEST_REDUNDANT_OPERATIONS] ) ) + + # Create ISR test components + for isr_config in config.get(CONF_ISR_COMPONENTS, []): + var = cg.new_Pvariable(isr_config[CONF_ID]) + await cg.register_component(var, isr_config) + cg.add(var.set_name(isr_config[CONF_NAME])) diff --git a/tests/integration/fixtures/external_components/loop_test_component/loop_test_isr_component.cpp b/tests/integration/fixtures/external_components/loop_test_component/loop_test_isr_component.cpp new file mode 100644 index 0000000000..30afec0422 --- /dev/null +++ b/tests/integration/fixtures/external_components/loop_test_component/loop_test_isr_component.cpp @@ -0,0 +1,80 @@ +#include "loop_test_isr_component.h" +#include "esphome/core/hal.h" +#include "esphome/core/application.h" + +namespace esphome { +namespace loop_test_component { + +static const char *const ISR_TAG = "loop_test_isr_component"; + +void LoopTestISRComponent::setup() { + ESP_LOGI(ISR_TAG, "[%s] ISR component setup called", this->name_.c_str()); + this->last_check_time_ = millis(); +} + +void LoopTestISRComponent::loop() { + this->loop_count_++; + ESP_LOGI(ISR_TAG, "[%s] ISR component loop count: %d", this->name_.c_str(), this->loop_count_); + + // Disable after 5 loops + if (this->loop_count_ == 5) { + ESP_LOGI(ISR_TAG, "[%s] Disabling after 5 loops", this->name_.c_str()); + this->disable_loop(); + this->last_disable_time_ = millis(); + // Simulate ISR after disabling + this->set_timeout("simulate_isr_1", 50, [this]() { + ESP_LOGI(ISR_TAG, "[%s] Simulating ISR enable", this->name_.c_str()); + this->simulate_isr_enable(); + // Test reentrancy - call enable_loop() directly after ISR + // This simulates another thread calling enable_loop while processing ISR enables + this->set_timeout("test_reentrant", 10, [this]() { + ESP_LOGI(ISR_TAG, "[%s] Testing reentrancy - calling enable_loop() directly", this->name_.c_str()); + this->enable_loop(); + }); + }); + } + + // If we get here after being disabled, it means ISR re-enabled us + if (this->loop_count_ > 5 && this->loop_count_ < 10) { + ESP_LOGI(ISR_TAG, "[%s] Running after ISR re-enable! ISR was called %d times", this->name_.c_str(), + this->isr_call_count_); + } + + // Disable again after 10 loops to test multiple ISR enables + if (this->loop_count_ == 10) { + ESP_LOGI(ISR_TAG, "[%s] Disabling again after 10 loops", this->name_.c_str()); + this->disable_loop(); + this->last_disable_time_ = millis(); + + // Test pure ISR enable without any main loop enable + this->set_timeout("simulate_isr_2", 50, [this]() { + ESP_LOGI(ISR_TAG, "[%s] Testing pure ISR enable (no main loop enable)", this->name_.c_str()); + this->simulate_isr_enable(); + // DO NOT call enable_loop() - test that ISR alone works + }); + } + + // Log when we're running after second ISR enable + if (this->loop_count_ > 10) { + ESP_LOGI(ISR_TAG, "[%s] Running after pure ISR re-enable! ISR was called %d times total", this->name_.c_str(), + this->isr_call_count_); + } +} + +void IRAM_ATTR LoopTestISRComponent::simulate_isr_enable() { + // This simulates what would happen in a real ISR + // In a real scenario, this would be called from an actual interrupt handler + + this->isr_call_count_++; + + // Call enable_loop_soon_any_context multiple times to test that it's safe + this->enable_loop_soon_any_context(); + this->enable_loop_soon_any_context(); // Test multiple calls + this->enable_loop_soon_any_context(); // Should be idempotent + + // Note: In a real ISR, we cannot use ESP_LOG* macros as they're not ISR-safe + // For testing, we'll track the call count and log it from the main loop +} + +} // namespace loop_test_component +} // namespace esphome diff --git a/tests/integration/fixtures/external_components/loop_test_component/loop_test_isr_component.h b/tests/integration/fixtures/external_components/loop_test_component/loop_test_isr_component.h new file mode 100644 index 0000000000..20e11b5ecd --- /dev/null +++ b/tests/integration/fixtures/external_components/loop_test_component/loop_test_isr_component.h @@ -0,0 +1,32 @@ +#pragma once + +#include "esphome/core/component.h" +#include "esphome/core/log.h" +#include "esphome/core/hal.h" + +namespace esphome { +namespace loop_test_component { + +class LoopTestISRComponent : public Component { + public: + void set_name(const std::string &name) { this->name_ = name; } + + void setup() override; + void loop() override; + + // Simulates an ISR calling enable_loop_soon_any_context + void simulate_isr_enable(); + + float get_setup_priority() const override { return setup_priority::DATA; } + + protected: + std::string name_; + int loop_count_{0}; + uint32_t last_disable_time_{0}; + uint32_t last_check_time_{0}; + bool isr_enable_pending_{false}; + int isr_call_count_{0}; +}; + +} // namespace loop_test_component +} // namespace esphome diff --git a/tests/integration/fixtures/loop_disable_enable.yaml b/tests/integration/fixtures/loop_disable_enable.yaml index 17010f7c34..f19d7f60ca 100644 --- a/tests/integration/fixtures/loop_disable_enable.yaml +++ b/tests/integration/fixtures/loop_disable_enable.yaml @@ -35,6 +35,11 @@ loop_test_component: test_redundant_operations: true disable_after: 10 + # ISR test component that uses enable_loop_soon_any_context + isr_components: + - id: isr_test + name: "isr_test" + # Interval to re-enable the self_disable_10 component after some time interval: - interval: 0.5s diff --git a/tests/integration/test_loop_disable_enable.py b/tests/integration/test_loop_disable_enable.py index 84301c25d8..d5f868aa93 100644 --- a/tests/integration/test_loop_disable_enable.py +++ b/tests/integration/test_loop_disable_enable.py @@ -41,17 +41,25 @@ async def test_loop_disable_enable( redundant_disable_tested = asyncio.Event() # Event fired when self_disable_10 component is re-enabled and runs again (count > 10) self_disable_10_re_enabled = asyncio.Event() + # Events for ISR component testing + isr_component_disabled = asyncio.Event() + isr_component_re_enabled = asyncio.Event() + isr_component_pure_re_enabled = asyncio.Event() # Track loop counts for components self_disable_10_counts: list[int] = [] normal_component_counts: list[int] = [] + isr_component_counts: list[int] = [] def on_log_line(line: str) -> None: """Process each log line from the process output.""" # Strip ANSI color codes clean_line = re.sub(r"\x1b\[[0-9;]*m", "", line) - if "loop_test_component" not in clean_line: + if ( + "loop_test_component" not in clean_line + and "loop_test_isr_component" not in clean_line + ): return log_messages.append(clean_line) @@ -92,6 +100,18 @@ async def test_loop_disable_enable( ): redundant_disable_tested.set() + # ISR component events + elif "[isr_test]" in clean_line: + if "ISR component loop count:" in clean_line: + count = int(clean_line.split("ISR component loop count: ")[1]) + isr_component_counts.append(count) + elif "Disabling after 5 loops" in clean_line: + isr_component_disabled.set() + elif "Running after ISR re-enable!" in clean_line: + isr_component_re_enabled.set() + elif "Running after pure ISR re-enable!" in clean_line: + isr_component_pure_re_enabled.set() + # Write, compile and run the ESPHome device with log callback async with ( run_compiled(yaml_config, line_callback=on_log_line), @@ -148,3 +168,40 @@ async def test_loop_disable_enable( assert later_self_disable_counts, ( "self_disable_10 was re-enabled but did not run additional times" ) + + # Test ISR component functionality + # Wait for ISR component to disable itself after 5 loops + try: + await asyncio.wait_for(isr_component_disabled.wait(), timeout=3.0) + except asyncio.TimeoutError: + pytest.fail("ISR component did not disable itself within 3 seconds") + + # Verify it ran exactly 5 times before disabling + first_run_counts = [c for c in isr_component_counts if c <= 5] + assert len(first_run_counts) == 5, ( + f"Expected 5 loops before disable, got {first_run_counts}" + ) + + # Wait for component to be re-enabled by periodic ISR simulation and run again + try: + await asyncio.wait_for(isr_component_re_enabled.wait(), timeout=2.0) + except asyncio.TimeoutError: + pytest.fail("ISR component was not re-enabled after ISR call") + + # Verify it's running again after ISR enable + count_after_isr = len(isr_component_counts) + assert count_after_isr > 5, ( + f"Component didn't run after ISR enable: got {count_after_isr} counts total" + ) + + # Wait for pure ISR enable (no main loop enable) to work + try: + await asyncio.wait_for(isr_component_pure_re_enabled.wait(), timeout=2.0) + except asyncio.TimeoutError: + pytest.fail("ISR component was not re-enabled by pure ISR call") + + # Verify it ran after pure ISR enable + final_count = len(isr_component_counts) + assert final_count > 10, ( + f"Component didn't run after pure ISR enable: got {final_count} counts total" + ) From 40a5638005fe3bcf00401c1e32ed8fa283e8a966 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 19 Jun 2025 03:33:00 +0200 Subject: [PATCH 8/9] Optimize OTA loop to avoid unnecessary stack allocations (#9129) --- .../components/esphome/ota/ota_esphome.cpp | 43 +++++++++++-------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/esphome/components/esphome/ota/ota_esphome.cpp b/esphome/components/esphome/ota/ota_esphome.cpp index 227cb676ff..4cc82b9094 100644 --- a/esphome/components/esphome/ota/ota_esphome.cpp +++ b/esphome/components/esphome/ota/ota_esphome.cpp @@ -26,19 +26,19 @@ void ESPHomeOTAComponent::setup() { ota::register_ota_platform(this); #endif - server_ = socket::socket_ip_loop_monitored(SOCK_STREAM, 0); // monitored for incoming connections - if (server_ == nullptr) { + this->server_ = socket::socket_ip_loop_monitored(SOCK_STREAM, 0); // monitored for incoming connections + if (this->server_ == nullptr) { ESP_LOGW(TAG, "Could not create socket"); this->mark_failed(); return; } int enable = 1; - int err = server_->setsockopt(SOL_SOCKET, SO_REUSEADDR, &enable, sizeof(int)); + int err = this->server_->setsockopt(SOL_SOCKET, SO_REUSEADDR, &enable, sizeof(int)); if (err != 0) { ESP_LOGW(TAG, "Socket unable to set reuseaddr: errno %d", err); // we can still continue } - err = server_->setblocking(false); + err = this->server_->setblocking(false); if (err != 0) { ESP_LOGW(TAG, "Socket unable to set nonblocking mode: errno %d", err); this->mark_failed(); @@ -54,14 +54,14 @@ void ESPHomeOTAComponent::setup() { return; } - err = server_->bind((struct sockaddr *) &server, sizeof(server)); + err = this->server_->bind((struct sockaddr *) &server, sizeof(server)); if (err != 0) { ESP_LOGW(TAG, "Socket unable to bind: errno %d", errno); this->mark_failed(); return; } - err = server_->listen(4); + err = this->server_->listen(4); if (err != 0) { ESP_LOGW(TAG, "Socket unable to listen: errno %d", errno); this->mark_failed(); @@ -82,7 +82,14 @@ void ESPHomeOTAComponent::dump_config() { #endif } -void ESPHomeOTAComponent::loop() { this->handle_(); } +void ESPHomeOTAComponent::loop() { + // Skip handle_() call if no client connected and no incoming connections + // This optimization reduces idle loop overhead when OTA is not active + // Note: No need to check server_ for null as the component is marked failed in setup() if server_ creation fails + if (this->client_ != nullptr || this->server_->ready()) { + this->handle_(); + } +} static const uint8_t FEATURE_SUPPORTS_COMPRESSION = 0x01; @@ -101,23 +108,21 @@ void ESPHomeOTAComponent::handle_() { size_t size_acknowledged = 0; #endif - if (client_ == nullptr) { - // Check if the server socket is ready before accepting - if (this->server_->ready()) { - struct sockaddr_storage source_addr; - socklen_t addr_len = sizeof(source_addr); - client_ = server_->accept((struct sockaddr *) &source_addr, &addr_len); - } + if (this->client_ == nullptr) { + // We already checked server_->ready() in loop(), so we can accept directly + struct sockaddr_storage source_addr; + socklen_t addr_len = sizeof(source_addr); + this->client_ = this->server_->accept((struct sockaddr *) &source_addr, &addr_len); + if (this->client_ == nullptr) + return; } - if (client_ == nullptr) - return; int enable = 1; - int err = client_->setsockopt(IPPROTO_TCP, TCP_NODELAY, &enable, sizeof(int)); + int err = this->client_->setsockopt(IPPROTO_TCP, TCP_NODELAY, &enable, sizeof(int)); if (err != 0) { ESP_LOGW(TAG, "Socket could not enable TCP nodelay, errno %d", errno); - client_->close(); - client_ = nullptr; + this->client_->close(); + this->client_ = nullptr; return; } From df56ca02362c9644c33132a263556a0f05b5f87a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 19 Jun 2025 03:41:25 +0200 Subject: [PATCH 9/9] remove redundant enable_loop, it must already be enabled to get here --- esphome/components/ethernet/ethernet_component.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/esphome/components/ethernet/ethernet_component.cpp b/esphome/components/ethernet/ethernet_component.cpp index 984a94b078..180a72ec7e 100644 --- a/esphome/components/ethernet/ethernet_component.cpp +++ b/esphome/components/ethernet/ethernet_component.cpp @@ -462,8 +462,6 @@ void EthernetComponent::start_connect_() { #endif /* USE_NETWORK_IPV6 */ this->connect_begin_ = millis(); this->status_set_warning("waiting for IP configuration"); - // Enable loop during connection phase - this->enable_loop(); esp_err_t err; err = esp_netif_set_hostname(this->eth_netif_, App.get_name().c_str());