Skip to content

[script] Fix script.wait hanging when triggered from on_boot #12102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
bdraco merged 1 commit into from
Nov 25, 2025
Merged

[script] Fix script.wait hanging when triggered from on_boot #12102

bdraco merged 1 commit into from
Nov 25, 2025

Conversation

@bdraco
Copy link
Member

@bdraco bdraco commented Nov 25, 2025

What does this implement/fix?

Applies the same fix from #11869 to ScriptWaitAction.

PR #11869 fixed WaitUntilAction hanging when triggered from on_boot, but the identical bug existed in ScriptWaitAction. The root cause is the same: setup() unconditionally disabled the loop, even if play_complex() had already been called and enabled it.

The fix adds the same conditional check to only disable the loop in setup() if num_running_ is 0.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code quality improvements to existing code or addition of tests
  • Other

Related issue or feature (if applicable):

Pull request in esphome-docs with documentation (if applicable):

  • N/A (no documentation changes needed)

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx
  • nRF52840
  • Host

Example entry for config.yaml:

esphome:
  name: test-device
  on_boot:
    then:
      - script.execute: show_start_page
      - script.wait: show_start_page
      - logger.log: "First script completed"

script:
  - id: show_start_page
    mode: single
    then:
      - logger.log: "Starting..."
      - delay: 100ms
      - logger.log: "Done"

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

  • Documentation added/updated in esphome-docs. (N/A - no user-facing changes)

Copilot AI review requested due to automatic review settings November 25, 2025 15:44
@bdraco bdraco requested a review from a team as a code owner November 25, 2025 15:44
@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2025

To use the changes from this PR as an external component, add the following to your ESPHome configuration YAML file:

external_components:
  - source: github://pr#12102
    components: [script]
    refresh: 1h

(Added by the PR bot)

@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2025

Memory Impact Analysis

Components: script
Platform: esp8266-ard

Metric Target Branch This PR Change
RAM 28,984 bytes 28,984 bytes ➡️ +0 bytes (0.00%)
Flash 279,095 bytes 279,111 bytes 📈 🔸 +16 bytes (+0.01%)
📊 Component Memory Breakdown
Component Target Flash PR Flash Change
[esphome]script 2,045 bytes 2,051 bytes 📈 🔸 +6 bytes (+0.29%)
[esphome]core 10,050 bytes 10,045 bytes 📉 ✅ -5 bytes (-0.05%)
🔍 Symbol-Level Changes (click to expand)

Changed Symbols

Symbol Target Size PR Size Change
esphome::script::ScriptWaitAction<esphome::script::SingleScript<>>::setup() 8 bytes 14 bytes 📈 +6 bytes (+75.00%)
void std::_Tuple_impl<1u, esphome::TemplatableValue, esphome::TemplatableValue<boo...void std::_Tuple_impl<1u, esphome::TemplatableValue, esphome::TemplatableValue >::_M_assign<esphome::TemplatableValue, esphome::TemplatableValue >(std::_Tuple_impl<1u, esphome::TemplatableValue, esphome::TemplatableValue >&&)
 140 bytes 136 bytes 📉 -4 bytes (-2.86%)
esphome::ActionList<std::__cxx11::basic_string<char, std::char_traits, std::allocator...esphome::ActionList<std::__cxx11::basic_string<char, std::char_traits, std::allocator >, unsigned char, bool>::play(std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&, unsigned char const&, bool const&)
 14 bytes 13 bytes 📉 -1 bytes (-7.14%)

Note: This analysis measures static RAM and Flash usage only (compile-time allocation).
Dynamic memory (heap) cannot be measured automatically.
⚠️ You must test this PR on a real device to measure free heap and ensure no runtime memory issues.

This analysis runs automatically when components change. Memory usage is measured from a representative test configuration.

@codecov-commenter
Copy link

codecov-commenter commented Nov 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.50%. Comparing base (c30b920) to head (2711dd9).

Additional details and impacted files 
@@            Coverage Diff             @@
##              dev   #12102      +/-   ##
==========================================
+ Coverage   72.47%   72.50%   +0.02%     
==========================================
  Files          53       53              
  Lines       11159    11159              
  Branches     1513     1513              
==========================================
+ Hits         8088     8091       +3     
+ Misses       2676     2674       -2     
+ Partials      395      394       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI reviewed Nov 25, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a race condition bug in ScriptWaitAction where script.wait would hang forever when triggered from on_boot. The issue is identical to the one previously fixed for WaitUntilAction in PR #11869.

Key Changes:

  • Added conditional check in ScriptWaitAction::setup() to only disable the loop if num_running_ is 0
  • Added comprehensive integration test to verify the fix works correctly
  • Test reproduces the exact scenario where the bug manifests (on_boot triggering script.wait at the same priority level)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
esphome/components/script/script.h Fixed race condition in ScriptWaitAction::setup() by checking num_running_ before disabling loop
tests/integration/test_script_wait_on_boot.py Added comprehensive integration test verifying script.wait works correctly when triggered from on_boot
tests/integration/fixtures/script_wait_on_boot.yaml Test fixture defining two scripts with delays to reproduce the hanging bug scenario

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@swoboda1337 swoboda1337 added this to the 2025.11.2 milestone Nov 25, 2025
@bdraco
Copy link
Member Author

bdraco commented Nov 25, 2025

thanks

@bdraco bdraco merged commit a571033 into dev Nov 25, 2025
42 checks passed
@bdraco bdraco deleted the script_wait_on_boot branch November 25, 2025 16:30
@github-actions github-actions bot locked and limited conversation to collaborators Nov 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@swoboda1337 swoboda1337 swoboda1337 approved these changes

Assignees

No one assigned

Labels

bugfix cherry-picked component: script has-tests

Projects

None yet

Milestone

2025.11.2

Development

Successfully merging this pull request may close these issues.

script.wait never finishes when called from on_boot in ESPHome 2025.11.0/2025.11.1

4 participants

@bdraco @codecov-commenter @swoboda1337