diff --git a/esphome/components/binary_sensor/__init__.py b/esphome/components/binary_sensor/__init__.py index 154b999124b..6c03d440e7a 100644 --- a/esphome/components/binary_sensor/__init__.py +++ b/esphome/components/binary_sensor/__init__.py @@ -264,12 +264,18 @@ async def autorepeat_filter_to_code(config: list[ConfigType], filter_id: ID) -> ) for conf in config ] + # A never-run delay ends the table, so the filter stores no count. + timings.append( + cg.StructInitializer( + AutorepeatFilterTiming, ("delay", cg.RawExpression("SCHEDULER_DONT_RUN")) + ) + ) table = cg.shared_progmem_array( "binary_sensor_autorepeat_timings", AutorepeatFilterTiming, cg.ArrayInitializer(*timings), ) - return cg.new_Pvariable(filter_id, table, len(timings)) + return cg.new_Pvariable(filter_id, table) @register_filter("lambda", LambdaFilter, cv.returning_lambda) diff --git a/esphome/components/binary_sensor/filter.cpp b/esphome/components/binary_sensor/filter.cpp index db8de5f7e8a..38149338ad4 100644 --- a/esphome/components/binary_sensor/filter.cpp +++ b/esphome/components/binary_sensor/filter.cpp @@ -83,12 +83,12 @@ optional AutorepeatFilter::new_value(bool value) { } void AutorepeatFilter::next_timing_() { - if (this->active_timing_ < this->timings_count_) { - App.scheduler.set_timeout(this, this->timings_[this->active_timing_].delay, [this]() { this->next_timing_(); }); - } - if (this->active_timing_ <= this->timings_count_) { - this->active_timing_++; + const uint32_t delay = this->timings_[this->active_timing_].delay; + if (delay != SCHEDULER_DONT_RUN) { + App.scheduler.set_timeout(this, delay, [this]() { this->next_timing_(); }); } + // Only reached with active_timing_ at most the end-marker index, so this never passes it. + this->active_timing_++; if (this->active_timing_ == 2) this->next_value_(false); } diff --git a/esphome/components/binary_sensor/filter.h b/esphome/components/binary_sensor/filter.h index 9f7d2aa5de0..34788323fac 100644 --- a/esphome/components/binary_sensor/filter.h +++ b/esphome/components/binary_sensor/filter.h @@ -93,14 +93,13 @@ static_assert(std::is_same_v std::is_same_v, "AutorepeatFilterTiming fields must stay uint32_t"); -/// Timings live in a PROGMEM table emitted by codegen. Every field is uint32_t, so aligned -/// loads straight from flash are safe on ESP8266. +/// Timings live in a PROGMEM table emitted by codegen, ended by an entry whose delay is +/// SCHEDULER_DONT_RUN (a step that could never advance anyway). Aligned loads are ESP8266 safe. /// The two scheduled timers are keyed off `this` and `&active_timing_`; since the address /// of `active_timing_` is taken as a scheduler key, the class must not be copied or moved. class AutorepeatFilter : public Filter { public: - AutorepeatFilter(const AutorepeatFilterTiming *timings, uint8_t timings_count) - : timings_(timings), timings_count_(timings_count) {} + explicit AutorepeatFilter(const AutorepeatFilterTiming *timings) : timings_(timings) {} AutorepeatFilter(const AutorepeatFilter &) = delete; AutorepeatFilter &operator=(const AutorepeatFilter &) = delete; @@ -110,9 +109,8 @@ class AutorepeatFilter : public Filter { void next_timing_(); void next_value_(bool val); + uint8_t active_timing_{0}; // before timings_ so it fills the padding after the Filter base const AutorepeatFilterTiming *timings_; - uint8_t timings_count_; - uint8_t active_timing_{0}; }; class LambdaFilter : public Filter { diff --git a/tests/component_tests/binary_sensor/test_autorepeat.py b/tests/component_tests/binary_sensor/test_autorepeat.py index a22abeb4260..53a5cb59c2f 100644 --- a/tests/component_tests/binary_sensor/test_autorepeat.py +++ b/tests/component_tests/binary_sensor/test_autorepeat.py @@ -16,9 +16,11 @@ def test_autorepeat_timings_shared_progmem_table( ) assert len(tables) == 2 shared, default = tables - assert main_cpp.count(f"AutorepeatFilter({shared}, 2);") == 2 - assert f"AutorepeatFilter({default}, 1);" in main_cpp + assert main_cpp.count(f"AutorepeatFilter({shared});") == 2 + assert f"AutorepeatFilter({default});" in main_cpp assert "binary_sensor_autorepeat_timings" not in tables default_table = main_cpp.split(f"{default}[] PROGMEM = ", 1)[1].split(";", 1)[0] for field in (".delay = 1000", ".time_off = 100", ".time_on = 900"): assert field in default_table + # Each table ends with a never-run delay instead of the filter storing a count. + assert main_cpp.count(".delay = SCHEDULER_DONT_RUN") == len(tables)