New to Arduino with a de-bouncing question

Hi all,

I am just working on my first real Arduino project. I have a switch connected between digital pin 2 and ground. I am using the attachInterrupt function to create a counter.

const int DebounceDelay = 250;
const int Counter1Pin = 2;

void setup()
{

  pinMode(Counter1Pin, INPUT);
  digitalWrite(Counter1Pin, HIGH);

  attachInterrupt(0, Counter1Handler, FALLING);

}

void loop ()
{
}

void Counter1Handler()
{
  static unsigned long LastInterruptTime = 0;
  unsigned long InterruptTime = millis();

  if (InterruptTime - LastInterruptTime > DebounceDelay) {
      Counter1 ++;
  }

  LastInterruptTime = InterruptTime;
}

This works in so far as it debounces the switch but if I hold my finger on the switch for a second or more then Counter1 increments again.

What am I doing wrong?

Z

This works in so far as it debounces the switch but if I hold my finger on the switch for a second or more then Counter1 increments again.

How can you see this as I cannot even compile your sketch above? Please upload the complete code.

If you're counter 1 pin is a input you shouldn't be setting it as high. As an input it would be expecting a current change, not creating it.

I believe what you want to do is rather than go from pin 2 to switch to ground instead go from +5v to switch to pin 2 and change your interrupt to RISING.

--- EDIT ---

I see, setting it HIGH set's a pullup resistor. It says there it's enough current to dimly light an LED so you might be getting fluctuation causing the interrupt to trigger?

Your code looks fine, the best explanation would be that you get an additional falling flank. Do you have a way of checking this?

Korman

Korman: I am not sure how to go about checking this. Any recommendations?

Biocow: I have just tried this and get the same result

Robtillaart: I have just posted the salient points of the code as it is quite a big sketch because it drives the ethernet shield as well as an LCD.

Z

Bits are free. Post it all up. (It's preferred that way.)

I have just posted the salient points of the code as it is quite a big sketch because it drives the ethernet shield as well as an LCD.

very good, leave out the ethernet but keep the LCD and take care it still compiles as this is essential for debugging.

Did you allready see this one http://www.arduino.cc/playground/Code/Debounce

I have stripped out the web page serving bits and here is the code. A bit of background, I am trying to monitor 6 digital IOs using interrupts and display the counters for each on the LCD.

#include <EEPROM.h>
#include <SPI.h>
#include <Ethernet.h>
#include <LiquidCrystal.h>

const int DebounceDelay = 250;

const int ShowIPPin = 26;

const int Counter1Pin = 2;
const int Counter2Pin = 3;
const int Counter3Pin = 18;
const int Counter4Pin = 19;
const int Counter5Pin = 20;
const int Counter6Pin = 21;
const char QUOTE = 34;

const int LED1Pin = 13;
const int LED2Pin = 53;

byte MACAddress[] = { 0xDE, 0xAD, 0xBE, 0xEF, 0xFE, 0xED };
byte IPAddress[4];
Server WebServer(80);

LiquidCrystal lcd(12, 11, 25, 24, 23, 22);

String MachineName1;
String MachineName2;
String MachineName3;
String MachineName4;
String MachineName5;
String MachineName6;

unsigned long Counter1 = 0;
unsigned long Counter2 = 0;
unsigned long Counter3 = 0;
unsigned long Counter4 = 0;
unsigned long Counter5 = 0;
unsigned long Counter6 = 0;

String CountersString;

void setup()
{
  
  Serial.begin(9600);
  Serial.println("Started");

  GetIPAddress();
  Ethernet.begin(MACAddress, IPAddress);
  WebServer.begin();
 
  pinMode(LED1Pin, OUTPUT);

  pinMode(ShowIPPin, INPUT);

  pinMode(Counter1Pin, INPUT);
  pinMode(Counter2Pin, INPUT);
  pinMode(Counter3Pin, INPUT);
  pinMode(Counter4Pin, INPUT);
  pinMode(Counter5Pin, INPUT);
  pinMode(Counter6Pin, INPUT);

  digitalWrite(ShowIPPin, HIGH);


  digitalWrite(Counter1Pin, HIGH);
  digitalWrite(Counter2Pin, HIGH);
  digitalWrite(Counter3Pin, HIGH);
  digitalWrite(Counter4Pin, HIGH);
  digitalWrite(Counter5Pin, HIGH);
  digitalWrite(Counter6Pin, HIGH);

  attachInterrupt(0, Counter1Handler, FALLING);
  attachInterrupt(1, Counter2Handler, FALLING);
  attachInterrupt(2, Counter3Handler, FALLING);
  attachInterrupt(3, Counter4Handler, FALLING);
  attachInterrupt(4, Counter5Handler, FALLING);
  attachInterrupt(5, Counter6Handler, FALLING);

  lcd.begin(20, 4);

}

void loop()
{

  Client IncomingClient = WebServer.available();
  String tmpCountersString;
  boolean currentLineIsBlank = true;
  char c;
  String QueryString;
  char WhichCounter;
  int tmpPos = 0;
 
  if (IncomingClient) {
    while (IncomingClient.connected()) {
      if (IncomingClient.available()) {
        c = IncomingClient.read();
        QueryString += c;
        if (c == '\n') {
          if (currentLineIsBlank) {
            WebPage(IncomingClient);
            break;
          }
          else {
            if ( QueryString.indexOf("ResetCounterValue") != -1 ) {
              tmpPos = QueryString.indexOf("=") + 1;
              WhichCounter = QueryString.charAt(tmpPos);
              ResetCounter(IncomingClient, WhichCounter);
              break;
            }
            else if ( QueryString.indexOf("SetCounterInitialValue") != -1 ) {
              Serial.println("  SetCounter");
              break;
            }
            else if (QueryString.indexOf("SetCounterName") != -1 ) {
              Serial.println("  Name Counter");
              break;
            }
            QueryString = "";
          }
        }
        if (c == '\n') {
          currentLineIsBlank = true;
        } 
        else if (c != '\r') {
          currentLineIsBlank = false;
        }
      }
    }
    delay(1);
    IncomingClient.stop();

  }
 
  ShowIPAddress(digitalRead(ShowIPPin) == HIGH);
  
  TimeToLCD(11, 0);

  lcd.setCursor(0, 2);
  lcd.print(CounterToString(Counter1));

  lcd.setCursor(7, 2);
  lcd.print(CounterToString(Counter2));

  lcd.setCursor(14, 2);
  lcd.print(CounterToString(Counter3));

  lcd.setCursor(0, 3);
  lcd.print(CounterToString(Counter4));

  lcd.setCursor(7, 3);
  lcd.print(CounterToString(Counter5));

  lcd.setCursor(14, 3);
  lcd.print(CounterToString(Counter6));

  //if (Serial.available())
    //ProcessSerialRequest();

}

void Counter1Handler()
{
  static unsigned long LastInterruptTime = 0;
  unsigned long InterruptTime = millis();

  if (InterruptTime - LastInterruptTime > DebounceDelay) {
      Counter1 ++;
  }

  LastInterruptTime = InterruptTime;
}

void Counter2Handler()
{
  static unsigned long LastInterruptTime = 0;
  unsigned long InterruptTime = millis();

  if (InterruptTime - LastInterruptTime > DebounceDelay) {
    Counter2 ++;
  }

  LastInterruptTime = InterruptTime;
}

void Counter3Handler()
{
  static unsigned long LastInterruptTime = 0;
  unsigned long InterruptTime = millis();

  if (InterruptTime - LastInterruptTime > DebounceDelay) {
    Counter3 ++;
  }

  LastInterruptTime = InterruptTime;
}

void Counter4Handler()
{
  static unsigned long LastInterruptTime = 0;
  unsigned long InterruptTime = millis();

  if (InterruptTime - LastInterruptTime > DebounceDelay) {
    Counter4 ++;
  }

  LastInterruptTime = InterruptTime;
}

void Counter5Handler()
{
  static unsigned long LastInterruptTime = 0;
  unsigned long InterruptTime = millis();

  if (InterruptTime - LastInterruptTime > DebounceDelay) {
    Counter5 ++;
  }

  LastInterruptTime = InterruptTime;
}

void Counter6Handler()
{
  static unsigned long LastInterruptTime = 0;
  unsigned long InterruptTime = millis();

  if (InterruptTime - LastInterruptTime > DebounceDelay) {
    Counter6 ++;
  }

  LastInterruptTime = InterruptTime;
}

void TimeToLCD(int Column, int Row)
{

  int SecondsSinceReset = 0;
  int HoursSinceReset = 0;
  int MinutesSinceReset = 0;

  SecondsSinceReset = millis() / 1000;
  MinutesSinceReset = SecondsSinceReset / 60;
  HoursSinceReset = MinutesSinceReset / 60;

  //if ( MinutesSinceReset > 59)
  //  MinutesSinceReset = 0;

  MinutesSinceReset = (MinutesSinceReset % 60);
 
  lcd.setCursor(Column, Row);
  if ( HoursSinceReset < 9)
    lcd.print("0"); 
  lcd.print(HoursSinceReset);

  lcd.print(":");
 
  if ( MinutesSinceReset < 10)
    lcd.print("0");
  lcd.print(MinutesSinceReset);

}

String CounterToString(unsigned long CounterValue)
{
  
  String tmpCounterString;
  String Result;

  tmpCounterString = "000000";
  tmpCounterString += CounterValue;
  tmpCounterString = tmpCounterString.substring(tmpCounterString.length() - 6, tmpCounterString.length());
  
  return tmpCounterString;
  
}

void WebPage(  Client IncomingClient)
{
}

void ResetCounter(Client IncomingClient, char WhichCounter)
{

  switch (WhichCounter) {
    case '1' :
      Counter1 = 0;
      break;

    case '2' :
      Counter2 = 0;
      break;

    case '3' :
      Counter3 = 0;
      break;
  
    case '4' :
      Counter4 = 0;
      break;
  
    case '5' :
      Counter5 = 0;
      break;
  
    case '6' :
      Counter6 = 0;
      break;
    }

}

void ShowIPAddress(boolean BlankLine)
{
  if ( BlankLine == true ) {
    lcd.setCursor(0, 1);
    lcd.print("                    ");
  }
  else {
    lcd.setCursor(0, 1);
    lcd.print("IP: ");
    lcd.print(IPAddress[0], DEC);
    lcd.print(".");
    lcd.print(IPAddress[1], DEC);
    lcd.print(".");
    lcd.print(IPAddress[2], DEC);
    lcd.print(".");
    lcd.print(IPAddress[3], DEC);
  }
}

void GetIPAddress()
{
  for (int ILoop = 0; ILoop < 4; ILoop ++)
    IPAddress[ILoop] = EEPROM.read(ILoop);
  
}

It compiles OK.

Robtillaart: I have looked at the code you linked and based my approach on that (modified for interrupt usage)

Z

The CounterN variables should be declared volatile, so that the current value is fetched every time CounterN is referenced.

Because the CounterN variables are larger than byte sized, you need to assure that changes to the variables are never interrupted. To do that, each interrupt handler needs to disable interrupts before updating CounterN, and enable them again afterward.

What is generating the interrupt? Does it really need to be debounced?

As I don't have a Mega I can't try it but here are my first remarks:

Counter1 (and the other 5) need to be declared as volatile. That means the compiler won't optimize access http://www.arduino.cc/en/Reference/Volatile

you declared - static unsigned long LastInterruptTime - in every IRQ function. It is a bit confusing (for me at least) but the compiler should take care that it are 6 different vars.

tmpCounterString = tmpCounterString.substring(tmpCounterString.length() - 6, tmpCounterString.length());
return tmpCounterString;

could be written as

return tmpCounterString.substring(tmpCounterString.length() - 6);

PaulS: The interrupts are being driven by simple switches and so it does need to be debounced. I have thought of "electrically" debouncing them but haven't got any capacitors to try.

Robtillaart: I will make the changes to the declarations and interrupt handlers and try it. I understand what you mean about the variable declarations all being the same names, I was just being lazy with cut and paste. As for the return statement, I was being over cautious its been 20 years since I last programmed in C and I didn't do a lot then!

Thanks guys for your help.

Z

PaulS: I have modified the code to declare the Counters as volatile and have detached and re-attached the interrupts but the problem still persists!

Z

Back to the code:

void Counter1Handler()
{
  static unsigned long LastInterruptTime = 0;
  unsigned long InterruptTime = millis();

  if (InterruptTime - LastInterruptTime > DebounceDelay) {
      Counter1 ++;
  }

  LastInterruptTime = InterruptTime;
}

In plain english:
CounterHandler counts the times it is called with at least 250 msec since the last call. If it is called faster it will not count.

So a switch that is pressed every 200msec will not be counted.

Is that what you meant it to do?

Your initial question

but if I hold my finger on the switch for a second or more then Counter1 increments again.

So far as I understand the code there must be a new interrupt to let this happen.

What kind of wiring are you using between switch and Arduino?

PaulS: The 250 msec was just an arbitrary figure I selected. I am wiring it with surface mount switches on a breadborad and jumper wires at the moment. Do you think it might be worth trying with better switches and wiring?

I have changed the code to attach the interrupts as RISING and it seems to work so far. I will return to it tomorrow and test some more.

Cheers

Z