Merge branch 'heap_scheduler_stress_component' into integration

esphome/components/scd4x/scd4x.cpp

@@ -58,7 +58,7 @@ void SCD4XComponent::setup() {
       }
 
       // If pressure compensation available use it, else use altitude
-      if (this->ambient_pressure_compensation_) {
+      if (this->ambient_pressure_) {
         if (!this->update_ambient_pressure_compensation_(this->ambient_pressure_)) {
           ESP_LOGE(TAG, "Error setting ambient pressure compensation");
           this->error_code_ = MEASUREMENT_INIT_FAILED;
@@ -137,7 +137,7 @@ void SCD4XComponent::dump_config() {
     ESP_LOGCONFIG(TAG, "  Dynamic ambient pressure compensation using '%s'",
                   this->ambient_pressure_source_->get_name().c_str());
   } else {
-    if (this->ambient_pressure_compensation_) {
+    if (this->ambient_pressure_) {
       ESP_LOGCONFIG(TAG,
                     "  Altitude compensation disabled\n"
                     "  Ambient pressure compensation: %dmBar",
@@ -230,7 +230,7 @@ bool SCD4XComponent::perform_forced_calibration(uint16_t current_co2_concentrati
       // frc takes 400 ms
       // because this method will be used very rarly
       // the simple approach with delay is ok
-      delay(400);  // NOLINT'
+      delay(400);  // NOLINT
       if (!this->start_measurement_()) {
         return false;
       } else {
@@ -267,8 +267,7 @@ bool SCD4XComponent::factory_reset() {
 }
 
 void SCD4XComponent::set_ambient_pressure_compensation(float pressure_in_hpa) {
-  ambient_pressure_compensation_ = true;
+  uint16_t new_ambient_pressure = static_cast<uint16_t>(pressure_in_hpa);
-  uint16_t new_ambient_pressure = (uint16_t) pressure_in_hpa;
   if (!this->initialized_) {
     this->ambient_pressure_ = new_ambient_pressure;
     return;

esphome/components/scd4x/scd4x.h

@@ -46,19 +46,17 @@ class SCD4XComponent : public PollingComponent, public sensirion_common::Sensiri
   bool update_ambient_pressure_compensation_(uint16_t pressure_in_hpa);
   bool start_measurement_();
 
-  uint16_t altitude_compensation_;
-  uint16_t ambient_pressure_;
-  bool initialized_{false};
-  bool ambient_pressure_compensation_;
-  bool enable_asc_;
-  float temperature_offset_;
-  ErrorCode error_code_;
-  MeasurementMode measurement_mode_{PERIODIC};
   sensor::Sensor *co2_sensor_{nullptr};
   sensor::Sensor *temperature_sensor_{nullptr};
   sensor::Sensor *humidity_sensor_{nullptr};
-  // used for compensation
+  sensor::Sensor *ambient_pressure_source_{nullptr};  // used for compensation
-  sensor::Sensor *ambient_pressure_source_{nullptr};
+  float temperature_offset_;
+  uint16_t altitude_compensation_{0};
+  uint16_t ambient_pressure_{0};  // Per datasheet, valid values are 700 to 1200 hPa; 0 is a valid sentinel value
+  bool initialized_{false};
+  bool enable_asc_{false};
+  ErrorCode error_code_;
+  MeasurementMode measurement_mode_{PERIODIC};
 };
 
 }  // namespace scd4x

esphome/components/sx127x/sx127x.cpp

@@ -318,24 +318,23 @@ void SX127x::loop() {
       uint8_t addr = this->read_register_(REG_FIFO_RX_CURR_ADDR);
       uint8_t rssi = this->read_register_(REG_PKT_RSSI_VALUE);
       int8_t snr = (int8_t) this->read_register_(REG_PKT_SNR_VALUE);
-      std::vector<uint8_t> packet(bytes);
+      this->packet_.resize(bytes);
       this->write_register_(REG_FIFO_ADDR_PTR, addr);
-      this->read_fifo_(packet);
+      this->read_fifo_(this->packet_);
       if (this->frequency_ > 700000000) {
-        this->call_listeners_(packet, (float) rssi - RSSI_OFFSET_HF, (float) snr / 4);
+        this->call_listeners_(this->packet_, (float) rssi - RSSI_OFFSET_HF, (float) snr / 4);
       } else {
-        this->call_listeners_(packet, (float) rssi - RSSI_OFFSET_LF, (float) snr / 4);
+        this->call_listeners_(this->packet_, (float) rssi - RSSI_OFFSET_LF, (float) snr / 4);
       }
     }
   } else if (this->packet_mode_) {
-    std::vector<uint8_t> packet;
     uint8_t payload_length = this->payload_length_;
     if (payload_length == 0) {
       payload_length = this->read_register_(REG_FIFO);
     }
-    packet.resize(payload_length);
+    this->packet_.resize(payload_length);
-    this->read_fifo_(packet);
+    this->read_fifo_(this->packet_);
-    this->call_listeners_(packet, 0.0f, 0.0f);
+    this->call_listeners_(this->packet_, 0.0f, 0.0f);
   }
 }
 
@@ -407,18 +406,6 @@ void SX127x::dump_config() {
   LOG_PIN("  CS Pin: ", this->cs_);
   LOG_PIN("  RST Pin: ", this->rst_pin_);
   LOG_PIN("  DIO0 Pin: ", this->dio0_pin_);
-  const char *shaping = "NONE";
-  if (this->shaping_ == CUTOFF_BR_X_2) {
-    shaping = "CUTOFF_BR_X_2";
-  } else if (this->shaping_ == CUTOFF_BR_X_1) {
-    shaping = "CUTOFF_BR_X_1";
-  } else if (this->shaping_ == GAUSSIAN_BT_0_3) {
-    shaping = "GAUSSIAN_BT_0_3";
-  } else if (this->shaping_ == GAUSSIAN_BT_0_5) {
-    shaping = "GAUSSIAN_BT_0_5";
-  } else if (this->shaping_ == GAUSSIAN_BT_1_0) {
-    shaping = "GAUSSIAN_BT_1_0";
-  }
   const char *pa_pin = "RFO";
   if (this->pa_pin_ == PA_PIN_BOOST) {
     pa_pin = "BOOST";
@@ -429,10 +416,9 @@ void SX127x::dump_config() {
                 "  Bandwidth: %" PRIu32 " Hz\n"
                 "  PA Pin: %s\n"
                 "  PA Power: %" PRIu8 " dBm\n"
-                "  PA Ramp: %" PRIu16 " us\n"
+                "  PA Ramp: %" PRIu16 " us",
-                "  Shaping: %s",
                 TRUEFALSE(this->auto_cal_), this->frequency_, BW_HZ[this->bandwidth_], pa_pin, this->pa_power_,
-                RAMP[this->pa_ramp_], shaping);
+                RAMP[this->pa_ramp_]);
   if (this->modulation_ == MOD_FSK) {
     ESP_LOGCONFIG(TAG, "  Deviation: %" PRIu32 " Hz", this->deviation_);
   }
@@ -459,14 +445,31 @@ void SX127x::dump_config() {
       ESP_LOGCONFIG(TAG, "  Sync Value: 0x%02x", this->sync_value_[0]);
     }
   } else {
+    const char *shaping = "NONE";
+    if (this->modulation_ == MOD_FSK) {
+      if (this->shaping_ == GAUSSIAN_BT_0_3) {
+        shaping = "GAUSSIAN_BT_0_3";
+      } else if (this->shaping_ == GAUSSIAN_BT_0_5) {
+        shaping = "GAUSSIAN_BT_0_5";
+      } else if (this->shaping_ == GAUSSIAN_BT_1_0) {
+        shaping = "GAUSSIAN_BT_1_0";
+      }
+    } else {
+      if (this->shaping_ == CUTOFF_BR_X_2) {
+        shaping = "CUTOFF_BR_X_2";
+      } else if (this->shaping_ == CUTOFF_BR_X_1) {
+        shaping = "CUTOFF_BR_X_1";
+      }
+    }
     ESP_LOGCONFIG(TAG,
+                  "  Shaping: %s\n"
                   "  Modulation: %s\n"
                   "  Bitrate: %" PRIu32 "b/s\n"
                   "  Bitsync: %s\n"
                   "  Rx Start: %s\n"
                   "  Rx Floor: %.1f dBm\n"
                   "  Packet Mode: %s",
-                  this->modulation_ == MOD_FSK ? "FSK" : "OOK", this->bitrate_, TRUEFALSE(this->bitsync_),
+                  shaping, this->modulation_ == MOD_FSK ? "FSK" : "OOK", this->bitrate_, TRUEFALSE(this->bitsync_),
                   TRUEFALSE(this->rx_start_), this->rx_floor_, TRUEFALSE(this->packet_mode_));
     if (this->packet_mode_) {
       ESP_LOGCONFIG(TAG, "  CRC Enable: %s", TRUEFALSE(this->crc_enable_));

esphome/components/sx127x/sx127x.h

@@ -96,6 +96,7 @@ class SX127x : public Component,
   uint8_t read_register_(uint8_t reg);
   Trigger<std::vector<uint8_t>, float, float> *packet_trigger_{new Trigger<std::vector<uint8_t>, float, float>()};
   std::vector<SX127xListener *> listeners_;
+  std::vector<uint8_t> packet_;
   std::vector<uint8_t> sync_value_;
   InternalGPIOPin *dio0_pin_{nullptr};
   InternalGPIOPin *rst_pin_{nullptr};

esphome/core/lock_free_queue.h

@@ -122,19 +122,17 @@ template<class T, uint8_t SIZE> class NotifyingLockFreeQueue : public LockFreeQu
     bool result = this->push_internal_(element, was_empty, old_tail);
 
     // Notify optimization: only notify if we need to
-    if (result && task_to_notify_ != nullptr) {
+    if (result && task_to_notify_ != nullptr &&
-      if (was_empty) {
+        (was_empty || this->head_.load(std::memory_order_acquire) == old_tail)) {
-        // Queue was empty - consumer might be going to sleep, must notify
+      // Notify in two cases:
-        xTaskNotifyGive(task_to_notify_);
+      // 1. Queue was empty - consumer might be going to sleep
-      } else if (this->head_.load(std::memory_order_acquire) == old_tail) {
+      // 2. Consumer just caught up to where tail was - might go to sleep
-        // Consumer just caught up to where tail was - might go to sleep, must notify
+      // Note: There's a benign race in case 2 - between reading head and calling
-        // Note: There's a benign race here - between reading head and calling
+      // xTaskNotifyGive(), the consumer could advance further. This would result
-        // xTaskNotifyGive(), the consumer could advance further. This would result
+      // in an unnecessary wake-up, but is harmless and extremely rare in practice.
-        // in an unnecessary wake-up, but is harmless and extremely rare in practice.
+      xTaskNotifyGive(task_to_notify_);
-        xTaskNotifyGive(task_to_notify_);
-      }
-      // Otherwise: consumer is still behind, no need to notify
     }
+    // Otherwise: consumer is still behind, no need to notify
 
     return result;
   }

tests/integration/fixtures/external_components/scheduler_string_lifetime_component/string_lifetime_component.cpp

@@ -38,6 +38,48 @@ void SchedulerStringLifetimeComponent::run_string_lifetime_test() {
   });
 }
 
+void SchedulerStringLifetimeComponent::run_test1() {
+  test_temporary_string_lifetime();
+  // Wait for all callbacks to execute
+  this->set_timeout("test1_complete", 10, [this]() { ESP_LOGI(TAG, "Test 1 complete"); });
+}
+
+void SchedulerStringLifetimeComponent::run_test2() {
+  test_scope_exit_string();
+  // Wait for all callbacks to execute
+  this->set_timeout("test2_complete", 20, [this]() { ESP_LOGI(TAG, "Test 2 complete"); });
+}
+
+void SchedulerStringLifetimeComponent::run_test3() {
+  test_vector_reallocation();
+  // Wait for all callbacks to execute
+  this->set_timeout("test3_complete", 60, [this]() { ESP_LOGI(TAG, "Test 3 complete"); });
+}
+
+void SchedulerStringLifetimeComponent::run_test4() {
+  test_string_move_semantics();
+  // Wait for all callbacks to execute
+  this->set_timeout("test4_complete", 35, [this]() { ESP_LOGI(TAG, "Test 4 complete"); });
+}
+
+void SchedulerStringLifetimeComponent::run_test5() {
+  test_lambda_capture_lifetime();
+  // Wait for all callbacks to execute
+  this->set_timeout("test5_complete", 50, [this]() { ESP_LOGI(TAG, "Test 5 complete"); });
+}
+
+void SchedulerStringLifetimeComponent::run_final_check() {
+  ESP_LOGI(TAG, "String lifetime tests complete");
+  ESP_LOGI(TAG, "Tests passed: %d", this->tests_passed_);
+  ESP_LOGI(TAG, "Tests failed: %d", this->tests_failed_);
+
+  if (this->tests_failed_ == 0) {
+    ESP_LOGI(TAG, "SUCCESS: All string lifetime tests passed!");
+  } else {
+    ESP_LOGE(TAG, "FAILURE: %d string lifetime tests failed!", this->tests_failed_);
+  }
+}
+
 void SchedulerStringLifetimeComponent::test_temporary_string_lifetime() {
   ESP_LOGI(TAG, "Test 1: Temporary string lifetime for timeout names");
 

tests/integration/fixtures/external_components/scheduler_string_lifetime_component/string_lifetime_component.h

@@ -14,6 +14,14 @@ class SchedulerStringLifetimeComponent : public Component {
 
   void run_string_lifetime_test();
 
+  // Individual test methods exposed as services
+  void run_test1();
+  void run_test2();
+  void run_test3();
+  void run_test4();
+  void run_test5();
+  void run_final_check();
+
  private:
   void test_temporary_string_lifetime();
   void test_scope_exit_string();

tests/integration/fixtures/scheduler_string_lifetime.yaml

@@ -21,3 +21,27 @@ api:
       then:
         - lambda: |-
             id(string_lifetime)->run_string_lifetime_test();
+    - service: run_test1
+      then:
+        - lambda: |-
+            id(string_lifetime)->run_test1();
+    - service: run_test2
+      then:
+        - lambda: |-
+            id(string_lifetime)->run_test2();
+    - service: run_test3
+      then:
+        - lambda: |-
+            id(string_lifetime)->run_test3();
+    - service: run_test4
+      then:
+        - lambda: |-
+            id(string_lifetime)->run_test4();
+    - service: run_test5
+      then:
+        - lambda: |-
+            id(string_lifetime)->run_test5();
+    - service: run_final_check
+      then:
+        - lambda: |-
+            id(string_lifetime)->run_final_check();

tests/integration/test_scheduler_string_lifetime.py

@@ -4,7 +4,6 @@ import asyncio
 from pathlib import Path
 import re
 
-from aioesphomeapi import UserService
 import pytest
 
 from .types import APIClientConnectedFactory, RunCompiledFunction
@@ -28,19 +27,42 @@ async def test_scheduler_string_lifetime(
         "EXTERNAL_COMPONENT_PATH", external_components_path
     )
 
-    # Create a future to signal test completion
+    # Create events for synchronization
-    loop = asyncio.get_running_loop()
+    test1_complete = asyncio.Event()
-    test_complete_future: asyncio.Future[None] = loop.create_future()
+    test2_complete = asyncio.Event()
+    test3_complete = asyncio.Event()
+    test4_complete = asyncio.Event()
+    test5_complete = asyncio.Event()
+    all_tests_complete = asyncio.Event()
 
     # Track test progress
     test_stats = {
         "tests_passed": 0,
         "tests_failed": 0,
         "errors": [],
-        "use_after_free_detected": False,
+        "current_test": None,
+        "test_callbacks_executed": {},
     }
 
     def on_log_line(line: str) -> None:
+        # Track test-specific events
+        if "Test 1 complete" in line:
+            test1_complete.set()
+        elif "Test 2 complete" in line:
+            test2_complete.set()
+        elif "Test 3 complete" in line:
+            test3_complete.set()
+        elif "Test 4 complete" in line:
+            test4_complete.set()
+        elif "Test 5 complete" in line:
+            test5_complete.set()
+
+        # Track individual callback executions
+        callback_match = re.search(r"Callback '(.+?)' executed", line)
+        if callback_match:
+            callback_name = callback_match.group(1)
+            test_stats["test_callbacks_executed"][callback_name] = True
+
         # Track test results from the C++ test output
         if "Tests passed:" in line and "string_lifetime" in line:
             # Extract the number from "Tests passed: 32"
@@ -68,16 +90,11 @@ async def test_scheduler_string_lifetime(
                 "invalid pointer",
             ]
         ):
-            test_stats["use_after_free_detected"] = True
+            pytest.fail(f"Memory corruption detected: {line}")
-            if not test_complete_future.done():
-                test_complete_future.set_exception(
-                    Exception(f"Memory corruption detected: {line}")
-                )
-            return
 
         # Check for completion
-        if "String lifetime tests complete" in line and not test_complete_future.done():
+        if "String lifetime tests complete" in line:
-            test_complete_future.set_result(None)
+            all_tests_complete.set()
 
     async with (
         run_compiled(yaml_config, line_callback=on_log_line),
@@ -93,29 +110,56 @@ async def test_scheduler_string_lifetime(
             client.list_entities_services(), timeout=5.0
         )
 
-        # Find our test service
+        # Find our test services
-        run_test_service: UserService | None = None
+        test_services = {}
         for service in services:
-            if service.name == "run_string_lifetime_test":
+            if service.name == "run_test1":
-                run_test_service = service
+                test_services["test1"] = service
-                break
+            elif service.name == "run_test2":
+                test_services["test2"] = service
+            elif service.name == "run_test3":
+                test_services["test3"] = service
+            elif service.name == "run_test4":
+                test_services["test4"] = service
+            elif service.name == "run_test5":
+                test_services["test5"] = service
+            elif service.name == "run_final_check":
+                test_services["final"] = service
 
-        assert run_test_service is not None, (
+        # Ensure all services are found
-            "run_string_lifetime_test service not found"
+        required_services = ["test1", "test2", "test3", "test4", "test5", "final"]
-        )
+        for service_name in required_services:
+            assert service_name in test_services, f"{service_name} service not found"
 
-        # Call the service to start the test
+        # Run tests sequentially, waiting for each to complete
-        client.execute_service(run_test_service, {})
-
-        # Wait for test to complete
         try:
-            await asyncio.wait_for(test_complete_future, timeout=30.0)
+            # Test 1
+            client.execute_service(test_services["test1"], {})
+            await asyncio.wait_for(test1_complete.wait(), timeout=5.0)
+
+            # Test 2
+            client.execute_service(test_services["test2"], {})
+            await asyncio.wait_for(test2_complete.wait(), timeout=5.0)
+
+            # Test 3
+            client.execute_service(test_services["test3"], {})
+            await asyncio.wait_for(test3_complete.wait(), timeout=5.0)
+
+            # Test 4
+            client.execute_service(test_services["test4"], {})
+            await asyncio.wait_for(test4_complete.wait(), timeout=5.0)
+
+            # Test 5
+            client.execute_service(test_services["test5"], {})
+            await asyncio.wait_for(test5_complete.wait(), timeout=5.0)
+
+            # Final check
+            client.execute_service(test_services["final"], {})
+            await asyncio.wait_for(all_tests_complete.wait(), timeout=5.0)
+
         except asyncio.TimeoutError:
             pytest.fail(f"String lifetime test timed out. Stats: {test_stats}")
 
-        # Check for use-after-free
-        assert not test_stats["use_after_free_detected"], "Use-after-free detected!"
-
         # Check for any errors
         assert test_stats["tests_failed"] == 0, f"Tests failed: {test_stats['errors']}"