From 00eb56d8db27af8001ba93fbb3552778eab750a2 Mon Sep 17 00:00:00 2001 From: Clyde Stubbs <2366188+clydebarrow@users.noreply.github.com> Date: Thu, 3 Jul 2025 00:08:10 +1000 Subject: [PATCH] [esp32_touch] Fix threshold (#9291) Co-authored-by: Keith Burzinski Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- esphome/components/esp32_touch/esp32_touch.h | 22 ++++---- .../esp32_touch/esp32_touch_common.cpp | 17 +++--- .../components/esp32_touch/esp32_touch_v1.cpp | 6 +- .../components/esp32_touch/esp32_touch_v2.cpp | 55 +++++++++---------- 4 files changed, 50 insertions(+), 50 deletions(-) diff --git a/esphome/components/esp32_touch/esp32_touch.h b/esphome/components/esp32_touch/esp32_touch.h index 576c1a5649..5a91b1c750 100644 --- a/esphome/components/esp32_touch/esp32_touch.h +++ b/esphome/components/esp32_touch/esp32_touch.h @@ -93,7 +93,6 @@ class ESP32TouchComponent : public Component { uint32_t last_release_check_{0}; uint32_t release_timeout_ms_{1500}; uint32_t release_check_interval_ms_{50}; - bool initial_state_published_[TOUCH_PAD_MAX] = {false}; // Common configuration parameters uint16_t sleep_cycle_{4095}; @@ -123,13 +122,6 @@ class ESP32TouchComponent : public Component { }; protected: - // Design note: last_touch_time_ does not require synchronization primitives because: - // 1. ESP32 guarantees atomic 32-bit aligned reads/writes - // 2. ISR only writes timestamps, main loop only reads - // 3. Timing tolerance allows for occasional stale reads (50ms check interval) - // 4. Queue operations provide implicit memory barriers - // Using atomic/critical sections would add overhead without meaningful benefit - uint32_t last_touch_time_[TOUCH_PAD_MAX] = {0}; uint32_t iir_filter_{0}; bool iir_filter_enabled_() const { return this->iir_filter_ > 0; } @@ -147,9 +139,6 @@ class ESP32TouchComponent : public Component { uint32_t intr_mask; }; - // Track last touch time for timeout-based release detection - uint32_t last_touch_time_[TOUCH_PAD_MAX] = {0}; - protected: // Filter configuration touch_filter_mode_t filter_mode_{TOUCH_PAD_FILTER_MAX}; @@ -255,11 +244,22 @@ class ESP32TouchBinarySensor : public binary_sensor::BinarySensor { touch_pad_t touch_pad_{TOUCH_PAD_MAX}; uint32_t threshold_{0}; + uint32_t benchmark_{}; #ifdef USE_ESP32_VARIANT_ESP32 uint32_t value_{0}; #endif bool last_state_{false}; const uint32_t wakeup_threshold_{0}; + + // Track last touch time for timeout-based release detection + // Design note: last_touch_time_ does not require synchronization primitives because: + // 1. ESP32 guarantees atomic 32-bit aligned reads/writes + // 2. ISR only writes timestamps, main loop only reads + // 3. Timing tolerance allows for occasional stale reads (50ms check interval) + // 4. Queue operations provide implicit memory barriers + // Using atomic/critical sections would add overhead without meaningful benefit + uint32_t last_touch_time_{}; + bool initial_state_published_{}; }; } // namespace esp32_touch diff --git a/esphome/components/esp32_touch/esp32_touch_common.cpp b/esphome/components/esp32_touch/esp32_touch_common.cpp index fd2cdfcbad..2d93de077e 100644 --- a/esphome/components/esp32_touch/esp32_touch_common.cpp +++ b/esphome/components/esp32_touch/esp32_touch_common.cpp @@ -22,16 +22,20 @@ void ESP32TouchComponent::dump_config_base_() { " Sleep cycle: %.2fms\n" " Low Voltage Reference: %s\n" " High Voltage Reference: %s\n" - " Voltage Attenuation: %s", + " Voltage Attenuation: %s\n" + " Release Timeout: %" PRIu32 "ms\n", this->meas_cycle_ / (8000000.0f / 1000.0f), this->sleep_cycle_ / (150000.0f / 1000.0f), lv_s, hv_s, - atten_s); + atten_s, this->release_timeout_ms_); } void ESP32TouchComponent::dump_config_sensors_() { for (auto *child : this->children_) { LOG_BINARY_SENSOR(" ", "Touch Pad", child); - ESP_LOGCONFIG(TAG, " Pad: T%" PRIu32, (uint32_t) child->get_touch_pad()); - ESP_LOGCONFIG(TAG, " Threshold: %" PRIu32, child->get_threshold()); + ESP_LOGCONFIG(TAG, + " Pad: T%u\n" + " Threshold: %" PRIu32 "\n" + " Benchmark: %" PRIu32, + (unsigned) child->touch_pad_, child->threshold_, child->benchmark_); } } @@ -112,12 +116,11 @@ bool ESP32TouchComponent::should_check_for_releases_(uint32_t now) { } void ESP32TouchComponent::publish_initial_state_if_needed_(ESP32TouchBinarySensor *child, uint32_t now) { - touch_pad_t pad = child->get_touch_pad(); - if (!this->initial_state_published_[pad]) { + if (!child->initial_state_published_) { // Check if enough time has passed since startup if (now > this->release_timeout_ms_) { child->publish_initial_state(false); - this->initial_state_published_[pad] = true; + child->initial_state_published_ = true; ESP_LOGV(TAG, "Touch Pad '%s' state: OFF (initial)", child->get_name().c_str()); } } diff --git a/esphome/components/esp32_touch/esp32_touch_v1.cpp b/esphome/components/esp32_touch/esp32_touch_v1.cpp index a6d499e9fa..6f05610ed6 100644 --- a/esphome/components/esp32_touch/esp32_touch_v1.cpp +++ b/esphome/components/esp32_touch/esp32_touch_v1.cpp @@ -104,7 +104,7 @@ void ESP32TouchComponent::loop() { // Track when we last saw this pad as touched if (new_state) { - this->last_touch_time_[event.pad] = now; + child->last_touch_time_ = now; } // Only publish if state changed - this filters out repeated events @@ -127,15 +127,13 @@ void ESP32TouchComponent::loop() { size_t pads_off = 0; for (auto *child : this->children_) { - touch_pad_t pad = child->get_touch_pad(); - // Handle initial state publication after startup this->publish_initial_state_if_needed_(child, now); if (child->last_state_) { // Pad is currently in touched state - check for release timeout // Using subtraction handles 32-bit rollover correctly - uint32_t time_diff = now - this->last_touch_time_[pad]; + uint32_t time_diff = now - child->last_touch_time_; // Check if we haven't seen this pad recently if (time_diff > this->release_timeout_ms_) { diff --git a/esphome/components/esp32_touch/esp32_touch_v2.cpp b/esphome/components/esp32_touch/esp32_touch_v2.cpp index ad77881724..afd2655fd7 100644 --- a/esphome/components/esp32_touch/esp32_touch_v2.cpp +++ b/esphome/components/esp32_touch/esp32_touch_v2.cpp @@ -14,19 +14,16 @@ static const char *const TAG = "esp32_touch"; void ESP32TouchComponent::update_touch_state_(ESP32TouchBinarySensor *child, bool is_touched) { // Always update timer when touched if (is_touched) { - this->last_touch_time_[child->get_touch_pad()] = App.get_loop_component_start_time(); + child->last_touch_time_ = App.get_loop_component_start_time(); } if (child->last_state_ != is_touched) { - // Read value for logging - uint32_t value = this->read_touch_value(child->get_touch_pad()); - child->last_state_ = is_touched; child->publish_state(is_touched); if (is_touched) { // ESP32-S2/S3 v2: touched when value > threshold ESP_LOGV(TAG, "Touch Pad '%s' state: ON (value: %" PRIu32 " > threshold: %" PRIu32 ")", child->get_name().c_str(), - value, child->get_threshold()); + this->read_touch_value(child->touch_pad_), child->threshold_ + child->benchmark_); } else { ESP_LOGV(TAG, "Touch Pad '%s' state: OFF", child->get_name().c_str()); } @@ -36,10 +33,13 @@ void ESP32TouchComponent::update_touch_state_(ESP32TouchBinarySensor *child, boo // Helper to read touch value and update state for a given child (used for timeout events) bool ESP32TouchComponent::check_and_update_touch_state_(ESP32TouchBinarySensor *child) { // Read current touch value - uint32_t value = this->read_touch_value(child->get_touch_pad()); + uint32_t value = this->read_touch_value(child->touch_pad_); - // ESP32-S2/S3 v2: Touch is detected when value > threshold - bool is_touched = value > child->get_threshold(); + // ESP32-S2/S3 v2: Touch is detected when value > threshold + benchmark + ESP_LOGV(TAG, + "Checking touch state for '%s' (T%d): value = %" PRIu32 ", threshold = %" PRIu32 ", benchmark = %" PRIu32, + child->get_name().c_str(), child->touch_pad_, value, child->threshold_, child->benchmark_); + bool is_touched = value > child->benchmark_ + child->threshold_; this->update_touch_state_(child, is_touched); return is_touched; @@ -61,9 +61,9 @@ void ESP32TouchComponent::setup() { // Configure each touch pad first for (auto *child : this->children_) { - esp_err_t config_err = touch_pad_config(child->get_touch_pad()); + esp_err_t config_err = touch_pad_config(child->touch_pad_); if (config_err != ESP_OK) { - ESP_LOGE(TAG, "Failed to configure touch pad %d: %s", child->get_touch_pad(), esp_err_to_name(config_err)); + ESP_LOGE(TAG, "Failed to configure touch pad %d: %s", child->touch_pad_, esp_err_to_name(config_err)); } } @@ -100,8 +100,8 @@ void ESP32TouchComponent::setup() { // Configure measurement parameters touch_pad_set_voltage(this->high_voltage_reference_, this->low_voltage_reference_, this->voltage_attenuation_); - // ESP32-S2/S3 always use the older API - touch_pad_set_meas_time(this->sleep_cycle_, this->meas_cycle_); + touch_pad_set_charge_discharge_times(this->meas_cycle_); + touch_pad_set_measurement_interval(this->sleep_cycle_); // Configure timeout if needed touch_pad_timeout_set(true, TOUCH_PAD_THRESHOLD_MAX); @@ -118,8 +118,8 @@ void ESP32TouchComponent::setup() { // Set thresholds for each pad BEFORE starting FSM for (auto *child : this->children_) { - if (child->get_threshold() != 0) { - touch_pad_set_thresh(child->get_touch_pad(), child->get_threshold()); + if (child->threshold_ != 0) { + touch_pad_set_thresh(child->touch_pad_, child->threshold_); } } @@ -277,6 +277,7 @@ void ESP32TouchComponent::loop() { // Process any queued touch events from interrupts TouchPadEventV2 event; while (xQueueReceive(this->touch_queue_, &event, 0) == pdTRUE) { + ESP_LOGD(TAG, "Event received, mask = 0x%" PRIx32 ", pad = %d", event.intr_mask, event.pad); // Handle timeout events if (event.intr_mask & TOUCH_PAD_INTR_MASK_TIMEOUT) { // Resume measurement after timeout @@ -289,18 +290,16 @@ void ESP32TouchComponent::loop() { // Find the child for the pad that triggered the interrupt for (auto *child : this->children_) { - if (child->get_touch_pad() != event.pad) { - continue; + if (child->touch_pad_ == event.pad) { + if (event.intr_mask & TOUCH_PAD_INTR_MASK_TIMEOUT) { + // For timeout events, we need to read the value to determine state + this->check_and_update_touch_state_(child); + } else if (event.intr_mask & TOUCH_PAD_INTR_MASK_ACTIVE) { + // We only get ACTIVE interrupts now, releases are detected by timeout + this->update_touch_state_(child, true); // Always touched for ACTIVE interrupts + } + break; } - - if (event.intr_mask & TOUCH_PAD_INTR_MASK_TIMEOUT) { - // For timeout events, we need to read the value to determine state - this->check_and_update_touch_state_(child); - } else if (event.intr_mask & TOUCH_PAD_INTR_MASK_ACTIVE) { - // We only get ACTIVE interrupts now, releases are detected by timeout - this->update_touch_state_(child, true); // Always touched for ACTIVE interrupts - } - break; } } @@ -311,15 +310,15 @@ void ESP32TouchComponent::loop() { size_t pads_off = 0; for (auto *child : this->children_) { - touch_pad_t pad = child->get_touch_pad(); - + if (child->benchmark_ == 0) + touch_pad_read_benchmark(child->touch_pad_, &child->benchmark_); // Handle initial state publication after startup this->publish_initial_state_if_needed_(child, now); if (child->last_state_) { // Pad is currently in touched state - check for release timeout // Using subtraction handles 32-bit rollover correctly - uint32_t time_diff = now - this->last_touch_time_[pad]; + uint32_t time_diff = now - child->last_touch_time_; // Check if we haven't seen this pad recently if (time_diff > this->release_timeout_ms_) {