Merge branch 'integration' into memory_api

This commit is contained in:
J. Nick Koston
2025-11-05 21:22:24 -06:00
14 changed files with 430 additions and 57 deletions

View File

@@ -261,8 +261,8 @@ bool ESP32BLE::ble_setup_() {
#endif
std::string name;
if (this->name_.has_value()) {
name = this->name_.value();
if (this->name_ != nullptr) {
name = this->name_;
if (App.is_name_add_mac_suffix_enabled()) {
// MAC address suffix length (last 6 characters of 12-char MAC address string)
constexpr size_t mac_address_suffix_len = 6;

View File

@@ -112,7 +112,7 @@ class ESP32BLE : public Component {
void loop() override;
void dump_config() override;
float get_setup_priority() const override;
void set_name(const std::string &name) { this->name_ = name; }
void set_name(const char *name) { this->name_ = name; }
#ifdef USE_ESP32_BLE_ADVERTISING
void advertising_start();
@@ -192,8 +192,8 @@ class ESP32BLE : public Component {
esphome::LockFreeQueue<BLEEvent, MAX_BLE_QUEUE_SIZE> ble_events_;
esphome::EventPool<BLEEvent, MAX_BLE_QUEUE_SIZE> ble_event_pool_;
// optional<string> (typically 16+ bytes on 32-bit, aligned to 4 bytes)
optional<std::string> name_;
// Pointer to compile-time string literal or nullptr (4 bytes on 32-bit)
const char *name_{nullptr};
// 4-byte aligned members
#ifdef USE_ESP32_BLE_ADVERTISING

View File

@@ -98,22 +98,28 @@ template<typename... Ts> class ForCondition : public Condition<Ts...>, public Co
TEMPLATABLE_VALUE(uint32_t, time);
void loop() override { this->check_internal(); }
float get_setup_priority() const override { return setup_priority::DATA; }
bool check_internal() {
bool cond = this->condition_->check();
if (!cond)
this->last_inactive_ = App.get_loop_component_start_time();
return cond;
void loop() override {
// Safe to use cached time - only called from Application::loop()
this->check_internal_(App.get_loop_component_start_time());
}
float get_setup_priority() const override { return setup_priority::DATA; }
bool check(const Ts &...x) override {
if (!this->check_internal())
auto now = millis();
if (!this->check_internal_(now))
return false;
return millis() - this->last_inactive_ >= this->time_.value(x...);
return now - this->last_inactive_ >= this->time_.value(x...);
}
protected:
bool check_internal_(uint32_t now) {
bool cond = this->condition_->check();
if (!cond)
this->last_inactive_ = now;
return cond;
}
Condition<> *condition_;
uint32_t last_inactive_{0};
};
@@ -424,34 +430,17 @@ template<typename... Ts> class WaitUntilAction : public Action<Ts...>, public Co
auto timeout = this->timeout_value_.optional_value(x...);
this->var_queue_.emplace_front(now, timeout, std::make_tuple(x...));
// Enable loop now that we have work to do
this->enable_loop();
this->loop();
// Do immediate check with fresh timestamp
if (this->process_queue_(now)) {
// Only enable loop if we still have pending items
this->enable_loop();
}
}
void loop() override {
if (this->num_running_ == 0)
return;
auto now = App.get_loop_component_start_time();
this->var_queue_.remove_if([&](auto &queued) {
auto start = std::get<uint32_t>(queued);
auto timeout = std::get<optional<uint32_t>>(queued);
auto &var = std::get<std::tuple<Ts...>>(queued);
auto expired = timeout && (now - start) >= *timeout;
if (!expired && !this->condition_->check_tuple(var)) {
return false;
}
this->play_next_tuple_(var);
return true;
});
// If queue is now empty, disable loop until next play_complex
if (this->var_queue_.empty()) {
// Safe to use cached time - only called from Application::loop()
if (this->num_running_ > 0 && !this->process_queue_(App.get_loop_component_start_time())) {
// If queue is now empty, disable loop until next play_complex
this->disable_loop();
}
}
@@ -467,6 +456,31 @@ template<typename... Ts> class WaitUntilAction : public Action<Ts...>, public Co
}
protected:
// Helper: Process queue, triggering completed items and removing them
// Returns true if queue still has pending items
bool process_queue_(uint32_t now) {
// Process each queued wait_until and remove completed ones
this->var_queue_.remove_if([&](auto &queued) {
auto start = std::get<uint32_t>(queued);
auto timeout = std::get<optional<uint32_t>>(queued);
auto &var = std::get<std::tuple<Ts...>>(queued);
// Check if timeout has expired
auto expired = timeout && (now - start) >= *timeout;
// Keep waiting if not expired and condition not met
if (!expired && !this->condition_->check_tuple(var)) {
return false;
}
// Condition met or timed out - trigger next action
this->play_next_tuple_(var);
return true;
});
return !this->var_queue_.empty();
}
Condition<Ts...> *condition_;
std::forward_list<std::tuple<uint32_t, optional<uint32_t>, std::tuple<Ts...>>> var_queue_{};
};

View File

@@ -12,6 +12,7 @@
#include <string>
#include <type_traits>
#include <vector>
#include <concepts>
#include "esphome/core/optional.h"
@@ -1172,7 +1173,20 @@ template<class T> class RAMAllocator {
template<class T> using ExternalRAMAllocator = RAMAllocator<T>;
/// @}
/**
* Functions to constrain the range of arithmetic values.
*/
template<std::totally_ordered T> T clamp_at_least(T value, T min) {
if (value < min)
return min;
return value;
}
template<std::totally_ordered T> T clamp_at_most(T value, T max) {
if (value > max)
return max;
return value;
}
/// @name Internal functions
///@{

View File

@@ -94,6 +94,7 @@ class Platform(StrEnum):
# Memory impact analysis constants
MEMORY_IMPACT_FALLBACK_COMPONENT = "api" # Representative component for core changes
MEMORY_IMPACT_FALLBACK_PLATFORM = Platform.ESP32_IDF # Most representative platform
MEMORY_IMPACT_MAX_COMPONENTS = 40 # Max components before results become nonsensical
# Platform-specific components that can only be built on their respective platforms
# These components contain platform-specific code and cannot be cross-compiled
@@ -555,6 +556,17 @@ def detect_memory_impact_config(
if not components_with_tests:
return {"should_run": "false"}
# Skip memory impact analysis if too many components changed
# Building 40+ components at once produces nonsensical memory impact results
# This typically happens with large refactorings or batch updates
if len(components_with_tests) > MEMORY_IMPACT_MAX_COMPONENTS:
print(
f"Memory impact: Skipping analysis for {len(components_with_tests)} components "
f"(limit is {MEMORY_IMPACT_MAX_COMPONENTS}, would give nonsensical results)",
file=sys.stderr,
)
return {"should_run": "false"}
# Find common platforms supported by ALL components
# This ensures we can build all components together in a merged config
common_platforms = set(MEMORY_IMPACT_PLATFORM_PREFERENCE)

View File

@@ -0,0 +1,13 @@
logger:
level: VERBOSE
esphome:
on_boot:
- lambda: |-
int x = 100;
x = clamp(x, 50, 90);
assert(x == 90);
x = clamp_at_least(x, 95);
assert(x == 95);
x = clamp_at_most(x, 40);
assert(x == 40);

View File

@@ -0,0 +1,13 @@
logger:
level: VERBOSE
esphome:
on_boot:
- lambda: |-
int x = 100;
x = clamp(x, 50, 90);
assert(x == 90);
x = clamp_at_least(x, 95);
assert(x == 95);
x = clamp_at_most(x, 40);
assert(x == 40);

View File

@@ -15,3 +15,10 @@ esphome:
static const uint8_t my_addr[6] = {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
if (!mac_address_is_valid(my_addr))
ESP_LOGD("test", "Invalid mac address %X", my_addr[0]); // etc.
int x = 100;
x = clamp(x, 50, 90);
assert(x == 90);
x = clamp_at_least(x, 95);
assert(x == 95);
x = clamp_at_most(x, 40);
assert(x == 40);

View File

@@ -0,0 +1,13 @@
logger:
level: VERBOSE
esphome:
on_boot:
- lambda: |-
int x = 100;
x = clamp(x, 50, 90);
assert(x == 90);
x = clamp_at_least(x, 95);
assert(x == 95);
x = clamp_at_most(x, 40);
assert(x == 40);

View File

@@ -1,3 +1,13 @@
esphome:
on_boot:
- lambda: |-
int x = 100;
x = clamp(x, 50, 90);
assert(x == 90);
x = clamp_at_least(x, 95);
assert(x == 95);
x = clamp_at_most(x, 40);
assert(x == 40);
nrf52:
dfu:
reset_pin:

View File

@@ -0,0 +1,13 @@
logger:
level: VERBOSE
esphome:
on_boot:
- lambda: |-
int x = 100;
x = clamp(x, 50, 90);
assert(x == 90);
x = clamp_at_least(x, 95);
assert(x == 95);
x = clamp_at_most(x, 40);
assert(x == 40);

View File

@@ -0,0 +1,109 @@
# Test for PR #11676 bug: wait_until timeout when triggered mid-component-loop
# This demonstrates that App.get_loop_component_start_time() is stale when
# wait_until is triggered partway through a component's loop execution
esphome:
name: wait-mid-loop
host:
api:
actions:
- action: test_mid_loop_timeout
then:
- logger.log: "=== Test: wait_until triggered mid-loop should timeout correctly ==="
# Reset test state
- globals.set:
id: test_complete
value: 'false'
# Trigger the slow script that will call wait_until mid-execution
- script.execute: slow_script
# Wait for test to complete (should take ~300ms: 100ms delay + 200ms timeout)
- wait_until:
condition:
lambda: return id(test_complete);
timeout: 2s
- if:
condition:
lambda: return id(test_complete);
then:
- logger.log: "✓ Test PASSED: wait_until timed out correctly"
else:
- logger.log: "✗ Test FAILED: wait_until did not complete properly"
logger:
level: DEBUG
globals:
- id: test_complete
type: bool
restore_value: false
initial_value: 'false'
- id: test_condition
type: bool
restore_value: false
initial_value: 'false'
- id: timeout_start_time
type: uint32_t
restore_value: false
initial_value: '0'
- id: timeout_end_time
type: uint32_t
restore_value: false
initial_value: '0'
script:
# This script simulates a component that takes time during its execution
# When wait_until is triggered mid-script, the loop_component_start_time
# will be stale (from when the script's component loop started)
- id: slow_script
then:
- logger.log: "Script: Starting, about to do some work..."
# Simulate component doing work for 100ms
# This represents time spent in a component's loop() before triggering wait_until
- delay: 100ms
- logger.log: "Script: 100ms elapsed, now starting wait_until with 200ms timeout"
- lambda: |-
// Record when timeout starts
id(timeout_start_time) = millis();
id(test_condition) = false;
# At this point:
# - Script component's loop started 100ms ago
# - App.loop_component_start_time_ = time from 100ms ago (stale!)
# - wait_until will capture millis() NOW (fresh)
# - BUG: loop() will use stale loop_component_start_time, causing immediate timeout
- wait_until:
condition:
lambda: return id(test_condition);
timeout: 200ms
- lambda: |-
// Record when timeout completes
id(timeout_end_time) = millis();
uint32_t elapsed = id(timeout_end_time) - id(timeout_start_time);
ESP_LOGD("TEST", "wait_until completed after %u ms (expected ~200ms)", elapsed);
// Check if timeout took approximately correct time
// Should be ~200ms, not <50ms (immediate timeout)
if (elapsed >= 150 && elapsed <= 250) {
ESP_LOGD("TEST", "✓ Timeout duration correct: %u ms", elapsed);
id(test_complete) = true;
} else {
ESP_LOGE("TEST", "✗ Timeout duration WRONG: %u ms (expected 150-250ms)", elapsed);
if (elapsed < 50) {
ESP_LOGE("TEST", " → Likely BUG: Immediate timeout due to stale loop_component_start_time");
}
id(test_complete) = false;
}

View File

@@ -0,0 +1,112 @@
"""Integration test for PR #11676 mid-loop timing bug.
This test validates that wait_until timeouts work correctly when triggered
mid-component-loop, where App.get_loop_component_start_time() is stale.
The bug: When wait_until is triggered partway through a component's loop execution
(e.g., from a script or automation), the cached loop_component_start_time_ is stale
relative to when the action was actually triggered. This causes timeout calculations
to underflow and timeout immediately instead of waiting the specified duration.
"""
from __future__ import annotations
import asyncio
import re
import pytest
from .types import APIClientConnectedFactory, RunCompiledFunction
@pytest.mark.asyncio
async def test_wait_until_mid_loop_timing(
yaml_config: str,
run_compiled: RunCompiledFunction,
api_client_connected: APIClientConnectedFactory,
) -> None:
"""Test that wait_until timeout works when triggered mid-component-loop.
This test:
1. Executes a script that delays 100ms (simulating component work)
2. Then starts wait_until with 200ms timeout
3. Verifies timeout takes ~200ms, not <50ms (immediate timeout bug)
"""
loop = asyncio.get_running_loop()
# Track test results
test_results = {
"timeout_duration": None,
"passed": False,
"failed": False,
"bug_detected": False,
}
# Patterns for log messages
timeout_duration = re.compile(r"wait_until completed after (\d+) ms")
test_pass = re.compile(r"✓ Timeout duration correct")
test_fail = re.compile(r"✗ Timeout duration WRONG")
bug_pattern = re.compile(r"Likely BUG: Immediate timeout")
test_passed = re.compile(r"✓ Test PASSED")
test_failed = re.compile(r"✗ Test FAILED")
test_complete = loop.create_future()
def check_output(line: str) -> None:
"""Check log output for test results."""
# Extract timeout duration
match = timeout_duration.search(line)
if match:
test_results["timeout_duration"] = int(match.group(1))
if test_pass.search(line):
test_results["passed"] = True
if test_fail.search(line):
test_results["failed"] = True
if bug_pattern.search(line):
test_results["bug_detected"] = True
# Final test result
if (
test_passed.search(line)
or test_failed.search(line)
and not test_complete.done()
):
test_complete.set_result(True)
async with (
run_compiled(yaml_config, line_callback=check_output),
api_client_connected() as client,
):
# Get the test service
_, services = await client.list_entities_services()
test_service = next(
(s for s in services if s.name == "test_mid_loop_timeout"), None
)
assert test_service is not None, "test_mid_loop_timeout service not found"
# Execute the test
client.execute_service(test_service, {})
# Wait for test to complete (100ms delay + 200ms timeout + margins = ~500ms)
await asyncio.wait_for(test_complete, timeout=5.0)
# Verify results
assert test_results["timeout_duration"] is not None, (
"Timeout duration not reported"
)
assert test_results["passed"], (
f"Test failed: wait_until took {test_results['timeout_duration']}ms, expected ~200ms. "
f"Bug detected: {test_results['bug_detected']}"
)
assert not test_results["bug_detected"], (
f"BUG DETECTED: wait_until timed out immediately ({test_results['timeout_duration']}ms) "
"instead of waiting 200ms. This indicates stale loop_component_start_time."
)
# Additional validation: timeout should be ~200ms (150-250ms range)
duration = test_results["timeout_duration"]
assert 150 <= duration <= 250, (
f"Timeout duration {duration}ms outside expected range (150-250ms). "
f"This suggests timing regression from PR #11676."
)

View File

@@ -71,9 +71,10 @@ def mock_changed_files() -> Generator[Mock, None, None]:
@pytest.fixture(autouse=True)
def clear_clang_tidy_cache() -> None:
"""Clear the clang-tidy full scan cache before each test."""
def clear_determine_jobs_caches() -> None:
"""Clear all cached functions before each test."""
determine_jobs._is_clang_tidy_full_scan.cache_clear()
determine_jobs._component_has_tests.cache_clear()
def test_main_all_tests_should_run(
@@ -565,7 +566,6 @@ def test_main_filters_components_without_tests(
patch.object(determine_jobs, "changed_files", return_value=[]),
):
# Clear the cache since we're mocking root_path
determine_jobs._component_has_tests.cache_clear()
determine_jobs.main()
# Check output
@@ -665,7 +665,6 @@ def test_main_detects_components_with_variant_tests(
patch.object(determine_jobs, "changed_files", return_value=[]),
):
# Clear the cache since we're mocking root_path
determine_jobs._component_has_tests.cache_clear()
determine_jobs.main()
# Check output
@@ -714,7 +713,6 @@ def test_detect_memory_impact_config_with_common_platform(tmp_path: Path) -> Non
"esphome/components/wifi/wifi.cpp",
"esphome/components/api/api.cpp",
]
determine_jobs._component_has_tests.cache_clear()
result = determine_jobs.detect_memory_impact_config()
@@ -744,7 +742,6 @@ def test_detect_memory_impact_config_core_only_changes(tmp_path: Path) -> None:
"esphome/core/application.cpp",
"esphome/core/component.h",
]
determine_jobs._component_has_tests.cache_clear()
result = determine_jobs.detect_memory_impact_config()
@@ -775,7 +772,6 @@ def test_detect_memory_impact_config_core_python_only_changes(tmp_path: Path) ->
"esphome/config.py",
"esphome/core/config.py",
]
determine_jobs._component_has_tests.cache_clear()
result = determine_jobs.detect_memory_impact_config()
@@ -808,7 +804,6 @@ def test_detect_memory_impact_config_no_common_platform(tmp_path: Path) -> None:
"esphome/components/wifi/wifi.cpp",
"esphome/components/logger/logger.cpp",
]
determine_jobs._component_has_tests.cache_clear()
result = determine_jobs.detect_memory_impact_config()
@@ -830,7 +825,6 @@ def test_detect_memory_impact_config_no_changes(tmp_path: Path) -> None:
patch.object(determine_jobs, "changed_files") as mock_changed_files,
):
mock_changed_files.return_value = []
determine_jobs._component_has_tests.cache_clear()
result = determine_jobs.detect_memory_impact_config()
@@ -855,7 +849,6 @@ def test_detect_memory_impact_config_no_components_with_tests(tmp_path: Path) ->
mock_changed_files.return_value = [
"esphome/components/my_custom_component/component.cpp",
]
determine_jobs._component_has_tests.cache_clear()
result = determine_jobs.detect_memory_impact_config()
@@ -895,7 +888,6 @@ def test_detect_memory_impact_config_includes_base_bus_components(
"esphome/components/uart/automation.h", # Header file with inline code
"esphome/components/wifi/wifi.cpp",
]
determine_jobs._component_has_tests.cache_clear()
result = determine_jobs.detect_memory_impact_config()
@@ -938,7 +930,6 @@ def test_detect_memory_impact_config_with_variant_tests(tmp_path: Path) -> None:
"esphome/components/improv_serial/improv_serial.cpp",
"esphome/components/ethernet/ethernet.cpp",
]
determine_jobs._component_has_tests.cache_clear()
result = determine_jobs.detect_memory_impact_config()
@@ -1168,7 +1159,6 @@ def test_detect_memory_impact_config_filters_incompatible_esp32_on_esp8266(
"tests/components/esp8266/test.esp8266-ard.yaml",
"esphome/core/helpers_esp8266.h", # ESP8266-specific file to hint platform
]
determine_jobs._component_has_tests.cache_clear()
result = determine_jobs.detect_memory_impact_config()
@@ -1222,7 +1212,6 @@ def test_detect_memory_impact_config_filters_incompatible_esp8266_on_esp32(
"esphome/components/wifi/wifi_component_esp_idf.cpp", # ESP-IDF hint
"esphome/components/ethernet/ethernet_esp32.cpp", # ESP32 hint
]
determine_jobs._component_has_tests.cache_clear()
result = determine_jobs.detect_memory_impact_config()
@@ -1257,7 +1246,6 @@ def test_detect_memory_impact_config_skips_release_branch(tmp_path: Path) -> Non
patch.object(determine_jobs, "get_target_branch", return_value="release"),
):
mock_changed_files.return_value = ["esphome/components/wifi/wifi.cpp"]
determine_jobs._component_has_tests.cache_clear()
result = determine_jobs.detect_memory_impact_config()
@@ -1280,7 +1268,6 @@ def test_detect_memory_impact_config_skips_beta_branch(tmp_path: Path) -> None:
patch.object(determine_jobs, "get_target_branch", return_value="beta"),
):
mock_changed_files.return_value = ["esphome/components/wifi/wifi.cpp"]
determine_jobs._component_has_tests.cache_clear()
result = determine_jobs.detect_memory_impact_config()
@@ -1303,10 +1290,66 @@ def test_detect_memory_impact_config_runs_for_dev_branch(tmp_path: Path) -> None
patch.object(determine_jobs, "get_target_branch", return_value="dev"),
):
mock_changed_files.return_value = ["esphome/components/wifi/wifi.cpp"]
determine_jobs._component_has_tests.cache_clear()
result = determine_jobs.detect_memory_impact_config()
# Memory impact should run for dev branch
assert result["should_run"] == "true"
assert result["components"] == ["wifi"]
def test_detect_memory_impact_config_skips_too_many_components(
tmp_path: Path,
) -> None:
"""Test that memory impact analysis is skipped when more than 40 components changed."""
# Create test directory structure with 41 components
tests_dir = tmp_path / "tests" / "components"
component_names = [f"component_{i}" for i in range(41)]
for component_name in component_names:
comp_dir = tests_dir / component_name
comp_dir.mkdir(parents=True)
(comp_dir / "test.esp32-idf.yaml").write_text(f"test: {component_name}")
with (
patch.object(determine_jobs, "root_path", str(tmp_path)),
patch.object(helpers, "root_path", str(tmp_path)),
patch.object(determine_jobs, "changed_files") as mock_changed_files,
patch.object(determine_jobs, "get_target_branch", return_value="dev"),
):
mock_changed_files.return_value = [
f"esphome/components/{name}/{name}.cpp" for name in component_names
]
result = determine_jobs.detect_memory_impact_config()
# Memory impact should be skipped for too many components (41 > 40)
assert result["should_run"] == "false"
def test_detect_memory_impact_config_runs_at_component_limit(tmp_path: Path) -> None:
"""Test that memory impact analysis runs with exactly 40 components (at limit)."""
# Create test directory structure with exactly 40 components
tests_dir = tmp_path / "tests" / "components"
component_names = [f"component_{i}" for i in range(40)]
for component_name in component_names:
comp_dir = tests_dir / component_name
comp_dir.mkdir(parents=True)
(comp_dir / "test.esp32-idf.yaml").write_text(f"test: {component_name}")
with (
patch.object(determine_jobs, "root_path", str(tmp_path)),
patch.object(helpers, "root_path", str(tmp_path)),
patch.object(determine_jobs, "changed_files") as mock_changed_files,
patch.object(determine_jobs, "get_target_branch", return_value="dev"),
):
mock_changed_files.return_value = [
f"esphome/components/{name}/{name}.cpp" for name in component_names
]
result = determine_jobs.detect_memory_impact_config()
# Memory impact should run at exactly 40 components (at limit but not over)
assert result["should_run"] == "true"
assert len(result["components"]) == 40