Fixed button handling in combination with sleep mode

Ready for prototype!

Source/src/buttons.c

@@ -1,6 +1,7 @@
 #include "buttons.h"
 #include <avr/interrupt.h>
 #include <util/atomic.h>
+#include <util/delay.h>
 
 
 ButtonState buttonUp;
@@ -8,24 +9,26 @@ ButtonState buttonDown;
 ButtonState buttonOption;
 
 
-void button_init(ButtonState* state)
+// Since it's a pull-up, the value is low when the button is pressed.
-{
+// This macro just helps make the code more readable.
-  (*state).flags = BSFLastReported;
+#define isPressed(state) (!state)
-  (*state).debounceTicks = 0;
+
-  (*state).repeatTicks = 0;
+#define hasFlag(flags, value) ((flags & value) != 0)
-}
+#define setFlag(flags, value) flags |= (value)
+#define clearFlag(flags, value) flags &= ~(value)
+
+
+uint8_t inactivityTicks = 0;
 
 
 void buttons_init()
 {
-  button_init(&buttonUp);
-  button_init(&buttonDown);
-  button_init(&buttonOption);
-
   // Input with internal pull-up resistor
   DDRB &= ~BUTTON_ALL;
   PORTB |= BUTTON_ALL;
 
+  buttons_reset();
+
   // Mode 2: CTC
   TCCR0A = _BV(WGM01);
 
@@ -41,24 +44,40 @@ void buttons_init()
 }
 
 
+void button_init(ButtonState* state, uint8_t readState)
+{
+  (*state).flags = readState ? BSFLastReported : 0;
+  (*state).debounceTicks = 0;
+  (*state).repeatTicks = 0;
+}
 
-// Since it's a pull-up, the value is low when the button is pressed.
-// This macro just helps make the code more readable.
-#define isPressed(state) (!state)
 
-#define flagSet(flags, value) ((flags & value) != 0)
+void buttons_reset()
-#define setFlag(flags, value) flags |= (value)
+{
-#define clearFlag(flags, value) flags &= ~(value)
+  uint8_t pinState = PINB;
+
+  button_init(&buttonUp, hasFlag(pinState, _BV(BUTTON_UP)));
+  button_init(&buttonDown, hasFlag(pinState, _BV(BUTTON_DOWN)));
+  button_init(&buttonOption, hasFlag(pinState, _BV(BUTTON_OPTION)));
+}
 
 
 
-void button_check(ButtonState* state, uint8_t readState)
+uint8_t buttons_active()
+{
+  return inactivityTicks <= DEBOUNCE_TIME;
+}
+
+
+
+uint8_t button_check(ButtonState* state, uint8_t readState)
 {
   #define stateRef (*state)
 
-  uint8_t lastState = flagSet(stateRef.flags, BSFLastReported);
+  uint8_t lastState = hasFlag(stateRef.flags, BSFLastReported);
   uint8_t newState = lastState;
 
+
   // Debounce
   if (readState != lastState)
   {
@@ -76,7 +95,7 @@ void button_check(ButtonState* state, uint8_t readState)
   }
   else
   {
-    // State changed, reset debounce timer
+    // State reverted, reset debounce timer
     stateRef.debounceTicks = 0;
   }
 
@@ -91,7 +110,7 @@ void button_check(ButtonState* state, uint8_t readState)
       setFlag(stateRef.flags, BSFPressed | BSFRepeatEnabled);
       stateRef.repeatTicks = REPEAT_START;
     }
-    else if (flagSet(stateRef.flags, BSFRepeatEnabled))
+    else if (hasFlag(stateRef.flags, BSFRepeatEnabled))
     {
       // Held pressed
       stateRef.repeatTicks--;
@@ -105,10 +124,12 @@ void button_check(ButtonState* state, uint8_t readState)
   }
   else if (isPressed(lastState))
   {
-    // Released
+    // Only trigger Released if Pressed is not yet removed by polling
+    if (hasFlag(stateRef.flags, BSFPressed))
       setFlag(stateRef.flags, BSFReleased);
   }
 
+  return isPressed(newState) || (stateRef.debounceTicks > 0);
   #undef stateRef
 }
 
@@ -118,10 +139,16 @@ void button_check(ButtonState* state, uint8_t readState)
 ISR(TIMER0_COMPA_vect)
 {
   uint8_t pinState = PINB;
+  uint8_t buttonActive;
 
-  button_check(&buttonUp, flagSet(pinState, _BV(BUTTON_UP)));
+  buttonActive = button_check(&buttonUp, hasFlag(pinState, _BV(BUTTON_UP)));
-  button_check(&buttonDown, flagSet(pinState, _BV(BUTTON_DOWN)));
+  buttonActive = button_check(&buttonDown, hasFlag(pinState, _BV(BUTTON_DOWN))) || buttonActive;
-  button_check(&buttonOption, flagSet(pinState, _BV(BUTTON_OPTION)));
+  buttonActive = button_check(&buttonOption, hasFlag(pinState, _BV(BUTTON_OPTION))) || buttonActive;
+
+  if (buttonActive)
+    inactivityTicks = 0;
+  else if (inactivityTicks <= DEBOUNCE_TIME)
+    inactivityTicks++;
 }
 
 
@@ -132,7 +159,7 @@ inline uint8_t readAndClearFlag(ButtonState* state, uint8_t flag)
 
   ATOMIC_BLOCK(ATOMIC_FORCEON)
   {
-    result = flagSet((*state).flags, flag);
+    result = hasFlag((*state).flags, flag);
     (*state).flags &= ~(flag);
   }
 
@@ -162,17 +189,18 @@ uint8_t button_is_pressed_or_repeated(ButtonState* state)
 }
 
 
-uint8_t button_is_released_short(ButtonState* state)
+uint8_t button_is_pressed_short(ButtonState* state)
 {
   uint8_t result;
 
   ATOMIC_BLOCK(ATOMIC_FORCEON)
   {
-    result = flagSet((*state).flags, BSFReleased) &&
+    result = hasFlag((*state).flags, BSFPressed) &&
-             (!flagSet((*state).flags, BSFRepeated));
+             hasFlag((*state).flags, BSFReleased) &&
+             (!hasFlag((*state).flags, BSFRepeated));
 
     if (result)
-      (*state).flags &= ~BSFReleased;
+      (*state).flags &= ~(BSFPressed | BSFReleased);
   }
 
   return result;
@@ -184,8 +212,8 @@ uint8_t button_is_pressed_long(ButtonState* state)
 
   ATOMIC_BLOCK(ATOMIC_FORCEON)
   {
-    result = flagSet((*state).flags, BSFPressed) &&
+    result = hasFlag((*state).flags, BSFPressed) &&
-             flagSet((*state).flags, BSFRepeated);
+             hasFlag((*state).flags, BSFRepeated);
 
     // Remove RepeatEnabled as well to prevent further repeat events
     if (result)

Source/src/buttons.h

@@ -70,8 +70,11 @@ extern ButtonState buttonOption;
 // every 200ms
 #define REPEAT_NEXT 20
 
-
 extern void buttons_init();
+extern void buttons_reset();
+
+// Returns true if sleep should be prevented
+extern uint8_t buttons_active();
 
 
 // Note: these functions act on events, not on the current state.
@@ -84,7 +87,7 @@ extern uint8_t button_is_repeated(ButtonState* state);
 
 extern uint8_t button_is_pressed_or_repeated(ButtonState* state);
 
-extern uint8_t button_is_released_short(ButtonState* state);
+extern uint8_t button_is_pressed_short(ButtonState* state);
 extern uint8_t button_is_pressed_long(ButtonState* state);
 
 #endif

Source/src/main.c

@@ -1,4 +1,5 @@
 #include <stdint.h>
+#include <stdlib.h>
 #include <util/delay.h>
 #include <ssd1306xled.h>
 #include "power.h"
@@ -29,6 +30,8 @@ int main()
       if (powerState == On)
       {
           handleCurrentScreen();
+
+          if (!buttons_active())
             sleepUntilButton(_BV(BUTTON_UP) | _BV(BUTTON_DOWN) | _BV(BUTTON_OPTION));
       }
     }
@@ -40,13 +43,11 @@ int main()
 
 uint8_t handleGlobalInput()
 {
-  if (button_is_released_short(&buttonOption))
+  if (button_is_pressed_short(&buttonOption))
   {
-    // Toggle power
+    // Turn off power to the screen
     if (powerState == On)
       setPowerState(ManualOff);
-    else if (powerState == ManualOff)
-      setPowerState(On);
 
     return 1;
   }

Source/src/power.c

@@ -3,6 +3,7 @@
 #include <avr/io.h>
 #include <avr/interrupt.h>
 #include <avr/sleep.h>
+#include <util/atomic.h>
 #include <util/delay.h>
 #include <ssd1306xled.h>
 
@@ -26,7 +27,6 @@
 
 PowerState powerState = BatteryLow;
 uint16_t vcc = 0;
-uint8_t screenInvalidated = 0;
 
 
 // Forward declarations
@@ -66,6 +66,7 @@ void checkPower()
 
     case ManualOff:
       sleepUntilButton(_BV(BUTTON_OPTION));
+      setPowerState(On);
       break;
   }
 }
@@ -78,8 +79,8 @@ void setPowerState(PowerState newState)
   switch (newState)
   {
     case On:
+      buttons_reset();
       ssd1306_send_command(SSD1306CommandOn);
-      //buttons_init(); TODO this doesn't help, fix it!
       break;
 
     case BatteryLow:
@@ -95,22 +96,7 @@ void setPowerState(PowerState newState)
 }
 
 
-uint8_t getScreenInvalidated()
-{
-  if (screenInvalidated)
-  {
-    screenInvalidated = 0;
-    return 1;
-  }
 
-  return 0;
-}
-
-
-void setScreenInvalidated()
-{
-  screenInvalidated = 1;
-}
 
 
 // Source: http://21stdigitalhome.blogspot.nl/2014/10/trinket-attiny85-internal-temperature.html
@@ -152,7 +138,7 @@ uint16_t readVCC() {
 }
 
 
-inline void sleepStart()
+static inline void sleepStart()
 {
   // Turn ADC off
   ADCSRA &= ~_BV(ADEN);
@@ -168,7 +154,7 @@ inline void sleepStart()
 }
 
 
-inline void sleepEnd()
+static inline void sleepEnd()
 {
   sleep_disable();
   ADCSRA |= _BV(ADEN);

Source/src/power.h

@@ -27,8 +27,4 @@ extern void sleepUntilButton(uint8_t pinMask);
 // 9 = 8 sec
 extern void sleepUntilTimer(uint8_t interval);
 
-
-extern uint8_t getScreenInvalidated();
-extern void setScreenInvalidated();
-
 #endif

Source/src/screen/counter.c

@@ -1,6 +1,7 @@
 #include "counter.h"
 #include <stdint.h>
 #include <ssd1306xled.h>
+#include "state.h"
 #include "../buttons.h"
 
 

Source/src/screen/state.c

@@ -0,0 +1,22 @@
+#include "state.h"
+
+
+uint8_t screenInvalidated = 0;
+
+
+uint8_t getScreenInvalidated()
+{
+  if (screenInvalidated)
+  {
+    screenInvalidated = 0;
+    return 1;
+  }
+
+  return 0;
+}
+
+
+void setScreenInvalidated()
+{
+  screenInvalidated = 1;
+}

Source/src/screen/state.h

@@ -0,0 +1,9 @@
+#ifndef __ScreenState
+#define __ScreenState
+
+#include <stdint.h>
+
+extern uint8_t getScreenInvalidated();
+extern void setScreenInvalidated();
+
+#endif