From 4fdd6b6b4fe7387e7e6a99d855c31c3d2bb98c77 Mon Sep 17 00:00:00 2001
From: Djuri Baars <dsbaars@users.noreply.github.com>
Date: Sat, 21 Dec 2024 17:36:07 +0100
Subject: [PATCH] Fix button handling

---
 src/lib/button_handler.cpp | 58 +++++++++++++++++++++++---------------
 src/lib/button_handler.hpp |  7 +++++
 src/lib/config.cpp         | 46 +++++++++++++++---------------
 3 files changed, 66 insertions(+), 45 deletions(-)

diff --git a/src/lib/button_handler.cpp b/src/lib/button_handler.cpp
index 72f1eaa..c2f2a21 100644
--- a/src/lib/button_handler.cpp
+++ b/src/lib/button_handler.cpp
@@ -19,41 +19,52 @@ TickType_t lastDebounceTime = 0;
 void buttonTask(void *parameter) {
   while (1) {
     ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
-    std::lock_guard<std::mutex> lock(mcpMutex);
-
+    
     TickType_t currentTime = xTaskGetTickCount();
-
+    
     if ((currentTime - lastDebounceTime) >= debounceDelay) {
       lastDebounceTime = currentTime;
-
+      
+      std::lock_guard<std::mutex> lock(mcpMutex);
+      
       if (!digitalRead(MCP_INT_PIN)) {
-        uint pin = mcp1.getInterruptFlagRegister();
-
-        switch (pin) {
-          case BTN_1:
-            toggleTimerActive();
-            break;
-          case BTN_2:
-            nextScreen();
-            break;
-          case BTN_3:
-            previousScreen();
-            break;
-          case BTN_4:
-            showSystemStatusScreen();
-            break;
-        }
+        uint16_t intFlags = mcp1.getInterruptFlagRegister();
+        uint16_t intCap = mcp1.getInterruptCaptureRegister();
+                
+        // Check each button individually
+        if (intFlags & BTN_1) handleButton1();
+        if (intFlags & BTN_2) handleButton2();
+        if (intFlags & BTN_3) handleButton3();
+        if (intFlags & BTN_4) handleButton4();
       }
-      mcp1.getInterruptCaptureRegister();
-    } else {
     }
-    // Very ugly, but for some reason this is necessary
+    
+    // Clear interrupt state
     while (!digitalRead(MCP_INT_PIN)) {
+      std::lock_guard<std::mutex> lock(mcpMutex);
       mcp1.getInterruptCaptureRegister();
+      delay(1);  // Small delay to prevent tight loop
     }
   }
 }
 
+// Helper functions to handle each button
+void handleButton1() {
+  toggleTimerActive();
+}
+
+void handleButton2() {
+  nextScreen();
+}
+
+void handleButton3() {
+  previousScreen();
+}
+
+void handleButton4() {
+  showSystemStatusScreen();
+}
+
 void IRAM_ATTR handleButtonInterrupt() {
   BaseType_t xHigherPriorityTaskWoken = pdFALSE;
   xTaskNotifyFromISR(buttonTaskHandle, 0, eNoAction, &xHigherPriorityTaskWoken);
@@ -67,4 +78,5 @@ void setupButtonTask() {
               &buttonTaskHandle);  // Create the FreeRTOS task
   // Use interrupt instead of task
   attachInterrupt(MCP_INT_PIN, handleButtonInterrupt, CHANGE);
+  Serial.printf("Button task created\n");
 }
diff --git a/src/lib/button_handler.hpp b/src/lib/button_handler.hpp
index 9f28d33..ed8ae38 100644
--- a/src/lib/button_handler.hpp
+++ b/src/lib/button_handler.hpp
@@ -8,6 +8,13 @@
 
 extern TaskHandle_t buttonTaskHandle;
 
+// Task and setup functions
 void buttonTask(void *pvParameters);
 void IRAM_ATTR handleButtonInterrupt();
 void setupButtonTask();
+
+// Individual button handlers
+void handleButton1();
+void handleButton2();
+void handleButton3();
+void handleButton4();
diff --git a/src/lib/config.cpp b/src/lib/config.cpp
index 9feb9a9..08f645e 100644
--- a/src/lib/config.cpp
+++ b/src/lib/config.cpp
@@ -466,32 +466,34 @@ void setupHardware()
 
   Wire.begin(I2C_SDA_PIN, I2C_SCK_PIN, 400000);
 
-  if (!mcp1.begin())
-  {
+  if (!mcp1.begin()) {
     Serial.println(F("Error MCP23017 1"));
-
-    // while (1)
-    //         ;
-  }
-  else
-  {
+  } else {
     pinMode(MCP_INT_PIN, INPUT_PULLUP);
-//    mcp1.setupInterrupts(false, false, LOW);
-    mcp1.enableControlRegister(MCP23x17_IOCR_ODR);
-
-    mcp1.mirrorInterrupts(true);
-
-    for (int i = 0; i < 4; i++)
-    {
-      mcp1.pinMode1(i, INPUT_PULLUP);
-      mcp1.enableInterrupt(i, LOW);
+    
+    // Enable mirrored interrupts (both INTA and INTB pins signal any interrupt)
+    if (!mcp1.mirrorInterrupts(true)) {
+        Serial.println(F("Error setting up mirrored interrupts"));
     }
-#ifndef IS_BTCLOCK_V8
-    for (int i = 8; i <= 14; i++)
-    {
-      mcp1.pinMode1(i, OUTPUT);
+
+    // Configure all 4 button pins as inputs with pullups and interrupts
+    for (int i = 0; i < 4; i++) {
+        if (!mcp1.pinMode1(i, INPUT_PULLUP)) {
+            Serial.printf("Error setting pin %d to input pull up\n", i);
+        }
+        // Enable interrupt on CHANGE for each pin
+        if (!mcp1.enableInterrupt(i, CHANGE)) {
+            Serial.printf("Error enabling interrupt for pin %d\n", i);
+        }
     }
-#endif
+
+    // Set interrupt pins as open drain with active-low polarity
+    if (!mcp1.setInterruptPolarity(2)) { // 2 = Open drain
+        Serial.println(F("Error setting interrupt polarity"));
+    }
+
+    // Clear any pending interrupts
+    mcp1.getInterruptCaptureRegister();
   }
 
 #ifdef IS_HW_REV_B