mirror of
				https://github.com/esphome/esphome.git
				synced 2025-10-30 22:53:59 +00:00 
			
		
		
		
	Merge remote-tracking branch 'upstream/dev' into zwave_proxy
This commit is contained in:
		
							
								
								
									
										24
									
								
								.github/workflows/needs-docs.yml
									
									
									
									
										vendored
									
									
								
							
							
						
						
									
										24
									
								
								.github/workflows/needs-docs.yml
									
									
									
									
										vendored
									
									
								
							| @@ -1,24 +0,0 @@ | |||||||
| name: Needs Docs |  | ||||||
|  |  | ||||||
| on: |  | ||||||
|   pull_request: |  | ||||||
|     types: [labeled, unlabeled] |  | ||||||
|  |  | ||||||
| jobs: |  | ||||||
|   check: |  | ||||||
|     name: Check |  | ||||||
|     runs-on: ubuntu-latest |  | ||||||
|     steps: |  | ||||||
|       - name: Check for needs-docs label |  | ||||||
|         uses: actions/github-script@v7.0.1 |  | ||||||
|         with: |  | ||||||
|           script: | |  | ||||||
|             const { data: labels } = await github.rest.issues.listLabelsOnIssue({ |  | ||||||
|               owner: context.repo.owner, |  | ||||||
|               repo: context.repo.repo, |  | ||||||
|               issue_number: context.issue.number |  | ||||||
|             }); |  | ||||||
|             const needsDocs = labels.find(label => label.name === 'needs-docs'); |  | ||||||
|             if (needsDocs) { |  | ||||||
|               core.setFailed('Pull request needs docs'); |  | ||||||
|             } |  | ||||||
							
								
								
									
										30
									
								
								.github/workflows/status-check-labels.yml
									
									
									
									
										vendored
									
									
										Normal file
									
								
							
							
						
						
									
										30
									
								
								.github/workflows/status-check-labels.yml
									
									
									
									
										vendored
									
									
										Normal file
									
								
							| @@ -0,0 +1,30 @@ | |||||||
|  | name: Status check labels | ||||||
|  |  | ||||||
|  | on: | ||||||
|  |   pull_request: | ||||||
|  |     types: [labeled, unlabeled] | ||||||
|  |  | ||||||
|  | jobs: | ||||||
|  |   check: | ||||||
|  |     name: Check ${{ matrix.label }} | ||||||
|  |     runs-on: ubuntu-latest | ||||||
|  |     strategy: | ||||||
|  |       fail-fast: false | ||||||
|  |       matrix: | ||||||
|  |         label: | ||||||
|  |           - needs-docs | ||||||
|  |           - merge-after-release | ||||||
|  |     steps: | ||||||
|  |       - name: Check for ${{ matrix.label }} label | ||||||
|  |         uses: actions/github-script@v7.0.1 | ||||||
|  |         with: | ||||||
|  |           script: | | ||||||
|  |             const { data: labels } = await github.rest.issues.listLabelsOnIssue({ | ||||||
|  |               owner: context.repo.owner, | ||||||
|  |               repo: context.repo.repo, | ||||||
|  |               issue_number: context.issue.number | ||||||
|  |             }); | ||||||
|  |             const hasLabel = labels.find(label => label.name === '${{ matrix.label }}'); | ||||||
|  |             if (hasLabel) { | ||||||
|  |               core.setFailed('Pull request cannot be merged, it is labeled as ${{ matrix.label }}'); | ||||||
|  |             } | ||||||
| @@ -476,7 +476,7 @@ def show_logs(config: ConfigType, args: ArgsProtocol, devices: list[str]) -> int | |||||||
|         from esphome.components.api.client import run_logs |         from esphome.components.api.client import run_logs | ||||||
|  |  | ||||||
|         return run_logs(config, addresses_to_use) |         return run_logs(config, addresses_to_use) | ||||||
|     if get_port_type(port) == "MQTT" and "mqtt" in config: |     if get_port_type(port) in ("NETWORK", "MQTT") and "mqtt" in config: | ||||||
|         from esphome import mqtt |         from esphome import mqtt | ||||||
|  |  | ||||||
|         return mqtt.show_logs( |         return mqtt.show_logs( | ||||||
|   | |||||||
| @@ -44,7 +44,7 @@ static constexpr size_t MAX_PACKETS_PER_BATCH = 64;  // ESP32 has 8KB+ stack, HO | |||||||
| static constexpr size_t MAX_PACKETS_PER_BATCH = 32;  // ESP8266/RP2040/etc have smaller stacks | static constexpr size_t MAX_PACKETS_PER_BATCH = 32;  // ESP8266/RP2040/etc have smaller stacks | ||||||
| #endif | #endif | ||||||
|  |  | ||||||
| class APIConnection : public APIServerConnection { | class APIConnection final : public APIServerConnection { | ||||||
|  public: |  public: | ||||||
|   friend class APIServer; |   friend class APIServer; | ||||||
|   friend class ListEntitiesIterator; |   friend class ListEntitiesIterator; | ||||||
|   | |||||||
| @@ -104,9 +104,9 @@ class APIFrameHelper { | |||||||
|   // The buffer contains all messages with appropriate padding before each |   // The buffer contains all messages with appropriate padding before each | ||||||
|   virtual APIError write_protobuf_packets(ProtoWriteBuffer buffer, std::span<const PacketInfo> packets) = 0; |   virtual APIError write_protobuf_packets(ProtoWriteBuffer buffer, std::span<const PacketInfo> packets) = 0; | ||||||
|   // Get the frame header padding required by this protocol |   // Get the frame header padding required by this protocol | ||||||
|   virtual uint8_t frame_header_padding() = 0; |   uint8_t frame_header_padding() const { return frame_header_padding_; } | ||||||
|   // Get the frame footer size required by this protocol |   // Get the frame footer size required by this protocol | ||||||
|   virtual uint8_t frame_footer_size() = 0; |   uint8_t frame_footer_size() const { return frame_footer_size_; } | ||||||
|   // Check if socket has data ready to read |   // Check if socket has data ready to read | ||||||
|   bool is_socket_ready() const { return socket_ != nullptr && socket_->ready(); } |   bool is_socket_ready() const { return socket_ != nullptr && socket_->ready(); } | ||||||
|  |  | ||||||
|   | |||||||
| @@ -7,7 +7,7 @@ | |||||||
|  |  | ||||||
| namespace esphome::api { | namespace esphome::api { | ||||||
|  |  | ||||||
| class APINoiseFrameHelper : public APIFrameHelper { | class APINoiseFrameHelper final : public APIFrameHelper { | ||||||
|  public: |  public: | ||||||
|   APINoiseFrameHelper(std::unique_ptr<socket::Socket> socket, std::shared_ptr<APINoiseContext> ctx, |   APINoiseFrameHelper(std::unique_ptr<socket::Socket> socket, std::shared_ptr<APINoiseContext> ctx, | ||||||
|                       const ClientInfo *client_info) |                       const ClientInfo *client_info) | ||||||
| @@ -25,10 +25,6 @@ class APINoiseFrameHelper : public APIFrameHelper { | |||||||
|   APIError read_packet(ReadPacketBuffer *buffer) override; |   APIError read_packet(ReadPacketBuffer *buffer) override; | ||||||
|   APIError write_protobuf_packet(uint8_t type, ProtoWriteBuffer buffer) override; |   APIError write_protobuf_packet(uint8_t type, ProtoWriteBuffer buffer) override; | ||||||
|   APIError write_protobuf_packets(ProtoWriteBuffer buffer, std::span<const PacketInfo> packets) override; |   APIError write_protobuf_packets(ProtoWriteBuffer buffer, std::span<const PacketInfo> packets) override; | ||||||
|   // Get the frame header padding required by this protocol |  | ||||||
|   uint8_t frame_header_padding() override { return frame_header_padding_; } |  | ||||||
|   // Get the frame footer size required by this protocol |  | ||||||
|   uint8_t frame_footer_size() override { return frame_footer_size_; } |  | ||||||
|  |  | ||||||
|  protected: |  protected: | ||||||
|   APIError state_action_(); |   APIError state_action_(); | ||||||
|   | |||||||
| @@ -5,7 +5,7 @@ | |||||||
|  |  | ||||||
| namespace esphome::api { | namespace esphome::api { | ||||||
|  |  | ||||||
| class APIPlaintextFrameHelper : public APIFrameHelper { | class APIPlaintextFrameHelper final : public APIFrameHelper { | ||||||
|  public: |  public: | ||||||
|   APIPlaintextFrameHelper(std::unique_ptr<socket::Socket> socket, const ClientInfo *client_info) |   APIPlaintextFrameHelper(std::unique_ptr<socket::Socket> socket, const ClientInfo *client_info) | ||||||
|       : APIFrameHelper(std::move(socket), client_info) { |       : APIFrameHelper(std::move(socket), client_info) { | ||||||
| @@ -22,9 +22,6 @@ class APIPlaintextFrameHelper : public APIFrameHelper { | |||||||
|   APIError read_packet(ReadPacketBuffer *buffer) override; |   APIError read_packet(ReadPacketBuffer *buffer) override; | ||||||
|   APIError write_protobuf_packet(uint8_t type, ProtoWriteBuffer buffer) override; |   APIError write_protobuf_packet(uint8_t type, ProtoWriteBuffer buffer) override; | ||||||
|   APIError write_protobuf_packets(ProtoWriteBuffer buffer, std::span<const PacketInfo> packets) override; |   APIError write_protobuf_packets(ProtoWriteBuffer buffer, std::span<const PacketInfo> packets) override; | ||||||
|   uint8_t frame_header_padding() override { return frame_header_padding_; } |  | ||||||
|   // Get the frame footer size required by this protocol |  | ||||||
|   uint8_t frame_footer_size() override { return frame_footer_size_; } |  | ||||||
|  |  | ||||||
|  protected: |  protected: | ||||||
|   APIError try_read_frame_(std::vector<uint8_t> *frame); |   APIError try_read_frame_(std::vector<uint8_t> *frame); | ||||||
|   | |||||||
										
											
												File diff suppressed because it is too large
												Load Diff
											
										
									
								
							| @@ -8,74 +8,70 @@ namespace esphome::api { | |||||||
| static const char *const TAG = "api.proto"; | static const char *const TAG = "api.proto"; | ||||||
|  |  | ||||||
| void ProtoDecodableMessage::decode(const uint8_t *buffer, size_t length) { | void ProtoDecodableMessage::decode(const uint8_t *buffer, size_t length) { | ||||||
|   uint32_t i = 0; |   const uint8_t *ptr = buffer; | ||||||
|   bool error = false; |   const uint8_t *end = buffer + length; | ||||||
|   while (i < length) { |  | ||||||
|  |   while (ptr < end) { | ||||||
|     uint32_t consumed; |     uint32_t consumed; | ||||||
|     auto res = ProtoVarInt::parse(&buffer[i], length - i, &consumed); |  | ||||||
|  |     // Parse field header | ||||||
|  |     auto res = ProtoVarInt::parse(ptr, end - ptr, &consumed); | ||||||
|     if (!res.has_value()) { |     if (!res.has_value()) { | ||||||
|       ESP_LOGV(TAG, "Invalid field start at %" PRIu32, i); |       ESP_LOGV(TAG, "Invalid field start at offset %ld", (long) (ptr - buffer)); | ||||||
|       break; |       return; | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     uint32_t field_type = (res->as_uint32()) & 0b111; |     uint32_t tag = res->as_uint32(); | ||||||
|     uint32_t field_id = (res->as_uint32()) >> 3; |     uint32_t field_type = tag & 0b111; | ||||||
|     i += consumed; |     uint32_t field_id = tag >> 3; | ||||||
|  |     ptr += consumed; | ||||||
|  |  | ||||||
|     switch (field_type) { |     switch (field_type) { | ||||||
|       case 0: {  // VarInt |       case 0: {  // VarInt | ||||||
|         res = ProtoVarInt::parse(&buffer[i], length - i, &consumed); |         res = ProtoVarInt::parse(ptr, end - ptr, &consumed); | ||||||
|         if (!res.has_value()) { |         if (!res.has_value()) { | ||||||
|           ESP_LOGV(TAG, "Invalid VarInt at %" PRIu32, i); |           ESP_LOGV(TAG, "Invalid VarInt at offset %ld", (long) (ptr - buffer)); | ||||||
|           error = true; |           return; | ||||||
|           break; |  | ||||||
|         } |         } | ||||||
|         if (!this->decode_varint(field_id, *res)) { |         if (!this->decode_varint(field_id, *res)) { | ||||||
|           ESP_LOGV(TAG, "Cannot decode VarInt field %" PRIu32 " with value %" PRIu32 "!", field_id, res->as_uint32()); |           ESP_LOGV(TAG, "Cannot decode VarInt field %" PRIu32 " with value %" PRIu32 "!", field_id, res->as_uint32()); | ||||||
|         } |         } | ||||||
|         i += consumed; |         ptr += consumed; | ||||||
|         break; |         break; | ||||||
|       } |       } | ||||||
|       case 2: {  // Length-delimited |       case 2: {  // Length-delimited | ||||||
|         res = ProtoVarInt::parse(&buffer[i], length - i, &consumed); |         res = ProtoVarInt::parse(ptr, end - ptr, &consumed); | ||||||
|         if (!res.has_value()) { |         if (!res.has_value()) { | ||||||
|           ESP_LOGV(TAG, "Invalid Length Delimited at %" PRIu32, i); |           ESP_LOGV(TAG, "Invalid Length Delimited at offset %ld", (long) (ptr - buffer)); | ||||||
|           error = true; |           return; | ||||||
|           break; |  | ||||||
|         } |         } | ||||||
|         uint32_t field_length = res->as_uint32(); |         uint32_t field_length = res->as_uint32(); | ||||||
|         i += consumed; |         ptr += consumed; | ||||||
|         if (field_length > length - i) { |         if (ptr + field_length > end) { | ||||||
|           ESP_LOGV(TAG, "Out-of-bounds Length Delimited at %" PRIu32, i); |           ESP_LOGV(TAG, "Out-of-bounds Length Delimited at offset %ld", (long) (ptr - buffer)); | ||||||
|           error = true; |           return; | ||||||
|           break; |  | ||||||
|         } |         } | ||||||
|         if (!this->decode_length(field_id, ProtoLengthDelimited(&buffer[i], field_length))) { |         if (!this->decode_length(field_id, ProtoLengthDelimited(ptr, field_length))) { | ||||||
|           ESP_LOGV(TAG, "Cannot decode Length Delimited field %" PRIu32 "!", field_id); |           ESP_LOGV(TAG, "Cannot decode Length Delimited field %" PRIu32 "!", field_id); | ||||||
|         } |         } | ||||||
|         i += field_length; |         ptr += field_length; | ||||||
|         break; |         break; | ||||||
|       } |       } | ||||||
|       case 5: {  // 32-bit |       case 5: {  // 32-bit | ||||||
|         if (length - i < 4) { |         if (ptr + 4 > end) { | ||||||
|           ESP_LOGV(TAG, "Out-of-bounds Fixed32-bit at %" PRIu32, i); |           ESP_LOGV(TAG, "Out-of-bounds Fixed32-bit at offset %ld", (long) (ptr - buffer)); | ||||||
|           error = true; |           return; | ||||||
|           break; |  | ||||||
|         } |         } | ||||||
|         uint32_t val = encode_uint32(buffer[i + 3], buffer[i + 2], buffer[i + 1], buffer[i]); |         uint32_t val = encode_uint32(ptr[3], ptr[2], ptr[1], ptr[0]); | ||||||
|         if (!this->decode_32bit(field_id, Proto32Bit(val))) { |         if (!this->decode_32bit(field_id, Proto32Bit(val))) { | ||||||
|           ESP_LOGV(TAG, "Cannot decode 32-bit field %" PRIu32 " with value %" PRIu32 "!", field_id, val); |           ESP_LOGV(TAG, "Cannot decode 32-bit field %" PRIu32 " with value %" PRIu32 "!", field_id, val); | ||||||
|         } |         } | ||||||
|         i += 4; |         ptr += 4; | ||||||
|         break; |         break; | ||||||
|       } |       } | ||||||
|       default: |       default: | ||||||
|         ESP_LOGV(TAG, "Invalid field type at %" PRIu32, i); |         ESP_LOGV(TAG, "Invalid field type %u at offset %ld", field_type, (long) (ptr - buffer)); | ||||||
|         error = true; |         return; | ||||||
|         break; |  | ||||||
|     } |  | ||||||
|     if (error) { |  | ||||||
|       break; |  | ||||||
|     } |     } | ||||||
|   } |   } | ||||||
| } | } | ||||||
|   | |||||||
| @@ -382,20 +382,15 @@ float ATM90E32Component::get_setup_priority() const { return setup_priority::IO; | |||||||
| // R/C registers can conly be cleared after the LastSPIData register is updated (register 78H) | // R/C registers can conly be cleared after the LastSPIData register is updated (register 78H) | ||||||
| // Peakdetect period: 05H. Bit 15:8 are PeakDet_period in ms. 7:0 are Sag_period | // Peakdetect period: 05H. Bit 15:8 are PeakDet_period in ms. 7:0 are Sag_period | ||||||
| // Default is 143FH (20ms, 63ms) | // Default is 143FH (20ms, 63ms) | ||||||
| uint16_t ATM90E32Component::read16_transaction_(uint16_t a_register) { | uint16_t ATM90E32Component::read16_(uint16_t a_register) { | ||||||
|  |   this->enable(); | ||||||
|  |   delay_microseconds_safe(1);  // min delay between CS low and first SCK is 200ns - 1us is plenty | ||||||
|   uint8_t addrh = (1 << 7) | ((a_register >> 8) & 0x03); |   uint8_t addrh = (1 << 7) | ((a_register >> 8) & 0x03); | ||||||
|   uint8_t addrl = (a_register & 0xFF); |   uint8_t addrl = (a_register & 0xFF); | ||||||
|   uint8_t data[4] = {addrh, addrl, 0x00, 0x00}; |   uint8_t data[4] = {addrh, addrl, 0x00, 0x00}; | ||||||
|   this->transfer_array(data, 4); |   this->transfer_array(data, 4); | ||||||
|   uint16_t output = encode_uint16(data[2], data[3]); |   uint16_t output = encode_uint16(data[2], data[3]); | ||||||
|   ESP_LOGVV(TAG, "read16_ 0x%04" PRIX16 " output 0x%04" PRIX16, a_register, output); |   ESP_LOGVV(TAG, "read16_ 0x%04" PRIX16 " output 0x%04" PRIX16, a_register, output); | ||||||
|   return output; |  | ||||||
| } |  | ||||||
|  |  | ||||||
| uint16_t ATM90E32Component::read16_(uint16_t a_register) { |  | ||||||
|   this->enable(); |  | ||||||
|   delay_microseconds_safe(1);  // min delay between CS low and first SCK is 200ns - 1us is plenty |  | ||||||
|   uint16_t output = this->read16_transaction_(a_register); |  | ||||||
|   delay_microseconds_safe(1);  // allow the last clock to propagate before releasing CS |   delay_microseconds_safe(1);  // allow the last clock to propagate before releasing CS | ||||||
|   this->disable(); |   this->disable(); | ||||||
|   delay_microseconds_safe(1);  // meet minimum CS high time before next transaction |   delay_microseconds_safe(1);  // meet minimum CS high time before next transaction | ||||||
| @@ -403,14 +398,8 @@ uint16_t ATM90E32Component::read16_(uint16_t a_register) { | |||||||
| } | } | ||||||
|  |  | ||||||
| int ATM90E32Component::read32_(uint16_t addr_h, uint16_t addr_l) { | int ATM90E32Component::read32_(uint16_t addr_h, uint16_t addr_l) { | ||||||
|   this->enable(); |   const uint16_t val_h = this->read16_(addr_h); | ||||||
|   delay_microseconds_safe(1); |   const uint16_t val_l = this->read16_(addr_l); | ||||||
|   const uint16_t val_h = this->read16_transaction_(addr_h); |  | ||||||
|   delay_microseconds_safe(1); |  | ||||||
|   const uint16_t val_l = this->read16_transaction_(addr_l); |  | ||||||
|   delay_microseconds_safe(1); |  | ||||||
|   this->disable(); |  | ||||||
|   delay_microseconds_safe(1); |  | ||||||
|   const int32_t val = (val_h << 16) | val_l; |   const int32_t val = (val_h << 16) | val_l; | ||||||
|  |  | ||||||
|   ESP_LOGVV(TAG, |   ESP_LOGVV(TAG, | ||||||
|   | |||||||
| @@ -140,7 +140,6 @@ class ATM90E32Component : public PollingComponent, | |||||||
|   number::Number *ref_currents_[3]{nullptr, nullptr, nullptr}; |   number::Number *ref_currents_[3]{nullptr, nullptr, nullptr}; | ||||||
| #endif | #endif | ||||||
|   uint16_t read16_(uint16_t a_register); |   uint16_t read16_(uint16_t a_register); | ||||||
|   uint16_t read16_transaction_(uint16_t a_register); |  | ||||||
|   int read32_(uint16_t addr_h, uint16_t addr_l); |   int read32_(uint16_t addr_h, uint16_t addr_l); | ||||||
|   void write16_(uint16_t a_register, uint16_t val, bool validate = true); |   void write16_(uint16_t a_register, uint16_t val, bool validate = true); | ||||||
|   float get_local_phase_voltage_(uint8_t phase); |   float get_local_phase_voltage_(uint8_t phase); | ||||||
|   | |||||||
| @@ -375,10 +375,19 @@ bool BluetoothConnection::gattc_event_handler(esp_gattc_cb_event_t event, esp_ga | |||||||
|  |  | ||||||
|   switch (event) { |   switch (event) { | ||||||
|     case ESP_GATTC_DISCONNECT_EVT: { |     case ESP_GATTC_DISCONNECT_EVT: { | ||||||
|       this->reset_connection_(param->disconnect.reason); |       // Don't reset connection yet - wait for CLOSE_EVT to ensure controller has freed resources | ||||||
|  |       // This prevents race condition where we mark slot as free before controller cleanup is complete | ||||||
|  |       ESP_LOGD(TAG, "[%d] [%s] Disconnect, reason=0x%02x", this->connection_index_, this->address_str_.c_str(), | ||||||
|  |                param->disconnect.reason); | ||||||
|  |       // Send disconnection notification but don't free the slot yet | ||||||
|  |       this->proxy_->send_device_connection(this->address_, false, 0, param->disconnect.reason); | ||||||
|       break; |       break; | ||||||
|     } |     } | ||||||
|     case ESP_GATTC_CLOSE_EVT: { |     case ESP_GATTC_CLOSE_EVT: { | ||||||
|  |       ESP_LOGD(TAG, "[%d] [%s] Close, reason=0x%02x, freeing slot", this->connection_index_, this->address_str_.c_str(), | ||||||
|  |                param->close.reason); | ||||||
|  |       // Now the GATT connection is fully closed and controller resources are freed | ||||||
|  |       // Safe to mark the connection slot as available | ||||||
|       this->reset_connection_(param->close.reason); |       this->reset_connection_(param->close.reason); | ||||||
|       break; |       break; | ||||||
|     } |     } | ||||||
|   | |||||||
| @@ -824,8 +824,9 @@ async def to_code(config): | |||||||
|     cg.set_cpp_standard("gnu++20") |     cg.set_cpp_standard("gnu++20") | ||||||
|     cg.add_build_flag("-DUSE_ESP32") |     cg.add_build_flag("-DUSE_ESP32") | ||||||
|     cg.add_define("ESPHOME_BOARD", config[CONF_BOARD]) |     cg.add_define("ESPHOME_BOARD", config[CONF_BOARD]) | ||||||
|     cg.add_build_flag(f"-DUSE_ESP32_VARIANT_{config[CONF_VARIANT]}") |     variant = config[CONF_VARIANT] | ||||||
|     cg.add_define("ESPHOME_VARIANT", VARIANT_FRIENDLY[config[CONF_VARIANT]]) |     cg.add_build_flag(f"-DUSE_ESP32_VARIANT_{variant}") | ||||||
|  |     cg.add_define("ESPHOME_VARIANT", VARIANT_FRIENDLY[variant]) | ||||||
|     cg.add_define(ThreadModel.MULTI_ATOMICS) |     cg.add_define(ThreadModel.MULTI_ATOMICS) | ||||||
|  |  | ||||||
|     cg.add_platformio_option("lib_ldf_mode", "off") |     cg.add_platformio_option("lib_ldf_mode", "off") | ||||||
| @@ -859,6 +860,7 @@ async def to_code(config): | |||||||
|         cg.add_platformio_option( |         cg.add_platformio_option( | ||||||
|             "platform_packages", ["espressif/toolchain-esp32ulp@2.35.0-20220830"] |             "platform_packages", ["espressif/toolchain-esp32ulp@2.35.0-20220830"] | ||||||
|         ) |         ) | ||||||
|  |         add_idf_sdkconfig_option(f"CONFIG_IDF_TARGET_{variant}", True) | ||||||
|         add_idf_sdkconfig_option( |         add_idf_sdkconfig_option( | ||||||
|             f"CONFIG_ESPTOOLPY_FLASHSIZE_{config[CONF_FLASH_SIZE]}", True |             f"CONFIG_ESPTOOLPY_FLASHSIZE_{config[CONF_FLASH_SIZE]}", True | ||||||
|         ) |         ) | ||||||
|   | |||||||
| @@ -8,6 +8,7 @@ | |||||||
| #include <cinttypes> | #include <cinttypes> | ||||||
| #include <vector> | #include <vector> | ||||||
| #include <string> | #include <string> | ||||||
|  | #include <memory> | ||||||
|  |  | ||||||
| namespace esphome { | namespace esphome { | ||||||
| namespace esp32 { | namespace esp32 { | ||||||
| @@ -156,20 +157,23 @@ class ESP32Preferences : public ESPPreferences { | |||||||
|     return failed == 0; |     return failed == 0; | ||||||
|   } |   } | ||||||
|   bool is_changed(const uint32_t nvs_handle, const NVSData &to_save) { |   bool is_changed(const uint32_t nvs_handle, const NVSData &to_save) { | ||||||
|     NVSData stored_data{}; |  | ||||||
|     size_t actual_len; |     size_t actual_len; | ||||||
|     esp_err_t err = nvs_get_blob(nvs_handle, to_save.key.c_str(), nullptr, &actual_len); |     esp_err_t err = nvs_get_blob(nvs_handle, to_save.key.c_str(), nullptr, &actual_len); | ||||||
|     if (err != 0) { |     if (err != 0) { | ||||||
|       ESP_LOGV(TAG, "nvs_get_blob('%s'): %s - the key might not be set yet", to_save.key.c_str(), esp_err_to_name(err)); |       ESP_LOGV(TAG, "nvs_get_blob('%s'): %s - the key might not be set yet", to_save.key.c_str(), esp_err_to_name(err)); | ||||||
|       return true; |       return true; | ||||||
|     } |     } | ||||||
|     stored_data.data.resize(actual_len); |     // Check size first before allocating memory | ||||||
|     err = nvs_get_blob(nvs_handle, to_save.key.c_str(), stored_data.data.data(), &actual_len); |     if (actual_len != to_save.data.size()) { | ||||||
|  |       return true; | ||||||
|  |     } | ||||||
|  |     auto stored_data = std::make_unique<uint8_t[]>(actual_len); | ||||||
|  |     err = nvs_get_blob(nvs_handle, to_save.key.c_str(), stored_data.get(), &actual_len); | ||||||
|     if (err != 0) { |     if (err != 0) { | ||||||
|       ESP_LOGV(TAG, "nvs_get_blob('%s') failed: %s", to_save.key.c_str(), esp_err_to_name(err)); |       ESP_LOGV(TAG, "nvs_get_blob('%s') failed: %s", to_save.key.c_str(), esp_err_to_name(err)); | ||||||
|       return true; |       return true; | ||||||
|     } |     } | ||||||
|     return to_save.data != stored_data.data; |     return memcmp(to_save.data.data(), stored_data.get(), to_save.data.size()) != 0; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   bool reset() override { |   bool reset() override { | ||||||
|   | |||||||
| @@ -306,7 +306,7 @@ void ESP32BLE::loop() { | |||||||
|       case BLEEvent::GATTS: { |       case BLEEvent::GATTS: { | ||||||
|         esp_gatts_cb_event_t event = ble_event->event_.gatts.gatts_event; |         esp_gatts_cb_event_t event = ble_event->event_.gatts.gatts_event; | ||||||
|         esp_gatt_if_t gatts_if = ble_event->event_.gatts.gatts_if; |         esp_gatt_if_t gatts_if = ble_event->event_.gatts.gatts_if; | ||||||
|         esp_ble_gatts_cb_param_t *param = ble_event->event_.gatts.gatts_param; |         esp_ble_gatts_cb_param_t *param = &ble_event->event_.gatts.gatts_param; | ||||||
|         ESP_LOGV(TAG, "gatts_event [esp_gatt_if: %d] - %d", gatts_if, event); |         ESP_LOGV(TAG, "gatts_event [esp_gatt_if: %d] - %d", gatts_if, event); | ||||||
|         for (auto *gatts_handler : this->gatts_event_handlers_) { |         for (auto *gatts_handler : this->gatts_event_handlers_) { | ||||||
|           gatts_handler->gatts_event_handler(event, gatts_if, param); |           gatts_handler->gatts_event_handler(event, gatts_if, param); | ||||||
| @@ -316,7 +316,7 @@ void ESP32BLE::loop() { | |||||||
|       case BLEEvent::GATTC: { |       case BLEEvent::GATTC: { | ||||||
|         esp_gattc_cb_event_t event = ble_event->event_.gattc.gattc_event; |         esp_gattc_cb_event_t event = ble_event->event_.gattc.gattc_event; | ||||||
|         esp_gatt_if_t gattc_if = ble_event->event_.gattc.gattc_if; |         esp_gatt_if_t gattc_if = ble_event->event_.gattc.gattc_if; | ||||||
|         esp_ble_gattc_cb_param_t *param = ble_event->event_.gattc.gattc_param; |         esp_ble_gattc_cb_param_t *param = &ble_event->event_.gattc.gattc_param; | ||||||
|         ESP_LOGV(TAG, "gattc_event [esp_gatt_if: %d] - %d", gattc_if, event); |         ESP_LOGV(TAG, "gattc_event [esp_gatt_if: %d] - %d", gattc_if, event); | ||||||
|         for (auto *gattc_handler : this->gattc_event_handlers_) { |         for (auto *gattc_handler : this->gattc_event_handlers_) { | ||||||
|           gattc_handler->gattc_event_handler(event, gattc_if, param); |           gattc_handler->gattc_event_handler(event, gattc_if, param); | ||||||
|   | |||||||
| @@ -3,8 +3,7 @@ | |||||||
| #ifdef USE_ESP32 | #ifdef USE_ESP32 | ||||||
|  |  | ||||||
| #include <cstddef>  // for offsetof | #include <cstddef>  // for offsetof | ||||||
| #include <vector> | #include <cstring>  // for memcpy | ||||||
|  |  | ||||||
| #include <esp_gap_ble_api.h> | #include <esp_gap_ble_api.h> | ||||||
| #include <esp_gattc_api.h> | #include <esp_gattc_api.h> | ||||||
| #include <esp_gatts_api.h> | #include <esp_gatts_api.h> | ||||||
| @@ -62,10 +61,24 @@ static_assert(offsetof(esp_ble_gap_cb_param_t, read_rssi_cmpl.rssi) == sizeof(es | |||||||
| static_assert(offsetof(esp_ble_gap_cb_param_t, read_rssi_cmpl.remote_addr) == sizeof(esp_bt_status_t) + sizeof(int8_t), | static_assert(offsetof(esp_ble_gap_cb_param_t, read_rssi_cmpl.remote_addr) == sizeof(esp_bt_status_t) + sizeof(int8_t), | ||||||
|               "remote_addr must follow rssi in read_rssi_cmpl"); |               "remote_addr must follow rssi in read_rssi_cmpl"); | ||||||
|  |  | ||||||
|  | // Param struct sizes on ESP32 | ||||||
|  | static constexpr size_t GATTC_PARAM_SIZE = 28; | ||||||
|  | static constexpr size_t GATTS_PARAM_SIZE = 32; | ||||||
|  |  | ||||||
|  | // Maximum size for inline storage of data | ||||||
|  | // GATTC: 80 - 28 (param) - 8 (other fields) = 44 bytes for data | ||||||
|  | // GATTS: 80 - 32 (param) - 8 (other fields) = 40 bytes for data | ||||||
|  | static constexpr size_t GATTC_INLINE_DATA_SIZE = 44; | ||||||
|  | static constexpr size_t GATTS_INLINE_DATA_SIZE = 40; | ||||||
|  |  | ||||||
|  | // Verify param struct sizes | ||||||
|  | static_assert(sizeof(esp_ble_gattc_cb_param_t) == GATTC_PARAM_SIZE, "GATTC param size unexpected"); | ||||||
|  | static_assert(sizeof(esp_ble_gatts_cb_param_t) == GATTS_PARAM_SIZE, "GATTS param size unexpected"); | ||||||
|  |  | ||||||
| // Received GAP, GATTC and GATTS events are only queued, and get processed in the main loop(). | // Received GAP, GATTC and GATTS events are only queued, and get processed in the main loop(). | ||||||
| // This class stores each event with minimal memory usage. | // This class stores each event with minimal memory usage. | ||||||
| // GAP events (99% of traffic) don't have the vector overhead. | // GAP events (99% of traffic) don't have the heap allocation overhead. | ||||||
| // GATTC/GATTS events use heap allocation for their param and data. | // GATTC/GATTS events use heap allocation for their param and inline storage for small data. | ||||||
| // | // | ||||||
| // Event flow: | // Event flow: | ||||||
| // 1. ESP-IDF BLE stack calls our static handlers in the BLE task context | // 1. ESP-IDF BLE stack calls our static handlers in the BLE task context | ||||||
| @@ -112,21 +125,21 @@ class BLEEvent { | |||||||
|     this->init_gap_data_(e, p); |     this->init_gap_data_(e, p); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   // Constructor for GATTC events - uses heap allocation |   // Constructor for GATTC events - param stored inline, data may use heap | ||||||
|   // IMPORTANT: The heap allocation is REQUIRED and must not be removed as an optimization. |   // IMPORTANT: We MUST copy the param struct because the pointer from ESP-IDF | ||||||
|   // The param pointer from ESP-IDF is only valid during the callback execution. |   // is only valid during the callback execution. Since BLE events are processed | ||||||
|   // Since BLE events are processed asynchronously in the main loop, we must create |   // asynchronously in the main loop, we store our own copy inline to ensure | ||||||
|   // our own copy to ensure the data remains valid until the event is processed. |   // the data remains valid until the event is processed. | ||||||
|   BLEEvent(esp_gattc_cb_event_t e, esp_gatt_if_t i, esp_ble_gattc_cb_param_t *p) { |   BLEEvent(esp_gattc_cb_event_t e, esp_gatt_if_t i, esp_ble_gattc_cb_param_t *p) { | ||||||
|     this->type_ = GATTC; |     this->type_ = GATTC; | ||||||
|     this->init_gattc_data_(e, i, p); |     this->init_gattc_data_(e, i, p); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   // Constructor for GATTS events - uses heap allocation |   // Constructor for GATTS events - param stored inline, data may use heap | ||||||
|   // IMPORTANT: The heap allocation is REQUIRED and must not be removed as an optimization. |   // IMPORTANT: We MUST copy the param struct because the pointer from ESP-IDF | ||||||
|   // The param pointer from ESP-IDF is only valid during the callback execution. |   // is only valid during the callback execution. Since BLE events are processed | ||||||
|   // Since BLE events are processed asynchronously in the main loop, we must create |   // asynchronously in the main loop, we store our own copy inline to ensure | ||||||
|   // our own copy to ensure the data remains valid until the event is processed. |   // the data remains valid until the event is processed. | ||||||
|   BLEEvent(esp_gatts_cb_event_t e, esp_gatt_if_t i, esp_ble_gatts_cb_param_t *p) { |   BLEEvent(esp_gatts_cb_event_t e, esp_gatt_if_t i, esp_ble_gatts_cb_param_t *p) { | ||||||
|     this->type_ = GATTS; |     this->type_ = GATTS; | ||||||
|     this->init_gatts_data_(e, i, p); |     this->init_gatts_data_(e, i, p); | ||||||
| @@ -136,25 +149,32 @@ class BLEEvent { | |||||||
|   ~BLEEvent() { this->release(); } |   ~BLEEvent() { this->release(); } | ||||||
|  |  | ||||||
|   // Default constructor for pre-allocation in pool |   // Default constructor for pre-allocation in pool | ||||||
|   BLEEvent() : type_(GAP) {} |   BLEEvent() : event_{}, type_(GAP) {} | ||||||
|  |  | ||||||
|   // Invoked on return to EventPool - clean up any heap-allocated data |   // Invoked on return to EventPool - clean up any heap-allocated data | ||||||
|   void release() { |   void release() { | ||||||
|     if (this->type_ == GAP) { |     switch (this->type_) { | ||||||
|       return; |       case GAP: | ||||||
|  |         // GAP events don't have heap allocations | ||||||
|  |         break; | ||||||
|  |       case GATTC: | ||||||
|  |         // Param is now stored inline, only delete heap data if it was heap-allocated | ||||||
|  |         if (!this->event_.gattc.is_inline && this->event_.gattc.data.heap_data != nullptr) { | ||||||
|  |           delete[] this->event_.gattc.data.heap_data; | ||||||
|         } |         } | ||||||
|     if (this->type_ == GATTC) { |         // Clear critical fields to prevent issues if type changes | ||||||
|       delete this->event_.gattc.gattc_param; |         this->event_.gattc.is_inline = false; | ||||||
|       delete this->event_.gattc.data; |         this->event_.gattc.data.heap_data = nullptr; | ||||||
|       this->event_.gattc.gattc_param = nullptr; |         break; | ||||||
|       this->event_.gattc.data = nullptr; |       case GATTS: | ||||||
|       return; |         // Param is now stored inline, only delete heap data if it was heap-allocated | ||||||
|  |         if (!this->event_.gatts.is_inline && this->event_.gatts.data.heap_data != nullptr) { | ||||||
|  |           delete[] this->event_.gatts.data.heap_data; | ||||||
|         } |         } | ||||||
|     if (this->type_ == GATTS) { |         // Clear critical fields to prevent issues if type changes | ||||||
|       delete this->event_.gatts.gatts_param; |         this->event_.gatts.is_inline = false; | ||||||
|       delete this->event_.gatts.data; |         this->event_.gatts.data.heap_data = nullptr; | ||||||
|       this->event_.gatts.gatts_param = nullptr; |         break; | ||||||
|       this->event_.gatts.data = nullptr; |  | ||||||
|     } |     } | ||||||
|   } |   } | ||||||
|  |  | ||||||
| @@ -206,19 +226,29 @@ class BLEEvent { | |||||||
|  |  | ||||||
|     // NOLINTNEXTLINE(readability-identifier-naming) |     // NOLINTNEXTLINE(readability-identifier-naming) | ||||||
|     struct gattc_event { |     struct gattc_event { | ||||||
|       esp_gattc_cb_event_t gattc_event; |       esp_ble_gattc_cb_param_t gattc_param;  // Stored inline (28 bytes) | ||||||
|       esp_gatt_if_t gattc_if; |       esp_gattc_cb_event_t gattc_event;      // 4 bytes | ||||||
|       esp_ble_gattc_cb_param_t *gattc_param;  // Heap-allocated |       union { | ||||||
|       std::vector<uint8_t> *data;             // Heap-allocated |         uint8_t *heap_data;                           // 4 bytes when heap-allocated | ||||||
|     } gattc;                                  // 16 bytes (pointers only) |         uint8_t inline_data[GATTC_INLINE_DATA_SIZE];  // 44 bytes when stored inline | ||||||
|  |       } data;                                         // 44 bytes total | ||||||
|  |       uint16_t data_len;                              // 2 bytes | ||||||
|  |       esp_gatt_if_t gattc_if;                         // 1 byte | ||||||
|  |       bool is_inline;                                 // 1 byte - true when data is stored inline | ||||||
|  |     } gattc;                                          // Total: 80 bytes | ||||||
|  |  | ||||||
|     // NOLINTNEXTLINE(readability-identifier-naming) |     // NOLINTNEXTLINE(readability-identifier-naming) | ||||||
|     struct gatts_event { |     struct gatts_event { | ||||||
|       esp_gatts_cb_event_t gatts_event; |       esp_ble_gatts_cb_param_t gatts_param;  // Stored inline (32 bytes) | ||||||
|       esp_gatt_if_t gatts_if; |       esp_gatts_cb_event_t gatts_event;      // 4 bytes | ||||||
|       esp_ble_gatts_cb_param_t *gatts_param;  // Heap-allocated |       union { | ||||||
|       std::vector<uint8_t> *data;             // Heap-allocated |         uint8_t *heap_data;                           // 4 bytes when heap-allocated | ||||||
|     } gatts;                                  // 16 bytes (pointers only) |         uint8_t inline_data[GATTS_INLINE_DATA_SIZE];  // 40 bytes when stored inline | ||||||
|  |       } data;                                         // 40 bytes total | ||||||
|  |       uint16_t data_len;                              // 2 bytes | ||||||
|  |       esp_gatt_if_t gatts_if;                         // 1 byte | ||||||
|  |       bool is_inline;                                 // 1 byte - true when data is stored inline | ||||||
|  |     } gatts;                                          // Total: 80 bytes | ||||||
|   } event_;                                           // 80 bytes |   } event_;                                           // 80 bytes | ||||||
|  |  | ||||||
|   ble_event_t type_; |   ble_event_t type_; | ||||||
| @@ -233,6 +263,29 @@ class BLEEvent { | |||||||
|   const esp_ble_sec_t &security() const { return event_.gap.security; } |   const esp_ble_sec_t &security() const { return event_.gap.security; } | ||||||
|  |  | ||||||
|  private: |  private: | ||||||
|  |   // Helper to copy data with inline storage optimization | ||||||
|  |   template<typename EventStruct, size_t InlineSize> | ||||||
|  |   void copy_data_with_inline_storage_(EventStruct &event, const uint8_t *src_data, uint16_t len, | ||||||
|  |                                       uint8_t **param_value_ptr) { | ||||||
|  |     event.data_len = len; | ||||||
|  |     if (len > 0) { | ||||||
|  |       if (len <= InlineSize) { | ||||||
|  |         event.is_inline = true; | ||||||
|  |         memcpy(event.data.inline_data, src_data, len); | ||||||
|  |         *param_value_ptr = event.data.inline_data; | ||||||
|  |       } else { | ||||||
|  |         event.is_inline = false; | ||||||
|  |         event.data.heap_data = new uint8_t[len]; | ||||||
|  |         memcpy(event.data.heap_data, src_data, len); | ||||||
|  |         *param_value_ptr = event.data.heap_data; | ||||||
|  |       } | ||||||
|  |     } else { | ||||||
|  |       event.is_inline = false; | ||||||
|  |       event.data.heap_data = nullptr; | ||||||
|  |       *param_value_ptr = nullptr; | ||||||
|  |     } | ||||||
|  |   } | ||||||
|  |  | ||||||
|   // Initialize GAP event data |   // Initialize GAP event data | ||||||
|   void init_gap_data_(esp_gap_ble_cb_event_t e, esp_ble_gap_cb_param_t *p) { |   void init_gap_data_(esp_gap_ble_cb_event_t e, esp_ble_gap_cb_param_t *p) { | ||||||
|     this->event_.gap.gap_event = e; |     this->event_.gap.gap_event = e; | ||||||
| @@ -317,35 +370,38 @@ class BLEEvent { | |||||||
|     this->event_.gattc.gattc_if = i; |     this->event_.gattc.gattc_if = i; | ||||||
|  |  | ||||||
|     if (p == nullptr) { |     if (p == nullptr) { | ||||||
|       this->event_.gattc.gattc_param = nullptr; |       // Zero out the param struct when null | ||||||
|       this->event_.gattc.data = nullptr; |       memset(&this->event_.gattc.gattc_param, 0, sizeof(this->event_.gattc.gattc_param)); | ||||||
|  |       this->event_.gattc.is_inline = false; | ||||||
|  |       this->event_.gattc.data.heap_data = nullptr; | ||||||
|  |       this->event_.gattc.data_len = 0; | ||||||
|       return;  // Invalid event, but we can't log in header file |       return;  // Invalid event, but we can't log in header file | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     // Heap-allocate param and data |     // Copy param struct inline (no heap allocation!) | ||||||
|     // Heap allocation is used because GATTC/GATTS events are rare (<1% of events) |     // GATTC/GATTS events are rare (<1% of events) but we can still store them inline | ||||||
|     // while GAP events (99%) are stored inline to minimize memory usage |     // along with small data payloads, eliminating all heap allocations for typical BLE operations | ||||||
|     // IMPORTANT: This heap allocation provides clear ownership semantics: |     // CRITICAL: This copy is REQUIRED for memory safety - the ESP-IDF param pointer | ||||||
|     // - The BLEEvent owns the allocated memory for its lifetime |     // is only valid during the callback and will be reused/freed after we return | ||||||
|     // - The data remains valid from the BLE callback context until processed in the main loop |     this->event_.gattc.gattc_param = *p; | ||||||
|     // - Without this copy, we'd have use-after-free bugs as ESP-IDF reuses the callback memory |  | ||||||
|     this->event_.gattc.gattc_param = new esp_ble_gattc_cb_param_t(*p); |  | ||||||
|  |  | ||||||
|     // Copy data for events that need it |     // Copy data for events that need it | ||||||
|     // The param struct contains pointers (e.g., notify.value) that point to temporary buffers. |     // The param struct contains pointers (e.g., notify.value) that point to temporary buffers. | ||||||
|     // We must copy this data to ensure it remains valid when the event is processed later. |     // We must copy this data to ensure it remains valid when the event is processed later. | ||||||
|     switch (e) { |     switch (e) { | ||||||
|       case ESP_GATTC_NOTIFY_EVT: |       case ESP_GATTC_NOTIFY_EVT: | ||||||
|         this->event_.gattc.data = new std::vector<uint8_t>(p->notify.value, p->notify.value + p->notify.value_len); |         copy_data_with_inline_storage_<decltype(this->event_.gattc), GATTC_INLINE_DATA_SIZE>( | ||||||
|         this->event_.gattc.gattc_param->notify.value = this->event_.gattc.data->data(); |             this->event_.gattc, p->notify.value, p->notify.value_len, &this->event_.gattc.gattc_param.notify.value); | ||||||
|         break; |         break; | ||||||
|       case ESP_GATTC_READ_CHAR_EVT: |       case ESP_GATTC_READ_CHAR_EVT: | ||||||
|       case ESP_GATTC_READ_DESCR_EVT: |       case ESP_GATTC_READ_DESCR_EVT: | ||||||
|         this->event_.gattc.data = new std::vector<uint8_t>(p->read.value, p->read.value + p->read.value_len); |         copy_data_with_inline_storage_<decltype(this->event_.gattc), GATTC_INLINE_DATA_SIZE>( | ||||||
|         this->event_.gattc.gattc_param->read.value = this->event_.gattc.data->data(); |             this->event_.gattc, p->read.value, p->read.value_len, &this->event_.gattc.gattc_param.read.value); | ||||||
|         break; |         break; | ||||||
|       default: |       default: | ||||||
|         this->event_.gattc.data = nullptr; |         this->event_.gattc.is_inline = false; | ||||||
|  |         this->event_.gattc.data.heap_data = nullptr; | ||||||
|  |         this->event_.gattc.data_len = 0; | ||||||
|         break; |         break; | ||||||
|     } |     } | ||||||
|   } |   } | ||||||
| @@ -356,30 +412,33 @@ class BLEEvent { | |||||||
|     this->event_.gatts.gatts_if = i; |     this->event_.gatts.gatts_if = i; | ||||||
|  |  | ||||||
|     if (p == nullptr) { |     if (p == nullptr) { | ||||||
|       this->event_.gatts.gatts_param = nullptr; |       // Zero out the param struct when null | ||||||
|       this->event_.gatts.data = nullptr; |       memset(&this->event_.gatts.gatts_param, 0, sizeof(this->event_.gatts.gatts_param)); | ||||||
|  |       this->event_.gatts.is_inline = false; | ||||||
|  |       this->event_.gatts.data.heap_data = nullptr; | ||||||
|  |       this->event_.gatts.data_len = 0; | ||||||
|       return;  // Invalid event, but we can't log in header file |       return;  // Invalid event, but we can't log in header file | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     // Heap-allocate param and data |     // Copy param struct inline (no heap allocation!) | ||||||
|     // Heap allocation is used because GATTC/GATTS events are rare (<1% of events) |     // GATTC/GATTS events are rare (<1% of events) but we can still store them inline | ||||||
|     // while GAP events (99%) are stored inline to minimize memory usage |     // along with small data payloads, eliminating all heap allocations for typical BLE operations | ||||||
|     // IMPORTANT: This heap allocation provides clear ownership semantics: |     // CRITICAL: This copy is REQUIRED for memory safety - the ESP-IDF param pointer | ||||||
|     // - The BLEEvent owns the allocated memory for its lifetime |     // is only valid during the callback and will be reused/freed after we return | ||||||
|     // - The data remains valid from the BLE callback context until processed in the main loop |     this->event_.gatts.gatts_param = *p; | ||||||
|     // - Without this copy, we'd have use-after-free bugs as ESP-IDF reuses the callback memory |  | ||||||
|     this->event_.gatts.gatts_param = new esp_ble_gatts_cb_param_t(*p); |  | ||||||
|  |  | ||||||
|     // Copy data for events that need it |     // Copy data for events that need it | ||||||
|     // The param struct contains pointers (e.g., write.value) that point to temporary buffers. |     // The param struct contains pointers (e.g., write.value) that point to temporary buffers. | ||||||
|     // We must copy this data to ensure it remains valid when the event is processed later. |     // We must copy this data to ensure it remains valid when the event is processed later. | ||||||
|     switch (e) { |     switch (e) { | ||||||
|       case ESP_GATTS_WRITE_EVT: |       case ESP_GATTS_WRITE_EVT: | ||||||
|         this->event_.gatts.data = new std::vector<uint8_t>(p->write.value, p->write.value + p->write.len); |         copy_data_with_inline_storage_<decltype(this->event_.gatts), GATTS_INLINE_DATA_SIZE>( | ||||||
|         this->event_.gatts.gatts_param->write.value = this->event_.gatts.data->data(); |             this->event_.gatts, p->write.value, p->write.len, &this->event_.gatts.gatts_param.write.value); | ||||||
|         break; |         break; | ||||||
|       default: |       default: | ||||||
|         this->event_.gatts.data = nullptr; |         this->event_.gatts.is_inline = false; | ||||||
|  |         this->event_.gatts.data.heap_data = nullptr; | ||||||
|  |         this->event_.gatts.data_len = 0; | ||||||
|         break; |         break; | ||||||
|     } |     } | ||||||
|   } |   } | ||||||
| @@ -389,6 +448,15 @@ class BLEEvent { | |||||||
| // The gap member in the union should be 80 bytes (including the gap_event enum) | // The gap member in the union should be 80 bytes (including the gap_event enum) | ||||||
| static_assert(sizeof(decltype(((BLEEvent *) nullptr)->event_.gap)) <= 80, "gap_event struct has grown beyond 80 bytes"); | static_assert(sizeof(decltype(((BLEEvent *) nullptr)->event_.gap)) <= 80, "gap_event struct has grown beyond 80 bytes"); | ||||||
|  |  | ||||||
|  | // Verify GATTC and GATTS structs don't exceed GAP struct size | ||||||
|  | // This ensures the union size is determined by GAP (the most common event type) | ||||||
|  | static_assert(sizeof(decltype(((BLEEvent *) nullptr)->event_.gattc)) <= | ||||||
|  |                   sizeof(decltype(((BLEEvent *) nullptr)->event_.gap)), | ||||||
|  |               "gattc_event struct exceeds gap_event size - union size would increase"); | ||||||
|  | static_assert(sizeof(decltype(((BLEEvent *) nullptr)->event_.gatts)) <= | ||||||
|  |                   sizeof(decltype(((BLEEvent *) nullptr)->event_.gap)), | ||||||
|  |               "gatts_event struct exceeds gap_event size - union size would increase"); | ||||||
|  |  | ||||||
| // Verify esp_ble_sec_t fits within our union | // Verify esp_ble_sec_t fits within our union | ||||||
| static_assert(sizeof(esp_ble_sec_t) <= 73, "esp_ble_sec_t is larger than BLEScanResult"); | static_assert(sizeof(esp_ble_sec_t) <= 73, "esp_ble_sec_t is larger than BLEScanResult"); | ||||||
|  |  | ||||||
|   | |||||||
| @@ -12,7 +12,7 @@ extern "C" { | |||||||
| #include "preferences.h" | #include "preferences.h" | ||||||
|  |  | ||||||
| #include <cstring> | #include <cstring> | ||||||
| #include <vector> | #include <memory> | ||||||
|  |  | ||||||
| namespace esphome { | namespace esphome { | ||||||
| namespace esp8266 { | namespace esp8266 { | ||||||
| @@ -67,6 +67,8 @@ static uint32_t get_esp8266_flash_sector() { | |||||||
| } | } | ||||||
| static uint32_t get_esp8266_flash_address() { return get_esp8266_flash_sector() * SPI_FLASH_SEC_SIZE; } | static uint32_t get_esp8266_flash_address() { return get_esp8266_flash_sector() * SPI_FLASH_SEC_SIZE; } | ||||||
|  |  | ||||||
|  | static inline size_t bytes_to_words(size_t bytes) { return (bytes + 3) / 4; } | ||||||
|  |  | ||||||
| template<class It> uint32_t calculate_crc(It first, It last, uint32_t type) { | template<class It> uint32_t calculate_crc(It first, It last, uint32_t type) { | ||||||
|   uint32_t crc = type; |   uint32_t crc = type; | ||||||
|   while (first != last) { |   while (first != last) { | ||||||
| @@ -123,41 +125,36 @@ class ESP8266PreferenceBackend : public ESPPreferenceBackend { | |||||||
|   size_t length_words = 0; |   size_t length_words = 0; | ||||||
|  |  | ||||||
|   bool save(const uint8_t *data, size_t len) override { |   bool save(const uint8_t *data, size_t len) override { | ||||||
|     if ((len + 3) / 4 != length_words) { |     if (bytes_to_words(len) != length_words) { | ||||||
|       return false; |       return false; | ||||||
|     } |     } | ||||||
|     std::vector<uint32_t> buffer; |     size_t buffer_size = length_words + 1; | ||||||
|     buffer.resize(length_words + 1); |     std::unique_ptr<uint32_t[]> buffer(new uint32_t[buffer_size]());  // Note the () for zero-initialization | ||||||
|     memcpy(buffer.data(), data, len); |     memcpy(buffer.get(), data, len); | ||||||
|     buffer[buffer.size() - 1] = calculate_crc(buffer.begin(), buffer.end() - 1, type); |     buffer[length_words] = calculate_crc(buffer.get(), buffer.get() + length_words, type); | ||||||
|  |  | ||||||
|     if (in_flash) { |     if (in_flash) { | ||||||
|       return save_to_flash(offset, buffer.data(), buffer.size()); |       return save_to_flash(offset, buffer.get(), buffer_size); | ||||||
|     } else { |  | ||||||
|       return save_to_rtc(offset, buffer.data(), buffer.size()); |  | ||||||
|     } |     } | ||||||
|  |     return save_to_rtc(offset, buffer.get(), buffer_size); | ||||||
|   } |   } | ||||||
|   bool load(uint8_t *data, size_t len) override { |   bool load(uint8_t *data, size_t len) override { | ||||||
|     if ((len + 3) / 4 != length_words) { |     if (bytes_to_words(len) != length_words) { | ||||||
|       return false; |       return false; | ||||||
|     } |     } | ||||||
|     std::vector<uint32_t> buffer; |     size_t buffer_size = length_words + 1; | ||||||
|     buffer.resize(length_words + 1); |     std::unique_ptr<uint32_t[]> buffer(new uint32_t[buffer_size]()); | ||||||
|     bool ret; |     bool ret = in_flash ? load_from_flash(offset, buffer.get(), buffer_size) | ||||||
|     if (in_flash) { |                         : load_from_rtc(offset, buffer.get(), buffer_size); | ||||||
|       ret = load_from_flash(offset, buffer.data(), buffer.size()); |  | ||||||
|     } else { |  | ||||||
|       ret = load_from_rtc(offset, buffer.data(), buffer.size()); |  | ||||||
|     } |  | ||||||
|     if (!ret) |     if (!ret) | ||||||
|       return false; |       return false; | ||||||
|  |  | ||||||
|     uint32_t crc = calculate_crc(buffer.begin(), buffer.end() - 1, type); |     uint32_t crc = calculate_crc(buffer.get(), buffer.get() + length_words, type); | ||||||
|     if (buffer[buffer.size() - 1] != crc) { |     if (buffer[length_words] != crc) { | ||||||
|       return false; |       return false; | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     memcpy(data, buffer.data(), len); |     memcpy(data, buffer.get(), len); | ||||||
|     return true; |     return true; | ||||||
|   } |   } | ||||||
| }; | }; | ||||||
| @@ -178,7 +175,7 @@ class ESP8266Preferences : public ESPPreferences { | |||||||
|   } |   } | ||||||
|  |  | ||||||
|   ESPPreferenceObject make_preference(size_t length, uint32_t type, bool in_flash) override { |   ESPPreferenceObject make_preference(size_t length, uint32_t type, bool in_flash) override { | ||||||
|     uint32_t length_words = (length + 3) / 4; |     uint32_t length_words = bytes_to_words(length); | ||||||
|     if (in_flash) { |     if (in_flash) { | ||||||
|       uint32_t start = current_flash_offset; |       uint32_t start = current_flash_offset; | ||||||
|       uint32_t end = start + length_words + 1; |       uint32_t end = start + length_words + 1; | ||||||
|   | |||||||
| @@ -764,7 +764,8 @@ void Nextion::process_nextion_commands_() { | |||||||
|         variable_name = to_process.substr(0, index); |         variable_name = to_process.substr(0, index); | ||||||
|         ++index; |         ++index; | ||||||
|  |  | ||||||
|         text_value = to_process.substr(index); |         // Get variable value without terminating NUL byte.  Length check above ensures substr len >= 0. | ||||||
|  |         text_value = to_process.substr(index, to_process_length - index - 1); | ||||||
|  |  | ||||||
|         ESP_LOGN(TAG, "Text sensor: %s='%s'", variable_name.c_str(), text_value.c_str()); |         ESP_LOGN(TAG, "Text sensor: %s='%s'", variable_name.c_str(), text_value.c_str()); | ||||||
|  |  | ||||||
|   | |||||||
| @@ -23,20 +23,18 @@ void Pipsolar::loop() { | |||||||
|   // Read message |   // Read message | ||||||
|   if (this->state_ == STATE_IDLE) { |   if (this->state_ == STATE_IDLE) { | ||||||
|     this->empty_uart_buffer_(); |     this->empty_uart_buffer_(); | ||||||
|     switch (this->send_next_command_()) { |  | ||||||
|       case 0: |     if (this->send_next_command_()) { | ||||||
|         // no command send (empty queue) time to poll |       // command sent | ||||||
|         if (millis() - this->last_poll_ > this->update_interval_) { |  | ||||||
|           this->send_next_poll_(); |  | ||||||
|           this->last_poll_ = millis(); |  | ||||||
|         } |  | ||||||
|       return; |       return; | ||||||
|         break; |  | ||||||
|       case 1: |  | ||||||
|         // command send |  | ||||||
|         return; |  | ||||||
|         break; |  | ||||||
|     } |     } | ||||||
|  |  | ||||||
|  |     if (this->send_next_poll_()) { | ||||||
|  |       // poll sent | ||||||
|  |       return; | ||||||
|  |     } | ||||||
|  |  | ||||||
|  |     return; | ||||||
|   } |   } | ||||||
|   if (this->state_ == STATE_COMMAND_COMPLETE) { |   if (this->state_ == STATE_COMMAND_COMPLETE) { | ||||||
|     if (this->check_incoming_length_(4)) { |     if (this->check_incoming_length_(4)) { | ||||||
| @@ -530,7 +528,7 @@ void Pipsolar::loop() { | |||||||
|         // '(00000000000000000000000000000000' |         // '(00000000000000000000000000000000' | ||||||
|         // iterate over all available flag (as not all models have all flags, but at least in the same order) |         // iterate over all available flag (as not all models have all flags, but at least in the same order) | ||||||
|         this->value_warnings_present_ = false; |         this->value_warnings_present_ = false; | ||||||
|         this->value_faults_present_ = true; |         this->value_faults_present_ = false; | ||||||
|  |  | ||||||
|         for (size_t i = 1; i < strlen(tmp); i++) { |         for (size_t i = 1; i < strlen(tmp); i++) { | ||||||
|           enabled = tmp[i] == '1'; |           enabled = tmp[i] == '1'; | ||||||
| @@ -708,6 +706,7 @@ void Pipsolar::loop() { | |||||||
|         return; |         return; | ||||||
|       } |       } | ||||||
|       // crc ok |       // crc ok | ||||||
|  |       this->used_polling_commands_[this->last_polling_command_].needs_update = false; | ||||||
|       this->state_ = STATE_POLL_CHECKED; |       this->state_ = STATE_POLL_CHECKED; | ||||||
|       return; |       return; | ||||||
|     } else { |     } else { | ||||||
| @@ -788,7 +787,7 @@ uint8_t Pipsolar::check_incoming_crc_() { | |||||||
| } | } | ||||||
|  |  | ||||||
| // send next command used | // send next command used | ||||||
| uint8_t Pipsolar::send_next_command_() { | bool Pipsolar::send_next_command_() { | ||||||
|   uint16_t crc16; |   uint16_t crc16; | ||||||
|   if (!this->command_queue_[this->command_queue_position_].empty()) { |   if (!this->command_queue_[this->command_queue_position_].empty()) { | ||||||
|     const char *command = this->command_queue_[this->command_queue_position_].c_str(); |     const char *command = this->command_queue_[this->command_queue_position_].c_str(); | ||||||
| @@ -809,20 +808,23 @@ uint8_t Pipsolar::send_next_command_() { | |||||||
|     // end Byte |     // end Byte | ||||||
|     this->write(0x0D); |     this->write(0x0D); | ||||||
|     ESP_LOGD(TAG, "Sending command from queue: %s with length %d", command, length); |     ESP_LOGD(TAG, "Sending command from queue: %s with length %d", command, length); | ||||||
|     return 1; |     return true; | ||||||
|   } |   } | ||||||
|   return 0; |   return false; | ||||||
| } | } | ||||||
|  |  | ||||||
| void Pipsolar::send_next_poll_() { | bool Pipsolar::send_next_poll_() { | ||||||
|   uint16_t crc16; |   uint16_t crc16; | ||||||
|   this->last_polling_command_ = (this->last_polling_command_ + 1) % 15; |  | ||||||
|  |   for (uint8_t i = 0; i < POLLING_COMMANDS_MAX; i++) { | ||||||
|  |     this->last_polling_command_ = (this->last_polling_command_ + 1) % POLLING_COMMANDS_MAX; | ||||||
|     if (this->used_polling_commands_[this->last_polling_command_].length == 0) { |     if (this->used_polling_commands_[this->last_polling_command_].length == 0) { | ||||||
|     this->last_polling_command_ = 0; |       // not enabled | ||||||
|  |       continue; | ||||||
|     } |     } | ||||||
|   if (this->used_polling_commands_[this->last_polling_command_].length == 0) { |     if (!this->used_polling_commands_[this->last_polling_command_].needs_update) { | ||||||
|     // no command specified |       // no update requested | ||||||
|     return; |       continue; | ||||||
|     } |     } | ||||||
|     this->state_ = STATE_POLL; |     this->state_ = STATE_POLL; | ||||||
|     this->command_start_millis_ = millis(); |     this->command_start_millis_ = millis(); | ||||||
| @@ -840,6 +842,9 @@ void Pipsolar::send_next_poll_() { | |||||||
|     ESP_LOGD(TAG, "Sending polling command : %s with length %d", |     ESP_LOGD(TAG, "Sending polling command : %s with length %d", | ||||||
|              this->used_polling_commands_[this->last_polling_command_].command, |              this->used_polling_commands_[this->last_polling_command_].command, | ||||||
|              this->used_polling_commands_[this->last_polling_command_].length); |              this->used_polling_commands_[this->last_polling_command_].length); | ||||||
|  |     return true; | ||||||
|  |   } | ||||||
|  |   return false; | ||||||
| } | } | ||||||
|  |  | ||||||
| void Pipsolar::queue_command_(const char *command, uint8_t length) { | void Pipsolar::queue_command_(const char *command, uint8_t length) { | ||||||
| @@ -869,7 +874,13 @@ void Pipsolar::dump_config() { | |||||||
|     } |     } | ||||||
|   } |   } | ||||||
| } | } | ||||||
| void Pipsolar::update() {} | void Pipsolar::update() { | ||||||
|  |   for (auto &used_polling_command : this->used_polling_commands_) { | ||||||
|  |     if (used_polling_command.length != 0) { | ||||||
|  |       used_polling_command.needs_update = true; | ||||||
|  |     } | ||||||
|  |   } | ||||||
|  | } | ||||||
|  |  | ||||||
| void Pipsolar::add_polling_command_(const char *command, ENUMPollingCommand polling_command) { | void Pipsolar::add_polling_command_(const char *command, ENUMPollingCommand polling_command) { | ||||||
|   for (auto &used_polling_command : this->used_polling_commands_) { |   for (auto &used_polling_command : this->used_polling_commands_) { | ||||||
| @@ -891,6 +902,7 @@ void Pipsolar::add_polling_command_(const char *command, ENUMPollingCommand poll | |||||||
|       used_polling_command.errors = 0; |       used_polling_command.errors = 0; | ||||||
|       used_polling_command.identifier = polling_command; |       used_polling_command.identifier = polling_command; | ||||||
|       used_polling_command.length = length - 1; |       used_polling_command.length = length - 1; | ||||||
|  |       used_polling_command.needs_update = true; | ||||||
|       return; |       return; | ||||||
|     } |     } | ||||||
|   } |   } | ||||||
|   | |||||||
| @@ -25,6 +25,7 @@ struct PollingCommand { | |||||||
|   uint8_t length = 0; |   uint8_t length = 0; | ||||||
|   uint8_t errors; |   uint8_t errors; | ||||||
|   ENUMPollingCommand identifier; |   ENUMPollingCommand identifier; | ||||||
|  |   bool needs_update; | ||||||
| }; | }; | ||||||
|  |  | ||||||
| #define PIPSOLAR_VALUED_ENTITY_(type, name, polling_command, value_type) \ | #define PIPSOLAR_VALUED_ENTITY_(type, name, polling_command, value_type) \ | ||||||
| @@ -189,14 +190,14 @@ class Pipsolar : public uart::UARTDevice, public PollingComponent { | |||||||
|   static const size_t PIPSOLAR_READ_BUFFER_LENGTH = 110;  // maximum supported answer length |   static const size_t PIPSOLAR_READ_BUFFER_LENGTH = 110;  // maximum supported answer length | ||||||
|   static const size_t COMMAND_QUEUE_LENGTH = 10; |   static const size_t COMMAND_QUEUE_LENGTH = 10; | ||||||
|   static const size_t COMMAND_TIMEOUT = 5000; |   static const size_t COMMAND_TIMEOUT = 5000; | ||||||
|   uint32_t last_poll_ = 0; |   static const size_t POLLING_COMMANDS_MAX = 15; | ||||||
|   void add_polling_command_(const char *command, ENUMPollingCommand polling_command); |   void add_polling_command_(const char *command, ENUMPollingCommand polling_command); | ||||||
|   void empty_uart_buffer_(); |   void empty_uart_buffer_(); | ||||||
|   uint8_t check_incoming_crc_(); |   uint8_t check_incoming_crc_(); | ||||||
|   uint8_t check_incoming_length_(uint8_t length); |   uint8_t check_incoming_length_(uint8_t length); | ||||||
|   uint16_t pipsolar_crc_(uint8_t *msg, uint8_t len); |   uint16_t pipsolar_crc_(uint8_t *msg, uint8_t len); | ||||||
|   uint8_t send_next_command_(); |   bool send_next_command_(); | ||||||
|   void send_next_poll_(); |   bool send_next_poll_(); | ||||||
|   void queue_command_(const char *command, uint8_t length); |   void queue_command_(const char *command, uint8_t length); | ||||||
|   std::string command_queue_[COMMAND_QUEUE_LENGTH]; |   std::string command_queue_[COMMAND_QUEUE_LENGTH]; | ||||||
|   uint8_t command_queue_position_ = 0; |   uint8_t command_queue_position_ = 0; | ||||||
| @@ -216,7 +217,7 @@ class Pipsolar : public uart::UARTDevice, public PollingComponent { | |||||||
|   }; |   }; | ||||||
|  |  | ||||||
|   uint8_t last_polling_command_ = 0; |   uint8_t last_polling_command_ = 0; | ||||||
|   PollingCommand used_polling_commands_[15]; |   PollingCommand used_polling_commands_[POLLING_COMMANDS_MAX]; | ||||||
| }; | }; | ||||||
|  |  | ||||||
| }  // namespace pipsolar | }  // namespace pipsolar | ||||||
|   | |||||||
| @@ -15,11 +15,11 @@ namespace safe_mode { | |||||||
| static const char *const TAG = "safe_mode"; | static const char *const TAG = "safe_mode"; | ||||||
|  |  | ||||||
| void SafeModeComponent::dump_config() { | void SafeModeComponent::dump_config() { | ||||||
|   ESP_LOGCONFIG(TAG, "Safe Mode:"); |  | ||||||
|   ESP_LOGCONFIG(TAG, |   ESP_LOGCONFIG(TAG, | ||||||
|                 "  Boot considered successful after %" PRIu32 " seconds\n" |                 "Safe Mode:\n" | ||||||
|                 "  Invoke after %u boot attempts\n" |                 "  Successful after: %" PRIu32 "s\n" | ||||||
|                 "  Remain for %" PRIu32 " seconds", |                 "  Invoke after: %u attempts\n" | ||||||
|  |                 "  Duration: %" PRIu32 "s", | ||||||
|                 this->safe_mode_boot_is_good_after_ / 1000,  // because milliseconds |                 this->safe_mode_boot_is_good_after_ / 1000,  // because milliseconds | ||||||
|                 this->safe_mode_num_attempts_, |                 this->safe_mode_num_attempts_, | ||||||
|                 this->safe_mode_enable_time_ / 1000);  // because milliseconds |                 this->safe_mode_enable_time_ / 1000);  // because milliseconds | ||||||
| @@ -27,7 +27,7 @@ void SafeModeComponent::dump_config() { | |||||||
|   if (this->safe_mode_rtc_value_ > 1 && this->safe_mode_rtc_value_ != SafeModeComponent::ENTER_SAFE_MODE_MAGIC) { |   if (this->safe_mode_rtc_value_ > 1 && this->safe_mode_rtc_value_ != SafeModeComponent::ENTER_SAFE_MODE_MAGIC) { | ||||||
|     auto remaining_restarts = this->safe_mode_num_attempts_ - this->safe_mode_rtc_value_; |     auto remaining_restarts = this->safe_mode_num_attempts_ - this->safe_mode_rtc_value_; | ||||||
|     if (remaining_restarts) { |     if (remaining_restarts) { | ||||||
|       ESP_LOGW(TAG, "Last reset occurred too quickly; will be invoked in %" PRIu32 " restarts", remaining_restarts); |       ESP_LOGW(TAG, "Last reset too quick; invoke in %" PRIu32 " restarts", remaining_restarts); | ||||||
|     } else { |     } else { | ||||||
|       ESP_LOGW(TAG, "SAFE MODE IS ACTIVE"); |       ESP_LOGW(TAG, "SAFE MODE IS ACTIVE"); | ||||||
|     } |     } | ||||||
| @@ -72,26 +72,33 @@ bool SafeModeComponent::should_enter_safe_mode(uint8_t num_attempts, uint32_t en | |||||||
|   this->safe_mode_boot_is_good_after_ = boot_is_good_after; |   this->safe_mode_boot_is_good_after_ = boot_is_good_after; | ||||||
|   this->safe_mode_num_attempts_ = num_attempts; |   this->safe_mode_num_attempts_ = num_attempts; | ||||||
|   this->rtc_ = global_preferences->make_preference<uint32_t>(233825507UL, false); |   this->rtc_ = global_preferences->make_preference<uint32_t>(233825507UL, false); | ||||||
|   this->safe_mode_rtc_value_ = this->read_rtc_(); |  | ||||||
|  |  | ||||||
|   bool is_manual_safe_mode = this->safe_mode_rtc_value_ == SafeModeComponent::ENTER_SAFE_MODE_MAGIC; |   uint32_t rtc_val = this->read_rtc_(); | ||||||
|  |   this->safe_mode_rtc_value_ = rtc_val; | ||||||
|  |  | ||||||
|   if (is_manual_safe_mode) { |   bool is_manual = rtc_val == SafeModeComponent::ENTER_SAFE_MODE_MAGIC; | ||||||
|     ESP_LOGI(TAG, "Safe mode invoked manually"); |  | ||||||
|  |   if (is_manual) { | ||||||
|  |     ESP_LOGI(TAG, "Manual mode"); | ||||||
|   } else { |   } else { | ||||||
|     ESP_LOGCONFIG(TAG, "There have been %" PRIu32 " suspected unsuccessful boot attempts", this->safe_mode_rtc_value_); |     ESP_LOGCONFIG(TAG, "Unsuccessful boot attempts: %" PRIu32, rtc_val); | ||||||
|  |   } | ||||||
|  |  | ||||||
|  |   if (rtc_val < num_attempts && !is_manual) { | ||||||
|  |     // increment counter | ||||||
|  |     this->write_rtc_(rtc_val + 1); | ||||||
|  |     return false; | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   if (this->safe_mode_rtc_value_ >= num_attempts || is_manual_safe_mode) { |  | ||||||
|   this->clean_rtc(); |   this->clean_rtc(); | ||||||
|  |  | ||||||
|     if (!is_manual_safe_mode) { |   if (!is_manual) { | ||||||
|       ESP_LOGE(TAG, "Boot loop detected. Proceeding"); |     ESP_LOGE(TAG, "Boot loop detected"); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|   this->status_set_error(); |   this->status_set_error(); | ||||||
|   this->set_timeout(enable_time, []() { |   this->set_timeout(enable_time, []() { | ||||||
|       ESP_LOGW(TAG, "Safe mode enable time has elapsed -- restarting"); |     ESP_LOGW(TAG, "Timeout, restarting"); | ||||||
|     App.reboot(); |     App.reboot(); | ||||||
|   }); |   }); | ||||||
|  |  | ||||||
| @@ -104,11 +111,6 @@ bool SafeModeComponent::should_enter_safe_mode(uint8_t num_attempts, uint32_t en | |||||||
|   this->safe_mode_callback_.call(); |   this->safe_mode_callback_.call(); | ||||||
|  |  | ||||||
|   return true; |   return true; | ||||||
|   } else { |  | ||||||
|     // increment counter |  | ||||||
|     this->write_rtc_(this->safe_mode_rtc_value_ + 1); |  | ||||||
|     return false; |  | ||||||
|   } |  | ||||||
| } | } | ||||||
|  |  | ||||||
| void SafeModeComponent::write_rtc_(uint32_t val) { | void SafeModeComponent::write_rtc_(uint32_t val) { | ||||||
|   | |||||||
| @@ -53,10 +53,14 @@ void SenseAirComponent::update() { | |||||||
|  |  | ||||||
|   this->status_clear_warning(); |   this->status_clear_warning(); | ||||||
|   const uint8_t length = response[2]; |   const uint8_t length = response[2]; | ||||||
|   const uint16_t status = (uint16_t(response[3]) << 8) | response[4]; |   const uint16_t status = encode_uint16(response[3], response[4]); | ||||||
|   const int16_t ppm = int16_t((response[length + 1] << 8) | response[length + 2]); |   const uint16_t ppm = encode_uint16(response[length + 1], response[length + 2]); | ||||||
|  |  | ||||||
|   ESP_LOGD(TAG, "SenseAir Received CO₂=%dppm Status=0x%02X", ppm, status); |   ESP_LOGD(TAG, "SenseAir Received CO₂=%uppm Status=0x%02X", ppm, status); | ||||||
|  |   if (ppm == 0 && (status & SenseAirStatus::OUT_OF_RANGE_ERROR) != 0) { | ||||||
|  |     ESP_LOGD(TAG, "Discarding 0 ppm reading with out-of-range status."); | ||||||
|  |     return; | ||||||
|  |   } | ||||||
|   if (this->co2_sensor_ != nullptr) |   if (this->co2_sensor_ != nullptr) | ||||||
|     this->co2_sensor_->publish_state(ppm); |     this->co2_sensor_->publish_state(ppm); | ||||||
| } | } | ||||||
|   | |||||||
| @@ -8,6 +8,17 @@ | |||||||
| namespace esphome { | namespace esphome { | ||||||
| namespace senseair { | namespace senseair { | ||||||
|  |  | ||||||
|  | enum SenseAirStatus : uint8_t { | ||||||
|  |   FATAL_ERROR = 1 << 0, | ||||||
|  |   OFFSET_ERROR = 1 << 1, | ||||||
|  |   ALGORITHM_ERROR = 1 << 2, | ||||||
|  |   OUTPUT_ERROR = 1 << 3, | ||||||
|  |   SELF_DIAGNOSTIC_ERROR = 1 << 4, | ||||||
|  |   OUT_OF_RANGE_ERROR = 1 << 5, | ||||||
|  |   MEMORY_ERROR = 1 << 6, | ||||||
|  |   RESERVED = 1 << 7 | ||||||
|  | }; | ||||||
|  |  | ||||||
| class SenseAirComponent : public PollingComponent, public uart::UARTDevice { | class SenseAirComponent : public PollingComponent, public uart::UARTDevice { | ||||||
|  public: |  public: | ||||||
|   void set_co2_sensor(sensor::Sensor *co2_sensor) { co2_sensor_ = co2_sensor; } |   void set_co2_sensor(sensor::Sensor *co2_sensor) { co2_sensor_ = co2_sensor; } | ||||||
|   | |||||||
| @@ -813,7 +813,7 @@ std::string WebServer::cover_state_json_generator(WebServer *web_server, void *s | |||||||
|   return web_server->cover_json((cover::Cover *) (source), DETAIL_STATE); |   return web_server->cover_json((cover::Cover *) (source), DETAIL_STATE); | ||||||
| } | } | ||||||
| std::string WebServer::cover_all_json_generator(WebServer *web_server, void *source) { | std::string WebServer::cover_all_json_generator(WebServer *web_server, void *source) { | ||||||
|   return web_server->cover_json((cover::Cover *) (source), DETAIL_STATE); |   return web_server->cover_json((cover::Cover *) (source), DETAIL_ALL); | ||||||
| } | } | ||||||
| std::string WebServer::cover_json(cover::Cover *obj, JsonDetail start_config) { | std::string WebServer::cover_json(cover::Cover *obj, JsonDetail start_config) { | ||||||
|   return json::build_json([this, obj, start_config](JsonObject root) { |   return json::build_json([this, obj, start_config](JsonObject root) { | ||||||
|   | |||||||
| @@ -475,11 +475,16 @@ bool Application::register_socket_fd(int fd) { | |||||||
|   if (fd < 0) |   if (fd < 0) | ||||||
|     return false; |     return false; | ||||||
|  |  | ||||||
|  | #ifndef USE_ESP32 | ||||||
|  |   // Only check on non-ESP32 platforms | ||||||
|  |   // On ESP32 (both Arduino and ESP-IDF), CONFIG_LWIP_MAX_SOCKETS is always <= FD_SETSIZE by design | ||||||
|  |   // (LWIP_SOCKET_OFFSET = FD_SETSIZE - CONFIG_LWIP_MAX_SOCKETS per lwipopts.h) | ||||||
|  |   // Other platforms may not have this guarantee | ||||||
|   if (fd >= FD_SETSIZE) { |   if (fd >= FD_SETSIZE) { | ||||||
|     ESP_LOGE(TAG, "Cannot monitor socket fd %d: exceeds FD_SETSIZE (%d)", fd, FD_SETSIZE); |     ESP_LOGE(TAG, "fd %d exceeds FD_SETSIZE %d", fd, FD_SETSIZE); | ||||||
|     ESP_LOGE(TAG, "Socket will not be monitored for data - may cause performance issues!"); |  | ||||||
|     return false; |     return false; | ||||||
|   } |   } | ||||||
|  | #endif | ||||||
|  |  | ||||||
|   this->socket_fds_.push_back(fd); |   this->socket_fds_.push_back(fd); | ||||||
|   this->socket_fds_changed_ = true; |   this->socket_fds_changed_ = true; | ||||||
|   | |||||||
| @@ -82,7 +82,13 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type | |||||||
|   item->set_name(name_cstr, !is_static_string); |   item->set_name(name_cstr, !is_static_string); | ||||||
|   item->type = type; |   item->type = type; | ||||||
|   item->callback = std::move(func); |   item->callback = std::move(func); | ||||||
|  |   // Initialize remove to false (though it should already be from constructor) | ||||||
|  |   // Not using mark_item_removed_ helper since we're setting to false, not true | ||||||
|  | #ifdef ESPHOME_THREAD_MULTI_ATOMICS | ||||||
|  |   item->remove.store(false, std::memory_order_relaxed); | ||||||
|  | #else | ||||||
|   item->remove = false; |   item->remove = false; | ||||||
|  | #endif | ||||||
|   item->is_retry = is_retry; |   item->is_retry = is_retry; | ||||||
|  |  | ||||||
| #ifndef ESPHOME_THREAD_SINGLE | #ifndef ESPHOME_THREAD_SINGLE | ||||||
| @@ -398,6 +404,31 @@ void HOT Scheduler::call(uint32_t now) { | |||||||
|         this->pop_raw_(); |         this->pop_raw_(); | ||||||
|         continue; |         continue; | ||||||
|       } |       } | ||||||
|  |  | ||||||
|  |       // Check if item is marked for removal | ||||||
|  |       // This handles two cases: | ||||||
|  |       // 1. Item was marked for removal after cleanup_() but before we got here | ||||||
|  |       // 2. Item is marked for removal but wasn't at the front of the heap during cleanup_() | ||||||
|  | #ifdef ESPHOME_THREAD_MULTI_NO_ATOMICS | ||||||
|  |       // Multi-threaded platforms without atomics: must take lock to safely read remove flag | ||||||
|  |       { | ||||||
|  |         LockGuard guard{this->lock_}; | ||||||
|  |         if (is_item_removed_(item.get())) { | ||||||
|  |           this->pop_raw_(); | ||||||
|  |           this->to_remove_--; | ||||||
|  |           continue; | ||||||
|  |         } | ||||||
|  |       } | ||||||
|  | #else | ||||||
|  |       // Single-threaded or multi-threaded with atomics: can check without lock | ||||||
|  |       if (is_item_removed_(item.get())) { | ||||||
|  |         LockGuard guard{this->lock_}; | ||||||
|  |         this->pop_raw_(); | ||||||
|  |         this->to_remove_--; | ||||||
|  |         continue; | ||||||
|  |       } | ||||||
|  | #endif | ||||||
|  |  | ||||||
| #ifdef ESPHOME_DEBUG_SCHEDULER | #ifdef ESPHOME_DEBUG_SCHEDULER | ||||||
|       const char *item_name = item->get_name(); |       const char *item_name = item->get_name(); | ||||||
|       ESP_LOGV(TAG, "Running %s '%s/%s' with interval=%" PRIu32 " next_execution=%" PRIu64 " (now=%" PRIu64 ")", |       ESP_LOGV(TAG, "Running %s '%s/%s' with interval=%" PRIu32 " next_execution=%" PRIu64 " (now=%" PRIu64 ")", | ||||||
| @@ -518,7 +549,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c | |||||||
|   if (type == SchedulerItem::TIMEOUT) { |   if (type == SchedulerItem::TIMEOUT) { | ||||||
|     for (auto &item : this->defer_queue_) { |     for (auto &item : this->defer_queue_) { | ||||||
|       if (this->matches_item_(item, component, name_cstr, type, match_retry)) { |       if (this->matches_item_(item, component, name_cstr, type, match_retry)) { | ||||||
|         item->remove = true; |         this->mark_item_removed_(item.get()); | ||||||
|         total_cancelled++; |         total_cancelled++; | ||||||
|       } |       } | ||||||
|     } |     } | ||||||
| @@ -528,7 +559,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c | |||||||
|   // Cancel items in the main heap |   // Cancel items in the main heap | ||||||
|   for (auto &item : this->items_) { |   for (auto &item : this->items_) { | ||||||
|     if (this->matches_item_(item, component, name_cstr, type, match_retry)) { |     if (this->matches_item_(item, component, name_cstr, type, match_retry)) { | ||||||
|       item->remove = true; |       this->mark_item_removed_(item.get()); | ||||||
|       total_cancelled++; |       total_cancelled++; | ||||||
|       this->to_remove_++;  // Track removals for heap items |       this->to_remove_++;  // Track removals for heap items | ||||||
|     } |     } | ||||||
| @@ -537,7 +568,7 @@ bool HOT Scheduler::cancel_item_locked_(Component *component, const char *name_c | |||||||
|   // Cancel items in to_add_ |   // Cancel items in to_add_ | ||||||
|   for (auto &item : this->to_add_) { |   for (auto &item : this->to_add_) { | ||||||
|     if (this->matches_item_(item, component, name_cstr, type, match_retry)) { |     if (this->matches_item_(item, component, name_cstr, type, match_retry)) { | ||||||
|       item->remove = true; |       this->mark_item_removed_(item.get()); | ||||||
|       total_cancelled++; |       total_cancelled++; | ||||||
|       // Don't track removals for to_add_ items |       // Don't track removals for to_add_ items | ||||||
|     } |     } | ||||||
|   | |||||||
| @@ -97,22 +97,42 @@ class Scheduler { | |||||||
|  |  | ||||||
|     std::function<void()> callback; |     std::function<void()> callback; | ||||||
|  |  | ||||||
|     // Bit-packed fields to minimize padding | #ifdef ESPHOME_THREAD_MULTI_ATOMICS | ||||||
|  |     // Multi-threaded with atomics: use atomic for lock-free access | ||||||
|  |     // Place atomic<bool> separately since it can't be packed with bit fields | ||||||
|  |     std::atomic<bool> remove{false}; | ||||||
|  |  | ||||||
|  |     // Bit-packed fields (3 bits used, 5 bits padding in 1 byte) | ||||||
|  |     enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1; | ||||||
|  |     bool name_is_dynamic : 1;  // True if name was dynamically allocated (needs delete[]) | ||||||
|  |     bool is_retry : 1;         // True if this is a retry timeout | ||||||
|  |                                // 5 bits padding | ||||||
|  | #else | ||||||
|  |     // Single-threaded or multi-threaded without atomics: can pack all fields together | ||||||
|  |     // Bit-packed fields (4 bits used, 4 bits padding in 1 byte) | ||||||
|     enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1; |     enum Type : uint8_t { TIMEOUT, INTERVAL } type : 1; | ||||||
|     bool remove : 1; |     bool remove : 1; | ||||||
|     bool name_is_dynamic : 1;  // True if name was dynamically allocated (needs delete[]) |     bool name_is_dynamic : 1;  // True if name was dynamically allocated (needs delete[]) | ||||||
|     bool is_retry : 1;         // True if this is a retry timeout |     bool is_retry : 1;         // True if this is a retry timeout | ||||||
|                                // 4 bits padding |                                // 4 bits padding | ||||||
|  | #endif | ||||||
|  |  | ||||||
|     // Constructor |     // Constructor | ||||||
|     SchedulerItem() |     SchedulerItem() | ||||||
|         : component(nullptr), |         : component(nullptr), | ||||||
|           interval(0), |           interval(0), | ||||||
|           next_execution_(0), |           next_execution_(0), | ||||||
|  | #ifdef ESPHOME_THREAD_MULTI_ATOMICS | ||||||
|  |           // remove is initialized in the member declaration as std::atomic<bool>{false} | ||||||
|  |           type(TIMEOUT), | ||||||
|  |           name_is_dynamic(false), | ||||||
|  |           is_retry(false) { | ||||||
|  | #else | ||||||
|           type(TIMEOUT), |           type(TIMEOUT), | ||||||
|           remove(false), |           remove(false), | ||||||
|           name_is_dynamic(false), |           name_is_dynamic(false), | ||||||
|           is_retry(false) { |           is_retry(false) { | ||||||
|  | #endif | ||||||
|       name_.static_name = nullptr; |       name_.static_name = nullptr; | ||||||
|     } |     } | ||||||
|  |  | ||||||
| @@ -219,6 +239,37 @@ class Scheduler { | |||||||
|     return item->remove || (item->component != nullptr && item->component->is_failed()); |     return item->remove || (item->component != nullptr && item->component->is_failed()); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  |   // Helper to check if item is marked for removal (platform-specific) | ||||||
|  |   // Returns true if item should be skipped, handles platform-specific synchronization | ||||||
|  |   // For ESPHOME_THREAD_MULTI_NO_ATOMICS platforms, the caller must hold the scheduler lock before calling this | ||||||
|  |   // function. | ||||||
|  |   bool is_item_removed_(SchedulerItem *item) const { | ||||||
|  | #ifdef ESPHOME_THREAD_MULTI_ATOMICS | ||||||
|  |     // Multi-threaded with atomics: use atomic load for lock-free access | ||||||
|  |     return item->remove.load(std::memory_order_acquire); | ||||||
|  | #else | ||||||
|  |     // Single-threaded (ESPHOME_THREAD_SINGLE) or | ||||||
|  |     // multi-threaded without atomics (ESPHOME_THREAD_MULTI_NO_ATOMICS): direct read | ||||||
|  |     // For ESPHOME_THREAD_MULTI_NO_ATOMICS, caller MUST hold lock! | ||||||
|  |     return item->remove; | ||||||
|  | #endif | ||||||
|  |   } | ||||||
|  |  | ||||||
|  |   // Helper to mark item for removal (platform-specific) | ||||||
|  |   // For ESPHOME_THREAD_MULTI_NO_ATOMICS platforms, the caller must hold the scheduler lock before calling this | ||||||
|  |   // function. | ||||||
|  |   void mark_item_removed_(SchedulerItem *item) { | ||||||
|  | #ifdef ESPHOME_THREAD_MULTI_ATOMICS | ||||||
|  |     // Multi-threaded with atomics: use atomic store | ||||||
|  |     item->remove.store(true, std::memory_order_release); | ||||||
|  | #else | ||||||
|  |     // Single-threaded (ESPHOME_THREAD_SINGLE) or | ||||||
|  |     // multi-threaded without atomics (ESPHOME_THREAD_MULTI_NO_ATOMICS): direct write | ||||||
|  |     // For ESPHOME_THREAD_MULTI_NO_ATOMICS, caller MUST hold lock! | ||||||
|  |     item->remove = true; | ||||||
|  | #endif | ||||||
|  |   } | ||||||
|  |  | ||||||
|   // Template helper to check if any item in a container matches our criteria |   // Template helper to check if any item in a container matches our criteria | ||||||
|   template<typename Container> |   template<typename Container> | ||||||
|   bool has_cancelled_timeout_in_container_(const Container &container, Component *component, const char *name_cstr, |   bool has_cancelled_timeout_in_container_(const Container &container, Component *component, const char *name_cstr, | ||||||
|   | |||||||
| @@ -1952,7 +1952,7 @@ def build_message_type( | |||||||
|     dump_impl += "}\n" |     dump_impl += "}\n" | ||||||
|  |  | ||||||
|     if base_class: |     if base_class: | ||||||
|         out = f"class {desc.name} : public {base_class} {{\n" |         out = f"class {desc.name} final : public {base_class} {{\n" | ||||||
|     else: |     else: | ||||||
|         # Check if message has any non-deprecated fields |         # Check if message has any non-deprecated fields | ||||||
|         has_fields = any(not field.options.deprecated for field in desc.field) |         has_fields = any(not field.options.deprecated for field in desc.field) | ||||||
| @@ -1961,7 +1961,7 @@ def build_message_type( | |||||||
|             base_class = "ProtoDecodableMessage" |             base_class = "ProtoDecodableMessage" | ||||||
|         else: |         else: | ||||||
|             base_class = "ProtoMessage" |             base_class = "ProtoMessage" | ||||||
|         out = f"class {desc.name} : public {base_class} {{\n" |         out = f"class {desc.name} final : public {base_class} {{\n" | ||||||
|     out += " public:\n" |     out += " public:\n" | ||||||
|     out += indent("\n".join(public_content)) + "\n" |     out += indent("\n".join(public_content)) + "\n" | ||||||
|     out += "\n" |     out += "\n" | ||||||
|   | |||||||
							
								
								
									
										139
									
								
								tests/integration/fixtures/scheduler_removed_item_race.yaml
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										139
									
								
								tests/integration/fixtures/scheduler_removed_item_race.yaml
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,139 @@ | |||||||
|  | esphome: | ||||||
|  |   name: scheduler-removed-item-race | ||||||
|  |  | ||||||
|  | host: | ||||||
|  |  | ||||||
|  | api: | ||||||
|  |   services: | ||||||
|  |     - service: run_test | ||||||
|  |       then: | ||||||
|  |         - script.execute: run_test_script | ||||||
|  |  | ||||||
|  | logger: | ||||||
|  |   level: DEBUG | ||||||
|  |  | ||||||
|  | globals: | ||||||
|  |   - id: test_passed | ||||||
|  |     type: bool | ||||||
|  |     initial_value: 'true' | ||||||
|  |   - id: removed_item_executed | ||||||
|  |     type: int | ||||||
|  |     initial_value: '0' | ||||||
|  |   - id: normal_item_executed | ||||||
|  |     type: int | ||||||
|  |     initial_value: '0' | ||||||
|  |  | ||||||
|  | sensor: | ||||||
|  |   - platform: template | ||||||
|  |     id: test_sensor | ||||||
|  |     name: "Test Sensor" | ||||||
|  |     update_interval: never | ||||||
|  |     lambda: return 0.0; | ||||||
|  |  | ||||||
|  | script: | ||||||
|  |   - id: run_test_script | ||||||
|  |     then: | ||||||
|  |       - logger.log: "=== Starting Removed Item Race Test ===" | ||||||
|  |  | ||||||
|  |       # This test creates a scenario where: | ||||||
|  |       # 1. First item in heap is NOT cancelled (cleanup stops immediately) | ||||||
|  |       # 2. Items behind it ARE cancelled (remain in heap after cleanup) | ||||||
|  |       # 3. All items execute at the same time, including cancelled ones | ||||||
|  |  | ||||||
|  |       - lambda: |- | ||||||
|  |           // The key to hitting the race: | ||||||
|  |           // 1. Add items in a specific order to control heap structure | ||||||
|  |           // 2. Cancel ONLY items that won't be at the front | ||||||
|  |           // 3. Ensure the first item stays non-cancelled so cleanup_() stops immediately | ||||||
|  |  | ||||||
|  |           // Schedule all items to execute at the SAME time (1ms from now) | ||||||
|  |           // Using 1ms instead of 0 to avoid defer queue on multi-core platforms | ||||||
|  |           // This ensures they'll all be ready together and go through the heap | ||||||
|  |           const uint32_t exec_time = 1; | ||||||
|  |  | ||||||
|  |           // CRITICAL: Add a non-cancellable item FIRST | ||||||
|  |           // This will be at the front of the heap and block cleanup_() | ||||||
|  |           App.scheduler.set_timeout(id(test_sensor), "blocker", exec_time, []() { | ||||||
|  |             ESP_LOGD("test", "Blocker timeout executed (expected) - was at front of heap"); | ||||||
|  |             id(normal_item_executed)++; | ||||||
|  |           }); | ||||||
|  |  | ||||||
|  |           // Now add items that we WILL cancel | ||||||
|  |           // These will be behind the blocker in the heap | ||||||
|  |           App.scheduler.set_timeout(id(test_sensor), "cancel_1", exec_time, []() { | ||||||
|  |             ESP_LOGE("test", "RACE: Cancelled timeout 1 executed after being cancelled!"); | ||||||
|  |             id(removed_item_executed)++; | ||||||
|  |             id(test_passed) = false; | ||||||
|  |           }); | ||||||
|  |  | ||||||
|  |           App.scheduler.set_timeout(id(test_sensor), "cancel_2", exec_time, []() { | ||||||
|  |             ESP_LOGE("test", "RACE: Cancelled timeout 2 executed after being cancelled!"); | ||||||
|  |             id(removed_item_executed)++; | ||||||
|  |             id(test_passed) = false; | ||||||
|  |           }); | ||||||
|  |  | ||||||
|  |           App.scheduler.set_timeout(id(test_sensor), "cancel_3", exec_time, []() { | ||||||
|  |             ESP_LOGE("test", "RACE: Cancelled timeout 3 executed after being cancelled!"); | ||||||
|  |             id(removed_item_executed)++; | ||||||
|  |             id(test_passed) = false; | ||||||
|  |           }); | ||||||
|  |  | ||||||
|  |           // Add some more normal items | ||||||
|  |           App.scheduler.set_timeout(id(test_sensor), "normal_1", exec_time, []() { | ||||||
|  |             ESP_LOGD("test", "Normal timeout 1 executed (expected)"); | ||||||
|  |             id(normal_item_executed)++; | ||||||
|  |           }); | ||||||
|  |  | ||||||
|  |           App.scheduler.set_timeout(id(test_sensor), "normal_2", exec_time, []() { | ||||||
|  |             ESP_LOGD("test", "Normal timeout 2 executed (expected)"); | ||||||
|  |             id(normal_item_executed)++; | ||||||
|  |           }); | ||||||
|  |  | ||||||
|  |           App.scheduler.set_timeout(id(test_sensor), "normal_3", exec_time, []() { | ||||||
|  |             ESP_LOGD("test", "Normal timeout 3 executed (expected)"); | ||||||
|  |             id(normal_item_executed)++; | ||||||
|  |           }); | ||||||
|  |  | ||||||
|  |           // Force items into the heap before cancelling | ||||||
|  |           App.scheduler.process_to_add(); | ||||||
|  |  | ||||||
|  |           // NOW cancel the items - they're behind "blocker" in the heap | ||||||
|  |           // When cleanup_() runs, it will see "blocker" (not removed) at the front | ||||||
|  |           // and stop immediately, leaving cancel_1, cancel_2, cancel_3 in the heap | ||||||
|  |           bool c1 = App.scheduler.cancel_timeout(id(test_sensor), "cancel_1"); | ||||||
|  |           bool c2 = App.scheduler.cancel_timeout(id(test_sensor), "cancel_2"); | ||||||
|  |           bool c3 = App.scheduler.cancel_timeout(id(test_sensor), "cancel_3"); | ||||||
|  |  | ||||||
|  |           ESP_LOGD("test", "Cancelled items (behind blocker): %s, %s, %s", | ||||||
|  |                    c1 ? "true" : "false", | ||||||
|  |                    c2 ? "true" : "false", | ||||||
|  |                    c3 ? "true" : "false"); | ||||||
|  |  | ||||||
|  |           // The heap now has: | ||||||
|  |           // - "blocker" at front (not cancelled) | ||||||
|  |           // - cancelled items behind it (marked remove=true but still in heap) | ||||||
|  |           // - When all execute at once, cleanup_() stops at "blocker" | ||||||
|  |           // - The loop then executes ALL ready items including cancelled ones | ||||||
|  |  | ||||||
|  |           ESP_LOGD("test", "Setup complete. Blocker at front prevents cleanup of cancelled items behind it"); | ||||||
|  |  | ||||||
|  |       # Wait for all timeouts to execute (or not) | ||||||
|  |       - delay: 20ms | ||||||
|  |  | ||||||
|  |       # Check results | ||||||
|  |       - lambda: |- | ||||||
|  |           ESP_LOGI("test", "=== Test Results ==="); | ||||||
|  |           ESP_LOGI("test", "Normal items executed: %d (expected 4)", id(normal_item_executed)); | ||||||
|  |           ESP_LOGI("test", "Removed items executed: %d (expected 0)", id(removed_item_executed)); | ||||||
|  |  | ||||||
|  |           if (id(removed_item_executed) > 0) { | ||||||
|  |             ESP_LOGE("test", "TEST FAILED: %d cancelled items were executed!", id(removed_item_executed)); | ||||||
|  |             id(test_passed) = false; | ||||||
|  |           } else if (id(normal_item_executed) != 4) { | ||||||
|  |             ESP_LOGE("test", "TEST FAILED: Expected 4 normal items, got %d", id(normal_item_executed)); | ||||||
|  |             id(test_passed) = false; | ||||||
|  |           } else { | ||||||
|  |             ESP_LOGI("test", "TEST PASSED: No cancelled items were executed"); | ||||||
|  |           } | ||||||
|  |  | ||||||
|  |           ESP_LOGI("test", "=== Test Complete ==="); | ||||||
							
								
								
									
										102
									
								
								tests/integration/test_scheduler_removed_item_race.py
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										102
									
								
								tests/integration/test_scheduler_removed_item_race.py
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,102 @@ | |||||||
|  | """Test for scheduler race condition where removed items still execute.""" | ||||||
|  |  | ||||||
|  | import asyncio | ||||||
|  | import re | ||||||
|  |  | ||||||
|  | import pytest | ||||||
|  |  | ||||||
|  | from .types import APIClientConnectedFactory, RunCompiledFunction | ||||||
|  |  | ||||||
|  |  | ||||||
|  | @pytest.mark.asyncio | ||||||
|  | async def test_scheduler_removed_item_race( | ||||||
|  |     yaml_config: str, | ||||||
|  |     run_compiled: RunCompiledFunction, | ||||||
|  |     api_client_connected: APIClientConnectedFactory, | ||||||
|  | ) -> None: | ||||||
|  |     """Test that items marked for removal don't execute. | ||||||
|  |  | ||||||
|  |     This test verifies the fix for a race condition where: | ||||||
|  |     1. cleanup_() only removes items from the front of the heap | ||||||
|  |     2. Items in the middle of the heap marked for removal still execute | ||||||
|  |     3. This causes cancelled timeouts to run when they shouldn't | ||||||
|  |     """ | ||||||
|  |  | ||||||
|  |     loop = asyncio.get_running_loop() | ||||||
|  |     test_complete_future: asyncio.Future[bool] = loop.create_future() | ||||||
|  |  | ||||||
|  |     # Track test results | ||||||
|  |     test_passed = False | ||||||
|  |     removed_executed = 0 | ||||||
|  |     normal_executed = 0 | ||||||
|  |  | ||||||
|  |     # Patterns to match | ||||||
|  |     race_pattern = re.compile(r"RACE: .* executed after being cancelled!") | ||||||
|  |     passed_pattern = re.compile(r"TEST PASSED") | ||||||
|  |     failed_pattern = re.compile(r"TEST FAILED") | ||||||
|  |     complete_pattern = re.compile(r"=== Test Complete ===") | ||||||
|  |     normal_count_pattern = re.compile(r"Normal items executed: (\d+)") | ||||||
|  |     removed_count_pattern = re.compile(r"Removed items executed: (\d+)") | ||||||
|  |  | ||||||
|  |     def check_output(line: str) -> None: | ||||||
|  |         """Check log output for test results.""" | ||||||
|  |         nonlocal test_passed, removed_executed, normal_executed | ||||||
|  |  | ||||||
|  |         if race_pattern.search(line): | ||||||
|  |             # Race condition detected - a cancelled item executed | ||||||
|  |             test_passed = False | ||||||
|  |  | ||||||
|  |         if passed_pattern.search(line): | ||||||
|  |             test_passed = True | ||||||
|  |         elif failed_pattern.search(line): | ||||||
|  |             test_passed = False | ||||||
|  |  | ||||||
|  |         normal_match = normal_count_pattern.search(line) | ||||||
|  |         if normal_match: | ||||||
|  |             normal_executed = int(normal_match.group(1)) | ||||||
|  |  | ||||||
|  |         removed_match = removed_count_pattern.search(line) | ||||||
|  |         if removed_match: | ||||||
|  |             removed_executed = int(removed_match.group(1)) | ||||||
|  |  | ||||||
|  |         if not test_complete_future.done() and complete_pattern.search(line): | ||||||
|  |             test_complete_future.set_result(True) | ||||||
|  |  | ||||||
|  |     async with ( | ||||||
|  |         run_compiled(yaml_config, line_callback=check_output), | ||||||
|  |         api_client_connected() as client, | ||||||
|  |     ): | ||||||
|  |         # Verify we can connect | ||||||
|  |         device_info = await client.device_info() | ||||||
|  |         assert device_info is not None | ||||||
|  |         assert device_info.name == "scheduler-removed-item-race" | ||||||
|  |  | ||||||
|  |         # List services | ||||||
|  |         _, services = await asyncio.wait_for( | ||||||
|  |             client.list_entities_services(), timeout=5.0 | ||||||
|  |         ) | ||||||
|  |  | ||||||
|  |         # Find run_test service | ||||||
|  |         run_test_service = next((s for s in services if s.name == "run_test"), None) | ||||||
|  |         assert run_test_service is not None, "run_test service not found" | ||||||
|  |  | ||||||
|  |         # Execute the test | ||||||
|  |         client.execute_service(run_test_service, {}) | ||||||
|  |  | ||||||
|  |         # Wait for test completion | ||||||
|  |         try: | ||||||
|  |             await asyncio.wait_for(test_complete_future, timeout=5.0) | ||||||
|  |         except TimeoutError: | ||||||
|  |             pytest.fail("Test did not complete within timeout") | ||||||
|  |  | ||||||
|  |         # Verify results | ||||||
|  |         assert test_passed, ( | ||||||
|  |             f"Test failed! Removed items executed: {removed_executed}, " | ||||||
|  |             f"Normal items executed: {normal_executed}" | ||||||
|  |         ) | ||||||
|  |         assert removed_executed == 0, ( | ||||||
|  |             f"Cancelled items should not execute, but {removed_executed} did" | ||||||
|  |         ) | ||||||
|  |         assert normal_executed == 4, ( | ||||||
|  |             f"Expected 4 normal items to execute, got {normal_executed}" | ||||||
|  |         ) | ||||||
		Reference in New Issue
	
	Block a user