diff --git a/esphome/components/http_request/__init__.py b/esphome/components/http_request/__init__.py index 2abf097aec..de35d52a40 100644 --- a/esphome/components/http_request/__init__.py +++ b/esphome/components/http_request/__init__.py @@ -17,12 +17,14 @@ from esphome.const import ( CONF_TIMEOUT, CONF_URL, CONF_WATCHDOG_TIMEOUT, + PLATFORM_ESP32, PLATFORM_HOST, PlatformFramework, __version__, ) -from esphome.core import CORE, ID, Lambda +from esphome.core import CORE, ID, Lambda, TimePeriodMilliseconds from esphome.cpp_generator import MockObj, TemplateArgsType +import esphome.final_validate as fv from esphome.helpers import IS_MACOS from esphome.types import ConfigType @@ -94,6 +96,34 @@ def validate_ssl_verification(config: ConfigType) -> ConfigType: return config +# esp_http_client_open() runs DNS, TCP connect and the TLS handshake with no +# watchdog feed in between; each can take up to `timeout` on ESP-IDF. +WATCHDOG_TIMEOUT_MULTIPLIER = 3 +# Headroom over the exact worst case so a fully stalled open does not land on +# the watchdog deadline. +WATCHDOG_TIMEOUT_MARGIN_MS = 1000 + + +def default_watchdog_timeout(config: ConfigType) -> None: + """Arm the request watchdog on ESP32 when the user did not set it. + + The default never goes below the platform task watchdog, so a user who + widened `esp32.watchdog_timeout` keeps that window during requests. + """ + if not CORE.is_esp32 or CONF_WATCHDOG_TIMEOUT in config: + return + derived_ms = ( + config[CONF_TIMEOUT].total_milliseconds * WATCHDOG_TIMEOUT_MULTIPLIER + + WATCHDOG_TIMEOUT_MARGIN_MS + ) + platform_ms = fv.full_config.get()[PLATFORM_ESP32][ + CONF_WATCHDOG_TIMEOUT + ].total_milliseconds + config[CONF_WATCHDOG_TIMEOUT] = TimePeriodMilliseconds( + milliseconds=max(derived_ms, platform_ms) + ) + + def _declare_request_class(value: Any) -> ID: if CORE.is_host: return cv.declare_id(HttpRequestHost)(value) @@ -153,6 +183,8 @@ CONFIG_SCHEMA = cv.All( validate_ssl_verification, ) +FINAL_VALIDATE_SCHEMA = default_watchdog_timeout + async def to_code(config: ConfigType) -> None: var = cg.new_Pvariable(config[CONF_ID]) diff --git a/esphome/components/http_request/http_request_idf.cpp b/esphome/components/http_request/http_request_idf.cpp index 470ed332f1..10313be89d 100644 --- a/esphome/components/http_request/http_request_idf.cpp +++ b/esphome/components/http_request/http_request_idf.cpp @@ -142,12 +142,13 @@ std::shared_ptr HttpRequestIDF::perform(const std::string &url, c const char *buf = body.c_str(); while (write_left > 0) { int written = esp_http_client_write(client, buf + write_index, write_left); - if (written < 0) { + if (written <= 0) { err = ESP_FAIL; break; } write_left -= written; write_index += written; + container->feed_wdt(); } } diff --git a/tests/component_tests/http_request/__init__.py b/tests/component_tests/http_request/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/component_tests/http_request/config/test_esp32_default.yaml b/tests/component_tests/http_request/config/test_esp32_default.yaml new file mode 100644 index 0000000000..86744dcb11 --- /dev/null +++ b/tests/component_tests/http_request/config/test_esp32_default.yaml @@ -0,0 +1,12 @@ +esphome: + name: test + +esp32: + board: nodemcu-32s + +wifi: + ssid: test + password: testtest + +http_request: + timeout: 10s diff --git a/tests/component_tests/http_request/config/test_esp32_explicit.yaml b/tests/component_tests/http_request/config/test_esp32_explicit.yaml new file mode 100644 index 0000000000..e0d0074caa --- /dev/null +++ b/tests/component_tests/http_request/config/test_esp32_explicit.yaml @@ -0,0 +1,13 @@ +esphome: + name: test + +esp32: + board: nodemcu-32s + +wifi: + ssid: test + password: testtest + +http_request: + timeout: 10s + watchdog_timeout: 20s diff --git a/tests/component_tests/http_request/config/test_esp32_platform_wider.yaml b/tests/component_tests/http_request/config/test_esp32_platform_wider.yaml new file mode 100644 index 0000000000..77a85da2ff --- /dev/null +++ b/tests/component_tests/http_request/config/test_esp32_platform_wider.yaml @@ -0,0 +1,13 @@ +esphome: + name: test + +esp32: + board: nodemcu-32s + watchdog_timeout: 60s + +wifi: + ssid: test + password: testtest + +http_request: + timeout: 10s diff --git a/tests/component_tests/http_request/config/test_esp32_stock.yaml b/tests/component_tests/http_request/config/test_esp32_stock.yaml new file mode 100644 index 0000000000..70d2701466 --- /dev/null +++ b/tests/component_tests/http_request/config/test_esp32_stock.yaml @@ -0,0 +1,11 @@ +esphome: + name: test + +esp32: + board: nodemcu-32s + +wifi: + ssid: test + password: testtest + +http_request: diff --git a/tests/component_tests/http_request/config/test_esp8266.yaml b/tests/component_tests/http_request/config/test_esp8266.yaml new file mode 100644 index 0000000000..d0698dc57e --- /dev/null +++ b/tests/component_tests/http_request/config/test_esp8266.yaml @@ -0,0 +1,13 @@ +esphome: + name: test + +esp8266: + board: d1_mini + +wifi: + ssid: test + password: testtest + +http_request: + timeout: 10s + verify_ssl: false diff --git a/tests/component_tests/http_request/config/test_rp2040.yaml b/tests/component_tests/http_request/config/test_rp2040.yaml new file mode 100644 index 0000000000..030736c30d --- /dev/null +++ b/tests/component_tests/http_request/config/test_rp2040.yaml @@ -0,0 +1,13 @@ +esphome: + name: test + +rp2: + board: rpipicow + +wifi: + ssid: test + password: testtest + +http_request: + timeout: 10s + verify_ssl: false diff --git a/tests/component_tests/http_request/test_init.py b/tests/component_tests/http_request/test_init.py new file mode 100644 index 0000000000..446c4acbd0 --- /dev/null +++ b/tests/component_tests/http_request/test_init.py @@ -0,0 +1,42 @@ +"""Tests for the http_request watchdog timeout default.""" + +from collections.abc import Callable +from pathlib import Path + +import pytest + +from esphome.config import read_config +from esphome.const import CONF_WATCHDOG_TIMEOUT +from esphome.core import CORE, TimePeriodMilliseconds + + +@pytest.mark.parametrize( + ("yaml_file", "expected_ms"), + [ + # stock 4.5s timeout: 3 x 4.5s plus 1s margin + ("test_esp32_stock.yaml", 14500), + # 3 x 10s plus 1s margin + ("test_esp32_default.yaml", 31000), + # esp32.watchdog_timeout: 60s is wider than the derived value and wins + ("test_esp32_platform_wider.yaml", 60000), + # explicit value is kept as is + ("test_esp32_explicit.yaml", 20000), + ], +) +def test_esp32_watchdog_timeout( + component_config_path: Callable[[str], Path], yaml_file: str, expected_ms: int +) -> None: + CORE.config_path = component_config_path(yaml_file) + config = read_config({}) + assert config["http_request"][CONF_WATCHDOG_TIMEOUT] == TimePeriodMilliseconds( + milliseconds=expected_ms + ) + + +@pytest.mark.parametrize("yaml_file", ["test_esp8266.yaml", "test_rp2040.yaml"]) +def test_other_platforms_leave_watchdog_unset( + component_config_path: Callable[[str], Path], yaml_file: str +) -> None: + CORE.config_path = component_config_path(yaml_file) + config = read_config({}) + assert CONF_WATCHDOG_TIMEOUT not in config["http_request"]