mirror of
https://github.com/esphome/esphome.git
synced 2026-05-24 18:06:27 +08:00
[core] coerce set_interval(0) / update_interval: 0ms to 1ms (#15799)
This commit is contained in:
committed by
Jesse Hills
parent
f5806818cd
commit
b26601a3dc
@@ -943,7 +943,26 @@ def time_period_in_minutes_(value):
|
||||
def update_interval(value):
|
||||
if value == "never":
|
||||
return TimePeriodMilliseconds(milliseconds=SCHEDULER_DONT_RUN)
|
||||
return positive_time_period_milliseconds(value)
|
||||
result = positive_time_period_milliseconds(value)
|
||||
# 0ms was historically (mis)used as a pseudo-loop() mechanism for
|
||||
# PollingComponents. Under the hood it calls set_interval(0), which
|
||||
# causes Scheduler::call() to spin (WDT reset in the field). Coerce
|
||||
# to 1ms so existing configs keep working at ~1kHz instead of
|
||||
# spinning. Don't hard-fail so configs don't break on upgrade;
|
||||
# authors should migrate to HighFrequencyLoopRequester (C++) for
|
||||
# true run-every-loop behaviour.
|
||||
if result.total_milliseconds == 0:
|
||||
_LOGGER.warning(
|
||||
"update_interval of 0ms is not supported - coercing to 1ms. "
|
||||
"A literal 0ms schedule would spin the main loop (the scheduled "
|
||||
"item would always be due, so the scheduler would never yield "
|
||||
"back) and trigger a watchdog reset. Set update_interval to a "
|
||||
"non-zero value such as 1ms or higher. (Custom C++ components "
|
||||
"that need true run-every-loop behaviour should override loop() "
|
||||
"with HighFrequencyLoopRequester instead.)"
|
||||
)
|
||||
return TimePeriodMilliseconds(milliseconds=1)
|
||||
return result
|
||||
|
||||
|
||||
time_period = Any(time_period_str_unit, time_period_str_colon, time_period_dict)
|
||||
|
||||
@@ -144,6 +144,19 @@ void HOT Scheduler::set_timer_common_(Component *component, SchedulerItem::Type
|
||||
return;
|
||||
}
|
||||
|
||||
// An interval of 0 means "fire every tick forever," which is misuse: the
|
||||
// item would always be due, causing Scheduler::call() to spin and starve
|
||||
// the main loop (WDT reset in the field). Coerce to 1ms so existing code
|
||||
// using update_interval=0ms as a pseudo-loop() continues to work at ~1kHz,
|
||||
// and warn so authors can migrate to HighFrequencyLoopRequester which is
|
||||
// the intended mechanism for running fast in the main loop. Zero-delay
|
||||
// timeouts (defer) remain legitimate one-shots and are not affected.
|
||||
if (type == SchedulerItem::INTERVAL && delay == 0) [[unlikely]] {
|
||||
ESP_LOGE(TAG, "[%s] set_interval(0) would spin main loop - coercing to 1ms (use HighFrequencyLoopRequester)",
|
||||
component ? LOG_STR_ARG(component->get_component_log_str()) : LOG_STR_LITERAL("?"));
|
||||
delay = 1;
|
||||
}
|
||||
|
||||
// Take lock early to protect scheduler_item_pool_ access and retry-cancelled check
|
||||
LockGuard guard{this->lock_};
|
||||
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
esphome:
|
||||
name: sched-interval-zero
|
||||
|
||||
host:
|
||||
api:
|
||||
logger:
|
||||
level: DEBUG
|
||||
|
||||
globals:
|
||||
- id: fire_count
|
||||
type: int
|
||||
initial_value: "0"
|
||||
|
||||
interval:
|
||||
# Deliberately configure 0ms — this path goes through the C++
|
||||
# Scheduler::set_timer_common_ coercion (not the Python cv.update_interval
|
||||
# path, since interval: doesn't call cv.update_interval — it's an intervals
|
||||
# component schema, not a PollingComponent's update_interval).
|
||||
# Expected: scheduler coerces to 1ms at registration, emits ESP_LOGE,
|
||||
# fires at ~1kHz instead of spinning.
|
||||
- interval: 0ms
|
||||
then:
|
||||
- lambda: |-
|
||||
id(fire_count) += 1;
|
||||
if (id(fire_count) == 50) {
|
||||
ESP_LOGI("test", "ZERO_INTERVAL_50_FIRES_REACHED");
|
||||
}
|
||||
@@ -0,0 +1,67 @@
|
||||
"""Test that Scheduler::set_timer_common_ coerces interval=0 to 1ms.
|
||||
|
||||
Regression test for the scheduler busy-loop when interval=0 was passed
|
||||
literally. Without the coercion, Scheduler::call() would spin forever
|
||||
because the item's next_execution == now_64 after re-scheduling, failing
|
||||
the loop's `> now_64` break condition. The device would fail to yield
|
||||
back to the main loop and trigger a WDT reset.
|
||||
|
||||
With the coercion, interval=0 becomes interval=1 and the scheduler
|
||||
fires at ~1kHz (bounded by the loop), the main loop continues to run,
|
||||
and the device stays responsive to API calls.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
|
||||
import pytest
|
||||
|
||||
from .types import APIClientConnectedFactory, RunCompiledFunction
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_scheduler_interval_zero_coerced(
|
||||
yaml_config: str,
|
||||
run_compiled: RunCompiledFunction,
|
||||
api_client_connected: APIClientConnectedFactory,
|
||||
) -> None:
|
||||
"""interval=0ms must be coerced to 1ms and not starve the main loop."""
|
||||
loop = asyncio.get_running_loop()
|
||||
reached_50: asyncio.Future[None] = loop.create_future()
|
||||
coerce_warning: asyncio.Future[None] = loop.create_future()
|
||||
|
||||
def on_log_line(line: str) -> None:
|
||||
if "ZERO_INTERVAL_50_FIRES_REACHED" in line and not reached_50.done():
|
||||
reached_50.set_result(None)
|
||||
if "would spin main loop" in line and not coerce_warning.done():
|
||||
coerce_warning.set_result(None)
|
||||
|
||||
async with (
|
||||
run_compiled(yaml_config, line_callback=on_log_line),
|
||||
api_client_connected() as client,
|
||||
):
|
||||
# The API-client connection itself is evidence that the main loop
|
||||
# is not starved — if set_interval(0) were spinning we could not
|
||||
# get here at all.
|
||||
device_info = await client.device_info()
|
||||
assert device_info is not None
|
||||
assert device_info.name == "sched-interval-zero"
|
||||
|
||||
# Coerce warning must fire at registration
|
||||
try:
|
||||
await asyncio.wait_for(coerce_warning, timeout=5.0)
|
||||
except TimeoutError:
|
||||
pytest.fail("Expected coerce warning 'would spin main loop' not seen")
|
||||
|
||||
# The coerced 1ms interval should fire 50 times quickly — this
|
||||
# confirms the callback actually runs (not just registered) and the
|
||||
# scheduler yields back to the main loop each time.
|
||||
try:
|
||||
await asyncio.wait_for(reached_50, timeout=5.0)
|
||||
except TimeoutError:
|
||||
pytest.fail(
|
||||
"Coerced interval=0→1ms did not reach 50 fires within 5s, "
|
||||
"which would indicate either the coercion failed or the "
|
||||
"main loop is still being starved."
|
||||
)
|
||||
@@ -24,6 +24,7 @@ from esphome.const import (
|
||||
PLATFORM_LN882X,
|
||||
PLATFORM_RP2040,
|
||||
PLATFORM_RTL87XX,
|
||||
SCHEDULER_DONT_RUN,
|
||||
)
|
||||
from esphome.core import CORE, HexInt, Lambda
|
||||
|
||||
@@ -765,3 +766,30 @@ def test_percentage_validators__raw_number_above_one_without_percent_sign(
|
||||
config_validation.unbounded_percentage(value)
|
||||
with pytest.raises(Invalid, match="percent sign"):
|
||||
config_validation.unbounded_possibly_negative_percentage(value)
|
||||
|
||||
|
||||
def test_update_interval__coerces_zero_to_one_ms(
|
||||
caplog: pytest.LogCaptureFixture,
|
||||
) -> None:
|
||||
"""update_interval: 0ms must be coerced to 1ms (not rejected) because a
|
||||
literal 0ms schedule causes Scheduler::call() to spin. Coercion keeps
|
||||
existing configs compiling on upgrade while emitting a user-facing
|
||||
warning that directs them to set a non-zero value."""
|
||||
with caplog.at_level("WARNING"):
|
||||
result = config_validation.update_interval("0ms")
|
||||
assert result.total_milliseconds == 1
|
||||
assert "update_interval of 0ms is not supported" in caplog.text
|
||||
assert "1ms" in caplog.text
|
||||
|
||||
|
||||
def test_update_interval__preserves_nonzero_values() -> None:
|
||||
"""Non-zero update_interval values must pass through unchanged."""
|
||||
assert config_validation.update_interval("1ms").total_milliseconds == 1
|
||||
assert config_validation.update_interval("50ms").total_milliseconds == 50
|
||||
assert config_validation.update_interval("60s").total_milliseconds == 60000
|
||||
|
||||
|
||||
def test_update_interval__never_passes_through() -> None:
|
||||
"""update_interval: never must still map to SCHEDULER_DONT_RUN."""
|
||||
result = config_validation.update_interval("never")
|
||||
assert result.total_milliseconds == SCHEDULER_DONT_RUN
|
||||
|
||||
Reference in New Issue
Block a user