Merge pull request #157 from jakub-k-dev/bugfix/fix-button-click-detection

Bugfix/fix button click detection

Author: edwardtfn

ESPHome/TX-Ultimate-Easy-ESPHome_hw_buttons.yaml

@@ -24,17 +24,15 @@ substitutions:
   BUTTON_3_ACTION_FAILSAFE_TEXT: "Relay 3 (API failsafe only)"
   BUTTON_4_ACTION_FAILSAFE_TEXT: "Relay 4 (API failsafe only)"
 
-  BUTTON_CLICK_MIN_LENGTH: '50'    # The minimum duration the click should last, in msec
-  BUTTON_CLICK_MAX_LENGTH: '350'   # The maximum duration the click should last, in msec
-  BUTTON_MULTI_CLICK_DELAY: '250'  # The time to wait for another click, in msec
-  BUTTON_PRESS_TIMEOUT: '10000'    # Ignore if button is pressed for longer than this time, in msec
-  BUTTON_LONG_PRESS_DELAY: '800'   # The time to wait to consider a long press, in msec
+  BUTTON_MULTI_CLICK_DELAY: "250" # The time to wait for another click, in msec
+  BUTTON_PRESS_TIMEOUT: "10000" # Ignore if button is pressed for longer than this time, in msec
+  BUTTON_LONG_PRESS_DELAY: "500" # The time to wait to consider a long press, in msec
 
   # Button numbers constants
-  BUTTON_1_ID: '1'
-  BUTTON_2_ID: '2'
-  BUTTON_3_ID: '3'
-  BUTTON_4_ID: '4'
+  BUTTON_1_ID: "1"
+  BUTTON_2_ID: "2"
+  BUTTON_3_ID: "3"
+  BUTTON_4_ID: "4"
 
   TAG_HW_BUTTONS: tx_ultimate_easy.hw.buttons
 
@@ -69,29 +67,36 @@ globals:
   - id: button_actions_packed
     type: uint16_t
     restore_value: true
-    initial_value: '4369'  # Binary: 0001000100010001 = all buttons set to option 1 (default action)
+    initial_value: "4369" # Binary: 0001000100010001 = all buttons set to option 1 (default action)
     # Bit layout: [button4(4)][button3(4)][button2(4)][button1(4)]
     # Each button uses 4 bits: 0=None, 1=Toggle, 2=Failsafe, 3-15=Reserved for future options
 
   - id: button_press_button
     type: uint8_t
     restore_value: false
-    initial_value: '0'
+    initial_value: "0"
 
   - id: button_press_position
     type: uint8_t
     restore_value: false
-    initial_value: '0'
+    initial_value: "0"
 
   - id: button_press_start_time
     type: uint32_t
     restore_value: false
-    initial_value: '0'
+    initial_value: "0"
+    # Used for: current press duration (press → release)
+
+  - id: last_release_time
+    type: uint32_t
+    restore_value: false
+    initial_value: "0"
+    # Used for: inter-click gap timing (time since last release)
 
   - id: click_counter
     type: uint8_t
     restore_value: false
-    initial_value: '0'
+    initial_value: "0"
 
 script:
   - id: !extend boot_initialize
@@ -180,6 +185,7 @@ script:
           id(button_press_button) = 0;
           id(button_press_position) = 0;
           id(button_press_start_time) = 0;
+          id(last_release_time) = 0;
           id(click_counter) = 0;
           buttons_release->execute();
 
@@ -210,6 +216,18 @@ script:
             }
           }
 
+  - id: buttons_reset_state
+    mode: restart
+    then:
+      - lambda: |-
+          // Reset all button-related global state
+          id(button_press_start_time) = 0;
+          id(last_release_time) = 0;
+          id(click_counter) = 0;
+          id(button_press_button) = 0;
+          id(button_press_position) = 0;
+          buttons_release->execute();
+
   - id: !extend dump_config
     then:
       - lambda: |-
@@ -275,7 +293,7 @@ script:
 
   - id: !extend touch_on_multi_touch_release
     then:
-      - script.execute: buttons_release
+      - script.execute: buttons_reset_state
 
   - id: !extend touch_on_press
     then:
@@ -291,15 +309,46 @@ script:
       position: uint8_t
     then:
       - lambda: |-
-          id(button_press_start_time) = millis();
-          id(button_press_button) = button;
-          // Update counters
-          if (id(button_press_position) == position) {
-            id(click_counter)++;
-          } else {
+          uint32_t current_time = millis();
+          // Check if we should start a new click sequence:
+          // 1. Previous cycle completed (start_time was reset to 0)
+          // 2. Position changed
+          // 3. Enough time has passed since last release (more than multi-click delay)
+          bool start_new_sequence = false;
+          if (id(button_press_start_time) == 0) {
+            // Previous cycle completed
+            start_new_sequence = true;
+          } else if (id(button_press_position) != position) {
+            // Position changed
+            start_new_sequence = true;
+          } else if (id(last_release_time) > 0) {
+            // Check if enough time has passed since last release (for inter-click gap)
+            // Use runtime value from HA instead of compile-time constant
+            uint32_t gap_window = (uint32_t) nr_button_multi_click_delay->state;
+            uint32_t time_since_last_release;
+            // Handle millis() overflow (wraps after ~49 days)
+            if (current_time >= id(last_release_time)) {
+              time_since_last_release = current_time - id(last_release_time);
+            } else {
+              // Overflow occurred, calculate correctly
+              time_since_last_release = (UINT32_MAX - id(last_release_time)) + current_time + 1;
+            }
+            if (time_since_last_release > gap_window) {
+              // Too much time passed since last release, start fresh
+              start_new_sequence = true;
+            }
+          }
+
+          if (start_new_sequence) {
             id(click_counter) = 1;
             id(button_press_position) = position;
+          } else {
+            // Same position and within multi-click window, increment counter
+            id(click_counter)++;
           }
+          // Set press start time for this press (for duration calculation)
+          id(button_press_start_time) = current_time;
+          id(button_press_button) = button;
           // Update binary sensor
           switch (button) {
             case 1:
@@ -321,34 +370,64 @@ script:
       - lambda: |-
           uint32_t current_time = millis();
           buttons_release->execute();
-          if (id(button_press_start_time) > 0 and
-              id(button_press_start_time) < current_time) {
-            uint32_t press_duration  = current_time - id(button_press_start_time);
-            // Handle overflow (optional, since it's unlikely to happen here)
+          if (id(button_press_start_time) > 0) {
+            uint32_t press_duration;
+            // Handle millis() overflow (wraps after ~49 days)
+            if (current_time >= id(button_press_start_time)) {
+              press_duration = current_time - id(button_press_start_time);
+            } else {
+              // Overflow occurred, calculate correctly
+              press_duration = (UINT32_MAX - id(button_press_start_time)) + current_time + 1;
+            }
             ESP_LOGI("${TAG_HW_BUTTONS}", "Button press duration: %" PRIu32 " ms", press_duration);
-            if (press_duration < ${BUTTON_CLICK_MIN_LENGTH}) {
-              ESP_LOGW("${TAG_HW_BUTTONS}", "Ignoring button press (too short)");
-            } else if (press_duration >= ${BUTTON_CLICK_MIN_LENGTH} and
-                        press_duration <= ${BUTTON_CLICK_MAX_LENGTH}) { // Short/normal click
-              button_click_event->execute(id(button_press_button), id(click_counter));
-            } else if (press_duration >= ${BUTTON_LONG_PRESS_DELAY} and press_duration <= ${BUTTON_PRESS_TIMEOUT}) {
-              button_action->execute(id(button_press_button), "long_press", 1);
+            // Update last release time for inter-click gap detection
+            id(last_release_time) = current_time;
+            if (press_duration <= ${BUTTON_LONG_PRESS_DELAY}) { // Short/normal click
+              // Validate button is still valid (in case swipe/multi-touch happened)
+              if (id(button_press_button) > 0 && id(button_press_button) <= 4) {
+                // Don't reset start_time here - let button_action reset it after multi-click delay
+                // This allows subsequent presses during the delay to increment the counter
+                button_click_event->execute(id(button_press_button), id(click_counter));
+              } else {
+                ESP_LOGW("${TAG_HW_BUTTONS}", "Invalid button state, resetting");
+                id(button_press_start_time) = 0;
+                id(last_release_time) = 0;
+                id(click_counter) = 0;
+                id(button_press_button) = 0;
+              }
+            } else if (press_duration > ${BUTTON_LONG_PRESS_DELAY} and press_duration <= ${BUTTON_PRESS_TIMEOUT}) {
+              // Long press: anything longer than BUTTON_LONG_PRESS_DELAY (and within timeout) is a long press
+              // Validate button is still valid (in case swipe/multi-touch happened)
+              if (id(button_press_button) > 0 && id(button_press_button) <= 4) {
+                button_action->execute(id(button_press_button), "long_press", 1);
+              } else {
+                ESP_LOGW("${TAG_HW_BUTTONS}", "Invalid button state during long press, resetting");
+                id(button_press_start_time) = 0;
+                id(last_release_time) = 0;
+                id(click_counter) = 0;
+                id(button_press_button) = 0;
+              }
             } else if (press_duration > ${BUTTON_PRESS_TIMEOUT}) { // Timeout or invalid
               ESP_LOGW("${TAG_HW_BUTTONS}",
                         "Button press cancelled or timed out after ${BUTTON_PRESS_TIMEOUT} ms");
+              id(button_press_start_time) = 0;
+              id(last_release_time) = 0;
+              id(click_counter) = 0;
             }
           } else {
             ESP_LOGW("${TAG_HW_BUTTONS}", "Press event timestamp not recorded yet");
+            id(button_press_start_time) = 0;
+            id(last_release_time) = 0;
+            id(click_counter) = 0;
           }
-          id(button_press_start_time) = 0;
 
   - id: !extend touch_swipe_${ID_SWIPE_LEFT_OR_UP_TEXT}
     then:
-      - script.execute: buttons_release
+      - script.execute: buttons_reset_state
 
   - id: !extend touch_swipe_${ID_SWIPE_RIGHT_OR_DOWN_TEXT}
     then:
-      - script.execute: buttons_release
+      - script.execute: buttons_reset_state
 
 select:
   - &button_select_action_base
@@ -416,4 +495,3 @@ select:
         - lambda: |-
             id(button_actions_packed) = (id(button_actions_packed) & ~(0x0F << 12)) |
                                         ((id(sl_button_4_action).active_index().value_or(1) & 0x0F) << 12);
-...