Bad programing or Button debouncing problem

[electron_misfire]

Hi Everyone,

I'm making a small LED cube that plays a sequence everytime a button is pressed. It's using the Attiny85.
So I got it working except with the attiny missing a few button presses.
To fix that I thought i'd make an interrupt for the button instead of polling it.

I made it worse.   

So now I'ts either missing button presses or registering them twice.

Here is my circuit layout:
http://123d.circuits.io/circuits/717787-the-unnamed-circuit


And here is my code (awful I know):

Code: [Select]

#include <avr/sleep.h>

int ledG = 0;
int ledY = 1;// the pin that the LED is attached to
int state = 0; //switch state
volatile int lastProgram = 0; //the last program
volatile int programAmount = 5; //the amount of sequences
int brightness = 0;    // how bright the LED is
int fadeAmount = 1;    // how many points to fade the LED by
int a = 0;

long intervalA = 30;
long intervalB = 500;
unsigned long currentMillis;
long previousMillis = 0;

// the setup routine runs once when you press reset:
void setup()  {
  // declare pin 9 to be an output:
  pinMode(ledG, OUTPUT);
  pinMode(ledY, OUTPUT);
  pinMode(2, INPUT_PULLUP);
  pinMode(3, INPUT); //the following outputs are useless, just for reduced power consumption
  pinMode(4, INPUT);
  pinMode(5, INPUT);
 
  GIMSK = 0b01000000;
  PCMSK = 0b00000001;
  sei();
  attachInterrupt(0, Bpress, LOW);
 
  ADCSRA &= ~(1<<ADEN);
  set_sleep_mode(SLEEP_MODE_PWR_DOWN);
}

// the loop routine runs over and over again forever:
void loop()  {
 
  currentMillis = millis();
    switch (lastProgram)
    {
      case 1:
        digitalWrite(ledG, LOW);
        digitalWrite(ledY, LOW);
        sleep_enable();
        sleep_cpu();
        break;
       
      case 2:
        fade();
        delay(7);
        break;
       
       case 3:
         flash();
         break;
       case 4:
         //dfdf
         digitalWrite(ledY, HIGH);
         break;
       case 5:
         //gchvh
         digitalWrite(ledG, HIGH);
         break;   
       default:
       //nothing
       break;
    }
}
    void Bpress()
    {
      if(lastProgram == programAmount)
      {
        lastProgram = 0;
      }
      lastProgram++;
    }
    void fade()
    {
      analogWrite(ledG, brightness);   
       
      brightness = brightness + fadeAmount;
         
      if (brightness == 0 || brightness == 255)
      {
       fadeAmount = -fadeAmount ;
      }
    }
   
    void flash()
    {
         if(currentMillis - previousMillis > intervalB && a == 0)
         {
           previousMillis = currentMillis;
           digitalWrite(ledG, HIGH);
           a = 1;
         }         
      if(currentMillis - previousMillis > intervalB && a == 1)
         {
           previousMillis = currentMillis;
           digitalWrite(ledG, LOW);
           a = 0;
         }
    }
   
Thanks for any help!

[Paul__B]

To fix that I thought I'd make an interrupt for the button instead of polling it.

I made it worse.  

Of course you did!  Using an interrupt on a push-button is a very, very bad blunder.

(Bet some wise guy pops up and says it is OK if you "do it properly". )

Please do try my super-paranoid button debounce code.  It is based not on the button being re-checked after a certain amount of time, but on it staying continuously pressed for a certain amount of time.

Code: [Select]

// Button toggle with extreme reliability!

const int ledpins[4] = {
  2, 3, 5, 6};
const int led1Pin =  13;    // LED pin number
const int buttons[2] =  {
  10, 11};

char bstates[2] = {
  0, 0};
char ledStates[2] = {
  0, 0};
unsigned long bcount[2] = {
  0, 0}; // button debounce timers.  Replicate as necessary.

// Have we completed the specified interval since last confirmed event?
// "marker" chooses which counter to check
// Routines by Paul__B of Arduino Forum
boolean timeout(unsigned long *marker, unsigned long interval) {
  if (millis() - *marker >= interval) {
    *marker += interval;    // move on ready for next interval
    return true;      
  }
  else return false;
}

// Deal with a button read; true if button pressed and debounced is a new event
// Uses reading of button input, debounce store, state store and debounce interval.
// Routines by Paul__B of Arduino Forum
boolean butndown(char button, unsigned long *marker, char *butnstate, unsigned long interval) {
  switch (*butnstate) {               // Odd states if was pressed, >= 2 if debounce in progress
  case 0: // Button up so far,
    if (button == HIGH) return false; // Nothing happening!
    else {
      *butnstate = 2;                 // record that is now pressed
      *marker = millis();             // note when was pressed
      return false;                   // and move on
    }

  case 1: // Button down so far,
    if (button == LOW) return false; // Nothing happening!
    else {
      *butnstate = 3;                 // record that is now released
      *marker = millis();             // note when was released
      return false;                   // and move on
    }

  case 2: // Button was up, now down.
    if (button == HIGH) {
      *butnstate = 0;                 // no, not debounced; revert the state
      return false;                   // False alarm!
    }
    else {
      if (millis() - *marker >= interval) {
        *butnstate = 1;               // jackpot!  update the state
        return true;                  // because we have the desired event!
      }
      else
        return false;                 // not done yet; just move on
    }

  case 3: // Button was down, now up.
    if (button == LOW) {
      *butnstate = 1;                 // no, not debounced; revert the state
      return false;                   // False alarm!
    }
    else {
      if (millis() - *marker >= interval) {
        *butnstate = 0;               // Debounced; update the state
        return false;                 // but it is not the event we want
      }
      else
        return false;                 // not done yet; just move on
    }
  default:                            // Error; recover anyway
    {  
      *butnstate = 0;
      return false;                   // Definitely false!
    }
  }
}

void setup() {
  for (int i=0; i < 4; i++) pinMode(ledpins[i], OUTPUT);      
  for (int i=0; i < 2; i++) {
    pinMode(buttons[i], INPUT);      
    digitalWrite(buttons[i],HIGH); // internal pullup all versions
  }        
}

void loop() {
  // Toggle LED if button debounced
  for (int i=0; i < 2; i++) {  
    if (butndown(digitalRead(buttons[i]), &bcount[i], &bstates[i], 10UL )) {
      if (ledStates[i] == LOW) {
        ledStates[i] = HIGH;
      }
      else {
        ledStates[i] = LOW;
      }
      digitalWrite(ledpins[i], ledStates[i]);
    }
  }
}

{Just why is Using an interrupt on a push-button so foolish?  Because it generates an interrupt for every single bounce of the button!}

[Paul__B]

Here's the single button, simplified version:

Code: [Select]

// Button toggle with extreme reliability!

const int led1Pin =  13;    // LED pin number
const int button1 =  10;
int led1State = LOW;        // initialise the LED
char bstate1 = 0;
unsigned long bcount1 = 0; // button debounce timer.  Replicate as necessary.

// Have we completed the specified interval since last confirmed event?
// "marker" chooses which counter to check
// Routines by Paul__B of Arduino Forum
boolean timeout(unsigned long *marker, unsigned long interval) {
  if (millis() - *marker >= interval) {
    *marker += interval;    // move on ready for next interval
    return true;       
  }
  else return false;
}

// Deal with a button read; true if button pressed and debounced is a new event
// Uses reading of button input, debounce store, state store and debounce interval.
// Routines by Paul__B of Arduino Forum
boolean butndown(char button, unsigned long *marker, char *butnstate, unsigned long interval) {
  switch (*butnstate) {               // Odd states if was pressed, >= 2 if debounce in progress
  case 0: // Button up so far,
    if (button == HIGH) return false; // Nothing happening!
    else {
      *butnstate = 2;                 // record that is now pressed
      *marker = millis();             // note when was pressed
      return false;                   // and move on
    }

  case 1: // Button down so far,
    if (button == LOW) return false; // Nothing happening!
    else {
      *butnstate = 3;                 // record that is now released
      *marker = millis();             // note when was released
      return false;                   // and move on
    }

  case 2: // Button was up, now down.
    if (button == HIGH) {
      *butnstate = 0;                 // no, not debounced; revert the state
      return false;                   // False alarm!
    }
    else {
      if (millis() - *marker >= interval) {
        *butnstate = 1;               // jackpot!  update the state
        return true;                  // because we have the desired event!
      }
      else
        return false;                 // not done yet; just move on
    }

  case 3: // Button was down, now up.
    if (button == LOW) {
      *butnstate = 1;                 // no, not debounced; revert the state
      return false;                   // False alarm!
    }
    else {
      if (millis() - *marker >= interval) {
        *butnstate = 0;               // Debounced; update the state
        return false;                 // but it is not the event we want
      }
      else
        return false;                 // not done yet; just move on
    }
  default:                            // Error; recover anyway
    { 
      *butnstate = 0;
      return false;                   // Definitely false!
    }
  }
}

void setup() {
  pinMode(led1Pin, OUTPUT);     
  pinMode(button1, INPUT);     
  digitalWrite(button1,HIGH);        // internal pullup all versions
}

void loop() {
  // Toggle LED if button debounced
  if (butndown(digitalRead(button1), &bcount1, &bstate1, 10UL )) {
    if (led1State == LOW) {
      led1State = HIGH;
    }
    else {
      led1State = LOW;
    }
    digitalWrite(led1Pin, led1State);
  }
}


Note that these contain the important non-blocking time delay "timeout()" function to be used for sequencing other events as well.