[binary_sensor] Share multi click timing tables in flash and skip the default cooldown setter (#20071)

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)
 

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);

esphome/components/binary_sensor/automation.h

@@ -1,7 +1,7 @@
 #pragma once
 
-#include <array>
 #include <cinttypes>
+#include <type_traits>
 #include <utility>
 
 #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<decltype(MultiClickTriggerEvent::state), uint32_t> &&
+                  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<size_t> 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<size_t N> class MultiClickTrigger final : public MultiClickTriggerBase {
- public:
-  MultiClickTrigger(BinarySensor *parent, std::initializer_list<MultiClickTriggerEvent> timing)
-      : MultiClickTriggerBase(parent) {
-    init_array_from(this->timing_storage_, timing);
-    this->timing_ = this->timing_storage_.data();
-    this->timing_count_ = N;
-  }
-
- protected:
-  std::array<MultiClickTriggerEvent, N> timing_storage_{};
-};
-
 }  // namespace esphome::binary_sensor

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

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