diff --git a/esphome/components/binary_sensor/__init__.py b/esphome/components/binary_sensor/__init__.py index 109a22c7bc..1384f8b008 100644 --- a/esphome/components/binary_sensor/__init__.py +++ b/esphome/components/binary_sensor/__init__.py @@ -74,6 +74,10 @@ from esphome.types import ConfigType from esphome.util import Registry CODEOWNERS = ["@esphome/core"] + +DEFAULT_INVALID_COOLDOWN_MS = ( + 1000 # Keep in sync with invalid_cooldown_ in automation.h +) DEVICE_CLASSES = [ DEVICE_CLASS_BATTERY, DEVICE_CLASS_BATTERY_CHARGING, @@ -131,10 +135,9 @@ ClickTrigger = binary_sensor_ns.class_("ClickTrigger", automation.Trigger.templa DoubleClickTrigger = binary_sensor_ns.class_( "DoubleClickTrigger", automation.Trigger.template() ) -MultiClickTriggerBase = binary_sensor_ns.class_( - "MultiClickTriggerBase", automation.Trigger.template(), cg.Component +MultiClickTrigger = binary_sensor_ns.class_( + "MultiClickTrigger", automation.Trigger.template(), cg.Component ) -MultiClickTrigger = binary_sensor_ns.class_("MultiClickTrigger", MultiClickTriggerBase) MultiClickTriggerEvent = binary_sensor_ns.struct("MultiClickTriggerEvent") @@ -492,7 +495,8 @@ _BINARY_SENSOR_SCHEMA = ( cv.Length(min=1, max=255), ), cv.Optional( - CONF_INVALID_COOLDOWN, default="1s" + CONF_INVALID_COOLDOWN, + default=f"{DEFAULT_INVALID_COOLDOWN_MS}ms", ): cv.positive_time_period_milliseconds, } ), @@ -586,11 +590,14 @@ async def _build_binary_sensor_automations(var, config): ) for tim in conf[CONF_TIMING] ] - trigger = cg.new_Pvariable( - conf[CONF_TRIGGER_ID], cg.TemplateArguments(len(timings)), var, timings + table = cg.shared_progmem_array( + "multi_click_timing", MultiClickTriggerEvent, cg.ArrayInitializer(*timings) ) - if CONF_INVALID_COOLDOWN in conf: - cg.add(trigger.set_invalid_cooldown(conf[CONF_INVALID_COOLDOWN])) + trigger = cg.new_Pvariable(conf[CONF_TRIGGER_ID], var, table, len(timings)) + if ( + cooldown := conf[CONF_INVALID_COOLDOWN] + ).total_milliseconds != DEFAULT_INVALID_COOLDOWN_MS: + cg.add(trigger.set_invalid_cooldown(cooldown)) await cg.register_component(trigger, conf) await automation.build_automation(trigger, [], conf) diff --git a/esphome/components/binary_sensor/automation.cpp b/esphome/components/binary_sensor/automation.cpp index d4b4906c30..46f7d9488c 100644 --- a/esphome/components/binary_sensor/automation.cpp +++ b/esphome/components/binary_sensor/automation.cpp @@ -18,7 +18,7 @@ constexpr uint32_t MULTICLICK_COOLDOWN_ID = 1; constexpr uint32_t MULTICLICK_IS_VALID_ID = 2; constexpr uint32_t MULTICLICK_IS_NOT_VALID_ID = 3; -void MultiClickTriggerBase::on_state_(bool state) { +void MultiClickTrigger::on_state_(bool state) { // Handle duplicate events if (state == this->last_state_) { return; @@ -30,7 +30,7 @@ void MultiClickTriggerBase::on_state_(bool state) { return; } - if (!this->at_index_.has_value()) { + if (this->at_index_ == 0) { // Start matching MultiClickTriggerEvent evt = this->timing_[0]; if (evt.state == state) { @@ -55,8 +55,7 @@ void MultiClickTriggerBase::on_state_(bool state) { return; } - // at_index_ has a value here (the !has_value() branch above returns). - size_t at_index = *this->at_index_; + uint8_t at_index = this->at_index_; if (at_index == this->timing_count_) { this->trigger_(); return; @@ -65,15 +64,15 @@ void MultiClickTriggerBase::on_state_(bool state) { MultiClickTriggerEvent evt = this->timing_[at_index]; if (evt.max_length != 4294967294UL) { - ESP_LOGV(TAG, "A i=%zu min=%" PRIu32 " max=%" PRIu32, at_index, evt.min_length, evt.max_length); // NOLINT + ESP_LOGV(TAG, "A i=%u min=%" PRIu32 " max=%" PRIu32, at_index, evt.min_length, evt.max_length); this->schedule_is_valid_(evt.min_length); this->schedule_is_not_valid_(evt.max_length); } else if (at_index + 1 != this->timing_count_) { - ESP_LOGV(TAG, "B i=%zu min=%" PRIu32, at_index, evt.min_length); // NOLINT + ESP_LOGV(TAG, "B i=%u min=%" PRIu32, at_index, evt.min_length); this->cancel_timeout(MULTICLICK_IS_NOT_VALID_ID); this->schedule_is_valid_(evt.min_length); } else { - ESP_LOGV(TAG, "C i=%zu min=%" PRIu32, at_index, evt.min_length); // NOLINT + ESP_LOGV(TAG, "C i=%u min=%" PRIu32, at_index, evt.min_length); this->is_valid_ = false; this->cancel_timeout(MULTICLICK_IS_NOT_VALID_ID); this->set_timeout(MULTICLICK_TRIGGER_ID, evt.min_length, [this]() { this->trigger_(); }); @@ -81,19 +80,19 @@ void MultiClickTriggerBase::on_state_(bool state) { this->at_index_ = at_index + 1; } -void MultiClickTriggerBase::schedule_cooldown_() { +void MultiClickTrigger::schedule_cooldown_() { ESP_LOGV(TAG, "Multi Click: Invalid length of press, starting cooldown of %" PRIu32 " ms", this->invalid_cooldown_); this->is_in_cooldown_ = true; this->set_timeout(MULTICLICK_COOLDOWN_ID, this->invalid_cooldown_, [this]() { ESP_LOGV(TAG, "Multi Click: Cooldown ended, matching is now enabled again."); this->is_in_cooldown_ = false; }); - this->at_index_.reset(); + this->at_index_ = 0; this->cancel_timeout(MULTICLICK_TRIGGER_ID); this->cancel_timeout(MULTICLICK_IS_VALID_ID); this->cancel_timeout(MULTICLICK_IS_NOT_VALID_ID); } -void MultiClickTriggerBase::schedule_is_valid_(uint32_t min_length) { +void MultiClickTrigger::schedule_is_valid_(uint32_t min_length) { if (min_length == 0) { this->is_valid_ = true; return; @@ -105,7 +104,7 @@ void MultiClickTriggerBase::schedule_is_valid_(uint32_t min_length) { this->is_valid_ = true; }); } -void MultiClickTriggerBase::schedule_is_not_valid_(uint32_t max_length) { +void MultiClickTrigger::schedule_is_not_valid_(uint32_t max_length) { this->set_timeout(MULTICLICK_IS_NOT_VALID_ID, max_length, [this]() { ESP_LOGV(TAG, "Multi Click: You waited too long to %s.", this->parent_->state ? LOG_STR_LITERAL("RELEASE") : LOG_STR_LITERAL("PRESS")); @@ -113,14 +112,14 @@ void MultiClickTriggerBase::schedule_is_not_valid_(uint32_t max_length) { this->schedule_cooldown_(); }); } -void MultiClickTriggerBase::cancel() { +void MultiClickTrigger::cancel() { ESP_LOGV(TAG, "Multi Click: Sequence explicitly cancelled."); this->is_valid_ = false; this->schedule_cooldown_(); } -void MultiClickTriggerBase::trigger_() { +void MultiClickTrigger::trigger_() { ESP_LOGV(TAG, "Multi Click: Hooray, multi click is valid. Triggering!"); - this->at_index_.reset(); + this->at_index_ = 0; this->cancel_timeout(MULTICLICK_TRIGGER_ID); this->cancel_timeout(MULTICLICK_IS_VALID_ID); this->cancel_timeout(MULTICLICK_IS_NOT_VALID_ID); diff --git a/esphome/components/binary_sensor/automation.h b/esphome/components/binary_sensor/automation.h index 49457200bc..7426cc5614 100644 --- a/esphome/components/binary_sensor/automation.h +++ b/esphome/components/binary_sensor/automation.h @@ -1,7 +1,7 @@ #pragma once -#include #include +#include #include #include "esphome/core/component.h" @@ -13,10 +13,14 @@ namespace esphome::binary_sensor { struct MultiClickTriggerEvent { - bool state; + uint32_t state; // Not bool: all-word fields allow aligned loads from PROGMEM on ESP8266 uint32_t min_length; uint32_t max_length; }; +// Read straight from PROGMEM; a byte-sized field would fault on ESP8266. +static_assert(std::is_same_v && + sizeof(MultiClickTriggerEvent) == 3 * sizeof(uint32_t), + "MultiClickTriggerEvent fields must all be uint32_t"); bool match_interval(uint32_t min_length, uint32_t max_length, uint32_t length); @@ -70,10 +74,11 @@ class DoubleClickTrigger final : public Trigger<> { uint32_t max_length_; /// Maximum length of click. 0 means no maximum. }; -/// Non-template base for MultiClickTrigger (keeps large method bodies out of the header). -class MultiClickTriggerBase : public Trigger<>, public Component { +class MultiClickTrigger final : public Trigger<>, public Component { public: - explicit MultiClickTriggerBase(BinarySensor *parent) : parent_(parent) {} + /// `timing` is a codegen PROGMEM table shared by triggers with the same timing. + MultiClickTrigger(BinarySensor *parent, const MultiClickTriggerEvent *timing, uint8_t timing_count) + : parent_(parent), timing_(timing), timing_count_(timing_count) {} void setup() override { this->last_state_ = this->parent_->get_state_default(false); @@ -85,8 +90,8 @@ class MultiClickTriggerBase : public Trigger<>, public Component { void set_invalid_cooldown(uint32_t invalid_cooldown) { this->invalid_cooldown_ = invalid_cooldown; } void cancel(); - MultiClickTriggerBase(const MultiClickTriggerBase &) = delete; - MultiClickTriggerBase &operator=(const MultiClickTriggerBase &) = delete; + MultiClickTrigger(const MultiClickTrigger &) = delete; + MultiClickTrigger &operator=(const MultiClickTrigger &) = delete; protected: void on_state_(bool state); @@ -96,28 +101,13 @@ class MultiClickTriggerBase : public Trigger<>, public Component { void trigger_(); BinarySensor *parent_; - const MultiClickTriggerEvent *timing_{nullptr}; - uint32_t invalid_cooldown_{1000}; - optional at_index_{}; - uint8_t timing_count_{0}; + const MultiClickTriggerEvent *timing_; + uint32_t invalid_cooldown_{1000}; // Must match DEFAULT_INVALID_COOLDOWN_MS in __init__.py + uint8_t timing_count_; + uint8_t at_index_{0}; // 0: not matching, otherwise the index of the next timing event bool last_state_{false}; bool is_in_cooldown_{false}; bool is_valid_{false}; }; -/// Template wrapper that provides inline std::array storage for timing events. -/// N is set by code generation to match the exact number of timing events configured in YAML. -template class MultiClickTrigger final : public MultiClickTriggerBase { - public: - MultiClickTrigger(BinarySensor *parent, std::initializer_list timing) - : MultiClickTriggerBase(parent) { - init_array_from(this->timing_storage_, timing); - this->timing_ = this->timing_storage_.data(); - this->timing_count_ = N; - } - - protected: - std::array timing_storage_{}; -}; - } // namespace esphome::binary_sensor diff --git a/tests/component_tests/binary_sensor/test_multi_click.py b/tests/component_tests/binary_sensor/test_multi_click.py new file mode 100644 index 0000000000..6f2556735f --- /dev/null +++ b/tests/component_tests/binary_sensor/test_multi_click.py @@ -0,0 +1,29 @@ +"""Tests for the multi click trigger codegen.""" + +from collections.abc import Callable +from pathlib import Path +import re + + +def test_multi_click_timing_tables(generate_main: Callable[[str | Path], str]) -> None: + """Identical timings share one PROGMEM table; the default cooldown is not set.""" + main_cpp = generate_main( + "tests/component_tests/binary_sensor/test_multi_click.yaml" + ) + + tables = re.findall( + r"static constexpr binary_sensor::MultiClickTriggerEvent (\w+)\[\] PROGMEM", + main_cpp, + ) + assert len(tables) == 2 + single, long_press = tables + triggers = re.findall( + r"binary_sensor::MultiClickTrigger\((\w+), (\w+), (\d+)\);", main_cpp + ) + assert triggers == [ + ("button_a", single, "2"), + ("button_a", long_press, "1"), + ("button_b", single, "2"), + ] + assert main_cpp.count("set_invalid_cooldown(") == 1 + assert "set_invalid_cooldown(500);" in main_cpp diff --git a/tests/component_tests/binary_sensor/test_multi_click.yaml b/tests/component_tests/binary_sensor/test_multi_click.yaml new file mode 100644 index 0000000000..c18737ea3c --- /dev/null +++ b/tests/component_tests/binary_sensor/test_multi_click.yaml @@ -0,0 +1,33 @@ +--- +esphome: + name: test + +esp32: + board: esp32dev + +logger: + +binary_sensor: + - platform: gpio + id: button_a + pin: GPIO32 + on_multi_click: + - timing: + - ON for at most 1s + - OFF for at least 0.25s + then: + - logger.log: a single + - timing: + - ON for at least 2s + invalid_cooldown: 500ms + then: + - logger.log: a long + - platform: gpio + id: button_b + pin: GPIO33 + on_multi_click: + - timing: + - ON for at most 1s + - OFF for at least 0.25s + then: + - logger.log: b single