Fix race condition causing lost events

ClickEncoder.cpp

@@ -15,8 +15,6 @@
 // Button configuration (values for 1ms timer service calls)
 //
 #define ENC_BUTTONINTERVAL    10  // check button every x milliseconds, also debouce time
-#define ENC_DOUBLECLICKTIME  600  // second click within 600ms
-#define ENC_HOLDTIME        1200  // report held button after 1.2s
 
 // ----------------------------------------------------------------------------
 // Acceleration configuration (for 1000Hz calls to ::service())
@@ -28,7 +26,7 @@
 // ----------------------------------------------------------------------------
 
 #if ENC_DECODER != ENC_NORMAL
-#  ifdef ENC_HALFSTEP
+#  if ENC_HALFSTEP
      // decoding table for hardware with flaky notch (half resolution)
      const int8_t ClickEncoder::table[16] __attribute__((__progmem__)) = { 
        0, 0, -1, 0, 0, 0, 0, 1, 1, 0, 0, 0, 0, -1, 0, 0 
@@ -43,16 +41,21 @@
 
 // ----------------------------------------------------------------------------
 
-ClickEncoder::ClickEncoder(uint8_t A, uint8_t B, uint8_t BTN, uint8_t stepsPerNotch, bool active)
-  : doubleClickEnabled(true), accelerationEnabled(true),
+ClickEncoder::ClickEncoder(int8_t A, int8_t B, int8_t BTN, uint8_t stepsPerNotch, bool active)
+  : doubleClickEnabled(true),buttonHeldEnabled(true), accelerationEnabled(true),
     delta(0), last(0), acceleration(0),
     button(Open), steps(stepsPerNotch),
     pinA(A), pinB(B), pinBTN(BTN), pinsActive(active)
+#ifndef WITHOUT_BUTTON
+    , analogInput(false)
+#endif
 {
   uint8_t configType = (pinsActive == LOW) ? INPUT_PULLUP : INPUT;
-  pinMode(pinA, configType);
-  pinMode(pinB, configType);
-  pinMode(pinBTN, configType);
+  if (pinA >= 0) {pinMode(pinA, configType);}
+  if (pinB >= 0) {pinMode(pinB, configType);}
+#ifndef WITHOUT_BUTTON
+  if (pinBTN >= 0) {pinMode(pinBTN, configType);}
+#endif
 
   if (digitalRead(pinA) == pinsActive) {
     last = 3;
@@ -63,14 +66,57 @@ ClickEncoder::ClickEncoder(uint8_t A, uint8_t B, uint8_t BTN, uint8_t stepsPerNo
   }
 }
 
+// ----------------------------------------------------------------------------
+#ifndef WITHOUT_BUTTON
+
+
+// Depricated.  Use DigitalButton instead
+ClickEncoder::ClickEncoder(int8_t BTN, bool active)
+  : doubleClickEnabled(true),buttonHeldEnabled(true), accelerationEnabled(true),
+    delta(0), last(0), acceleration(0),
+    button(Open), steps(1), analogInput(false),
+    pinA(-1), pinB(-1), pinBTN(BTN), pinsActive(active)
+{
+  uint8_t configType = (pinsActive == LOW) ? INPUT_PULLUP : INPUT;
+  if (pinBTN >= 0) {pinMode(pinBTN, configType);}
+}
+
+// ----------------------------------------------------------------------------
+// Constructor for using digital input as a button
+
+DigitalButton::DigitalButton(int8_t BTN, bool active) : ClickEncoder(BTN, active)
+
+{
+
+}
+
+// ----------------------------------------------------------------------------
+// Constructor for using analog input range as a button
+
+AnalogButton::AnalogButton(int8_t BTN, int16_t rangeLow, int16_t rangeHigh) : ClickEncoder(BTN, false)
+{
+  pinMode(pinBTN, INPUT);
+
+  anlogActiveRangeLow = rangeLow;
+  anlogActiveRangeHigh = rangeHigh;
+  analogInput = true;
+
+  if (anlogActiveRangeLow > anlogActiveRangeHigh) {    // swap values if provided in the wrong order
+	  int16_t t = anlogActiveRangeLow;
+	  anlogActiveRangeLow = anlogActiveRangeHigh;
+	  anlogActiveRangeHigh = t;
+  }
+}
+#endif
+
 // ----------------------------------------------------------------------------
 // call this every 1 millisecond via timer ISR
 //
 void ClickEncoder::service(void)
 {
   bool moved = false;
-  unsigned long now = millis();
 
+  if (pinA >= 0 && pinA >= 0) {
   if (accelerationEnabled) { // decelerate every tick
     acceleration -= ENC_ACCEL_DEC;
     if (acceleration & 0x8000) { // handle overflow of MSB is set
@@ -89,7 +135,7 @@ void ClickEncoder::service(void)
     last |= 1;
   }
 
-  uint8_t tbl = pgm_read_byte(&table[last]); 
+  int8_t tbl = pgm_read_byte(&table[last]); 
   if (tbl) {
     delta += tbl;
     moved = true;
@@ -122,42 +168,42 @@ void ClickEncoder::service(void)
       acceleration += ENC_ACCEL_INC;
     }
   }
-
+}
   // handle button
   //
 #ifndef WITHOUT_BUTTON
-  static uint16_t keyDownTicks = 0;
-  static uint8_t doubleClickTicks = 0;
-  static unsigned long lastButtonCheck = 0;
-
-  if (pinBTN > 0 // check button only, if a pin has been provided
-      && (now - lastButtonCheck) >= ENC_BUTTONINTERVAL) // checking button is sufficient every 10-30ms
+  unsigned long currentMillis = millis();
+  if (currentMillis < lastButtonCheck) lastButtonCheck = 0;        // Handle case when millis() wraps back around to zero
+  if ((pinBTN > 0 || (pinBTN == 0 && buttonOnPinZeroEnabled))        // check button only, if a pin has been provided
+      && ((currentMillis - lastButtonCheck) >= ENC_BUTTONINTERVAL))            // checking button is sufficient every 10-30ms
   { 
-    lastButtonCheck = now;
+    lastButtonCheck = currentMillis;
+
+    bool pinRead = getPinState();
 
-    if (digitalRead(pinBTN) == pinsActive) { // key is down
+    if (pinRead == pinsActive) { // key is down
       keyDownTicks++;
-      if (keyDownTicks > (ENC_HOLDTIME / ENC_BUTTONINTERVAL)) {
+      if ((keyDownTicks > (buttonHoldTime / ENC_BUTTONINTERVAL)) && (buttonHeldEnabled)) {
         button = Held;
       }
     }
 
-    if (digitalRead(pinBTN) == !pinsActive) { // key is now up
-      if (keyDownTicks /*> ENC_BUTTONINTERVAL*/) {
+    if (pinRead == !pinsActive) { // key is now up
+      if (keyDownTicks > 1) {               //Make sure key was down through 1 complete tick to prevent random transients from registering as click
         if (button == Held) {
           button = Released;
           doubleClickTicks = 0;
         }
         else {
           #define ENC_SINGLECLICKONLY 1
           if (doubleClickTicks > ENC_SINGLECLICKONLY) {   // prevent trigger in single click mode
-            if (doubleClickTicks < (ENC_DOUBLECLICKTIME / ENC_BUTTONINTERVAL)) {
+            if (doubleClickTicks < (buttonDoubleClickTime / ENC_BUTTONINTERVAL)) {
               button = DoubleClicked;
               doubleClickTicks = 0;
             }
           }
           else {
-            doubleClickTicks = (doubleClickEnabled) ? (ENC_DOUBLECLICKTIME / ENC_BUTTONINTERVAL) : ENC_SINGLECLICKONLY;
+            doubleClickTicks = (doubleClickEnabled) ? (buttonDoubleClickTime / ENC_BUTTONINTERVAL) : ENC_SINGLECLICKONLY;
           }
         }
       }
@@ -167,7 +213,7 @@ void ClickEncoder::service(void)
 
     if (doubleClickTicks > 0) {
       doubleClickTicks--;
-      if (--doubleClickTicks == 0) {
+      if (doubleClickTicks == 0) {
         button = Clicked;
       }
     }
@@ -182,15 +228,13 @@ int16_t ClickEncoder::getValue(void)
 {
   int16_t val;
 
-  cli();
+  noInterrupts();
   val = delta;
 
   if (steps == 2) delta = val & 1;
   else if (steps == 4) delta = val & 3;
   else delta = 0; // default to 1 step per notch
 
-  sei();
-
   if (steps == 4) val >>= 2;
   if (steps == 2) val >>= 1;
 
@@ -203,6 +247,7 @@ int16_t ClickEncoder::getValue(void)
   else if (val > 0) {
     r += 1 + accel;
   }
+  interrupts();
 
   return r;
 }
@@ -212,10 +257,25 @@ int16_t ClickEncoder::getValue(void)
 #ifndef WITHOUT_BUTTON
 ClickEncoder::Button ClickEncoder::getButton(void)
 {
+  noInterrupts();
   ClickEncoder::Button ret = button;
-  if (button != ClickEncoder::Held) {
+  if (button != ClickEncoder::Held && ret != ClickEncoder::Open) {
     button = ClickEncoder::Open; // reset
   }
+  interrupts();
+
   return ret;
 }
+
+bool ClickEncoder::getPinState() {
+  bool pinState;
+  if (analogInput) {
+    int16_t pinValue = analogRead(pinBTN);
+    pinState = ((pinValue >= anlogActiveRangeLow) && (pinValue <= anlogActiveRangeHigh)) ?  LOW : HIGH;    // set result to LOW (button pressed) if analog input is in range
+  } else {
+    pinState = digitalRead(pinBTN);
+  }
+  return pinState;
+}
+
 #endif