From 4e4056d79095d7b5eb0dd84e68ea30cd37255fae Mon Sep 17 00:00:00 2001 From: Bascht74 <66269310+Bascht74@users.noreply.github.com> Date: Sat, 10 Oct 2026 03:11:48 +0200 Subject: [PATCH 01/17] [uart] Share uart_tcp's write pacing and uart_id matcher (#20464) --- .../cdc_acm_uart/bridge/__init__.py | 15 +---- esphome/components/uart/__init__.py | 19 ++++++ esphome/components/uart/uart_component.cpp | 19 ++++++ esphome/components/uart/uart_component.h | 4 ++ esphome/components/uart_tcp/__init__.py | 14 +---- esphome/components/uart_tcp/uart_tcp.cpp | 14 +---- .../components/uart/uart_paced_write_room.cpp | 62 +++++++++++++++++++ tests/unit_tests/components/test_uart.py | 24 +++++++ 8 files changed, 131 insertions(+), 40 deletions(-) create mode 100644 tests/components/uart/uart_paced_write_room.cpp create mode 100644 tests/unit_tests/components/test_uart.py diff --git a/esphome/components/cdc_acm_uart/bridge/__init__.py b/esphome/components/cdc_acm_uart/bridge/__init__.py index cee048df5d..133ea92a48 100644 --- a/esphome/components/cdc_acm_uart/bridge/__init__.py +++ b/esphome/components/cdc_acm_uart/bridge/__init__.py @@ -36,19 +36,6 @@ CONFIG_SCHEMA = cv.All( ) -def _subtree_references_uart(node: object, uart_id: str) -> bool: - """Return True if any dict in the subtree has a uart_id entry naming this bus.""" - if isinstance(node, dict): - return any( - (key == CONF_UART_ID and str(value) == uart_id) - or _subtree_references_uart(value, uart_id) - for key, value in node.items() - ) - if isinstance(node, list): - return any(_subtree_references_uart(item, uart_id) for item in node) - return False - - def _reject_debug(uart_conf: ConfigType) -> ConfigType: # The worker tasks use the IDF driver directly, so the uart debugger never sees # bridge traffic and its dummy_receiver would drain RX bytes on the main loop. @@ -86,7 +73,7 @@ def _final_validate(config: ConfigType) -> ConfigType: for domain, domain_conf in full_config.items(): if domain == BRIDGE_DOMAIN: continue - if _subtree_references_uart(domain_conf, owned_id): + if uart.subtree_references_uart(domain_conf, owned_id): raise cv.Invalid( f"The {label} '{owned_id}' is also used by '{domain}'; a bridge " f"requires exclusive use of its {label}.", diff --git a/esphome/components/uart/__init__.py b/esphome/components/uart/__init__.py index 25fbfb7599..2250b9d508 100644 --- a/esphome/components/uart/__init__.py +++ b/esphome/components/uart/__init__.py @@ -533,6 +533,25 @@ def final_validate_device_schema( ) +def subtree_references_uart( + node: object, uart_id: str, conf_key: str = CONF_UART_ID +) -> bool: + """Return True if any dict in the subtree has a conf_key entry naming this bus. + + For the final validation of a component that needs a UART for itself. Bare + `id:` references (a uart.write action) and lambdas are not found. + """ + if isinstance(node, dict): + return any( + (key == conf_key and str(value) == uart_id) + or subtree_references_uart(value, uart_id, conf_key) + for key, value in node.items() + ) + if isinstance(node, list): + return any(subtree_references_uart(item, uart_id, conf_key) for item in node) + return False + + async def register_uart_device(var, config): """Register a UART device, setting up all the internal values. diff --git a/esphome/components/uart/uart_component.cpp b/esphome/components/uart/uart_component.cpp index 6516174635..82bc592702 100644 --- a/esphome/components/uart/uart_component.cpp +++ b/esphome/components/uart/uart_component.cpp @@ -1,9 +1,15 @@ #include "uart_component.h" +#include "esphome/core/application.h" + +#include namespace esphome::uart { ESPHOME_LOG_TAG(TAG, "uart"); +// Keeps the pacing product in 32 bits up to about 10 Mbaud. +static constexpr uint32_t MAX_PACE_SPAN_MS = 4000; + bool UARTComponent::check_read_timeout_(size_t len) { if (this->available() >= len) return true; @@ -19,6 +25,19 @@ bool UARTComponent::check_read_timeout_(size_t len) { return true; } +size_t UARTComponent::paced_write_room(uint32_t last_write_ms) { + size_t room = this->available_for_write(); + if (room != SIZE_MAX) { + return room; + } + // A pass woken early writes little. + uint32_t span = + std::min({App.get_loop_component_start_time() - last_write_ms, App.get_loop_interval(), MAX_PACE_SPAN_MS}); + // 10 bits per byte on the line. + uint32_t paced = this->baud_rate_ / 10 * span / 1000; + return std::max(1, paced); +} + void UARTComponent::set_rx_full_threshold_ms(uint8_t time) { uint8_t bytelength = this->data_bits_ + this->stop_bits_ + 1; if (this->parity_ != UARTParityOptions::UART_CONFIG_PARITY_NONE) diff --git a/esphome/components/uart/uart_component.h b/esphome/components/uart/uart_component.h index 8f143b7977..2543cb5476 100644 --- a/esphome/components/uart/uart_component.h +++ b/esphome/components/uart/uart_component.h @@ -86,6 +86,10 @@ class UARTComponent { // Platforms that cannot tell return SIZE_MAX: write_array() takes everything and may block. virtual size_t available_for_write() { return SIZE_MAX; } + // Bytes a write can take now. Where available_for_write() cannot tell (SIZE_MAX), paces to the line time since + // last_write_ms, a loop start time: at most one loop interval and 4 s, at least one byte. + size_t paced_write_room(uint32_t last_write_ms); + // Pure virtual method to block until all bytes have been written to the UART bus. // @return UARTFlushResult indicating whether the flush was confirmed, timed out, failed, or assumed successful. virtual UARTFlushResult flush() = 0; diff --git a/esphome/components/uart_tcp/__init__.py b/esphome/components/uart_tcp/__init__.py index 601fdeb327..9d62d48d7c 100644 --- a/esphome/components/uart_tcp/__init__.py +++ b/esphome/components/uart_tcp/__init__.py @@ -71,18 +71,6 @@ CONFIG_SCHEMA = cv.All( ) -def _subtree_references_uart(node: object, uart_id: str) -> bool: - if isinstance(node, dict): - return any( - (key == CONF_UART_ID and str(value) == uart_id) - or _subtree_references_uart(value, uart_id) - for key, value in node.items() - ) - if isinstance(node, list): - return any(_subtree_references_uart(item, uart_id) for item in node) - return False - - def _reject_dummy_receiver(uart_conf: ConfigType) -> ConfigType: debug = uart_conf.get(CONF_DEBUG) if isinstance(debug, dict) and debug.get(CONF_DUMMY_RECEIVER): @@ -111,7 +99,7 @@ def _final_validate(config: ConfigType) -> ConfigType: # Bare `id:` references (a uart.write action) and lambdas are not caught. if not CORE.testing_mode: for domain, domain_conf in full_config.items(): - if domain != DOMAIN and _subtree_references_uart(domain_conf, uart_id): + if domain != DOMAIN and uart.subtree_references_uart(domain_conf, uart_id): raise cv.Invalid( f"The UART '{uart_id}' is also used by '{domain}'. " "uart_tcp requires exclusive use of that UART.", diff --git a/esphome/components/uart_tcp/uart_tcp.cpp b/esphome/components/uart_tcp/uart_tcp.cpp index bc75eb04c7..d634ac579f 100644 --- a/esphome/components/uart_tcp/uart_tcp.cpp +++ b/esphome/components/uart_tcp/uart_tcp.cpp @@ -11,9 +11,6 @@ namespace esphome::uart_tcp { ESPHOME_LOG_TAG(TAG, "uart_tcp"); -// Keeps the pacing product in 32 bits up to about 10 Mbaud. -static constexpr uint32_t MAX_PACE_SPAN_MS = 4000; - void UartTcp::setup() { this->link_.begin(TAG); #ifdef USE_SOCKET_TCP_LISTENER @@ -75,16 +72,7 @@ void UartTcp::sync_link_() { void UartTcp::read_socket_() { // A hardware write blocks until the driver takes every byte. Leave what does // not fit in the socket, so TCP flow control throttles the peer. - size_t room = this->parent_->available_for_write(); - if (room == SIZE_MAX) { - // Capacity unknown on this platform; pace to the UART time since the last write, - // at most one loop interval and 4 s, so a pass woken early by the socket writes little. - uint32_t span = std::min( - {App.get_loop_component_start_time() - this->last_write_ms_, App.get_loop_interval(), MAX_PACE_SPAN_MS}); - // 10 bits per byte on the line. - uint32_t paced = this->parent_->get_baud_rate() / 10 * span / 1000; - room = std::max(1, paced); - } + size_t room = this->parent_->paced_write_room(this->last_write_ms_); if (room == 0) { this->rx_pending_ = true; return; diff --git a/tests/components/uart/uart_paced_write_room.cpp b/tests/components/uart/uart_paced_write_room.cpp new file mode 100644 index 0000000000..ed64695dd0 --- /dev/null +++ b/tests/components/uart/uart_paced_write_room.cpp @@ -0,0 +1,62 @@ +#include + +#include "common.h" +#include "esphome/core/application.h" + +namespace esphome::uart::testing { + +class RoomUART : public MockUARTComponent { + public: + size_t available_for_write() override { return this->room; } + size_t room{SIZE_MAX}; +}; + +class PacedWriteRoom : public ::testing::Test { + protected: + void SetUp() override { + App.set_loop_interval(16); + this->uart_.set_baud_rate(9600); + } + void TearDown() override { + App.set_loop_interval(16); + this->at_(0); + } + // Publishes now_ms as the loop start time, as Application::loop() does. + void at_(uint32_t now_ms) { LoopBlockingGuard dispatch{nullptr, nullptr, now_ms}; } + + RoomUART uart_; +}; + +TEST_F(PacedWriteRoom, KnownRoomIsReturned) { + this->uart_.room = 7; + this->at_(1000); + EXPECT_EQ(this->uart_.paced_write_room(0), 7u); + this->uart_.room = 0; + EXPECT_EQ(this->uart_.paced_write_room(0), 0u); +} + +TEST_F(PacedWriteRoom, UnknownRoomTakesOneLoopInterval) { + this->at_(1000); + // 9600 baud at 10 bits per byte for 16 ms. + EXPECT_EQ(this->uart_.paced_write_room(0), 15u); +} + +TEST_F(PacedWriteRoom, EarlyPassTakesTheTimeSinceTheLastWrite) { + this->at_(1005); + EXPECT_EQ(this->uart_.paced_write_room(1000), 4u); +} + +TEST_F(PacedWriteRoom, AtLeastOneByte) { + this->at_(1000); + EXPECT_EQ(this->uart_.paced_write_room(1000), 1u); +} + +TEST_F(PacedWriteRoom, SpanCapsAtFourSeconds) { + App.set_loop_interval(10000); + this->uart_.set_baud_rate(300); + this->at_(6000); + // 30 bytes/s for 4 s. + EXPECT_EQ(this->uart_.paced_write_room(0), 120u); +} + +} // namespace esphome::uart::testing diff --git a/tests/unit_tests/components/test_uart.py b/tests/unit_tests/components/test_uart.py new file mode 100644 index 0000000000..4b062b4729 --- /dev/null +++ b/tests/unit_tests/components/test_uart.py @@ -0,0 +1,24 @@ +"""Tests for the uart helpers shared by components that own a UART.""" + +from esphome.components.uart import subtree_references_uart +from esphome.core import ID + + +def test_finds_a_nested_uart_id() -> None: + config = {"modbus": [{"id": ID("hub"), "uart_id": ID("bus")}]} + assert subtree_references_uart(config, "bus") + + +def test_ignores_other_uarts_and_other_keys() -> None: + config = {"modbus": [{"uart_id": ID("other")}], "sensor": [{"id": ID("bus")}]} + assert not subtree_references_uart(config, "bus") + + +def test_finds_another_key() -> None: + config = {"modbus_tcp_uart": [{"tcp_uart_id": ID("bus")}]} + assert subtree_references_uart(config, "bus", "tcp_uart_id") + assert not subtree_references_uart(config, "bus") + + +def test_handles_scalars() -> None: + assert not subtree_references_uart("bus", "bus") From 1b49e79bf8da3f10dc19c31b33f3ff268f7ee763 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:35:38 -1000 Subject: [PATCH 02/17] [microphone] Build on_data with build_callback_automation (#20451) --- esphome/components/microphone/__init__.py | 18 ++++-------------- esphome/components/microphone/automation.h | 17 ----------------- 2 files changed, 4 insertions(+), 31 deletions(-) delete mode 100644 esphome/components/microphone/automation.h diff --git a/esphome/components/microphone/__init__.py b/esphome/components/microphone/__init__.py index 6fa14cdd8b..a83827ef5d 100644 --- a/esphome/components/microphone/__init__.py +++ b/esphome/components/microphone/__init__.py @@ -12,7 +12,6 @@ from esphome.const import ( CONF_ID, CONF_MICROPHONE, CONF_ON_DATA, - CONF_TRIGGER_ID, ) from esphome.core import CORE from esphome.coroutine import CoroPriority, coroutine_with_priority @@ -30,17 +29,12 @@ microphone_ns = cg.esphome_ns.namespace("microphone") Microphone = microphone_ns.class_("Microphone") MicrophoneSource = microphone_ns.class_("MicrophoneSource") -DataTrigger = microphone_ns.class_( - "DataTrigger", - automation.Trigger.template(cg.std_vector.template(cg.uint8).operator("ref")), -) - async def setup_microphone_core_(var: MockObj, config: ConfigType) -> None: for conf in config.get(CONF_ON_DATA, []): - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) - await automation.build_automation( - trigger, + await automation.build_callback_automation( + var, + "add_data_callback", [(cg.std_vector.template(cg.uint8).operator("ref").operator("const"), "x")], conf, ) @@ -54,11 +48,7 @@ async def register_microphone(var: MockObj, config: ConfigType) -> None: MICROPHONE_SCHEMA = cv.Schema.extend(audio.AUDIO_COMPONENT_SCHEMA).extend( { - cv.Optional(CONF_ON_DATA): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(DataTrigger), - } - ), + cv.Optional(CONF_ON_DATA): automation.validate_automation({}), } ) diff --git a/esphome/components/microphone/automation.h b/esphome/components/microphone/automation.h deleted file mode 100644 index 0fac18d828..0000000000 --- a/esphome/components/microphone/automation.h +++ /dev/null @@ -1,17 +0,0 @@ -#pragma once - -#include "esphome/core/automation.h" -#include "microphone.h" - -#include - -namespace esphome::microphone { - -class DataTrigger final : public Trigger &> { - public: - explicit DataTrigger(Microphone *mic) { - mic->add_data_callback([this](const std::vector &data) { this->trigger(data); }); - } -}; - -} // namespace esphome::microphone From 5d2433f69ad99b3b95d1457bb4cb23ede78409fd Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:36:57 -1000 Subject: [PATCH 03/17] [graphical_display_menu] Build on_redraw with build_callback_automation (#20452) --- .../graphical_display_menu/__init__.py | 17 +++---------- .../graphical_display_menu.cpp | 2 +- .../graphical_display_menu.h | 12 +--------- .../common-triggers.yaml | 24 +++++++++++++++++++ .../test-triggers.esp32-idf.yaml | 6 +++++ 5 files changed, 35 insertions(+), 26 deletions(-) create mode 100644 tests/components/graphical_display_menu/common-triggers.yaml create mode 100644 tests/components/graphical_display_menu/test-triggers.esp32-idf.yaml diff --git a/esphome/components/graphical_display_menu/__init__.py b/esphome/components/graphical_display_menu/__init__.py index 59e8f66c50..a398aea619 100644 --- a/esphome/components/graphical_display_menu/__init__.py +++ b/esphome/components/graphical_display_menu/__init__.py @@ -13,7 +13,6 @@ from esphome.const import ( CONF_FONT, CONF_FOREGROUND_COLOR, CONF_ID, - CONF_TRIGGER_ID, ) from esphome.types import ConfigType @@ -29,9 +28,6 @@ MenuItemValueArguments = graphical_display_menu_ns.struct("MenuItemValueArgument MenuItemValueArgumentsConstPtr = MenuItemValueArguments.operator("ptr").operator( "const" ) -GraphicalDisplayMenuOnRedrawTrigger = graphical_display_menu_ns.class_( - "GraphicalDisplayMenuOnRedrawTrigger", automation.Trigger -) CODEOWNERS = ["@MrMDavidson"] DOMAIN = "graphical_display_menu" @@ -49,13 +45,7 @@ CONFIG_SCHEMA = DISPLAY_MENU_BASE_SCHEMA.extend( cv.Optional(CONF_MENU_ITEM_VALUE): cv.templatable(cv.string), cv.Optional(CONF_FOREGROUND_COLOR): cv.use_id(color.ColorStruct), cv.Optional(CONF_BACKGROUND_COLOR): cv.use_id(color.ColorStruct), - cv.Optional(CONF_ON_REDRAW): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id( - GraphicalDisplayMenuOnRedrawTrigger - ) - } - ), + cv.Optional(CONF_ON_REDRAW): automation.validate_automation({}), } ) ) @@ -92,9 +82,8 @@ async def to_code(config: ConfigType) -> None: cg.add(var.set_background_color(background_color)) for conf in config.get(CONF_ON_REDRAW, []): - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) - await automation.build_automation( - trigger, [(GraphicalDisplayMenuConstPtr, "it")], conf + await automation.build_callback_automation( + var, "add_on_redraw_callback", [(GraphicalDisplayMenuConstPtr, "it")], conf ) await display_menu_to_code(var, config) diff --git a/esphome/components/graphical_display_menu/graphical_display_menu.cpp b/esphome/components/graphical_display_menu/graphical_display_menu.cpp index 8761e5fb4c..5f5b3e8d42 100644 --- a/esphome/components/graphical_display_menu/graphical_display_menu.cpp +++ b/esphome/components/graphical_display_menu/graphical_display_menu.cpp @@ -241,6 +241,6 @@ void GraphicalDisplayMenu::draw_item(const display_menu_base::MenuItem *item, co "draw_item should be called."); } -void GraphicalDisplayMenu::update() { this->on_redraw_callbacks_.call(); } +void GraphicalDisplayMenu::update() { this->on_redraw_callbacks_.call(this); } } // namespace esphome::graphical_display_menu diff --git a/esphome/components/graphical_display_menu/graphical_display_menu.h b/esphome/components/graphical_display_menu/graphical_display_menu.h index 13c0f9d73f..1f1d8ff3fc 100644 --- a/esphome/components/graphical_display_menu/graphical_display_menu.h +++ b/esphome/components/graphical_display_menu/graphical_display_menu.h @@ -70,17 +70,7 @@ class GraphicalDisplayMenu final : public display_menu_base::DisplayMenuComponen Color foreground_color_{COLOR_ON}; Color background_color_{COLOR_OFF}; - CallbackManager on_redraw_callbacks_{}; -}; - -class GraphicalDisplayMenuOnRedrawTrigger final : public Trigger { - public: - explicit GraphicalDisplayMenuOnRedrawTrigger(GraphicalDisplayMenu *parent) : parent_(parent) { - parent->add_on_redraw_callback([this]() { this->trigger(this->parent_); }); - } - - protected: - GraphicalDisplayMenu *parent_; + LazyCallbackManager on_redraw_callbacks_{}; }; } // namespace graphical_display_menu diff --git a/tests/components/graphical_display_menu/common-triggers.yaml b/tests/components/graphical_display_menu/common-triggers.yaml new file mode 100644 index 0000000000..ea4daeb162 --- /dev/null +++ b/tests/components/graphical_display_menu/common-triggers.yaml @@ -0,0 +1,24 @@ +display: + - platform: ssd1306_i2c + i2c_id: i2c_bus + id: graphical_display_menu_triggers_display + model: SSD1306_128X64 + reset_pin: ${reset_pin} + lambda: |- + it.rectangle(0, 0, it.get_width(), it.get_height()); + +font: + - file: "gfonts://Roboto" + id: graphical_display_menu_triggers_font + size: 20 + +graphical_display_menu: + id: graphical_display_menu_triggers_menu + display: graphical_display_menu_triggers_display + font: graphical_display_menu_triggers_font + on_redraw: + then: + lambda: 'ESP_LOGI("graphical_display_menu", "redraw, active: %d", it->is_active());' + items: + - type: label + text: "Label" diff --git a/tests/components/graphical_display_menu/test-triggers.esp32-idf.yaml b/tests/components/graphical_display_menu/test-triggers.esp32-idf.yaml new file mode 100644 index 0000000000..2adb772294 --- /dev/null +++ b/tests/components/graphical_display_menu/test-triggers.esp32-idf.yaml @@ -0,0 +1,6 @@ +substitutions: + reset_pin: GPIO15 + +packages: + i2c: !include ../../test_build_components/common/i2c/esp32-idf.yaml + graphical_display_menu: !include common-triggers.yaml From 79fd5edc7f44cc9a93b7a8be3dcea96c54b14f15 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:37:10 -1000 Subject: [PATCH 04/17] [core] Bind InternalGPIOPin to the platform pin class instead of a virtual base (#20455) --- esphome/components/esp32/__init__.py | 8 - esphome/components/esp32/gpio.cpp | 11 +- esphome/components/esp32/gpio.h | 19 +- esphome/components/esp32/gpio.py | 1 - esphome/components/esp8266/gpio.cpp | 3 +- esphome/components/esp8266/gpio.h | 18 +- esphome/components/host/gpio.cpp | 3 +- esphome/components/host/gpio.h | 18 +- esphome/components/libretiny/gpio_arduino.cpp | 3 +- esphome/components/libretiny/gpio_arduino.h | 18 +- esphome/components/rp2/gpio.cpp | 3 +- esphome/components/rp2/gpio.h | 18 +- esphome/components/zephyr/gpio.cpp | 3 +- esphome/components/zephyr/gpio.h | 21 +- esphome/core/defines.h | 1 - esphome/core/gpio.h | 184 ++++-------------- esphome/core/gpio_pin.h | 147 ++++++++++++++ .../deep_sleep/test-timer-only.esp32-idf.yaml | 4 + .../power_supply/test_setup_priority.cpp | 25 +-- 19 files changed, 278 insertions(+), 230 deletions(-) create mode 100644 esphome/core/gpio_pin.h create mode 100644 tests/components/deep_sleep/test-timer-only.esp32-idf.yaml diff --git a/esphome/components/esp32/__init__.py b/esphome/components/esp32/__init__.py index cfa79e3cf1..aa4f78de69 100644 --- a/esphome/components/esp32/__init__.py +++ b/esphome/components/esp32/__init__.py @@ -15,7 +15,6 @@ from esphome.components.const import ( CONF_ENABLE_OTA_DOWNGRADE_PROTECTION, CONF_IGNORE_NOT_FOUND, ) -from esphome.config_helpers import filter_source_files_from_defines import esphome.config_validation as cv from esphome.const import ( CONF_ADVANCED, @@ -3967,10 +3966,3 @@ def process_stacktrace(config, line, backtrace_state): _decode_pc(config, addr.group()) return backtrace_state - - -# gpio.cpp only implements ESP32InternalGPIOPin and its ISR helpers, which -# are instantiated solely by the pin schema codegen (esp32_pin_to_code) -FILTER_SOURCE_FILES = filter_source_files_from_defines( - {"gpio.cpp": "USE_ESP32_INTERNAL_GPIO"} -) diff --git a/esphome/components/esp32/gpio.cpp b/esphome/components/esp32/gpio.cpp index 6e4de6aa51..d9c9b86185 100644 --- a/esphome/components/esp32/gpio.cpp +++ b/esphome/components/esp32/gpio.cpp @@ -1,9 +1,10 @@ #include "esphome/core/defines.h" -// Also defines the core ISRInternalGPIOPin methods; those are only reachable -// via ESP32InternalGPIOPin::to_isr(), so the same define gates both safely. -#if defined(USE_ESP32) && defined(USE_ESP32_INTERNAL_GPIO) +// Always built: InternalGPIOPin is this class, so components that merely hold a +// pointer to it reference these methods even in configs without a pin. +#ifdef USE_ESP32 #include "gpio.h" +#include "esphome/core/hal.h" #include "esphome/core/log.h" #include "driver/gpio.h" #include "driver/rtc_io.h" @@ -68,7 +69,7 @@ ISRInternalGPIOPin ESP32InternalGPIOPin::to_isr() const { return ISRInternalGPIOPin((void *) arg); } -void ESP32InternalGPIOPin::attach_interrupt(void (*func)(void *), void *arg, gpio::InterruptType type) const { +void ESP32InternalGPIOPin::attach_interrupt_(void (*func)(void *), void *arg, gpio::InterruptType type) const { gpio_int_type_t idf_type = GPIO_INTR_ANYEDGE; switch (type) { case gpio::INTERRUPT_RISING_EDGE: @@ -207,4 +208,4 @@ void IRAM_ATTR ISRInternalGPIOPin::pin_mode(gpio::Flags flags) { } // namespace esphome -#endif // USE_ESP32 && USE_ESP32_INTERNAL_GPIO +#endif // USE_ESP32 diff --git a/esphome/components/esp32/gpio.h b/esphome/components/esp32/gpio.h index 9c661b4d9e..954b213d7f 100644 --- a/esphome/components/esp32/gpio.h +++ b/esphome/components/esp32/gpio.h @@ -1,7 +1,8 @@ #pragma once #ifdef USE_ESP32 -#include "esphome/core/hal.h" +#include "esphome/core/gpio_pin.h" +#include #include namespace esphome::esp32 { @@ -10,7 +11,7 @@ namespace esphome::esp32 { static_assert(GPIO_NUM_MAX <= 256, "gpio_num_t has too many values for uint8_t"); static_assert(GPIO_DRIVE_CAP_MAX <= 4, "gpio_drive_cap_t has too many values for 2-bit field"); -class ESP32InternalGPIOPin final : public InternalGPIOPin { +class ESP32InternalGPIOPin final : public GPIOPin { public: void set_pin(gpio_num_t pin) { this->pin_ = static_cast(pin); } void set_inverted(bool inverted) { this->pin_flags_.inverted = inverted; } @@ -24,11 +25,15 @@ class ESP32InternalGPIOPin final : public InternalGPIOPin { bool digital_read() override; void digital_write(bool value) override; size_t dump_summary(char *buffer, size_t len) const override; - void detach_interrupt() const override; - ISRInternalGPIOPin to_isr() const override; - uint8_t get_pin() const override { return this->pin_; } + void detach_interrupt() const; + template void attach_interrupt(void (*func)(T *), T *arg, gpio::InterruptType type) const { + this->attach_interrupt_(reinterpret_cast(func), arg, type); + } + ISRInternalGPIOPin to_isr() const; + uint8_t get_pin() const { return this->pin_; } gpio::Flags get_flags() const override { return this->flags_; } - bool is_inverted() const override { return this->pin_flags_.inverted; } + bool is_inverted() const { return this->pin_flags_.inverted; } + bool is_internal() override { return true; } gpio_num_t get_pin_num() const { return static_cast(this->pin_); } gpio_drive_cap_t get_drive_strength() const { return static_cast(this->pin_flags_.drive_strength); } #ifdef USE_GPIO_HOLD @@ -41,7 +46,7 @@ class ESP32InternalGPIOPin final : public InternalGPIOPin { #endif protected: - void attach_interrupt(void (*func)(void *), void *arg, gpio::InterruptType type) const override; + void attach_interrupt_(void (*func)(void *), void *arg, gpio::InterruptType type) const; // Memory layout: 8 bytes total on 32-bit systems // - 3 bytes for members below diff --git a/esphome/components/esp32/gpio.py b/esphome/components/esp32/gpio.py index 98aac209ec..321dd3d498 100644 --- a/esphome/components/esp32/gpio.py +++ b/esphome/components/esp32/gpio.py @@ -257,7 +257,6 @@ ESP32_PIN_SCHEMA = cv.All( @pins.PIN_SCHEMA_REGISTRY.register(PLATFORM_ESP32, ESP32_PIN_SCHEMA) async def esp32_pin_to_code(config): - cg.add_define("USE_ESP32_INTERNAL_GPIO") var = cg.new_Pvariable(config[CONF_ID]) num = config[CONF_NUMBER] cg.add(var.set_pin(getattr(gpio_num_t, f"GPIO_NUM_{num}"))) diff --git a/esphome/components/esp8266/gpio.cpp b/esphome/components/esp8266/gpio.cpp index ae09bdb91c..1d97b2c1e5 100644 --- a/esphome/components/esp8266/gpio.cpp +++ b/esphome/components/esp8266/gpio.cpp @@ -1,6 +1,7 @@ #ifdef USE_ESP8266 #include "gpio.h" +#include "esphome/core/hal.h" #include "esphome/core/log.h" namespace esphome::esp8266 { @@ -74,7 +75,7 @@ ISRInternalGPIOPin ESP8266GPIOPin::to_isr() const { return ISRInternalGPIOPin((void *) arg); } -void ESP8266GPIOPin::attach_interrupt(void (*func)(void *), void *arg, gpio::InterruptType type) const { +void ESP8266GPIOPin::attach_interrupt_(void (*func)(void *), void *arg, gpio::InterruptType type) const { uint8_t arduino_mode = 0; switch (type) { case gpio::INTERRUPT_RISING_EDGE: diff --git a/esphome/components/esp8266/gpio.h b/esphome/components/esp8266/gpio.h index 57ef06106a..84968d9752 100644 --- a/esphome/components/esp8266/gpio.h +++ b/esphome/components/esp8266/gpio.h @@ -2,12 +2,12 @@ #ifdef USE_ESP8266 -#include "esphome/core/hal.h" +#include "esphome/core/gpio_pin.h" #include namespace esphome::esp8266 { -class ESP8266GPIOPin final : public InternalGPIOPin { +class ESP8266GPIOPin final : public GPIOPin { public: void set_pin(uint8_t pin) { pin_ = pin; } void set_inverted(bool inverted) { inverted_ = inverted; } @@ -18,14 +18,18 @@ class ESP8266GPIOPin final : public InternalGPIOPin { bool digital_read() override; void digital_write(bool value) override; size_t dump_summary(char *buffer, size_t len) const override; - void detach_interrupt() const override; - ISRInternalGPIOPin to_isr() const override; - uint8_t get_pin() const override { return pin_; } + void detach_interrupt() const; + template void attach_interrupt(void (*func)(T *), T *arg, gpio::InterruptType type) const { + this->attach_interrupt_(reinterpret_cast(func), arg, type); + } + ISRInternalGPIOPin to_isr() const; + uint8_t get_pin() const { return pin_; } gpio::Flags get_flags() const override { return flags_; } - bool is_inverted() const override { return inverted_; } + bool is_inverted() const { return inverted_; } + bool is_internal() override { return true; } protected: - void attach_interrupt(void (*func)(void *), void *arg, gpio::InterruptType type) const override; + void attach_interrupt_(void (*func)(void *), void *arg, gpio::InterruptType type) const; uint8_t pin_; bool inverted_{}; diff --git a/esphome/components/host/gpio.cpp b/esphome/components/host/gpio.cpp index c1fd56e945..b57bd41188 100644 --- a/esphome/components/host/gpio.cpp +++ b/esphome/components/host/gpio.cpp @@ -1,6 +1,7 @@ #ifdef USE_HOST #include "gpio.h" +#include "esphome/core/hal.h" #include "esphome/core/log.h" namespace esphome { @@ -20,7 +21,7 @@ ISRInternalGPIOPin HostGPIOPin::to_isr() const { return ISRInternalGPIOPin((void *) arg); } -void HostGPIOPin::attach_interrupt(void (*func)(void *), void *arg, gpio::InterruptType type) const { +void HostGPIOPin::attach_interrupt_(void (*func)(void *), void *arg, gpio::InterruptType type) const { ESP_LOGD(TAG, "Attaching interrupt %p to pin %d and mode %d", func, pin_, (uint32_t) type); } void HostGPIOPin::pin_mode(gpio::Flags flags) { ESP_LOGD(TAG, "Setting pin %d mode to %02X", pin_, (uint32_t) flags); } diff --git a/esphome/components/host/gpio.h b/esphome/components/host/gpio.h index bd2d09257b..fd3ebefe1f 100644 --- a/esphome/components/host/gpio.h +++ b/esphome/components/host/gpio.h @@ -2,11 +2,11 @@ #ifdef USE_HOST -#include "esphome/core/hal.h" +#include "esphome/core/gpio_pin.h" namespace esphome::host { -class HostGPIOPin final : public InternalGPIOPin { +class HostGPIOPin final : public GPIOPin { public: void set_pin(uint8_t pin) { pin_ = pin; } void set_inverted(bool inverted) { inverted_ = inverted; } @@ -17,14 +17,18 @@ class HostGPIOPin final : public InternalGPIOPin { bool digital_read() override; void digital_write(bool value) override; size_t dump_summary(char *buffer, size_t len) const override; - void detach_interrupt() const override; - ISRInternalGPIOPin to_isr() const override; - uint8_t get_pin() const override { return pin_; } + void detach_interrupt() const; + template void attach_interrupt(void (*func)(T *), T *arg, gpio::InterruptType type) const { + this->attach_interrupt_(reinterpret_cast(func), arg, type); + } + ISRInternalGPIOPin to_isr() const; + uint8_t get_pin() const { return pin_; } gpio::Flags get_flags() const override { return flags_; } - bool is_inverted() const override { return inverted_; } + bool is_inverted() const { return inverted_; } + bool is_internal() override { return true; } protected: - void attach_interrupt(void (*func)(void *), void *arg, gpio::InterruptType type) const override; + void attach_interrupt_(void (*func)(void *), void *arg, gpio::InterruptType type) const; uint8_t pin_; bool inverted_{}; diff --git a/esphome/components/libretiny/gpio_arduino.cpp b/esphome/components/libretiny/gpio_arduino.cpp index ff333aacef..9884c37b85 100644 --- a/esphome/components/libretiny/gpio_arduino.cpp +++ b/esphome/components/libretiny/gpio_arduino.cpp @@ -1,6 +1,7 @@ #ifdef USE_LIBRETINY #include "gpio_arduino.h" +#include "esphome/core/hal.h" #include "esphome/core/log.h" namespace esphome::libretiny { @@ -37,7 +38,7 @@ ISRInternalGPIOPin ArduinoInternalGPIOPin::to_isr() const { return ISRInternalGPIOPin((void *) arg); } -void ArduinoInternalGPIOPin::attach_interrupt(void (*func)(void *), void *arg, gpio::InterruptType type) const { +void ArduinoInternalGPIOPin::attach_interrupt_(void (*func)(void *), void *arg, gpio::InterruptType type) const { PinStatus arduino_mode = (PinStatus) 255; switch (type) { case gpio::INTERRUPT_RISING_EDGE: diff --git a/esphome/components/libretiny/gpio_arduino.h b/esphome/components/libretiny/gpio_arduino.h index da477fde36..b5ccf544ba 100644 --- a/esphome/components/libretiny/gpio_arduino.h +++ b/esphome/components/libretiny/gpio_arduino.h @@ -1,11 +1,11 @@ #pragma once #ifdef USE_LIBRETINY -#include "esphome/core/hal.h" +#include "esphome/core/gpio_pin.h" namespace esphome::libretiny { -class ArduinoInternalGPIOPin final : public InternalGPIOPin { +class ArduinoInternalGPIOPin final : public GPIOPin { public: void set_pin(uint8_t pin) { pin_ = pin; } void set_inverted(bool inverted) { inverted_ = inverted; } @@ -16,14 +16,18 @@ class ArduinoInternalGPIOPin final : public InternalGPIOPin { bool digital_read() override; void digital_write(bool value) override; size_t dump_summary(char *buffer, size_t len) const override; - void detach_interrupt() const override; - ISRInternalGPIOPin to_isr() const override; - uint8_t get_pin() const override { return pin_; } + void detach_interrupt() const; + template void attach_interrupt(void (*func)(T *), T *arg, gpio::InterruptType type) const { + this->attach_interrupt_(reinterpret_cast(func), arg, type); + } + ISRInternalGPIOPin to_isr() const; + uint8_t get_pin() const { return pin_; } gpio::Flags get_flags() const override { return flags_; } - bool is_inverted() const override { return inverted_; } + bool is_inverted() const { return inverted_; } + bool is_internal() override { return true; } protected: - void attach_interrupt(void (*func)(void *), void *arg, gpio::InterruptType type) const override; + void attach_interrupt_(void (*func)(void *), void *arg, gpio::InterruptType type) const; uint8_t pin_; bool inverted_{}; diff --git a/esphome/components/rp2/gpio.cpp b/esphome/components/rp2/gpio.cpp index 93bdba1fca..2eeb460e95 100644 --- a/esphome/components/rp2/gpio.cpp +++ b/esphome/components/rp2/gpio.cpp @@ -1,6 +1,7 @@ #ifdef USE_RP2 #include "gpio.h" +#include "esphome/core/hal.h" #include "esphome/core/log.h" namespace esphome { @@ -40,7 +41,7 @@ ISRInternalGPIOPin RP2GPIOPin::to_isr() const { return ISRInternalGPIOPin((void *) arg); } -void RP2GPIOPin::attach_interrupt(void (*func)(void *), void *arg, gpio::InterruptType type) const { +void RP2GPIOPin::attach_interrupt_(void (*func)(void *), void *arg, gpio::InterruptType type) const { PinStatus arduino_mode = LOW; switch (type) { case gpio::INTERRUPT_RISING_EDGE: diff --git a/esphome/components/rp2/gpio.h b/esphome/components/rp2/gpio.h index 538fef619a..b1d4c9a8f6 100644 --- a/esphome/components/rp2/gpio.h +++ b/esphome/components/rp2/gpio.h @@ -3,11 +3,11 @@ #ifdef USE_RP2 #include -#include "esphome/core/hal.h" +#include "esphome/core/gpio_pin.h" namespace esphome::rp2 { -class RP2GPIOPin final : public InternalGPIOPin { +class RP2GPIOPin final : public GPIOPin { public: void set_pin(uint8_t pin) { pin_ = pin; } void set_inverted(bool inverted) { inverted_ = inverted; } @@ -18,14 +18,18 @@ class RP2GPIOPin final : public InternalGPIOPin { bool digital_read() override; void digital_write(bool value) override; size_t dump_summary(char *buffer, size_t len) const override; - void detach_interrupt() const override; - ISRInternalGPIOPin to_isr() const override; - uint8_t get_pin() const override { return pin_; } + void detach_interrupt() const; + template void attach_interrupt(void (*func)(T *), T *arg, gpio::InterruptType type) const { + this->attach_interrupt_(reinterpret_cast(func), arg, type); + } + ISRInternalGPIOPin to_isr() const; + uint8_t get_pin() const { return pin_; } gpio::Flags get_flags() const override { return flags_; } - bool is_inverted() const override { return inverted_; } + bool is_inverted() const { return inverted_; } + bool is_internal() override { return true; } protected: - void attach_interrupt(void (*func)(void *), void *arg, gpio::InterruptType type) const override; + void attach_interrupt_(void (*func)(void *), void *arg, gpio::InterruptType type) const; uint8_t pin_; bool inverted_{}; diff --git a/esphome/components/zephyr/gpio.cpp b/esphome/components/zephyr/gpio.cpp index 4400f0a0c5..57a31e302e 100644 --- a/esphome/components/zephyr/gpio.cpp +++ b/esphome/components/zephyr/gpio.cpp @@ -1,5 +1,6 @@ #ifdef USE_ZEPHYR #include "gpio.h" +#include "esphome/core/hal.h" #include #include #include "esphome/core/log.h" @@ -79,7 +80,7 @@ ISRInternalGPIOPin ZephyrGPIOPin::to_isr() const { return ISRInternalGPIOPin((void *) arg); } -void ZephyrGPIOPin::attach_interrupt(void (*func)(void *), void *arg, gpio::InterruptType type) const { +void ZephyrGPIOPin::attach_interrupt_(void (*func)(void *), void *arg, gpio::InterruptType type) const { if (!device_is_ready(this->gpio_)) { ESP_LOGE(TAG, "Cannot attach interrupt: GPIO device not ready"); return; diff --git a/esphome/components/zephyr/gpio.h b/esphome/components/zephyr/gpio.h index 71d1620a67..dd26e34eb8 100644 --- a/esphome/components/zephyr/gpio.h +++ b/esphome/components/zephyr/gpio.h @@ -1,7 +1,7 @@ #pragma once #ifdef USE_ZEPHYR -#include "esphome/core/hal.h" +#include "esphome/core/gpio_pin.h" #include #include namespace esphome::zephyr { @@ -16,7 +16,7 @@ struct ZephyrGPIOInterrupt { void *arg{nullptr}; }; -class ZephyrGPIOPin final : public InternalGPIOPin { +class ZephyrGPIOPin final : public GPIOPin { public: ZephyrGPIOPin(const device *gpio, int gpio_size, const char *pin_name_prefix) { this->gpio_ = gpio; @@ -32,14 +32,18 @@ class ZephyrGPIOPin final : public InternalGPIOPin { bool digital_read() override; void digital_write(bool value) override; size_t dump_summary(char *buffer, size_t len) const override; - void detach_interrupt() const override; - ISRInternalGPIOPin to_isr() const override; - uint8_t get_pin() const override { return this->pin_; } - bool is_inverted() const override { return this->inverted_; } + void detach_interrupt() const; + template void attach_interrupt(void (*func)(T *), T *arg, gpio::InterruptType type) const { + this->attach_interrupt_(reinterpret_cast(func), arg, type); + } + ISRInternalGPIOPin to_isr() const; + uint8_t get_pin() const { return this->pin_; } + bool is_inverted() const { return this->inverted_; } + bool is_internal() override { return true; } gpio::Flags get_flags() const override { return flags_; } protected: - void attach_interrupt(void (*func)(void *), void *arg, gpio::InterruptType type) const override; + void attach_interrupt_(void (*func)(void *), void *arg, gpio::InterruptType type) const; const device *gpio_{nullptr}; const char *pin_name_prefix_{nullptr}; gpio::Flags flags_{}; @@ -48,8 +52,7 @@ class ZephyrGPIOPin final : public InternalGPIOPin { bool inverted_{}; bool value_{false}; - // attach_interrupt()/detach_interrupt() are const (matching the base class), so - // the interrupt state they manage has to be mutable. + // attach_interrupt_()/detach_interrupt() are const, so their interrupt state is mutable. mutable ZephyrGPIOInterrupt interrupt_{}; }; diff --git a/esphome/core/defines.h b/esphome/core/defines.h index d4932ae749..731c0d2017 100644 --- a/esphome/core/defines.h +++ b/esphome/core/defines.h @@ -364,7 +364,6 @@ #ifdef USE_ESP32 #define USE_ESP32_CRASH_HANDLER #define USE_ESP32_VASPRINTF_STUB -#define USE_ESP32_INTERNAL_GPIO #define USE_MQTT_IDF_ENQUEUE #define USE_ESPHOME_TASK_LOG_BUFFER #define ESPHOME_TASK_LOG_BUFFER_SIZE 768 diff --git a/esphome/core/gpio.h b/esphome/core/gpio.h index 7daf1917a6..8ed0d70f6b 100644 --- a/esphome/core/gpio.h +++ b/esphome/core/gpio.h @@ -1,150 +1,44 @@ #pragma once -#include -#include +#include "esphome/core/gpio_pin.h" -#include "esphome/core/defines.h" -#include "esphome/core/helpers.h" -#include "esphome/core/log.h" +// Each platform has exactly one internal pin class, so InternalGPIOPin is an alias for +// it instead of an abstract base class: calls through the alias are direct and can be inlined. +#if defined(USE_ESP32) +#include "esphome/components/esp32/gpio.h" +namespace esphome { +using InternalGPIOPin = esp32::ESP32InternalGPIOPin; +} +#elif defined(USE_ESP8266) +#include "esphome/components/esp8266/gpio.h" +namespace esphome { +using InternalGPIOPin = esp8266::ESP8266GPIOPin; +} +#elif defined(USE_LIBRETINY) +#include "esphome/components/libretiny/gpio_arduino.h" +namespace esphome { +using InternalGPIOPin = libretiny::ArduinoInternalGPIOPin; +} +#elif defined(USE_RP2) +#include "esphome/components/rp2/gpio.h" +namespace esphome { +using InternalGPIOPin = rp2::RP2GPIOPin; +} +#elif defined(USE_HOST) +#include "esphome/components/host/gpio.h" +namespace esphome { +using InternalGPIOPin = host::HostGPIOPin; +} +#elif defined(USE_ZEPHYR) +#include "esphome/components/zephyr/gpio.h" +namespace esphome { +using InternalGPIOPin = zephyr::ZephyrGPIOPin; +} +#else +#error "gpio.h: no internal pin class for this platform" +#endif namespace esphome { - -/// Maximum buffer size for dump_summary output -inline constexpr size_t GPIO_SUMMARY_MAX_LEN = 48; - -#ifdef USE_ESP8266 -#define LOG_PIN(prefix, pin) log_pin(TAG, F(prefix), pin) -#else -#define LOG_PIN(prefix, pin) log_pin(TAG, prefix, pin) -#endif - -// put GPIO flags in a namespace to not pollute esphome namespace -namespace gpio { - -enum Flags : uint8_t { - // Can't name these just INPUT because of Arduino defines :( - FLAG_NONE = 0x00, - FLAG_INPUT = 0x01, - FLAG_OUTPUT = 0x02, - FLAG_OPEN_DRAIN = 0x04, - FLAG_PULLUP = 0x08, - FLAG_PULLDOWN = 0x10, - FLAG_HOLD = 0x20, -}; - -class FlagsHelper { - public: - constexpr FlagsHelper(Flags val) : val_(val) {} - constexpr operator Flags() const { return val_; } - - protected: - Flags val_; -}; -constexpr FlagsHelper operator&(Flags lhs, Flags rhs) { - return static_cast(static_cast(lhs) & static_cast(rhs)); -} -constexpr FlagsHelper operator|(Flags lhs, Flags rhs) { - return static_cast(static_cast(lhs) | static_cast(rhs)); -} - -enum InterruptType : uint8_t { - INTERRUPT_RISING_EDGE = 1, - INTERRUPT_FALLING_EDGE = 2, - INTERRUPT_ANY_EDGE = 3, - INTERRUPT_LOW_LEVEL = 4, - INTERRUPT_HIGH_LEVEL = 5, -}; - -} // namespace gpio - -class GPIOPin { - public: - virtual void setup() = 0; - - virtual void pin_mode(gpio::Flags flags) = 0; - - /** - * @brief Retrieve GPIO pin flags. - * - * @return The GPIO flags describing the pin mode and properties. - */ - virtual gpio::Flags get_flags() const = 0; - - virtual bool digital_read() = 0; - - virtual void digital_write(bool value) = 0; - - /// Write a summary of this pin to the provided buffer. - /// @param buffer The buffer to write to - /// @param len The size of the buffer (must be > 0) - /// @return The number of characters that would be written (excluding null terminator), - /// which may exceed len-1 if truncation occurred (snprintf semantics) - virtual size_t dump_summary(char *buffer, size_t len) const; - - virtual bool is_internal() { return false; } -#ifdef USE_GPIO_HOLD - bool get_hold() const { return this->get_flags() & gpio::FLAG_HOLD; } - virtual bool is_held() const { return false; } -#else - bool get_hold() const { return false; } - bool is_held() const { return false; } -#endif -}; - -/// Copy of GPIOPin that is safe to use from ISRs (with no virtual functions) -class ISRInternalGPIOPin { - public: - ISRInternalGPIOPin() = default; - ISRInternalGPIOPin(void *arg) : arg_(arg) {} - bool digital_read(); - void digital_write(bool value); - void clear_interrupt(); - void pin_mode(gpio::Flags flags); - - protected: - void *arg_{nullptr}; -}; - -class InternalGPIOPin : public GPIOPin { - public: - template void attach_interrupt(void (*func)(T *), T *arg, gpio::InterruptType type) const { - this->attach_interrupt(reinterpret_cast(func), arg, type); - } - - virtual void detach_interrupt() const = 0; - - virtual ISRInternalGPIOPin to_isr() const = 0; - - virtual uint8_t get_pin() const = 0; - - bool is_internal() override { return true; } - - virtual bool is_inverted() const = 0; - - protected: - virtual void attach_interrupt(void (*func)(void *), void *arg, gpio::InterruptType type) const = 0; -}; - -// Inline default implementation for GPIOPin::dump_summary. -// Writes an empty summary; subclasses override to provide pin details. -inline size_t GPIOPin::dump_summary(char *buffer, size_t len) const { - if (len > 0) - buffer[0] = '\0'; - return 0; -} - -// Inline helper for log_pin - allows compiler to inline into log_pin in gpio.cpp -inline void log_pin_with_prefix(const char *tag, const char *prefix, GPIOPin *pin) { - char buffer[GPIO_SUMMARY_MAX_LEN]; - size_t len = pin->dump_summary(buffer, sizeof(buffer)); - len = std::min(len, sizeof(buffer) - 1); - esp_log_printf_(ESPHOME_LOG_LEVEL_CONFIG, tag, __LINE__, "%s%.*s", prefix, (int) len, buffer); -} - -// log_pin function declarations - implementation in gpio.cpp -#ifdef USE_ESP8266 -void log_pin(const char *tag, const __FlashStringHelper *prefix, GPIOPin *pin); -#else -void log_pin(const char *tag, const char *prefix, GPIOPin *pin); -#endif - +static_assert(InternalGPIOPinContract, + "The platform's internal pin class is missing part of the InternalGPIOPin surface " + "(esphome/core/gpio_pin.h)"); } // namespace esphome diff --git a/esphome/core/gpio_pin.h b/esphome/core/gpio_pin.h new file mode 100644 index 0000000000..6c9acfa8d6 --- /dev/null +++ b/esphome/core/gpio_pin.h @@ -0,0 +1,147 @@ +#pragma once +#include +#include +#include +#include + +#include "esphome/core/defines.h" +#include "esphome/core/helpers.h" +#include "esphome/core/log.h" + +namespace esphome { + +/// Maximum buffer size for dump_summary output +inline constexpr size_t GPIO_SUMMARY_MAX_LEN = 48; + +#ifdef USE_ESP8266 +#define LOG_PIN(prefix, pin) log_pin(TAG, F(prefix), pin) +#else +#define LOG_PIN(prefix, pin) log_pin(TAG, prefix, pin) +#endif + +// put GPIO flags in a namespace to not pollute esphome namespace +namespace gpio { + +enum Flags : uint8_t { + // Can't name these just INPUT because of Arduino defines :( + FLAG_NONE = 0x00, + FLAG_INPUT = 0x01, + FLAG_OUTPUT = 0x02, + FLAG_OPEN_DRAIN = 0x04, + FLAG_PULLUP = 0x08, + FLAG_PULLDOWN = 0x10, + FLAG_HOLD = 0x20, +}; + +class FlagsHelper { + public: + constexpr FlagsHelper(Flags val) : val_(val) {} + constexpr operator Flags() const { return val_; } + + protected: + Flags val_; +}; +constexpr FlagsHelper operator&(Flags lhs, Flags rhs) { + return static_cast(static_cast(lhs) & static_cast(rhs)); +} +constexpr FlagsHelper operator|(Flags lhs, Flags rhs) { + return static_cast(static_cast(lhs) | static_cast(rhs)); +} + +enum InterruptType : uint8_t { + INTERRUPT_RISING_EDGE = 1, + INTERRUPT_FALLING_EDGE = 2, + INTERRUPT_ANY_EDGE = 3, + INTERRUPT_LOW_LEVEL = 4, + INTERRUPT_HIGH_LEVEL = 5, +}; + +} // namespace gpio + +class GPIOPin { + public: + virtual void setup() = 0; + + virtual void pin_mode(gpio::Flags flags) = 0; + + /** + * @brief Retrieve GPIO pin flags. + * + * @return The GPIO flags describing the pin mode and properties. + */ + virtual gpio::Flags get_flags() const = 0; + + virtual bool digital_read() = 0; + + virtual void digital_write(bool value) = 0; + + /// Write a summary of this pin to the provided buffer. + /// @param buffer The buffer to write to + /// @param len The size of the buffer (must be > 0) + /// @return The number of characters that would be written (excluding null terminator), + /// which may exceed len-1 if truncation occurred (snprintf semantics) + virtual size_t dump_summary(char *buffer, size_t len) const; + + /// Only the platform pin class behind the InternalGPIOPin alias may return true: + /// callers static_cast to InternalGPIOPin on it. + virtual bool is_internal() { return false; } +#ifdef USE_GPIO_HOLD + bool get_hold() const { return this->get_flags() & gpio::FLAG_HOLD; } + virtual bool is_held() const { return false; } +#else + bool get_hold() const { return false; } + bool is_held() const { return false; } +#endif +}; + +/// Copy of GPIOPin that is safe to use from ISRs (with no virtual functions) +class ISRInternalGPIOPin { + public: + ISRInternalGPIOPin() = default; + ISRInternalGPIOPin(void *arg) : arg_(arg) {} + bool digital_read(); + void digital_write(bool value); + void clear_interrupt(); + void pin_mode(gpio::Flags flags); + + protected: + void *arg_{nullptr}; +}; + +// What callers of the InternalGPIOPin alias (bound in esphome/core/gpio.h) rely on beyond +// GPIOPin. The class must be final so calls to the GPIOPin virtuals through the alias are direct. +// int is arbitrary: any T * exercises the public attach_interrupt template. +template +concept InternalGPIOPinContract = std::derived_from && std::is_final_v && + requires(const T pin, void (*func)(int *), int *arg, gpio::InterruptType type) { + { pin.get_pin() } -> std::same_as; + { pin.is_inverted() } -> std::same_as; + { pin.to_isr() } -> std::same_as; + { pin.detach_interrupt() } -> std::same_as; + { pin.attach_interrupt(func, arg, type) } -> std::same_as; +}; + +// Inline default implementation for GPIOPin::dump_summary. +// Writes an empty summary; subclasses override to provide pin details. +inline size_t GPIOPin::dump_summary(char *buffer, size_t len) const { + if (len > 0) + buffer[0] = '\0'; + return 0; +} + +// Inline helper for log_pin - allows compiler to inline into log_pin in gpio.cpp +inline void log_pin_with_prefix(const char *tag, const char *prefix, GPIOPin *pin) { + char buffer[GPIO_SUMMARY_MAX_LEN]; + size_t len = pin->dump_summary(buffer, sizeof(buffer)); + len = std::min(len, sizeof(buffer) - 1); + esp_log_printf_(ESPHOME_LOG_LEVEL_CONFIG, tag, __LINE__, "%s%.*s", prefix, (int) len, buffer); +} + +// log_pin function declarations - implementation in gpio.cpp +#ifdef USE_ESP8266 +void log_pin(const char *tag, const __FlashStringHelper *prefix, GPIOPin *pin); +#else +void log_pin(const char *tag, const char *prefix, GPIOPin *pin); +#endif + +} // namespace esphome diff --git a/tests/components/deep_sleep/test-timer-only.esp32-idf.yaml b/tests/components/deep_sleep/test-timer-only.esp32-idf.yaml new file mode 100644 index 0000000000..ad93ea225f --- /dev/null +++ b/tests/components/deep_sleep/test-timer-only.esp32-idf.yaml @@ -0,0 +1,4 @@ +# No pin anywhere in the config: the pin class methods deep_sleep calls must still link +deep_sleep: + run_duration: 10s + sleep_duration: 50s diff --git a/tests/components/power_supply/test_setup_priority.cpp b/tests/components/power_supply/test_setup_priority.cpp index 401fc72654..736fdf99b0 100644 --- a/tests/components/power_supply/test_setup_priority.cpp +++ b/tests/components/power_supply/test_setup_priority.cpp @@ -1,3 +1,4 @@ +#ifdef USE_HOST #include #include "esphome/components/power_supply/power_supply.h" @@ -6,28 +7,9 @@ namespace esphome::power_supply::testing { -// Minimal dummy internal GPIO pin implementation for testing -class DummyInternalPin : public InternalGPIOPin { - public: - DummyInternalPin() = default; - void setup() override {} - void pin_mode(esphome::gpio::Flags) override {} - esphome::gpio::Flags get_flags() const override { return esphome::gpio::FLAG_NONE; } - bool digital_read() override { return false; } - void digital_write(bool) override {} - void detach_interrupt() const override {} - ISRInternalGPIOPin to_isr() const override { return ISRInternalGPIOPin(); } - uint8_t get_pin() const override { return 0; } - bool is_inverted() const override { return false; } - - protected: - // Implement protected attach_interrupt required by InternalGPIOPin - void attach_interrupt(void (*func)(void *), void *arg, esphome::gpio::InterruptType type) const override {} -}; - TEST(PowerSupply, HasHigherPriorityThanBusWhenInternalAndEnableOnBoot) { power_supply::PowerSupply ps; - DummyInternalPin pin; + InternalGPIOPin pin; ps.set_pin(&pin); ps.set_enable_on_boot(true); @@ -37,7 +19,7 @@ TEST(PowerSupply, HasHigherPriorityThanBusWhenInternalAndEnableOnBoot) { TEST(PowerSupply, FallsBackToIOWhenNotEnableOnBoot) { power_supply::PowerSupply ps; - DummyInternalPin pin; + InternalGPIOPin pin; ps.set_pin(&pin); ps.set_enable_on_boot(false); @@ -45,3 +27,4 @@ TEST(PowerSupply, FallsBackToIOWhenNotEnableOnBoot) { } } // namespace esphome::power_supply::testing +#endif // USE_HOST From 59963dd246715a999feb13b7943beb64b6f40770 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:38:16 -1000 Subject: [PATCH 05/17] [espidf] Declare the map byproduct only when IDF has not (#20462) --- esphome/build_gen/espidf.py | 17 +++++++++++++---- tests/unit_tests/build_gen/test_espidf.py | 13 +++++++++++-- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/esphome/build_gen/espidf.py b/esphome/build_gen/espidf.py index 759d9a0bef..a826a2b628 100644 --- a/esphome/build_gen/espidf.py +++ b/esphome/build_gen/espidf.py @@ -425,15 +425,24 @@ project({CORE.name}) # Emit per-memory-type JSON size data for ESPHome to read post-build. # json2 stays small; raw dumps every symbol (~2s on a large map) and # this command runs inside the link edge, blocking everything downstream. -# The map is a BYPRODUCT so ninja knows the link writes it; IDF's size -# target depends on the map and can then be built in the same run as all. -# IDF's cmakev2 declares the map itself, so drop this line on that switch. +# IDF's size target depends on the map, so ninja has to know the link +# writes it for size to build in the same run as all. IDF declares the +# map itself since espressif/esp-idf#19201 (and in cmakev2), and ninja +# rejects two rules for one output, so declare it here only when IDF has +# not. add_custom_command(OUTPUT) marks its outputs GENERATED. +get_source_file_property(esphome_map_declared + ${{CMAKE_BINARY_DIR}}/${{CMAKE_PROJECT_NAME}}.map GENERATED) +if(esphome_map_declared) + set(esphome_map_byproducts) +else() + set(esphome_map_byproducts BYPRODUCTS ${{CMAKE_BINARY_DIR}}/${{CMAKE_PROJECT_NAME}}.map) +endif() add_custom_command( TARGET ${{CMAKE_PROJECT_NAME}}.elf POST_BUILD COMMAND ${{PYTHON}} -m esp_idf_size {size_ng_flag} --format=json2 -o ${{CMAKE_BINARY_DIR}}/esp_idf_size.json ${{CMAKE_PROJECT_NAME}}.map - BYPRODUCTS ${{CMAKE_BINARY_DIR}}/${{CMAKE_PROJECT_NAME}}.map + ${{esphome_map_byproducts}} WORKING_DIRECTORY ${{CMAKE_BINARY_DIR}} VERBATIM ) diff --git a/tests/unit_tests/build_gen/test_espidf.py b/tests/unit_tests/build_gen/test_espidf.py index c439b87d16..9d92988da0 100644 --- a/tests/unit_tests/build_gen/test_espidf.py +++ b/tests/unit_tests/build_gen/test_espidf.py @@ -242,9 +242,18 @@ def test_get_project_cmakelists_drops_empty_lwip_sources() -> None: def test_get_project_cmakelists_declares_map_as_link_byproduct() -> None: - """The link declares the map so size can build in the same ninja run.""" + """The map is a link byproduct only when IDF has not declared it itself.""" content = _render() - assert "BYPRODUCTS ${CMAKE_BINARY_DIR}/${CMAKE_PROJECT_NAME}.map" in content + assert ( + "get_source_file_property(esphome_map_declared\n" + " ${CMAKE_BINARY_DIR}/${CMAKE_PROJECT_NAME}.map GENERATED)" + ) in content + assert ( + "set(esphome_map_byproducts BYPRODUCTS " + "${CMAKE_BINARY_DIR}/${CMAKE_PROJECT_NAME}.map)" + ) in content + assert " ${esphome_map_byproducts}\n WORKING_DIRECTORY" in content + assert "BYPRODUCTS ${CMAKE_BINARY_DIR}/${CMAKE_PROJECT_NAME}.map\n" not in content def test_get_project_cmakelists_uses_supplied_builtin_components() -> None: From 95dc9a749b07a0250f8a1c34448e020efd888208 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:39:00 -1000 Subject: [PATCH 06/17] [display_menu_base] Build the menu item triggers with build_parent_callback_automation (#20443) --- .../components/display_menu_base/__init__.py | 128 ++++-------------- .../components/display_menu_base/automation.h | 58 -------- 2 files changed, 30 insertions(+), 156 deletions(-) delete mode 100644 esphome/components/display_menu_base/automation.h diff --git a/esphome/components/display_menu_base/__init__.py b/esphome/components/display_menu_base/__init__.py index 9d046dbc19..791242cda1 100644 --- a/esphome/components/display_menu_base/__init__.py +++ b/esphome/components/display_menu_base/__init__.py @@ -20,9 +20,10 @@ from esphome.const import ( CONF_ON_VALUE, CONF_SWITCH, CONF_TEXT, - CONF_TRIGGER_ID, CONF_TYPE, ) +from esphome.cpp_generator import MockObj +from esphome.types import ConfigType CODEOWNERS = ["@numo68"] DOMAIN = "display_menu_base" @@ -86,26 +87,6 @@ MENU_MODES = { CONF_JOYSTICK: MenuMode.MENU_MODE_JOYSTICK, } -DisplayMenuOnEnterTrigger = display_menu_base_ns.class_( - "DisplayMenuOnEnterTrigger", automation.Trigger -) - -DisplayMenuOnLeaveTrigger = display_menu_base_ns.class_( - "DisplayMenuOnLeaveTrigger", automation.Trigger -) - -DisplayMenuOnValueTrigger = display_menu_base_ns.class_( - "DisplayMenuOnValueTrigger", automation.Trigger -) - -DisplayMenuOnNextTrigger = display_menu_base_ns.class_( - "DisplayMenuOnNextTrigger", automation.Trigger -) - -DisplayMenuOnPrevTrigger = display_menu_base_ns.class_( - "DisplayMenuOnPrevTrigger", automation.Trigger -) - def validate_format(format): if re.search(r"^%[+-]*(\d+)?(\.\d+)?[fg]$", format) is None: @@ -129,44 +110,20 @@ MENU_ITEM_COMMON_SCHEMA = cv.Schema( MENU_ITEM_ENTER_LEAVE_SCHEMA = MENU_ITEM_COMMON_SCHEMA.extend( { - cv.Optional(CONF_ON_ENTER): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id( - DisplayMenuOnEnterTrigger - ), - } - ), - cv.Optional(CONF_ON_LEAVE): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id( - DisplayMenuOnLeaveTrigger - ), - } - ), + cv.Optional(CONF_ON_ENTER): automation.validate_automation({}), + cv.Optional(CONF_ON_LEAVE): automation.validate_automation({}), } ) MENU_ITEM_VALUE_SCHEMA = MENU_ITEM_COMMON_SCHEMA.extend( { - cv.Optional(CONF_ON_VALUE): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id( - DisplayMenuOnValueTrigger - ), - } - ), + cv.Optional(CONF_ON_VALUE): automation.validate_automation({}), } ) MENU_ITEM_ENTER_LEAVE_VALUE_SCHEMA = MENU_ITEM_ENTER_LEAVE_SCHEMA.extend( { - cv.Optional(CONF_ON_VALUE): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id( - DisplayMenuOnValueTrigger - ), - } - ), + cv.Optional(CONF_ON_VALUE): automation.validate_automation({}), } ) @@ -230,20 +187,8 @@ MENU_ITEM_SCHEMA = cv.typed_schema( cv.GenerateID(CONF_ID): cv.declare_id(MenuItemCustom), cv.Optional(CONF_IMMEDIATE_EDIT, default=False): cv.boolean, cv.Optional(CONF_VALUE_LAMBDA): cv.returning_lambda, - cv.Optional(CONF_ON_NEXT): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id( - DisplayMenuOnNextTrigger - ), - } - ), - cv.Optional(CONF_ON_PREV): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id( - DisplayMenuOnPrevTrigger - ), - } - ), + cv.Optional(CONF_ON_NEXT): automation.validate_automation({}), + cv.Optional(CONF_ON_PREV): automation.validate_automation({}), } ), }, @@ -256,20 +201,8 @@ DISPLAY_MENU_BASE_SCHEMA = cv.Schema( cv.Optional(CONF_ACTIVE, default=True): cv.boolean, cv.GenerateID(CONF_ROOT_ITEM_ID): cv.declare_id(MenuItemMenu), cv.Optional(CONF_MODE, default=CONF_ROTARY): cv.enum(MENU_MODES), - cv.Optional(CONF_ON_ENTER): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id( - DisplayMenuOnEnterTrigger - ), - } - ), - cv.Optional(CONF_ON_LEAVE): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id( - DisplayMenuOnLeaveTrigger - ), - } - ), + cv.Optional(CONF_ON_ENTER): automation.validate_automation({}), + cv.Optional(CONF_ON_LEAVE): automation.validate_automation({}), cv.Required(CONF_ITEMS): cv.All( cv.ensure_list(MENU_ITEM_SCHEMA), cv.Length(min=1) ), @@ -302,6 +235,24 @@ automation.register_apply_condition( ) +# Each item callback carries nothing; the automation receives the item itself. +_ITEM_CALLBACKS = ( + (CONF_ON_ENTER, "add_on_enter_callback"), + (CONF_ON_LEAVE, "add_on_leave_callback"), + (CONF_ON_VALUE, "add_on_value_callback"), + (CONF_ON_NEXT, "add_on_next_callback"), + (CONF_ON_PREV, "add_on_prev_callback"), +) + + +async def _build_item_automations(item: MockObj, config: ConfigType) -> None: + for conf_key, callback_method in _ITEM_CALLBACKS: + for conf in config.get(conf_key, []): + await automation.build_parent_callback_automation( + item, callback_method, (MenuItemConstPtr, "it"), conf + ) + + async def menu_item_to_code(menu, config, parent): if config[CONF_TYPE] in MENU_ITEMS_WITH_SPECIALIZED_CLASSES: item = cg.new_Pvariable(config[CONF_ID]) @@ -340,21 +291,7 @@ async def menu_item_to_code(menu, config, parent): cg.add(item.set_switch_variable(var)) cg.add(item.set_on_text(config[CONF_ON_TEXT])) cg.add(item.set_off_text(config[CONF_OFF_TEXT])) - for conf in config.get(CONF_ON_ENTER, []): - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], item) - await automation.build_automation(trigger, [(MenuItemConstPtr, "it")], conf) - for conf in config.get(CONF_ON_LEAVE, []): - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], item) - await automation.build_automation(trigger, [(MenuItemConstPtr, "it")], conf) - for conf in config.get(CONF_ON_VALUE, []): - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], item) - await automation.build_automation(trigger, [(MenuItemConstPtr, "it")], conf) - for conf in config.get(CONF_ON_NEXT, []): - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], item) - await automation.build_automation(trigger, [(MenuItemConstPtr, "it")], conf) - for conf in config.get(CONF_ON_PREV, []): - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], item) - await automation.build_automation(trigger, [(MenuItemConstPtr, "it")], conf) + await _build_item_automations(item, config) async def display_menu_to_code(menu, config): @@ -364,9 +301,4 @@ async def display_menu_to_code(menu, config): cg.add(menu.set_mode(config[CONF_MODE])) for c in config[CONF_ITEMS]: await menu_item_to_code(menu, c, root_item) - for conf in config.get(CONF_ON_ENTER, []): - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], root_item) - await automation.build_automation(trigger, [(MenuItemConstPtr, "it")], conf) - for conf in config.get(CONF_ON_LEAVE, []): - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], root_item) - await automation.build_automation(trigger, [(MenuItemConstPtr, "it")], conf) + await _build_item_automations(root_item, config) diff --git a/esphome/components/display_menu_base/automation.h b/esphome/components/display_menu_base/automation.h deleted file mode 100644 index 477a971d47..0000000000 --- a/esphome/components/display_menu_base/automation.h +++ /dev/null @@ -1,58 +0,0 @@ -#pragma once - -#include "esphome/core/automation.h" -#include "display_menu_base.h" - -namespace esphome::display_menu_base { - -class DisplayMenuOnEnterTrigger final : public Trigger { - public: - explicit DisplayMenuOnEnterTrigger(MenuItem *parent) : parent_(parent) { - parent->add_on_enter_callback([this]() { this->trigger(this->parent_); }); - } - - protected: - MenuItem *parent_; -}; - -class DisplayMenuOnLeaveTrigger final : public Trigger { - public: - explicit DisplayMenuOnLeaveTrigger(MenuItem *parent) : parent_(parent) { - parent->add_on_leave_callback([this]() { this->trigger(this->parent_); }); - } - - protected: - MenuItem *parent_; -}; - -class DisplayMenuOnValueTrigger final : public Trigger { - public: - explicit DisplayMenuOnValueTrigger(MenuItem *parent) : parent_(parent) { - parent->add_on_value_callback([this]() { this->trigger(this->parent_); }); - } - - protected: - MenuItem *parent_; -}; - -class DisplayMenuOnNextTrigger final : public Trigger { - public: - explicit DisplayMenuOnNextTrigger(MenuItemCustom *parent) : parent_(parent) { - parent->add_on_next_callback([this]() { this->trigger(this->parent_); }); - } - - protected: - MenuItemCustom *parent_; -}; - -class DisplayMenuOnPrevTrigger final : public Trigger { - public: - explicit DisplayMenuOnPrevTrigger(MenuItemCustom *parent) : parent_(parent) { - parent->add_on_prev_callback([this]() { this->trigger(this->parent_); }); - } - - protected: - MenuItemCustom *parent_; -}; - -} // namespace esphome::display_menu_base From f1a23a02c24203c306c3a7c9f970abe71ee5e1cf Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:39:06 -1000 Subject: [PATCH 07/17] [time] Build on_time_sync with build_callback_automation (#20441) --- esphome/components/time/__init__.py | 23 +++++++------------ esphome/components/time/automation.cpp | 4 ---- esphome/components/time/automation.h | 8 ------- tests/component_tests/time/test_triggers.py | 6 ++--- .../time/test-sync.esp8266-ard.yaml | 8 +++++++ 5 files changed, 19 insertions(+), 30 deletions(-) create mode 100644 tests/components/time/test-sync.esp8266-ard.yaml diff --git a/esphome/components/time/__init__.py b/esphome/components/time/__init__.py index cedb9b3c9b..a017109e7d 100644 --- a/esphome/components/time/__init__.py +++ b/esphome/components/time/__init__.py @@ -46,7 +46,6 @@ DOMAIN = "time" time_ns = cg.esphome_ns.namespace("time") RealTimeClock = time_ns.class_("RealTimeClock", cg.PollingComponent) CronTrigger = time_ns.class_("CronTrigger", automation.Trigger.template(), cg.Component) -SyncTrigger = time_ns.class_("SyncTrigger", automation.Trigger.template(), cg.Component) # C++ types for pre-parsed timezone struct generation DSTRuleType_cpp = time_ns.enum("DSTRuleType", is_class=True) @@ -357,11 +356,7 @@ TIME_SCHEMA = cv.Schema( }, validate_cron_keys, ), - cv.Optional(CONF_ON_TIME_SYNC): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(SyncTrigger), - } - ), + cv.Optional(CONF_ON_TIME_SYNC): automation.validate_automation({}), } ).extend( # ``visibility=ADVANCED`` flags the inherited ``update_interval`` @@ -424,8 +419,7 @@ async def setup_time_core_(time_var, config): _emit_parsed_timezone_fields(parsed) on_time = config.get(CONF_ON_TIME, []) - on_time_sync = config.get(CONF_ON_TIME_SYNC, []) - if on_time or on_time_sync: + if on_time: cg.add_define("USE_TIME_TRIGGERS") for conf in on_time: @@ -447,11 +441,10 @@ async def setup_time_core_(time_var, config): await cg.register_component(trigger, conf) await automation.build_automation(trigger, [], conf) - for conf in on_time_sync: - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], time_var) - - await cg.register_component(trigger, conf) - await automation.build_automation(trigger, [], conf) + for conf in config.get(CONF_ON_TIME_SYNC, []): + await automation.build_callback_automation( + time_var, "add_on_time_sync_callback", [], conf + ) async def register_time(time_var, config): @@ -478,8 +471,8 @@ automation.register_apply_condition( # posix_tz.cpp is fully #ifdef'd on USE_TIME_TIMEZONE, set only when a -# timezone is configured or detected; automation.cpp holds the on_time and -# on_time_sync triggers and is #ifdef'd on USE_TIME_TRIGGERS. +# timezone is configured or detected; automation.cpp holds the on_time +# trigger and is #ifdef'd on USE_TIME_TRIGGERS. FILTER_SOURCE_FILES = filter_source_files_from_defines( { "posix_tz.cpp": "USE_TIME_TIMEZONE", diff --git a/esphome/components/time/automation.cpp b/esphome/components/time/automation.cpp index 26956703e7..1c3d462566 100644 --- a/esphome/components/time/automation.cpp +++ b/esphome/components/time/automation.cpp @@ -94,10 +94,6 @@ void CronTrigger::add_days_of_week(const std::vector &days_of_week) { } float CronTrigger::get_setup_priority() const { return setup_priority::HARDWARE; } -SyncTrigger::SyncTrigger(RealTimeClock *rtc) : rtc_(rtc) { - rtc->add_on_time_sync_callback([this]() { this->trigger(); }); -} - } // namespace esphome::time #endif // USE_TIME_TRIGGERS diff --git a/esphome/components/time/automation.h b/esphome/components/time/automation.h index 5f980690a4..5e70000ac3 100644 --- a/esphome/components/time/automation.h +++ b/esphome/components/time/automation.h @@ -43,14 +43,6 @@ class CronTrigger final : public Trigger<>, public Component { RealTimeClock *rtc_; optional last_check_; }; - -class SyncTrigger final : public Trigger<>, public Component { - public: - explicit SyncTrigger(RealTimeClock *rtc); - - protected: - RealTimeClock *rtc_; -}; } // namespace esphome::time #endif // USE_TIME_TRIGGERS diff --git a/tests/component_tests/time/test_triggers.py b/tests/component_tests/time/test_triggers.py index 34fd5a3157..d6c3eed584 100644 --- a/tests/component_tests/time/test_triggers.py +++ b/tests/component_tests/time/test_triggers.py @@ -1,5 +1,5 @@ -"""automation.cpp (CronTrigger and SyncTrigger) is only compiled when an -on_time or on_time_sync automation exists, so the define must follow them.""" +"""automation.cpp (CronTrigger) is only compiled when an on_time automation +exists, so the define must follow it; on_time_sync needs no class.""" from collections.abc import Callable from pathlib import Path @@ -14,7 +14,7 @@ from esphome.core import CORE [ ("no_triggers.yaml", False), ("on_time.yaml", True), - ("on_time_sync.yaml", True), + ("on_time_sync.yaml", False), ], ) def test_triggers_define_follows_automations( diff --git a/tests/components/time/test-sync.esp8266-ard.yaml b/tests/components/time/test-sync.esp8266-ard.yaml new file mode 100644 index 0000000000..6240fcd73e --- /dev/null +++ b/tests/components/time/test-sync.esp8266-ard.yaml @@ -0,0 +1,8 @@ +wifi: + ssid: MySSID + password: password1 + +time: + - platform: sntp + on_time_sync: + - logger.log: Time synchronized From e8943975035e93923830820428b06b3677bab8f9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:39:25 -1000 Subject: [PATCH 08/17] [datetime] Build on_value with build_callback_automation (#20445) --- esphome/components/datetime/__init__.py | 20 +++++++++---------- esphome/components/datetime/datetime_base.h | 10 ---------- .../components/datetime/common-triggers.yaml | 10 ++++++++++ .../datetime/test-triggers.esp8266-ard.yaml | 3 +++ 4 files changed, 23 insertions(+), 20 deletions(-) create mode 100644 tests/components/datetime/common-triggers.yaml create mode 100644 tests/components/datetime/test-triggers.esp8266-ard.yaml diff --git a/esphome/components/datetime/__init__.py b/esphome/components/datetime/__init__.py index de79c05a9f..20fd258cb0 100644 --- a/esphome/components/datetime/__init__.py +++ b/esphome/components/datetime/__init__.py @@ -43,9 +43,6 @@ DateEntity = datetime_ns.class_("DateEntity", DateTimeBase) TimeEntity = datetime_ns.class_("TimeEntity", DateTimeBase) DateTimeEntity = datetime_ns.class_("DateTimeEntity", DateTimeBase) -DateTimeStateTrigger = datetime_ns.class_( - "DateTimeStateTrigger", automation.Trigger.template(cg.ESPTime) -) OnTimeTrigger = datetime_ns.class_( "OnTimeTrigger", automation.Trigger, cg.Component, cg.Parented.template(TimeEntity) @@ -75,11 +72,7 @@ def _validate_time_present(config: ConfigType) -> ConfigType: _DATETIME_SCHEMA = cv.ENTITY_BASE_SCHEMA.extend( cv.Schema( { - cv.Optional(CONF_ON_VALUE): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(DateTimeStateTrigger), - } - ), + cv.Optional(CONF_ON_VALUE): automation.validate_automation({}), cv.Optional(CONF_TIME_ID): cv.use_id(time.RealTimeClock), } ) @@ -144,9 +137,16 @@ async def setup_datetime_core_(var: MockObj, config: ConfigType) -> None: await mqtt.register_mqtt_component(mqtt_, config) if web_server_config := config.get(CONF_WEB_SERVER): await web_server.add_entity_config(var, web_server_config) + # The state callback carries nothing; the automation gets the value as an ESPTime. for conf in config.get(CONF_ON_VALUE, []): - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) - await automation.build_automation(trigger, [(cg.ESPTime, "x")], conf) + await automation.build_callback_automation( + var, + "add_on_state_callback", + [(cg.ESPTime, "x")], + conf, + params=[], + forward=[automation.parent_ref(var).state_as_esptime()], + ) if CONF_TIME_ID in config: rtc = await cg.get_variable(config[CONF_TIME_ID]) diff --git a/esphome/components/datetime/datetime_base.h b/esphome/components/datetime/datetime_base.h index f99debb692..fdbb193006 100644 --- a/esphome/components/datetime/datetime_base.h +++ b/esphome/components/datetime/datetime_base.h @@ -31,14 +31,4 @@ class DateTimeBase : public EntityBase { #endif }; -class DateTimeStateTrigger final : public Trigger { - public: - explicit DateTimeStateTrigger(DateTimeBase *parent) : parent_(parent) { - parent->add_on_state_callback([this]() { this->trigger(this->parent_->state_as_esptime()); }); - } - - protected: - DateTimeBase *parent_; -}; - } // namespace esphome::datetime diff --git a/tests/components/datetime/common-triggers.yaml b/tests/components/datetime/common-triggers.yaml new file mode 100644 index 0000000000..e93e8b2b51 --- /dev/null +++ b/tests/components/datetime/common-triggers.yaml @@ -0,0 +1,10 @@ +datetime: + - platform: template + name: Datetime Trigger Test + id: datetime_trigger_test + type: datetime + optimistic: true + on_value: + - logger.log: + format: "New value %04d-%02d-%02d" + args: [x.year, x.month, x.day_of_month] diff --git a/tests/components/datetime/test-triggers.esp8266-ard.yaml b/tests/components/datetime/test-triggers.esp8266-ard.yaml new file mode 100644 index 0000000000..3829bedb1a --- /dev/null +++ b/tests/components/datetime/test-triggers.esp8266-ard.yaml @@ -0,0 +1,3 @@ +packages: + datetime: !include common.yaml + triggers: !include common-triggers.yaml From bc44e8b5504fc01775355d443bea0d5be5cc7b70 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:40:40 -1000 Subject: [PATCH 09/17] [display] Test on_page_change with each from and to combination (#20457) --- .../display/common-page-change.yaml | 26 +++++++++++++++++++ .../display/test-page-change.esp32-idf.yaml | 6 +++++ 2 files changed, 32 insertions(+) create mode 100644 tests/components/display/common-page-change.yaml create mode 100644 tests/components/display/test-page-change.esp32-idf.yaml diff --git a/tests/components/display/common-page-change.yaml b/tests/components/display/common-page-change.yaml new file mode 100644 index 0000000000..649cad767b --- /dev/null +++ b/tests/components/display/common-page-change.yaml @@ -0,0 +1,26 @@ +display: + - platform: ssd1306_i2c + i2c_id: i2c_bus + model: SSD1306_128X64 + reset_pin: ${reset_pin} + id: display_page_change_display + pages: + - id: display_page_change_page1 + lambda: |- + it.rectangle(0, 0, it.get_width(), it.get_height()); + - id: display_page_change_page2 + lambda: |- + it.rectangle(0, 0, 10, 10); + on_page_change: + - then: + lambda: 'ESP_LOGD("display", "any change, %p %p", from, to);' + - from: display_page_change_page1 + then: + lambda: 'ESP_LOGD("display", "left page 1, %p %p", from, to);' + - to: display_page_change_page1 + then: + lambda: 'ESP_LOGD("display", "entered page 1, %p %p", from, to);' + - from: display_page_change_page1 + to: display_page_change_page2 + then: + lambda: 'ESP_LOGD("display", "1 -> 2, %p %p", from, to);' diff --git a/tests/components/display/test-page-change.esp32-idf.yaml b/tests/components/display/test-page-change.esp32-idf.yaml new file mode 100644 index 0000000000..b44cc076b2 --- /dev/null +++ b/tests/components/display/test-page-change.esp32-idf.yaml @@ -0,0 +1,6 @@ +substitutions: + reset_pin: GPIO15 + +packages: + i2c: !include ../../test_build_components/common/i2c/esp32-idf.yaml + display: !include common-page-change.yaml From fdc163a30d764045ce16d0a5f7cbb88f29bbeaf3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:42:43 -1000 Subject: [PATCH 10/17] [tuya] Build on_datapoint_update with build_trigger_callback (#20453) --- esphome/components/tuya/__init__.py | 85 +++++++------------------- esphome/components/tuya/automation.cpp | 65 -------------------- esphome/components/tuya/automation.h | 53 ---------------- esphome/components/tuya/tuya.cpp | 19 ++++-- esphome/components/tuya/tuya.h | 10 ++- tests/components/tuya/common.yaml | 45 +++++++++++++- 6 files changed, 87 insertions(+), 190 deletions(-) delete mode 100644 esphome/components/tuya/automation.cpp delete mode 100644 esphome/components/tuya/automation.h diff --git a/esphome/components/tuya/__init__.py b/esphome/components/tuya/__init__.py index 6957b57e0e..bf392c7826 100644 --- a/esphome/components/tuya/__init__.py +++ b/esphome/components/tuya/__init__.py @@ -2,7 +2,8 @@ from esphome import automation, pins import esphome.codegen as cg from esphome.components import time, uart import esphome.config_validation as cv -from esphome.const import CONF_ID, CONF_SENSOR_DATAPOINT, CONF_TIME_ID, CONF_TRIGGER_ID +from esphome.const import CONF_ID, CONF_SENSOR_DATAPOINT, CONF_TIME_ID +from esphome.cpp_generator import MockObj DEPENDENCIES = ["uart"] DOMAIN = "tuya" @@ -17,6 +18,8 @@ tuya_ns = cg.esphome_ns.namespace("tuya") TuyaDatapointType = tuya_ns.enum("TuyaDatapointType", is_class=True) Tuya = tuya_ns.class_("Tuya", cg.Component, uart.UARTDevice) +TuyaDatapoint = tuya_ns.struct("TuyaDatapoint") + DPTYPE_ANY = "any" DPTYPE_RAW = "raw" DPTYPE_BOOL = "bool" @@ -26,60 +29,18 @@ DPTYPE_STRING = "string" DPTYPE_ENUM = "enum" DPTYPE_BITMASK = "bitmask" +# Automation argument type, plus the expected TuyaDatapointType and the field forwarded DATAPOINT_TYPES = { - DPTYPE_ANY: tuya_ns.struct("TuyaDatapoint"), - DPTYPE_RAW: cg.std_vector.template(cg.uint8), - DPTYPE_BOOL: cg.bool_, - DPTYPE_INT: cg.int_, - DPTYPE_UINT: cg.uint32, - DPTYPE_STRING: cg.std_string, - DPTYPE_ENUM: cg.uint8, - DPTYPE_BITMASK: cg.uint32, + DPTYPE_ANY: (TuyaDatapoint, None, None), + DPTYPE_RAW: (cg.std_vector.template(cg.uint8), TuyaDatapointType.RAW, "value_raw"), + DPTYPE_BOOL: (cg.bool_, TuyaDatapointType.BOOLEAN, "value_bool"), + DPTYPE_INT: (cg.int_, TuyaDatapointType.INTEGER, "value_int"), + DPTYPE_UINT: (cg.uint32, TuyaDatapointType.INTEGER, "value_uint"), + DPTYPE_STRING: (cg.std_string, TuyaDatapointType.STRING, "value_string"), + DPTYPE_ENUM: (cg.uint8, TuyaDatapointType.ENUM, "value_enum"), + DPTYPE_BITMASK: (cg.uint32, TuyaDatapointType.BITMASK, "value_bitmask"), } -DATAPOINT_TRIGGERS = { - DPTYPE_ANY: tuya_ns.class_( - "TuyaDatapointUpdateTrigger", - automation.Trigger.template(DATAPOINT_TYPES[DPTYPE_ANY]), - ), - DPTYPE_RAW: tuya_ns.class_( - "TuyaRawDatapointUpdateTrigger", - automation.Trigger.template(DATAPOINT_TYPES[DPTYPE_RAW]), - ), - DPTYPE_BOOL: tuya_ns.class_( - "TuyaBoolDatapointUpdateTrigger", - automation.Trigger.template(DATAPOINT_TYPES[DPTYPE_BOOL]), - ), - DPTYPE_INT: tuya_ns.class_( - "TuyaIntDatapointUpdateTrigger", - automation.Trigger.template(DATAPOINT_TYPES[DPTYPE_INT]), - ), - DPTYPE_UINT: tuya_ns.class_( - "TuyaUIntDatapointUpdateTrigger", - automation.Trigger.template(DATAPOINT_TYPES[DPTYPE_UINT]), - ), - DPTYPE_STRING: tuya_ns.class_( - "TuyaStringDatapointUpdateTrigger", - automation.Trigger.template(DATAPOINT_TYPES[DPTYPE_STRING]), - ), - DPTYPE_ENUM: tuya_ns.class_( - "TuyaEnumDatapointUpdateTrigger", - automation.Trigger.template(DATAPOINT_TYPES[DPTYPE_ENUM]), - ), - DPTYPE_BITMASK: tuya_ns.class_( - "TuyaBitmaskDatapointUpdateTrigger", - automation.Trigger.template(DATAPOINT_TYPES[DPTYPE_BITMASK]), - ), -} - - -def assign_declare_id(value): - value = value.copy() - value[CONF_TRIGGER_ID] = cv.declare_id( - DATAPOINT_TRIGGERS[value[CONF_DATAPOINT_TYPE]] - )(value[CONF_TRIGGER_ID].id) - return value - CONF_TUYA_ID = "tuya_id" CONFIG_SCHEMA = ( @@ -93,15 +54,11 @@ CONFIG_SCHEMA = ( cv.Optional(CONF_STATUS_PIN): pins.gpio_output_pin_schema, cv.Optional(CONF_ON_DATAPOINT_UPDATE): automation.validate_automation( { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id( - DATAPOINT_TRIGGERS[DPTYPE_ANY] - ), cv.Required(CONF_SENSOR_DATAPOINT): cv.uint8_t, cv.Optional(CONF_DATAPOINT_TYPE, default=DPTYPE_ANY): cv.one_of( - *DATAPOINT_TRIGGERS, lower=True + *DATAPOINT_TYPES, lower=True ), - }, - extra_validators=assign_declare_id, + } ), } ) @@ -124,9 +81,11 @@ async def to_code(config): for dp in config[CONF_IGNORE_MCU_UPDATE_ON_DATAPOINTS]: cg.add(var.add_ignore_mcu_update_on_datapoints(dp)) for conf in config.get(CONF_ON_DATAPOINT_UPDATE, []): - trigger = cg.new_Pvariable( - conf[CONF_TRIGGER_ID], var, conf[CONF_SENSOR_DATAPOINT] - ) - await automation.build_automation( - trigger, [(DATAPOINT_TYPES[conf[CONF_DATAPOINT_TYPE]], "x")], conf + type_, expected, field = DATAPOINT_TYPES[conf[CONF_DATAPOINT_TYPE]] + forward = None + if expected is not None: + forward = [getattr(MockObj("x", ".").expect_type(expected), field)] + callback = await automation.build_trigger_callback( + [(type_, "x")], conf, params=[(TuyaDatapoint, "x")], forward=forward ) + cg.add(var.register_listener(conf[CONF_SENSOR_DATAPOINT], callback)) diff --git a/esphome/components/tuya/automation.cpp b/esphome/components/tuya/automation.cpp deleted file mode 100644 index a7eacf8e9a..0000000000 --- a/esphome/components/tuya/automation.cpp +++ /dev/null @@ -1,65 +0,0 @@ -#include "esphome/core/log.h" - -#include "automation.h" - -ESPHOME_LOG_TAG(TAG, "tuya.automation"); - -namespace esphome::tuya { - -void check_expected_datapoint(const TuyaDatapoint &dp, TuyaDatapointType expected) { - if (dp.type != expected) { - ESP_LOGW(TAG, "Tuya sensor %u expected datapoint type %#02hhX but got %#02hhX", dp.id, - static_cast(expected), static_cast(dp.type)); - } -} - -TuyaRawDatapointUpdateTrigger::TuyaRawDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id) { - parent->register_listener(sensor_id, [this](const TuyaDatapoint &dp) { - check_expected_datapoint(dp, TuyaDatapointType::RAW); - this->trigger(dp.value_raw); - }); -} - -TuyaBoolDatapointUpdateTrigger::TuyaBoolDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id) { - parent->register_listener(sensor_id, [this](const TuyaDatapoint &dp) { - check_expected_datapoint(dp, TuyaDatapointType::BOOLEAN); - this->trigger(dp.value_bool); - }); -} - -TuyaIntDatapointUpdateTrigger::TuyaIntDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id) { - parent->register_listener(sensor_id, [this](const TuyaDatapoint &dp) { - check_expected_datapoint(dp, TuyaDatapointType::INTEGER); - this->trigger(dp.value_int); - }); -} - -TuyaUIntDatapointUpdateTrigger::TuyaUIntDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id) { - parent->register_listener(sensor_id, [this](const TuyaDatapoint &dp) { - check_expected_datapoint(dp, TuyaDatapointType::INTEGER); - this->trigger(dp.value_uint); - }); -} - -TuyaStringDatapointUpdateTrigger::TuyaStringDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id) { - parent->register_listener(sensor_id, [this](const TuyaDatapoint &dp) { - check_expected_datapoint(dp, TuyaDatapointType::STRING); - this->trigger(dp.value_string); - }); -} - -TuyaEnumDatapointUpdateTrigger::TuyaEnumDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id) { - parent->register_listener(sensor_id, [this](const TuyaDatapoint &dp) { - check_expected_datapoint(dp, TuyaDatapointType::ENUM); - this->trigger(dp.value_enum); - }); -} - -TuyaBitmaskDatapointUpdateTrigger::TuyaBitmaskDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id) { - parent->register_listener(sensor_id, [this](const TuyaDatapoint &dp) { - check_expected_datapoint(dp, TuyaDatapointType::BITMASK); - this->trigger(dp.value_bitmask); - }); -} - -} // namespace esphome::tuya diff --git a/esphome/components/tuya/automation.h b/esphome/components/tuya/automation.h deleted file mode 100644 index 0cd63a76be..0000000000 --- a/esphome/components/tuya/automation.h +++ /dev/null @@ -1,53 +0,0 @@ -#pragma once - -#include "esphome/core/component.h" -#include "esphome/core/automation.h" -#include "tuya.h" - -#include - -namespace esphome::tuya { - -class TuyaDatapointUpdateTrigger final : public Trigger { - public: - explicit TuyaDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id) { - parent->register_listener(sensor_id, [this](const TuyaDatapoint &dp) { this->trigger(dp); }); - } -}; - -class TuyaRawDatapointUpdateTrigger final : public Trigger> { - public: - explicit TuyaRawDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id); -}; - -class TuyaBoolDatapointUpdateTrigger final : public Trigger { - public: - explicit TuyaBoolDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id); -}; - -class TuyaIntDatapointUpdateTrigger final : public Trigger { - public: - explicit TuyaIntDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id); -}; - -class TuyaUIntDatapointUpdateTrigger final : public Trigger { - public: - explicit TuyaUIntDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id); -}; - -class TuyaStringDatapointUpdateTrigger final : public Trigger { - public: - explicit TuyaStringDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id); -}; - -class TuyaEnumDatapointUpdateTrigger final : public Trigger { - public: - explicit TuyaEnumDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id); -}; - -class TuyaBitmaskDatapointUpdateTrigger final : public Trigger { - public: - explicit TuyaBitmaskDatapointUpdateTrigger(Tuya *parent, uint8_t sensor_id); -}; - -} // namespace esphome::tuya diff --git a/esphome/components/tuya/tuya.cpp b/esphome/components/tuya/tuya.cpp index 784aea5c50..b23dcc946b 100644 --- a/esphome/components/tuya/tuya.cpp +++ b/esphome/components/tuya/tuya.cpp @@ -482,7 +482,7 @@ void Tuya::handle_datapoints_(const uint8_t *buffer, size_t len) { // Run through listeners for (auto &listener : this->listeners_) { if (listener.datapoint_id == datapoint.id) - listener.on_datapoint(datapoint); + listener.on_datapoint.call(datapoint); } } } @@ -796,20 +796,27 @@ void Tuya::send_datapoint_command_(uint8_t datapoint_id, TuyaDatapointType datap this->send_command_(TuyaCommand{.cmd = TuyaCommandType::DATAPOINT_DELIVER, .payload = buffer}); } -void Tuya::register_listener(uint8_t datapoint_id, const std::function &func) { - auto listener = TuyaDatapointListener{ +void Tuya::register_listener_(uint8_t datapoint_id, Callback func) { + this->listeners_.push_back(TuyaDatapointListener{ .datapoint_id = datapoint_id, .on_datapoint = func, - }; - this->listeners_.push_back(listener); + }); // Run through existing datapoints for (auto &datapoint : this->datapoints_) { if (datapoint.id == datapoint_id) - func(datapoint); + func.call(datapoint); } } +const TuyaDatapoint &TuyaDatapoint::expect_type(TuyaDatapointType expected) const { + if (this->type != expected) { + ESP_LOGW(TAG, "Tuya sensor %u expected datapoint type %#02hhX but got %#02hhX", this->id, + static_cast(expected), static_cast(this->type)); + } + return *this; +} + TuyaInitState Tuya::get_init_state() { return this->init_state_; } } // namespace esphome::tuya diff --git a/esphome/components/tuya/tuya.h b/esphome/components/tuya/tuya.h index b8bf4e0ab1..f074d30162 100644 --- a/esphome/components/tuya/tuya.h +++ b/esphome/components/tuya/tuya.h @@ -37,11 +37,14 @@ struct TuyaDatapoint { }; std::string value_string; std::vector value_raw; + + /// Log a warning when this datapoint is not of type `expected`, and return it. + const TuyaDatapoint &expect_type(TuyaDatapointType expected) const; }; struct TuyaDatapointListener { uint8_t datapoint_id; - std::function on_datapoint; + Callback on_datapoint; }; enum class TuyaCommandType : uint8_t { @@ -91,7 +94,9 @@ class Tuya final : public Component, public uart::UARTDevice { void setup() override; void loop() override; void dump_config() override; - void register_listener(uint8_t datapoint_id, const std::function &func); + template void register_listener(uint8_t datapoint_id, F &&func) { + this->register_listener_(datapoint_id, Callback::create(std::forward(func))); + } void set_raw_datapoint_value(uint8_t datapoint_id, const std::vector &value); void set_boolean_datapoint_value(uint8_t datapoint_id, bool value); void set_integer_datapoint_value(uint8_t datapoint_id, uint32_t value); @@ -117,6 +122,7 @@ class Tuya final : public Component, public uart::UARTDevice { } protected: + void register_listener_(uint8_t datapoint_id, Callback func); void handle_char_(uint8_t c); void handle_datapoints_(const uint8_t *buffer, size_t len); optional get_datapoint_(uint8_t datapoint_id); diff --git a/tests/components/tuya/common.yaml b/tests/components/tuya/common.yaml index f52d47e7a0..1d0fc473ac 100644 --- a/tests/components/tuya/common.yaml +++ b/tests/components/tuya/common.yaml @@ -10,7 +10,50 @@ tuya: - sensor_datapoint: 6 datapoint_type: raw then: - - logger.log: Datapoint 6 updated + - logger.log: + format: "Datapoint 6 updated, %u bytes" + args: [x.size()] + - sensor_datapoint: 1 + then: + - logger.log: + format: "Datapoint %u updated" + args: [x.id] + - sensor_datapoint: 1 + datapoint_type: bool + then: + - logger.log: + format: "Datapoint 1 is %s" + args: ['x ? "on" : "off"'] + - sensor_datapoint: 3 + datapoint_type: int + then: + - logger.log: + format: "Datapoint 3 is %d" + args: [x] + - sensor_datapoint: 3 + datapoint_type: uint + then: + - logger.log: + format: "Datapoint 3 is %u" + args: [x] + - sensor_datapoint: 7 + datapoint_type: string + then: + - logger.log: + format: "Datapoint 7 is %s" + args: [x.c_str()] + - sensor_datapoint: 8 + datapoint_type: enum + then: + - logger.log: + format: "Datapoint 8 is %u" + args: [x] + - sensor_datapoint: 9 + datapoint_type: bitmask + then: + - logger.log: + format: "Datapoint 9 is %u" + args: [x] binary_sensor: - platform: tuya From 02bfa0781961e2ff1b0a78bc2dd9ea2a8907edae Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:42:55 -1000 Subject: [PATCH 11/17] [opentherm] Build before_send and before_process_response with build_callback_automation (#20450) --- esphome/components/opentherm/__init__.py | 44 ++++++++--------------- esphome/components/opentherm/automation.h | 23 ------------ tests/components/opentherm/common.yaml | 2 ++ 3 files changed, 16 insertions(+), 53 deletions(-) delete mode 100644 esphome/components/opentherm/automation.h diff --git a/esphome/components/opentherm/__init__.py b/esphome/components/opentherm/__init__.py index 439ec03245..cfcdfe7783 100644 --- a/esphome/components/opentherm/__init__.py +++ b/esphome/components/opentherm/__init__.py @@ -5,7 +5,7 @@ import esphome.codegen as cg from esphome.components import sensor from esphome.components.esp32 import include_builtin_idf_component import esphome.config_validation as cv -from esphome.const import CONF_ID, CONF_TRIGGER_ID, PLATFORM_ESP32, PLATFORM_ESP8266 +from esphome.const import CONF_ID, PLATFORM_ESP32, PLATFORM_ESP8266 from esphome.core import CORE from . import const, generate, schema, validate @@ -27,14 +27,16 @@ CONF_SYNC_MODE = "sync_mode" CONF_BEFORE_SEND = "before_send" CONF_BEFORE_PROCESS_RESPONSE = "before_process_response" -# Triggers -BeforeSendTrigger = generate.opentherm_ns.class_( - "BeforeSendTrigger", - automation.Trigger.template(generate.OpenthermData.operator("ref")), -) -BeforeProcessResponseTrigger = generate.opentherm_ns.class_( - "BeforeProcessResponseTrigger", - automation.Trigger.template(generate.OpenthermData.operator("ref")), +_DATA_ARGS = [(generate.OpenthermData.operator("ref"), "x")] +_CALLBACK_AUTOMATIONS = ( + automation.CallbackAutomation( + CONF_BEFORE_SEND, "add_on_before_send_callback", _DATA_ARGS + ), + automation.CallbackAutomation( + CONF_BEFORE_PROCESS_RESPONSE, + "add_on_before_process_response_callback", + _DATA_ARGS, + ), ) CONFIG_SCHEMA = cv.All( @@ -51,17 +53,9 @@ CONFIG_SCHEMA = cv.All( cv.Optional(CONF_SUMMER_MODE_ACTIVE, False): cv.boolean, cv.Optional(CONF_DHW_BLOCK, False): cv.boolean, cv.Optional(CONF_SYNC_MODE, False): cv.boolean, - cv.Optional(CONF_BEFORE_SEND): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(BeforeSendTrigger), - } - ), + cv.Optional(CONF_BEFORE_SEND): automation.validate_automation({}), cv.Optional(CONF_BEFORE_PROCESS_RESPONSE): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id( - BeforeProcessResponseTrigger - ), - } + {} ), } ) @@ -132,14 +126,4 @@ async def to_code(config: dict[str, Any]) -> None: generate.define_setting_readers(const.SETTING, settings) generate.add_messages(var, settings, schema.SETTINGS) - for conf in config.get(CONF_BEFORE_SEND, []): - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) - await automation.build_automation( - trigger, [(generate.OpenthermData.operator("ref"), "x")], conf - ) - - for conf in config.get(CONF_BEFORE_PROCESS_RESPONSE, []): - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) - await automation.build_automation( - trigger, [(generate.OpenthermData.operator("ref"), "x")], conf - ) + await automation.build_callback_automations(var, config, _CALLBACK_AUTOMATIONS) diff --git a/esphome/components/opentherm/automation.h b/esphome/components/opentherm/automation.h deleted file mode 100644 index 365992b280..0000000000 --- a/esphome/components/opentherm/automation.h +++ /dev/null @@ -1,23 +0,0 @@ -#pragma once - -#include "esphome/core/automation.h" -#include "hub.h" -#include "opentherm.h" - -namespace esphome::opentherm { - -class BeforeSendTrigger final : public Trigger { - public: - BeforeSendTrigger(OpenthermHub *hub) { - hub->add_on_before_send_callback([this](OpenthermData &x) { this->trigger(x); }); - } -}; - -class BeforeProcessResponseTrigger final : public Trigger { - public: - BeforeProcessResponseTrigger(OpenthermHub *hub) { - hub->add_on_before_process_response_callback([this](OpenthermData &x) { this->trigger(x); }); - } -}; - -} // namespace esphome::opentherm diff --git a/tests/components/opentherm/common.yaml b/tests/components/opentherm/common.yaml index fb5fb39eb8..1ae9ea4a4d 100644 --- a/tests/components/opentherm/common.yaml +++ b/tests/components/opentherm/common.yaml @@ -25,6 +25,8 @@ opentherm: then: - lambda: |- ESP_LOGW("OT", ">> Sending message %d", x.id); + if (x.id == 0) + x.valueHB |= 0x01; before_process_response: then: - lambda: |- From 073394b1eb50e265df13ba7bc8fac5aba54daa1c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:43:22 -1000 Subject: [PATCH 12/17] [core] Let a trigger filter name its parent with `{parent}` (#20448) --- esphome/automation.py | 39 +++++++++----- tests/unit_tests/test_automation.py | 83 ++++++++++++++++++++++++++--- 2 files changed, 102 insertions(+), 20 deletions(-) diff --git a/esphome/automation.py b/esphome/automation.py index 859ec2f1f3..6b6af4ed72 100644 --- a/esphome/automation.py +++ b/esphome/automation.py @@ -264,13 +264,17 @@ def string_ref_literal(config: ConfigType, value: str) -> str: return f"StringRef({literal_with_length(config, value)})" +def _names_parent(text: str) -> bool: + return any(f == "parent" for _, f, _, _ in string.Formatter().parse(text)) + + @dataclass(frozen=True) class ApplyCall: """One statement from config keys, e.g. ``"set_range({}, {})"`` with ``((CONF_LOW, cg.float_), ...)``. Each arg is ``(conf_key, type_)`` or ``(conf_key, type_, const_fn)``. A ``conf_key`` may be a - path into nested sections. A plain ``str`` ``type_`` is raw C++ type text and may use - ``{parent}``. ``const_fn(config, value)`` renders a constant's argument text; a lambda or an + path into nested sections. ``target`` and a plain ``str`` ``type_`` (raw C++ type text) may + name the parent object as ``{parent}``. ``const_fn(config, value)`` renders a constant's argument text; a lambda or an id bypasses it. The statement is skipped when none of its keys is set, always emitted when it has no keys, and a partial set is a config error. """ @@ -282,13 +286,13 @@ class ApplyCall: fields = [ f for _, f, _, _ in string.Formatter().parse(self.target) if f is not None ] - if any(fields): + if any(f not in ("", "parent") for f in fields): raise ValueError( - f"apply target {self.target!r}: only bare {{}} placeholders" + f"apply target {self.target!r}: only {{}} and {{parent}} placeholders" ) - if len(fields) != len(self.args): + if (count := fields.count("")) != len(self.args): raise ValueError( - f"apply target {self.target!r} has {len(fields)} " + f"apply target {self.target!r} has {count} " f"placeholder(s) for {len(self.args)} config key(s)" ) if any(len(arg) not in (2, 3) for arg in self.args): @@ -296,6 +300,12 @@ class ApplyCall: f"apply target {self.target!r}: each arg is (conf_key, type_[, const_fn])" ) + @property + def names_parent(self) -> bool: + return _names_parent(self.target) or any( + isinstance(arg[1], str) and _names_parent(arg[1]) for arg in self.args + ) + @property def members(self) -> list[tuple[Any, Any, Any]]: """Each arg as ``(conf_key, type_, const_fn or None)``.""" @@ -494,7 +504,7 @@ def register_apply_action( exprs = await _render_values( name, target, members, values, config, parent, lambda_args ) - statements.append(f"{receiver}{target.format(*exprs)};") + statements.append(f"{receiver}{target.format(*exprs, parent=parent)};") if call: statements = [ f"auto apply_call = {parent}->{call}();", @@ -526,7 +536,7 @@ async def _render_check( exprs = await _render_values( name, target, members, values, config, parent, lambda_args, compare=True ) - return target.format(*exprs) + return target.format(*exprs, parent=parent) def register_apply_condition( @@ -1061,21 +1071,22 @@ async def build_trigger_callback( params: TemplateArgsType, forward: Sequence[str | Expression] | None = None, when: str | ApplyCall | None = None, + parent: MockObj | None = None, ) -> LambdaExpression: """Build the Automation for ``config`` and return a stateless callback that triggers it. ``params`` are the parent callback's parameters, ``forward`` the expressions passed to ``trigger()`` (default: the parameter names; write the parent as ``parent_ref(var)``), ``when`` a filter the callback returns early on, skipped like any ``ApplyCall`` when none - of its keys is set. + of its keys is set. ``when`` may name ``parent`` as ``{parent}``, e.g. + ``"{parent}->is_fully_open()"``. """ members: list[tuple[Any, Any, Any]] = [] if when is not None: call = when if isinstance(when, ApplyCall) else ApplyCall(when) members = call.members - # A trigger callback has no parent for a str type to name. - if any(isinstance(t, str) and "{parent}" in t for _, t, _ in members): - raise ValueError(f"trigger filter {call.target!r}: a type names {{parent}}") + if parent is None and call.names_parent: + raise ValueError(f"trigger filter {call.target!r} names {{parent}}") obj = await _new_automation(args, config) lambda_args = _apply_lambda_args(params) statements: list[str] = [] @@ -1088,7 +1099,7 @@ async def build_trigger_callback( members, values, config, - None, + None if parent is None else str(parent_ref(parent)), lambda_args, ) statements.append(f"if (!({check}))\n return;") @@ -1137,7 +1148,7 @@ async def build_callback_automation( "forwarder cannot be combined with params, forward or when" ) callback = await build_trigger_callback( - args, config, args if params is None else params, forward, when + args, config, args if params is None else params, forward, when, parent ) cg.add(getattr(parent, callback_method)(callback)) return diff --git a/tests/unit_tests/test_automation.py b/tests/unit_tests/test_automation.py index ddd777fdb0..3cd70c082d 100644 --- a/tests/unit_tests/test_automation.py +++ b/tests/unit_tests/test_automation.py @@ -943,6 +943,30 @@ async def test_apply_action_call_shape( assert positions == sorted(positions) +@pytest.mark.asyncio +async def test_apply_target_names_parent( + registries: tuple[Registry, Registry], mock_cg: MockCodegen +) -> None: + """{parent} in a target is the global-scoped parent, also beside a call object.""" + fields = (ApplyCall("set_peer({parent}->id(), {})", (("level", cg.float_),)),) + await _run_apply_action(registries, fields, {"level": 0.5}) + assert ( + f"::{PARENT_OBJ}->set_peer(::{PARENT_OBJ}->id(), 0.5f);" + in _apply_definition(mock_cg) + ) + mock_cg.new_pvariable.reset_mock() + await _run_apply_action(registries, fields, {"level": 0.5}, call="make_call") + assert f"apply_call.set_peer(::{PARENT_OBJ}->id(), 0.5f);" in _apply_definition( + mock_cg + ) + mock_cg.new_pvariable.reset_mock() + await _run_apply_condition(registries, "is_peer({parent}->id())", {}) + assert ( + f"return ::{PARENT_OBJ}->is_peer(::{PARENT_OBJ}->id());" + in _apply_definition(mock_cg) + ) + + @pytest.mark.asyncio async def test_apply_field_nested_key_const_fn_and_type_string( registries: tuple[Registry, Registry], mock_cg: MockCodegen @@ -979,8 +1003,10 @@ async def test_apply_field_nested_key_const_fn_and_type_string( def test_apply_registration_checks(registries: tuple[Registry, Registry]) -> None: with pytest.raises(ValueError, match="2 placeholder"): ApplyCall("set_range({}, {})", (("low", cg.float_),)) - with pytest.raises(ValueError, match="only bare"): - ApplyCall("if ({}) {parent}->reset()", (("reset", cg.bool_),)) + with pytest.raises(ValueError, match="only {} and {parent}"): + ApplyCall("if ({}) {other}->reset()", (("reset", cg.bool_),)) + assert ApplyCall("if ({}) {parent}->reset()", (("reset", cg.bool_),)).names_parent + assert not ApplyCall("set_flags({{parent}})").names_parent ApplyCall("set_flags({{{}}})", (("flags", cg.int_),)) with pytest.raises(ValueError, match="each arg is"): ApplyCall("set_kp({})", (("kp", cg.float_, None, "extra"),)) @@ -1242,12 +1268,57 @@ async def test_trigger_callback_reshapes_and_filters(mock_cg: MockCodegen) -> No @pytest.mark.asyncio async def test_trigger_callback_filter_has_no_parent(mock_cg: MockCodegen) -> None: - """A filter type that names {parent} is rejected up front, a trigger callback has none.""" - when = ApplyCall("mode == {}", (("mode", "{parent}::Mode"),)) - with pytest.raises(ValueError, match="names {parent}"): + """A filter that names {parent} is rejected up front when no parent is given.""" + for when in ( + ApplyCall("mode == {}", (("mode", "{parent}::Mode"),)), + "{parent}->is_failed() == false", + ): + with pytest.raises(ValueError, match="names {parent}"): + await build_trigger_callback( + [], + {**TRIGGER_CONF, "mode": 1}, + [(cg.int_, "mode")], + forward=[], + when=when, + ) + + +@pytest.mark.asyncio +async def test_trigger_callback_filter_names_parent(mock_cg: MockCodegen) -> None: + """With the parent given, {parent} in a filter is the global-scoped parent.""" + text = _squash( await build_trigger_callback( - [], {**TRIGGER_CONF, "mode": 1}, [(cg.int_, "mode")], forward=[], when=when + [], + TRIGGER_CONF, + [(cg.int_, "state")], + forward=[], + when="state == 1 && {parent}->is_failed() == false", + parent=MockObj("improv", "->"), ) + ) + assert ( + "if (!(state == 1 && ::improv->is_failed() == false)) return; " + f"::{NEW_OBJ}->trigger();" + ) in text + + +@pytest.mark.asyncio +async def test_build_callback_automations_when_names_parent( + mock_cg: MockCodegen, +) -> None: + """A table entry names the parent through {parent}, so it can live at module level.""" + entries = ( + CallbackAutomation( + "on_open", "add_on_state_callback", when="{parent}->is_fully_open()" + ), + ) + await build_callback_automations( + MockObj("valve", "->"), {"on_open": [TRIGGER_CONF]}, entries + ) + assert _squash(mock_cg.add.call_args.args[0]) == ( + "valve->add_on_state_callback([]() -> void { " + f"if (!(::valve->is_fully_open())) return; ::{NEW_OBJ}->trigger(); }})" + ) @pytest.mark.asyncio From 4d187a2bb70e832022eb93c531c29bec532003a7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:43:22 -1000 Subject: [PATCH 13/17] [valve] Build on_open and on_closed with build_callback_automation (#20442) --- esphome/components/valve/__init__.py | 39 +++++++------------ esphome/components/valve/automation.h | 37 ------------------ tests/components/valve/common-triggers.yaml | 9 +++++ .../valve/test-triggers.esp8266-ard.yaml | 3 ++ 4 files changed, 25 insertions(+), 63 deletions(-) delete mode 100644 esphome/components/valve/automation.h create mode 100644 tests/components/valve/common-triggers.yaml create mode 100644 tests/components/valve/test-triggers.esp8266-ard.yaml diff --git a/esphome/components/valve/__init__.py b/esphome/components/valve/__init__.py index b000f6dd3f..41e25d63b1 100644 --- a/esphome/components/valve/__init__.py +++ b/esphome/components/valve/__init__.py @@ -15,7 +15,6 @@ from esphome.const import ( CONF_POSITION_STATE_TOPIC, CONF_STATE, CONF_STOP, - CONF_TRIGGER_ID, CONF_WEB_SERVER, DEVICE_CLASS_EMPTY, DEVICE_CLASS_GAS, @@ -62,17 +61,18 @@ VALVE_OPERATIONS = { } validate_valve_operation = cv.enum(VALVE_OPERATIONS, upper=True) -# Actions -ValvePublishAction = valve_ns.class_("ValvePublishAction", automation.Action) - -# Triggers -ValveOpenTrigger = valve_ns.class_("ValveOpenTrigger", automation.Trigger.template()) -ValveClosedTrigger = valve_ns.class_( - "ValveClosedTrigger", automation.Trigger.template() -) - CONF_ON_CLOSED = "on_closed" +# The state callback carries nothing; each automation fires only in its end state. +_CALLBACK_AUTOMATIONS = ( + automation.CallbackAutomation( + CONF_ON_OPEN, "add_on_state_callback", when="{parent}->is_fully_open()" + ), + automation.CallbackAutomation( + CONF_ON_CLOSED, "add_on_state_callback", when="{parent}->is_fully_closed()" + ), +) + _VALVE_SCHEMA = ( cv.ENTITY_BASE_SCHEMA.extend(web_server.WEBSERVER_SORTING_SCHEMA) .extend(cv.MQTT_COMMAND_COMPONENT_SCHEMA) @@ -89,16 +89,8 @@ _VALVE_SCHEMA = ( cv.Optional(CONF_POSITION_STATE_TOPIC): cv.All( cv.requires_component("mqtt"), cv.subscribe_topic ), - cv.Optional(CONF_ON_OPEN): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(ValveOpenTrigger), - } - ), - cv.Optional(CONF_ON_CLOSED): automation.validate_automation( - { - cv.GenerateID(CONF_TRIGGER_ID): cv.declare_id(ValveClosedTrigger), - } - ), + cv.Optional(CONF_ON_OPEN): automation.validate_automation({}), + cv.Optional(CONF_ON_CLOSED): automation.validate_automation({}), } ) ) @@ -134,12 +126,7 @@ def valve_schema( async def _setup_valve_core(var, config): setup_device_class(config) - for conf in config.get(CONF_ON_OPEN, []): - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) - await automation.build_automation(trigger, [], conf) - for conf in config.get(CONF_ON_CLOSED, []): - trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var) - await automation.build_automation(trigger, [], conf) + await automation.build_callback_automations(var, config, _CALLBACK_AUTOMATIONS) if mqtt_id_config := config.get(CONF_MQTT_ID): mqtt_ = cg.new_Pvariable(mqtt_id_config, var) diff --git a/esphome/components/valve/automation.h b/esphome/components/valve/automation.h deleted file mode 100644 index 598bbc1678..0000000000 --- a/esphome/components/valve/automation.h +++ /dev/null @@ -1,37 +0,0 @@ -#pragma once - -#include "esphome/core/automation.h" -#include "esphome/core/component.h" -#include "valve.h" - -namespace esphome::valve { - -class ValveOpenTrigger final : public Trigger<> { - public: - ValveOpenTrigger(Valve *a_valve) : valve_(a_valve) { - a_valve->add_on_state_callback([this]() { - if (this->valve_->is_fully_open()) { - this->trigger(); - } - }); - } - - protected: - Valve *valve_; -}; - -class ValveClosedTrigger final : public Trigger<> { - public: - ValveClosedTrigger(Valve *a_valve) : valve_(a_valve) { - a_valve->add_on_state_callback([this]() { - if (this->valve_->is_fully_closed()) { - this->trigger(); - } - }); - } - - protected: - Valve *valve_; -}; - -} // namespace esphome::valve diff --git a/tests/components/valve/common-triggers.yaml b/tests/components/valve/common-triggers.yaml new file mode 100644 index 0000000000..452cc2eac8 --- /dev/null +++ b/tests/components/valve/common-triggers.yaml @@ -0,0 +1,9 @@ +valve: + - platform: template + name: Trigger Test Valve + id: trigger_test_valve + optimistic: true + on_open: + - logger.log: Valve fully open + on_closed: + - logger.log: Valve fully closed diff --git a/tests/components/valve/test-triggers.esp8266-ard.yaml b/tests/components/valve/test-triggers.esp8266-ard.yaml new file mode 100644 index 0000000000..63ca1042de --- /dev/null +++ b/tests/components/valve/test-triggers.esp8266-ard.yaml @@ -0,0 +1,3 @@ +packages: + valve: !include common.yaml + triggers: !include common-triggers.yaml From 8567e69d46fe043156a4f16385e6043da520d6f4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:51:38 -1000 Subject: [PATCH 14/17] [core] Add status_momentary_warning/error overloads without a name (#20362) --- esphome/core/component.cpp | 28 ++++-- esphome/core/component.h | 14 +++ .../fixtures/status_momentary.yaml | 40 +++++++++ tests/integration/test_status_momentary.py | 87 +++++++++++++++++++ 4 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 tests/integration/fixtures/status_momentary.yaml create mode 100644 tests/integration/test_status_momentary.py diff --git a/esphome/core/component.cpp b/esphome/core/component.cpp index 0aa2dc21a2..edcd922aeb 100644 --- a/esphome/core/component.cpp +++ b/esphome/core/component.cpp @@ -292,14 +292,17 @@ void Component::status_set_warning(const LogString *message) { } void Component::status_set_error() { this->status_set_error((const LogString *) nullptr); } void Component::status_set_error(const LogString *message) { - if (!this->set_status_flag_(STATUS_LED_ERROR)) - return; - ESP_LOGE(TAG, "%s set Error flag: %s", LOG_STR_ARG(this->get_component_log_str()), - message ? LOG_STR_ARG(message) : LOG_STR_LITERAL("unspecified")); - if (message != nullptr) { + if (this->set_error_flag_(message) && message != nullptr) { store_component_error_message(this, message); } } +bool Component::set_error_flag_(const LogString *message) { + if (!this->set_status_flag_(STATUS_LED_ERROR)) + return false; + ESP_LOGE(TAG, "%s set Error flag: %s", LOG_STR_ARG(this->get_component_log_str()), + message ? LOG_STR_ARG(message) : LOG_STR_LITERAL("unspecified")); + return true; +} void Component::status_clear_warning_slow_path_() { this->component_state_ &= ~STATUS_LED_WARNING; // Clear the app-wide STATUS_LED_WARNING bit only if setup has finished @@ -329,6 +332,21 @@ void Component::status_momentary_error(const char *name, uint32_t length) { this->status_set_error(); this->set_timeout(name, length, [this]() { this->status_clear_error(); }); } +void Component::status_momentary_warning(uint32_t length) { + this->status_momentary_warning(static_cast(nullptr), length); +} +void Component::status_momentary_warning(const LogString *message, uint32_t length) { + this->status_set_warning(message); + this->set_timeout(InternalSchedulerID::STATUS_WARNING, length, [this]() { this->status_clear_warning(); }); +} +void Component::status_momentary_error(uint32_t length) { + this->status_momentary_error(static_cast(nullptr), length); +} +void Component::status_momentary_error(const LogString *message, uint32_t length) { + // Not stored: a cleared momentary error must not show up later as a FAILED reason + this->set_error_flag_(message); + this->set_timeout(InternalSchedulerID::STATUS_ERROR, length, [this]() { this->status_clear_error(); }); +} void Component::dump_config() {} // Function implementation of LOG_UPDATE_INTERVAL macro to reduce code size diff --git a/esphome/core/component.h b/esphome/core/component.h index ecaf863ecf..94ccec13f8 100644 --- a/esphome/core/component.h +++ b/esphome/core/component.h @@ -67,6 +67,8 @@ inline constexpr uint32_t SCHEDULER_DONT_RUN = 4294967295UL; /// with component-level NUMERIC_ID values, even if the uint32_t values overlap. enum class InternalSchedulerID : uint32_t { POLLING_UPDATE = 0, // PollingComponent interval + STATUS_WARNING = 1, // status_momentary_warning() without a name + STATUS_ERROR = 2, // status_momentary_error() without a name }; // Forward declaration @@ -307,6 +309,11 @@ class Component { */ void status_momentary_warning(const char *name, uint32_t length = 5000); + /// Set warning status flag and clear it after `length` ms. A new call restarts the timeout. + /// `message` is logged in place of "unspecified" when the flag gets set. + void status_momentary_warning(uint32_t length = 5000); + void status_momentary_warning(const LogString *message, uint32_t length = 5000); + /** Set error status flag and automatically clear it after a timeout. * * @param name Identifier for the timeout (used to cancel/replace existing timeouts with the same name). @@ -316,6 +323,11 @@ class Component { */ void status_momentary_error(const char *name, uint32_t length = 5000); + /// Set error status flag and clear it after `length` ms. A new call restarts the timeout. + /// `message` is logged in place of "unspecified"; unlike status_set_error() it is not stored. + void status_momentary_error(uint32_t length = 5000); + void status_momentary_error(const LogString *message, uint32_t length = 5000); + bool has_overridden_loop() const { return (this->component_state_ & COMPONENT_HAS_LOOP) != 0; } /** Get the integration where this component was declared as a LogString for logging. @@ -356,6 +368,8 @@ class Component { /// Note: Callers often use the return value to decide whether to log a warning/error, /// so once a flag is set, subsequent (potentially different) messages may be suppressed. bool set_status_flag_(uint8_t flag); + /// Set the error flag and log it; true when it was not set before. + bool set_error_flag_(const LogString *message); /** Set an interval function with a const char* name. Empty name means no cancelling possible. * diff --git a/tests/integration/fixtures/status_momentary.yaml b/tests/integration/fixtures/status_momentary.yaml new file mode 100644 index 0000000000..424a73ce3b --- /dev/null +++ b/tests/integration/fixtures/status_momentary.yaml @@ -0,0 +1,40 @@ +esphome: + name: status-momentary-test + +host: +api: + actions: + - action: momentary_warning + variables: + length: int + then: + - lambda: "id(probe)->status_momentary_warning(length);" + - action: momentary_error + variables: + length: int + then: + - lambda: "id(probe)->status_momentary_error(length);" + - action: momentary_warning_message + then: + - lambda: 'id(probe)->status_momentary_warning(LOG_STR("probe warning reason"), 300);' + - action: momentary_error_message + then: + - lambda: 'id(probe)->status_momentary_error(LOG_STR("probe error reason"), 300);' + +logger: + +sensor: + - platform: template + id: probe + update_interval: 24h + lambda: return 1.0; + +binary_sensor: + - platform: template + name: Probe Warning + id: probe_warning + lambda: return id(probe)->status_has_warning(); + - platform: template + name: Probe Error + id: probe_error + lambda: return id(probe)->status_has_error(); diff --git a/tests/integration/test_status_momentary.py b/tests/integration/test_status_momentary.py new file mode 100644 index 0000000000..0bf3a820be --- /dev/null +++ b/tests/integration/test_status_momentary.py @@ -0,0 +1,87 @@ +"""Integration test for status_momentary_warning/error without a name.""" + +from __future__ import annotations + +import asyncio + +from aioesphomeapi import BinarySensorState, EntityState +import pytest + +from .state_utils import InitialStateHelper +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_status_momentary( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + loop = asyncio.get_running_loop() + warning_logged = loop.create_future() + error_logged = loop.create_future() + + def on_log_line(line: str) -> None: + if ( + "set Warning flag: probe warning reason" in line + and not warning_logged.done() + ): + warning_logged.set_result(True) + if "set Error flag: probe error reason" in line and not error_logged.done(): + error_logged.set_result(True) + + async with ( + run_compiled(yaml_config, line_callback=on_log_line), + api_client_connected() as client, + ): + entities, services = await client.list_entities_services() + svc = {s.name: s for s in services} + keys = {e.object_id: e.key for e in entities} + warning_key = keys["probe_warning"] + error_key = keys["probe_error"] + + changes: asyncio.Queue[tuple[int, bool, float]] = asyncio.Queue() + + def on_state(state: EntityState) -> None: + if isinstance(state, BinarySensorState): + changes.put_nowait((state.key, state.state, loop.time())) + + initial_state_helper = InitialStateHelper(entities) + client.subscribe_states(initial_state_helper.on_state_wrapper(on_state)) + await initial_state_helper.wait_for_initial_states() + assert initial_state_helper.initial_states[warning_key].state is False + assert initial_state_helper.initial_states[error_key].state is False + + async def next_change(key: int) -> tuple[bool, float]: + while True: + k, value, when = await asyncio.wait_for(changes.get(), timeout=5) + if k == key: + return value, when + + # The flag sets at once and clears after the length + for service, key in ( + ("momentary_warning", warning_key), + ("momentary_error", error_key), + ): + start = loop.time() + await client.execute_service(svc[service], {"length": 300}) + assert (await next_change(key))[0] is True + value, cleared = await next_change(key) + assert value is False + assert cleared - start >= 0.25 + + # A second call restarts the timeout instead of adding a second one + start = loop.time() + await client.execute_service(svc["momentary_warning"], {"length": 1000}) + assert (await next_change(warning_key))[0] is True + await asyncio.sleep(0.6) + await client.execute_service(svc["momentary_warning"], {"length": 1000}) + value, cleared = await next_change(warning_key) + assert value is False + assert cleared - start >= 1.4 + + # A message replaces "unspecified" in the log line + await client.execute_service(svc["momentary_warning_message"], {}) + await asyncio.wait_for(warning_logged, timeout=5) + await client.execute_service(svc["momentary_error_message"], {}) + await asyncio.wait_for(error_logged, timeout=5) From 640d677913de685169d0d4dc0def96d14dd35a3c Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:51:55 -1000 Subject: [PATCH 15/17] [st7735][st7789v][waveshare_epaper] Declare log tags with ESPHOME_LOG_TAG (#20295) --- esphome/components/st7735/st7735.cpp | 2 +- esphome/components/st7789v/st7789v.cpp | 2 +- esphome/components/waveshare_epaper/waveshare_213v3.cpp | 2 +- esphome/components/waveshare_epaper/waveshare_epaper.cpp | 2 +- script/ci-custom.py | 6 ------ 5 files changed, 4 insertions(+), 10 deletions(-) diff --git a/esphome/components/st7735/st7735.cpp b/esphome/components/st7735/st7735.cpp index 7eb70c5c23..94b94d17a9 100644 --- a/esphome/components/st7735/st7735.cpp +++ b/esphome/components/st7735/st7735.cpp @@ -218,7 +218,7 @@ static constexpr uint8_t PROGMEM 100 }; // 100 ms delay // clang-format on -static const char *const TAG = "st7735"; +ESPHOME_LOG_TAG(TAG, "st7735"); ST7735::ST7735(ST7735Model model, int width, int height, int colstart, int rowstart, bool eightbitcolor, bool usebgr, bool invert_colors) diff --git a/esphome/components/st7789v/st7789v.cpp b/esphome/components/st7789v/st7789v.cpp index 2e07e24522..343d196b50 100644 --- a/esphome/components/st7789v/st7789v.cpp +++ b/esphome/components/st7789v/st7789v.cpp @@ -4,7 +4,7 @@ namespace esphome::st7789v { -static const char *const TAG = "st7789v"; +ESPHOME_LOG_TAG(TAG, "st7789v"); #ifdef USE_ESP32 static constexpr size_t TEMP_BUFFER_SIZE = 1024; #else diff --git a/esphome/components/waveshare_epaper/waveshare_213v3.cpp b/esphome/components/waveshare_epaper/waveshare_213v3.cpp index cc9c34cb42..5e19782f6d 100644 --- a/esphome/components/waveshare_epaper/waveshare_213v3.cpp +++ b/esphome/components/waveshare_epaper/waveshare_213v3.cpp @@ -4,7 +4,7 @@ namespace esphome::waveshare_epaper { -static const char *const TAG = "waveshare_2.13v3"; +ESPHOME_LOG_TAG(TAG, "waveshare_2.13v3"); static const uint8_t PARTIAL_LUT[] = { 0x32, // cmd diff --git a/esphome/components/waveshare_epaper/waveshare_epaper.cpp b/esphome/components/waveshare_epaper/waveshare_epaper.cpp index 93f23424c0..60d711d1cd 100644 --- a/esphome/components/waveshare_epaper/waveshare_epaper.cpp +++ b/esphome/components/waveshare_epaper/waveshare_epaper.cpp @@ -7,7 +7,7 @@ namespace esphome::waveshare_epaper { -static const char *const TAG = "waveshare_epaper"; +ESPHOME_LOG_TAG(TAG, "waveshare_epaper"); static const uint8_t LUT_SIZE_WAVESHARE = 30; diff --git a/script/ci-custom.py b/script/ci-custom.py index 1b3abc40f2..7839038a8c 100755 --- a/script/ci-custom.py +++ b/script/ci-custom.py @@ -1273,12 +1273,6 @@ def lint_no_std_nothrow(fname, match): r"\s*(?:=\s*\{?|[{(])\s*\"", prefilter="TAG", include=["esphome/components/*.cpp", "esphome/components/**/*.cpp"], - # Deprecated components, converted in a follow-up PR - exclude=[ - "esphome/components/st7735/*", - "esphome/components/st7789v/*", - "esphome/components/waveshare_epaper/*", - ], ) def lint_log_tag_macro(fname, match): return ( From 6b03d68fac862101393f3601b0182e43e7689ef2 Mon Sep 17 00:00:00 2001 From: Zhaoqi Xu Date: Sat, 10 Oct 2026 09:53:38 +0800 Subject: [PATCH 16/17] [heatpumpir] Fix fan speeds for mitsubishi_msc protocol (#20460) --- esphome/components/heatpumpir/heatpumpir.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/esphome/components/heatpumpir/heatpumpir.cpp b/esphome/components/heatpumpir/heatpumpir.cpp index eb23eb8ed5..2376a67416 100644 --- a/esphome/components/heatpumpir/heatpumpir.cpp +++ b/esphome/components/heatpumpir/heatpumpir.cpp @@ -187,15 +187,18 @@ void HeatpumpIRClimate::transmit_state() { swing_h_cmd = HDIR_SWING; } + // MitsubishiMSCHeatpumpIR only knows FAN_1..FAN_3 and sends anything else as auto, + // so low/medium/high have to start at FAN_1 there instead of FAN_2. + const bool three_speed_fan = this->protocol_ == PROTOCOL_MITSUBISHI_MSC; switch (this->fan_mode.value_or(climate::CLIMATE_FAN_AUTO)) { case climate::CLIMATE_FAN_LOW: - fan_speed_cmd = FAN_2; + fan_speed_cmd = three_speed_fan ? FAN_1 : FAN_2; break; case climate::CLIMATE_FAN_MEDIUM: - fan_speed_cmd = FAN_3; + fan_speed_cmd = three_speed_fan ? FAN_2 : FAN_3; break; case climate::CLIMATE_FAN_HIGH: - fan_speed_cmd = FAN_4; + fan_speed_cmd = three_speed_fan ? FAN_3 : FAN_4; break; case climate::CLIMATE_FAN_AUTO: default: From 41cc96bbb8cd83368093d0af89499bf4ae95d071 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 9 Oct 2026 15:55:09 -1000 Subject: [PATCH 17/17] [light] Keep the effect list in a flash table (#20204) Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> --- esphome/codegen.py | 1 + esphome/components/api/api.proto | 2 +- esphome/components/api/api_connection.cpp | 11 +---- esphome/components/api/api_pb2.cpp | 8 +-- esphome/components/api/api_pb2.h | 2 +- esphome/components/api/api_pb2_includes.h | 1 + esphome/components/light/__init__.py | 16 +++++- esphome/components/light/light_call.cpp | 2 +- esphome/components/light/light_effect.h | 2 + esphome/components/light/light_effect_names.h | 44 +++++++++++++++++ esphome/components/light/light_state.cpp | 12 ----- esphome/components/light/light_state.h | 11 +++-- esphome/core/hal.h | 8 +++ esphome/cpp_generator.py | 49 ++++++++++++++++--- .../components/api/bench_list_entities.cpp | 18 +++++-- .../light/test_default_setters.py | 26 +++++++++- tests/components/light/common.yaml | 3 +- tests/components/light/test_light_state.cpp | 29 ----------- tests/unit_tests/test_cpp_generator.py | 42 +++++++++++++++- 19 files changed, 207 insertions(+), 80 deletions(-) create mode 100644 esphome/components/light/light_effect_names.h diff --git a/esphome/codegen.py b/esphome/codegen.py index d57ec39aab..4db01a2174 100644 --- a/esphome/codegen.py +++ b/esphome/codegen.py @@ -34,6 +34,7 @@ from esphome.cpp_generator import ( # noqa: F401 extern_progmem_array, get_variable, get_variable_with_full_id, + is_static_pointer, is_template, new_Pvariable, new_variable, diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index 76b82810a4..d4ae82fd9f 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -760,7 +760,7 @@ message ListEntitiesLightResponse { bool legacy_supports_color_temperature = 8 [deprecated=true]; float min_mireds = 9; float max_mireds = 10; - repeated string effects = 11 [(container_pointer_no_template) = "FixedVector"]; + repeated string effects = 11 [(container_pointer_no_template) = "light::LightEffectNames"]; bool disabled_by_default = 13; string icon = 14 [(field_ifdef) = "USE_ENTITY_ICON", (max_data_length) = 63]; EntityCategory entity_category = 15; diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 940c8847ed..fd51aae1be 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -644,16 +644,7 @@ uint16_t APIConnection::try_send_light_info(EntityBase *entity, APIConnection *c msg.min_mireds = traits.get_min_mireds(); msg.max_mireds = traits.get_max_mireds(); } - FixedVector effects_list; - if (light->supports_effects()) { - auto &light_effects = light->get_effects(); - effects_list.init(light_effects.size() + 1); - effects_list.push_back("None"); - for (auto *effect : light_effects) { - // c_str() is safe as effect names are null-terminated strings from codegen - effects_list.push_back(effect->get_name().c_str()); - } - } + light::LightEffectNames effects_list(light->get_effects()); msg.effects = &effects_list; return fill_and_encode_entity_info(light, msg, conn, remaining_size); } diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index b59663aeb5..8847249b17 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -688,8 +688,8 @@ uint8_t *ListEntitiesLightResponse::encode_msg(const void *self, uint8_t *__rest if (uint32_t raw = float_to_raw(msg.max_mireds); raw != 0) [[likely]] { pos = ProtoEncode::write_tag_and_fixed32(pos PROTO_ENCODE_DEBUG_ARG, 85, raw); } - for (const char *it : *msg.effects) { - pos = ProtoEncode::encode_string_force(pos PROTO_ENCODE_DEBUG_ARG, 11, it, strlen(it)); + for (const auto &it : *msg.effects) { + pos = ProtoEncode::encode_string_force(pos PROTO_ENCODE_DEBUG_ARG, 11, it); } pos = ProtoEncode::encode_bool(pos PROTO_ENCODE_DEBUG_ARG, 13, msg.disabled_by_default); #ifdef USE_ENTITY_ICON @@ -713,8 +713,8 @@ uint32_t ListEntitiesLightResponse::calc_size_msg(const void *self) { size += ProtoSize::calc_float(1, msg.min_mireds); size += ProtoSize::calc_float(1, msg.max_mireds); if (!msg.effects->empty()) { - for (const char *it : *msg.effects) { - size += ProtoSize::calc_length_force(1, strlen(it)); + for (const auto &it : *msg.effects) { + size += ProtoSize::calc_length_force(1, it.size()); } } size += ProtoSize::calc_bool(1, msg.disabled_by_default); diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index 421c3fccb3..17436d33aa 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -1059,7 +1059,7 @@ class ListEntitiesLightResponse final : public InfoResponseProtoMessage { const light::ColorModeMask *supported_color_modes{}; float min_mireds{0.0f}; float max_mireds{0.0f}; - const FixedVector *effects{}; + const light::LightEffectNames *effects{}; static uint8_t *encode_msg(const void *self, uint8_t *pos PROTO_ENCODE_DEBUG_PARAM); uint8_t *encode(ProtoWriteBuffer &buffer PROTO_ENCODE_DEBUG_PARAM) const { return encode_msg(this, buffer.get_pos() PROTO_ENCODE_DEBUG_ARG); diff --git a/esphome/components/api/api_pb2_includes.h b/esphome/components/api/api_pb2_includes.h index 833e6529e2..e076f98fe3 100644 --- a/esphome/components/api/api_pb2_includes.h +++ b/esphome/components/api/api_pb2_includes.h @@ -15,6 +15,7 @@ #endif #ifdef USE_LIGHT +#include "esphome/components/light/light_effect_names.h" #include "esphome/components/light/light_traits.h" #endif diff --git a/esphome/components/light/__init__.py b/esphome/components/light/__init__.py index b175620bbf..c490300b58 100644 --- a/esphome/components/light/__init__.py +++ b/esphome/components/light/__init__.py @@ -41,7 +41,7 @@ from esphome.core.entity_helpers import ( queue_entity_register, setup_entity, ) -from esphome.cpp_generator import MockObjClass +from esphome.cpp_generator import MockObj, MockObjClass import esphome.final_validate as fv from esphome.types import ConfigType @@ -71,6 +71,7 @@ from .types import ( # noqa: F401 ChannelColors, ColorMode, GammaTable, + LightEffect, LightOutput, LightState, LightStateRTCState, @@ -519,6 +520,17 @@ def validate_color_temperature_channels(value): return value +def _effects_table(effects: list[MockObj]) -> MockObj: + """Emit the flash table of a light's effect pointers; each must be a static address.""" + return cg.shared_progmem_array( + "light_effects", + LightEffect.operator("ptr"), + cg.ArrayInitializer(*effects), + share=False, + constexpr=False, + ) + + @setup_entity("light") async def setup_light_core_(light_var, config, output_var): # All 8 legacy restore_mode values, and the restore_state key, are just different @@ -586,7 +598,7 @@ async def setup_light_core_(light_var, config, output_var): EFFECTS_REGISTRY, config.get(CONF_EFFECTS, []) ) if effects: - cg.add(light_var.add_effects(effects)) + cg.add(light_var.add_effects(_effects_table(effects), len(effects))) for conf in config.get(CONF_ON_TURN_ON, []): trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], light_var) diff --git a/esphome/components/light/light_call.cpp b/esphome/components/light/light_call.cpp index 5ad9215eed..0970c513a2 100644 --- a/esphome/components/light/light_call.cpp +++ b/esphome/components/light/light_call.cpp @@ -147,7 +147,7 @@ void LightCall::perform() { // EFFECT StringRef effect_s; if (this->effect_ == 0u) { - effect_s = StringRef::from_lit("None"); + effect_s = EFFECT_NONE_REF; } else { effect_s = this->parent_->effects_[this->effect_ - 1]->get_name(); } diff --git a/esphome/components/light/light_effect.h b/esphome/components/light/light_effect.h index a89e3fec5a..3fabcf5d51 100644 --- a/esphome/components/light/light_effect.h +++ b/esphome/components/light/light_effect.h @@ -5,6 +5,8 @@ namespace esphome::light { +inline constexpr StringRef EFFECT_NONE_REF = StringRef::from_lit("None"); + class LightState; class LightEffect { diff --git a/esphome/components/light/light_effect_names.h b/esphome/components/light/light_effect_names.h new file mode 100644 index 0000000000..7ddf459b24 --- /dev/null +++ b/esphome/components/light/light_effect_names.h @@ -0,0 +1,44 @@ +#pragma once + +#include "esphome/core/helpers.h" +#include "esphome/core/string_ref.h" +#include "light_effect.h" + +namespace esphome::light { + +/// Effect names as the native API lists them: "None" first, then each effect; empty without effects. +/// Reads the light's effect table directly, so listing entities allocates nothing. Holds a reference: +/// it only lives for one synchronous encode, and by value costs flash on every copy. +class LightEffectNames { + public: + explicit LightEffectNames(const ConstVector &effects) : effects_(effects) {} + + class Iterator { + public: + ESPHOME_ALWAYS_INLINE Iterator(const ConstVector &effects, size_t index) + : effects_(effects), index_(index) {} + ESPHOME_ALWAYS_INLINE StringRef operator*() const { + return this->index_ == 0 ? EFFECT_NONE_REF : this->effects_[this->index_ - 1]->get_name(); + } + ESPHOME_ALWAYS_INLINE Iterator &operator++() { + ++this->index_; + return *this; + } + ESPHOME_ALWAYS_INLINE bool operator!=(const Iterator &other) const { return this->index_ != other.index_; } + + protected: + const ConstVector &effects_; + size_t index_; + }; + + ESPHOME_ALWAYS_INLINE Iterator begin() const { return {this->effects_, 0}; } + ESPHOME_ALWAYS_INLINE Iterator end() const { + return {this->effects_, this->effects_.empty() ? 0 : this->effects_.size() + 1}; + } + ESPHOME_ALWAYS_INLINE bool empty() const { return this->effects_.empty(); } + + protected: + const ConstVector &effects_; +}; + +} // namespace esphome::light diff --git a/esphome/components/light/light_state.cpp b/esphome/components/light/light_state.cpp index 9fe483a291..885440187f 100644 --- a/esphome/components/light/light_state.cpp +++ b/esphome/components/light/light_state.cpp @@ -9,8 +9,6 @@ #include "light_output.h" #include "transformers.h" -#include - namespace esphome::light { ESPHOME_LOG_TAG(TAG, "light"); @@ -196,8 +194,6 @@ void LightState::publish_state() { LightOutput *LightState::get_output() const { return this->output_; } -static constexpr auto EFFECT_NONE_REF = StringRef::from_lit("None"); - StringRef LightState::get_effect_name() { if (this->active_effect_index_ > 0) { return this->effects_[this->active_effect_index_ - 1]->get_name(); @@ -218,11 +214,6 @@ void LightState::add_target_state_reached_listener(LightTargetStateReachedListen this->target_state_reached_listeners_->push_back(listener); } -void LightState::add_effects(const std::initializer_list &effects) { - // Called once from Python codegen during setup with all effects from YAML config - this->effects_ = effects; -} - void LightState::current_values_as_brightness(float *brightness) { this->current_values.as_brightness(brightness); *brightness = this->gamma_correct_lut(*brightness); @@ -352,9 +343,6 @@ float LightState::gamma_uncorrect_lut(float value) const { #endif // USE_LIGHT_GAMMA_LUT void LightState::start_effect_(uint32_t effect_index) { - // An external add_effects() can exceed the codegen cap; ignore an index the uint16_t can't hold - if (effect_index > std::numeric_limits::max()) - return; this->stop_effect_(); if (effect_index == 0) return; diff --git a/esphome/components/light/light_state.h b/esphome/components/light/light_state.h index fcb2ca40c9..2a99943e1d 100644 --- a/esphome/components/light/light_state.h +++ b/esphome/components/light/light_state.h @@ -217,10 +217,13 @@ class LightState : public EntityBase, public Component { bool supports_effects() const { return !this->effects_.empty(); } /// Get all effects for this light state. - const FixedVector &get_effects() const { return this->effects_; } + const ConstVector &get_effects() const { return this->effects_; } - /// Add effects for this light state. - void add_effects(const std::initializer_list &effects); + /// Codegen only: point at the generated table of this light's effects, which must outlive it. + /// Codegen caps the count at 65535, so an effect index always fits the uint16_t active index. + void add_effects(LightEffect *const *effects, size_t count) { + this->effects_ = ConstVector(effects, count); + } /// Get the total number of effects available for this light. size_t get_effect_count() const { return this->effects_.size(); } @@ -354,7 +357,7 @@ class LightState : public EntityBase, public Component { /// The currently active transformer for this light (transition/flash). std::unique_ptr transformer_{nullptr}; /// List of effects for this light. - FixedVector effects_; + ConstVector effects_; /// Object used to store the persisted values of the light. ESPPreferenceObject rtc_; diff --git a/esphome/core/hal.h b/esphome/core/hal.h index 4c5a19c6d1..dd3618f623 100644 --- a/esphome/core/hal.h +++ b/esphome/core/hal.h @@ -4,9 +4,17 @@ #include #include "gpio.h" #include "esphome/core/defines.h" + #include "esphome/core/time_64.h" #include "esphome/core/time_conversion.h" +/// Address tables in ESP8266 flash must be constant-initialized: a runtime initializer would write to flash. +#if defined(USE_ESP8266) && defined(__GNUC__) && !defined(__clang__) +#define ESPHOME_FLASH_CONSTINIT constinit +#else +#define ESPHOME_FLASH_CONSTINIT +#endif + // Per-platform HAL bits (IRAM_ATTR / PROGMEM macros, in_isr_context(), // inline yield/delay/micros/millis/millis_64 wrappers, ESP8266 progmem // helpers) live next to each platform component as components//hal.h diff --git a/esphome/cpp_generator.py b/esphome/cpp_generator.py index f5113d8098..9c9fed6601 100644 --- a/esphome/cpp_generator.py +++ b/esphome/cpp_generator.py @@ -10,6 +10,7 @@ from esphome.core import ( ID, Define, EnumValue, + EsphomeError, HexInt, Lambda, Library, @@ -447,13 +448,16 @@ class LineComment(Statement): class ProgmemAssignmentExpression(AssignmentExpression): - __slots__ = () + __slots__ = ("constexpr",) - def __init__(self, type_, name, rhs): + def __init__(self, type_, name, rhs, constexpr: bool = True): super().__init__(type_, "", name, rhs) + self.constexpr = constexpr def __str__(self): - return f"static constexpr {self.type} {self.name}[] PROGMEM = {self.rhs}" + if self.constexpr: + return f"static constexpr {self.type} {self.name}[] PROGMEM = {self.rhs}" + return f"ESPHOME_FLASH_CONSTINIT static {self.type} const {self.name}[] PROGMEM = {self.rhs}" class StaticConstAssignmentExpression(AssignmentExpression): @@ -493,20 +497,34 @@ def extern_progmem_array( def shared_progmem_array( - name: str, type_: "MockObjClass", rhs: SafeExpType, *, share: bool = True + name: str, + type_: "MockObjClass", + rhs: SafeExpType, + *, + share: bool = True, + constexpr: bool = True, ) -> "MockObj": """Emit a global PROGMEM array once per distinct type and contents; later calls reuse it. - The array is ``static constexpr``, so elements must be constant expressions and lambdas - must be captureless. Its name is made unique against every config id and variable. + The array is ``static constexpr`` by default, so elements must be constant expressions and + lambdas must be captureless. Its name is made unique against every config id and variable. ``share=False`` always emits a new array, e.g. for lambdas that may keep static state. + ``constexpr=False`` is for tables of generated object pointers: each top level variable + element must pass ``is_static_pointer`` or ``EsphomeError`` is raised. """ from esphome.config import iter_ids from esphome.config_validation import RESERVED_IDS arrays: dict[str, MockObj] = CORE.data.setdefault("shared_progmem_array", {}) rhs = safe_exp(rhs) - key = f"{type_} {rhs}" + if not constexpr and isinstance(rhs, ArrayInitializer): + for arg in rhs.args: + if isinstance(arg, MockObj) and not is_static_pointer(arg): + raise EsphomeError( + f"'{arg}' must be created with cg.new_Pvariable so its address is known " + "at compile time" + ) + key = f"{type_} {rhs} {constexpr}" if share and (array := arrays.get(key)) is not None: return array used = {str(i) for i, _ in iter_ids(CORE.config)} @@ -514,7 +532,7 @@ def shared_progmem_array( used |= set(RESERVED_IDS) | CORE.loaded_integrations id_ = ID(ensure_unique_string(name, used), is_declaration=True, type=type_) # Global, so any scope can use it; anything a lambda references is already declared. - CORE.add_global(ProgmemAssignmentExpression(type_, id_, rhs)) + CORE.add_global(ProgmemAssignmentExpression(type_, id_, rhs, constexpr)) array = MockObj(id_, ".") CORE.register_variable(id_, array) if share: @@ -703,6 +721,7 @@ def Pvariable(id_: ID, rhs: SafeExpType, type_: "MockObj" = None) -> "MockObj": ) placement_new = CallExpression(f"new({id_.id}) {actual_type}", *call_expr.args) CORE.add(ExpressionStatement(placement_new)) + CORE.data.setdefault(_STATIC_POINTER_IDS, set()).add(id_.id) else: decl = VariableDeclarationExpression(id_.type, "*", id_, static=True) CORE.add_global(decl) @@ -712,6 +731,20 @@ def Pvariable(id_: ID, rhs: SafeExpType, type_: "MockObj" = None) -> "MockObj": return obj +_STATIC_POINTER_IDS = "static_pointer_ids" + + +def is_static_pointer(obj: SafeExpType) -> bool: + """True if ``obj`` is a Pvariable whose object was placement constructed in static storage. + + Its pointer is then an address known at compile time and may appear in a ``constexpr=False`` + PROGMEM table; a pointer assigned in ``setup()`` may not. + """ + return isinstance(obj, MockObj) and str(obj.base) in CORE.data.get( + _STATIC_POINTER_IDS, () + ) + + def new_Pvariable(id_: ID, *args: SafeExpType) -> "MockObj": """Declare a new pointer variable in the code generation by calling it's constructor with the given arguments. diff --git a/tests/benchmarks/components/api/bench_list_entities.cpp b/tests/benchmarks/components/api/bench_list_entities.cpp index 4c445c2bb6..8a4cac1e2e 100644 --- a/tests/benchmarks/components/api/bench_list_entities.cpp +++ b/tests/benchmarks/components/api/bench_list_entities.cpp @@ -3,6 +3,8 @@ #include "esphome/components/api/api_pb2.h" #include "esphome/components/api/api_buffer.h" #include "esphome/components/light/color_mode.h" +#include "esphome/components/light/light_effect.h" +#include "esphome/components/light/light_effect_names.h" namespace esphome::api::benchmarks { @@ -150,7 +152,17 @@ BENCHMARK(CalcAndEncode_ListEntitiesBinarySensorResponse); // --- ListEntitiesLightResponse --- static light::ColorModeMask light_color_modes; -static FixedVector light_effects; + +class BenchEffect : public light::LightEffect { + public: + explicit BenchEffect(const char *name) : LightEffect(name) {} + void apply() override {} +}; +static BenchEffect rainbow_effect("Rainbow"); +static BenchEffect strobe_effect("Strobe"); +static light::LightEffect *const LIGHT_EFFECT_TABLE[] = {&rainbow_effect, &strobe_effect}; +static const ConstVector light_effect_list(LIGHT_EFFECT_TABLE, 2); +static const light::LightEffectNames light_effects(light_effect_list); static ListEntitiesLightResponse make_light_response() { // Initialize static data on first call @@ -158,10 +170,6 @@ static ListEntitiesLightResponse make_light_response() { if (!initialized) { light_color_modes.insert(light::ColorMode::RGB_WHITE); light_color_modes.insert(light::ColorMode::COLOR_TEMPERATURE); - light_effects.init(3); - light_effects.push_back("None"); - light_effects.push_back("Rainbow"); - light_effects.push_back("Strobe"); initialized = true; } diff --git a/tests/component_tests/light/test_default_setters.py b/tests/component_tests/light/test_default_setters.py index a4fc24a7cb..cbbecb632f 100644 --- a/tests/component_tests/light/test_default_setters.py +++ b/tests/component_tests/light/test_default_setters.py @@ -2,6 +2,14 @@ from collections.abc import Callable from pathlib import Path +import re + +import pytest + +import esphome.codegen as cg +from esphome.components import light +from esphome.components.light import types as light_types +from esphome.core import ID, EsphomeError def test_default_flash_length_and_empty_effects_are_not_emitted( @@ -16,4 +24,20 @@ def test_default_flash_length_and_empty_effects_are_not_emitted( assert "bare_light->set_flash_transition_length(" not in main_cpp assert "bare_light->add_effects(" not in main_cpp assert "fancy_light->set_flash_transition_length(500);" in main_cpp - assert "fancy_light->add_effects({" in main_cpp + call = re.search(r"fancy_light->add_effects\((\w+), (\d+)\);", main_cpp) + assert call is not None + # The effect pointers form a flash table; they are address constants, not constexpr + assert re.search( + rf"static light::LightEffect \* const {call.group(1)}\[\] PROGMEM = \{{[^}}]+\}};", + main_cpp, + ) + + +def test_effect_assigned_in_setup_is_rejected() -> None: + """A pointer assigned in setup() would make the flash table need dynamic init.""" + dynamic_effect = cg.Pvariable( + ID("dynamic_effect", is_declaration=True, type=light_types.LightEffect), + cg.RawExpression("make_effect()"), + ) + with pytest.raises(EsphomeError, match="dynamic_effect"): + light._effects_table([dynamic_effect]) diff --git a/tests/components/light/common.yaml b/tests/components/light/common.yaml index 64406afda8..c1e58c673f 100644 --- a/tests/components/light/common.yaml +++ b/tests/components/light/common.yaml @@ -32,6 +32,7 @@ esphome: - lambda: |- const auto &effects = id(test_monochromatic_light).get_effects(); auto &same = id(test_monochromatic_light).get_effects(); + const auto copy = id(test_monochromatic_light).get_effects(); // the view is copyable uint32_t total = effects.size(); for (auto *effect : effects) { ESP_LOGD("test", "Effect %s", effect->get_name().c_str()); @@ -39,7 +40,7 @@ esphome: if (total > 0) { ESP_LOGD("test", "First %s", effects.at(0)->get_name().c_str()); // Raw pointer iterators on purpose: external code binds std::find's result to `const auto *` - auto *it = std::find(same.begin(), same.end(), effects[0]); + auto *it = std::find(same.begin(), same.end(), copy[0]); ESP_LOGD("test", "Index %d", (int) (it - same.begin())); } diff --git a/tests/components/light/test_light_state.cpp b/tests/components/light/test_light_state.cpp index 85bfbd09ef..076438c7d2 100644 --- a/tests/components/light/test_light_state.cpp +++ b/tests/components/light/test_light_state.cpp @@ -1,6 +1,5 @@ #include -#include "esphome/components/light/light_effect.h" #include "esphome/components/light/light_output.h" #include "esphome/components/light/light_state.h" @@ -18,36 +17,8 @@ class BrightnessOutput : public LightOutput { void write_state(LightState *state) override {} }; -class NoopEffect : public LightEffect { - public: - using LightEffect::LightEffect; - void apply() override {} -}; - -// start_effect_() is where the uint32_t index is narrowed to the stored uint16_t. -class TestableLightState : public LightState { - public: - using LightState::LightState; - using LightState::start_effect_; -}; - } // namespace -// add_effects() is public, so an external component can exceed the codegen cap on effect count; -// an index the uint16_t can't hold must be ignored rather than wrap onto another effect. -TEST(LightStateEffect, IndexAboveUint16IsIgnoredAndKeepsTheActiveEffect) { - BrightnessOutput output; - TestableLightState state(&output); - NoopEffect effect("Noop"); - state.add_effects({&effect}); - - state.start_effect_(1); - ASSERT_EQ(state.get_current_effect_index(), 1u); - - state.start_effect_(0x10000u); // unchecked narrowing wraps this to 0, which stops the effect - EXPECT_EQ(state.get_current_effect_index(), 1u); -} - // get_gamma_correct() reads the gamma codegen stores after the lookup table, rounded to two decimals. TEST(LightStateGamma, ReadsTheGammaStoredWithTheTable) { static constexpr GammaTable TABLE{{}, 280}; diff --git a/tests/unit_tests/test_cpp_generator.py b/tests/unit_tests/test_cpp_generator.py index b6f8fd1ec1..6fdd197c95 100644 --- a/tests/unit_tests/test_cpp_generator.py +++ b/tests/unit_tests/test_cpp_generator.py @@ -4,7 +4,7 @@ import math import pytest from esphome import cpp_generator as cg, cpp_types as ct -from esphome.core import CORE, ID +from esphome.core import CORE, ID, EsphomeError class TestExpressions: @@ -829,6 +829,46 @@ class TestSharedProgmemArray: assert str(a) != str(b) assert sum("PROGMEM" in str(st) for st in CORE.global_statements) == 2 + def test_is_static_pointer_only_for_placement_new(self) -> None: + CORE.config = {} + static = cg.new_Pvariable(ID("static_obj", is_declaration=True, type=ct.uint8)) + dynamic = cg.Pvariable( + ID("dynamic_obj", is_declaration=True, type=ct.uint8), + cg.RawExpression("nullptr"), + ) + assert cg.is_static_pointer(static) + assert not cg.is_static_pointer(dynamic) + + def test_constexpr_and_const_arrays_are_not_shared(self) -> None: + CORE.config = {} + a = cg.shared_progmem_array("table", ct.uint8, [1], constexpr=False) + b = cg.shared_progmem_array("table", ct.uint8, [1]) + assert str(a) != str(b) + + def test_constexpr_false_rejects_dynamic_pointers(self) -> None: + CORE.config = {} + dynamic = cg.Pvariable( + ID("dynamic_obj", is_declaration=True, type=ct.uint8), + cg.RawExpression("nullptr"), + ) + with pytest.raises(EsphomeError, match="dynamic_obj"): + cg.shared_progmem_array( + "table", + ct.uint8.operator("ptr"), + cg.ArrayInitializer(dynamic), + constexpr=False, + ) + + def test_constexpr_false_emits_a_const_array(self) -> None: + CORE.config = {} + cg.shared_progmem_array("table", ct.uint8, [1], constexpr=False) + (statement,) = ( + str(st) for st in CORE.global_statements if "PROGMEM" in str(st) + ) + assert statement.startswith( + "ESPHOME_FLASH_CONSTINIT static uint8_t const table[] PROGMEM = {1}" + ) + def test_same_contents_different_type_are_separate(self) -> None: CORE.config = {} a = cg.shared_progmem_array("table", ct.uint8, [1])