diff --git a/esphome/components/scd4x/scd4x.cpp b/esphome/components/scd4x/scd4x.cpp index 4a700b70c2..06db70e3f3 100644 --- a/esphome/components/scd4x/scd4x.cpp +++ b/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 = (uint16_t) pressure_in_hpa; + uint16_t new_ambient_pressure = static_cast(pressure_in_hpa); if (!this->initialized_) { this->ambient_pressure_ = new_ambient_pressure; return; diff --git a/esphome/components/scd4x/scd4x.h b/esphome/components/scd4x/scd4x.h index 237d226107..ab5d72aeec 100644 --- a/esphome/components/scd4x/scd4x.h +++ b/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}; + sensor::Sensor *ambient_pressure_source_{nullptr}; // used for compensation + 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 diff --git a/esphome/components/sx127x/sx127x.cpp b/esphome/components/sx127x/sx127x.cpp index 7f62ee2bd3..2d2326549b 100644 --- a/esphome/components/sx127x/sx127x.cpp +++ b/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 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 packet; uint8_t payload_length = this->payload_length_; if (payload_length == 0) { payload_length = this->read_register_(REG_FIFO); } - packet.resize(payload_length); - this->read_fifo_(packet); - this->call_listeners_(packet, 0.0f, 0.0f); + this->packet_.resize(payload_length); + this->read_fifo_(this->packet_); + 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" - " Shaping: %s", + " PA Ramp: %" PRIu16 " us", 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_)); diff --git a/esphome/components/sx127x/sx127x.h b/esphome/components/sx127x/sx127x.h index 4cc7c9b6d3..0600b51201 100644 --- a/esphome/components/sx127x/sx127x.h +++ b/esphome/components/sx127x/sx127x.h @@ -96,6 +96,7 @@ class SX127x : public Component, uint8_t read_register_(uint8_t reg); Trigger, float, float> *packet_trigger_{new Trigger, float, float>()}; std::vector listeners_; + std::vector packet_; std::vector sync_value_; InternalGPIOPin *dio0_pin_{nullptr}; InternalGPIOPin *rst_pin_{nullptr}; diff --git a/esphome/core/lock_free_queue.h b/esphome/core/lock_free_queue.h index df38ad9148..f35cfa5af9 100644 --- a/esphome/core/lock_free_queue.h +++ b/esphome/core/lock_free_queue.h @@ -122,19 +122,17 @@ template 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 (was_empty) { - // Queue was empty - consumer might be going to sleep, must notify - xTaskNotifyGive(task_to_notify_); - } else if (this->head_.load(std::memory_order_acquire) == old_tail) { - // Consumer just caught up to where tail was - might go to sleep, must notify - // Note: There's a benign race here - between reading head and calling - // xTaskNotifyGive(), the consumer could advance further. This would result - // in an unnecessary wake-up, but is harmless and extremely rare in practice. - xTaskNotifyGive(task_to_notify_); - } - // Otherwise: consumer is still behind, no need to notify + if (result && task_to_notify_ != nullptr && + (was_empty || this->head_.load(std::memory_order_acquire) == old_tail)) { + // Notify in two cases: + // 1. Queue was empty - consumer might be going to sleep + // 2. Consumer just caught up to where tail was - might go to sleep + // Note: There's a benign race in case 2 - between reading head and calling + // xTaskNotifyGive(), the consumer could advance further. This would result + // in an unnecessary wake-up, but is harmless and extremely rare in practice. + xTaskNotifyGive(task_to_notify_); } + // Otherwise: consumer is still behind, no need to notify return result; } diff --git a/tests/integration/fixtures/external_components/scheduler_string_lifetime_component/string_lifetime_component.cpp b/tests/integration/fixtures/external_components/scheduler_string_lifetime_component/string_lifetime_component.cpp index 7a3561c6f6..5464772f2c 100644 --- a/tests/integration/fixtures/external_components/scheduler_string_lifetime_component/string_lifetime_component.cpp +++ b/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"); diff --git a/tests/integration/fixtures/external_components/scheduler_string_lifetime_component/string_lifetime_component.h b/tests/integration/fixtures/external_components/scheduler_string_lifetime_component/string_lifetime_component.h index 4fe462cea6..95532328bb 100644 --- a/tests/integration/fixtures/external_components/scheduler_string_lifetime_component/string_lifetime_component.h +++ b/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(); diff --git a/tests/integration/fixtures/scheduler_string_lifetime.yaml b/tests/integration/fixtures/scheduler_string_lifetime.yaml index a16f46f144..ebd5052b8b 100644 --- a/tests/integration/fixtures/scheduler_string_lifetime.yaml +++ b/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(); diff --git a/tests/integration/test_scheduler_string_lifetime.py b/tests/integration/test_scheduler_string_lifetime.py index 78f4e2486c..4d77abd954 100644 --- a/tests/integration/test_scheduler_string_lifetime.py +++ b/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 - loop = asyncio.get_running_loop() - test_complete_future: asyncio.Future[None] = loop.create_future() + # Create events for synchronization + test1_complete = asyncio.Event() + 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 - if not test_complete_future.done(): - test_complete_future.set_exception( - Exception(f"Memory corruption detected: {line}") - ) - return + pytest.fail(f"Memory corruption detected: {line}") # Check for completion - if "String lifetime tests complete" in line and not test_complete_future.done(): - test_complete_future.set_result(None) + if "String lifetime tests complete" in line: + 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 - run_test_service: UserService | None = None + # Find our test services + test_services = {} for service in services: - if service.name == "run_string_lifetime_test": - run_test_service = service - break + if service.name == "run_test1": + test_services["test1"] = service + 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, ( - "run_string_lifetime_test service not found" - ) + # Ensure all services are 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 - client.execute_service(run_test_service, {}) - - # Wait for test to complete + # Run tests sequentially, waiting for each 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']}"