Unreliable debouncing in interupt routine MEGA256

Hi,
I want to monitor the tips of a TBRG (Tipping Bucket Rain Gauge).
The reed switch is conneted to ground on one side and on pin 2 of the MEGA256.
Every time 0.2mm of rain has fallen into the rain gauge the bucket will tip and the reed switch will close momentarily.
I am using an digital interrupt to monitor pin 2. The moment the bucket tips it will generate a pulse and the ISR will start to run. It will increase the count value by one.
To avoid switch bounce I have a capacitor of 0.1uF over the reed switch and also have incorporated a software delay of 100 milliseconds. However this is not always reliable. Sometimes it counts a couple of tips too many.

Could you please look over my code and see where it goes wrong?

Here is the code:

/* Tipping Bucket Rain Gauge sketch for Arduino MEGA256*/
                 
const int tbrgPin = 2;     // Rain gauge is connected to this pin

// Declaring variables:
volatile int state = LOW;
volatile int buttonState = 0;         // variable for reading the pushbutton status
int tbrgCount = 0;
volatile int val = 0;
int old_val = 0;

void setup() {
  Serial.begin(9600);
  pinMode(tbrgPin, INPUT_PULLUP);// initialize the TBRG pin as an input:
  attachInterrupt(digitalPinToInterrupt(tbrgPin), tbrg_ISR, FALLING); //Declare interrupt
}

void loop() { 
  Serial.println("Just wasting time...");
  delay(5000);
}

void tbrg_ISR() {
val = digitalRead(tbrgPin);  //Read the status of the reed switch
delay(100);                  // Delay put in to deal with any "bouncing" in the switch.
tbrgCount = tbrgCount + 1;   //Add 1 to the count of bucket tips
old_val = val;              //Make the old value equal to the current value
Serial.print("Tips recorded = "); Serial.println(tbrgCount);  //Output the count to the serial monitor
}

I found a snippet of code on the forum which seems to be perfect for this situation however I have no idea how to incorporate this snippet in my code:

/*/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
//New code for switch deboucing
//Needs to be activated when inetrrupt is made
            volatile int pulseCount_ISR;

            void reedSwitch_ISR()
            {
              static unsigned long lastReedSwitchTime;
              if (labs(millis()-lastReedSwitchTime)>250)  // debounce for a quarter second = max. 4 counts per second
            {
              pulseCount_ISR++;
              lastReedSwitchTime=millis();
            }
            }
//xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx*/

The idea is that I will be building a small weather station. In the main loop I will be reading temperatures and humidity. Later on I will be adding an anemometer as well (Also via an interrupt).

As always, your feedback will be greatly appreciated.
Cheers,

Luc

Maybe something like this might work (untested):

volatile uint32_t currentMillis;
volatile uint16_t pulseCount;
volatile bool timing = false;
byte dbTime = 100;

void setup()
{
  Serial.begin(9600);
  // setup interupt pin //
}

void loop()
{
  if (millis() - currentMillis > dbTime)
    timing = false;
}
void dbISR ()
{
  if (timing)
    return;
  else
  {
    timing = true;
    currentMillis = millis();
    ++pulseCount;
  }
}

Dear outsider,

Thank you very much for your suggestion.
I spent a couple of hours on this and I'm able to compile it without any errors.
However, I don't understand why and when to call the dbISR function.
Is that something that should be integrated in my TBRG() function? Or should I leave it as a separate function?
I think I understand the whole interrupt process but to alter the code to suit my needs is more complex then I expected :slight_smile:
Thanks for your time and suggestions.
Cheers,

Luc

In void serial() make your interrupt line like:

attachInterrupt(digitalPinToInterrupt(tbrgPin), dbISR, FALLING);

Change variable names to match yours.
Post the errors and the code you are currently trying.

Thanks outsider!
I feel like I'm going to make a break through. Close, but no cigar yet.

Here is my revised code:

/*TBRG is connected between ground and pin 2 on UNO
Whenever the TBRG makes a tip an interrupt will override the main program
*/

volatile uint32_t currentMillis;
volatile uint16_t pulseCount;
volatile bool timing = false;
byte dbTime = 100;

const byte tbrgPin = 2;
volatile byte state = LOW;
volatile int val = 0;            //Current value of reed switch
int old_val = 0;                //Old value of reed switch
int TBRGCOUNT = 0;              //This is the variable that hold the count of switching

void dbISR ()
{
  if (timing)
    return;
  else
  {
    timing = true;
    currentMillis = millis();
    ++pulseCount;
  }
}

void setup() {
  Serial.begin(9600);
  pinMode(tbrgPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(tbrgPin), dbISR, FALLING);
}

void loop() {
  if (millis() - currentMillis > dbTime)
    timing = false;

else {
    void TBRG();
  //digitalWrite(tbrgPin, state);
}
  Serial.println("Just wasting time...");
  delay(5000);
}


void TBRG() {
//state = !state;
//val = digitalRead(tbrgPin);      //Read the status of the Reed switch
//delay(20);                   // Delay put in to deal with any "bouncing" in the switch.
TBRGCOUNT = TBRGCOUNT + 1;   //Add 1 to the count of bucket tips
//old_val = val;              //Make the old value equal to the current value
Serial.print("Tips recorded = "); Serial.println(TBRGCOUNT);  //Output the count to the serial monitor
}

It compiles without errors and it prints "Just wasting time..." but no action on the interrupt.
Any further ideas?
Cheers,

Luc

This compiles, but I don't have the hardware to test, try it and let me know, good luck.

/*TBRG is connected between ground and pin 2 on UNO
Whenever the TBRG makes a tip an interrupt will override the main program
*/

volatile uint32_t currentMillis;
volatile uint16_t TBRGCOUNT;
volatile bool timing = false;
const byte dbTime = 100;

const byte tbrgPin = 2;
//volatile byte state = LOW;
//volatile int val = 0;            //Current value of reed switch
//int old_val = 0;                //Old value of reed switch
//int TBRGCOUNT = 0;              //This is the variable that hold the count of switching

void dbISR ()
{
  if (timing)
    return;
  else
  {
    timing = true;
    currentMillis = millis();
    ++TBRGCOUNT;
  }
}

void setup() {
  Serial.begin(9600);
  pinMode(tbrgPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(tbrgPin), dbISR, FALLING);
}

void loop() {
  if (millis() - currentMillis > dbTime)
    timing = false;
  Serial.println(TBRGCOUNT);
  Serial.println("Just wasting time...");
  delay(5000);
}

Hi outsider,

The program works to a certain extend.
It shows me the amount of accumulated tips and then prints the line "Just wasting time..."
However, it's restricted to adding one count per each five seconds.
In reality the rain bucket can make multiple tips during this five second period but these are not taken in account.
Eg:
On the serial monitor I can see something like this:

0
wasting time...
0
Just wasting time...
0
Just wasting time...
1
Just wasting time...
2
Just wasting time...

However I made like four tips between the count from 1 to 2 and it only shows 1 extra tip.
Is there any chance that the code can be modified so that it can read every tip possible?

The TBRG is a slow moving counter. At maximum speed it probably makes one tip per second. (That would be equal to 720mm of rainfall per hour!!!!(Each tip is equal to 0.2mm of precipitation)).
Therefore I was looking for a reed switch de-bounce time of about 100 milliseconds. I probably can even bring that up to 500 milliseconds without any risk of losing future tip events.

The "Just wasting time..." is just there to fill up for the yet to be developed code.
The objective is to read the temperature and humidity sensor every 5 seconds or so. although most likely I will poll these sensors only once a minute.
I will also add an RTC so that every event will be time stamped.

I can't express enough how much I appreciate your time and effort; Thank you very much for helping out a noob like myself.
Cheers,

Luc

Another stab, I changed some variable names, modified the ISR and replaced the dreaded delay with a millis timer.

/*TBRG is connected between ground and pin 2 on UNO
Whenever the TBRG makes a tip an interrupt will override the main program
*/
uint32_t displayTimer;
uint16_t displayInterval = 5000;
volatile uint32_t dbTimer;
volatile uint16_t TBRGCOUNT;
volatile bool timing = false;
const byte dbTime = 100;

const byte tbrgPin = 2;
//volatile byte state = LOW;
//volatile int val = 0;            //Current value of reed switch
//int old_val = 0;                //Old value of reed switch
//int TBRGCOUNT = 0;              //This is the variable that hold the count of switching

void dbISR ()
{
  if (!timing)
  {
    timing = true;
    dbTimer = millis();
    ++TBRGCOUNT;
  }
}

void setup() {
  Serial.begin(9600);
  pinMode(tbrgPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(tbrgPin), dbISR, FALLING);
}

void loop() {
  if (millis() - dbTimer > dbTime)
    timing = false;
  if(millis() - displayTimer > displayInterval)
  {
    Serial.println(TBRGCOUNT);
    Serial.println("Just wasting time...");
    displayTimer += displayInterval;
  }
}

The good news is: It works!
The bad news is: It's not accurate at all.

Let me explain:
The rain gauge I'm using is a unit manufactured by McVan Instruments (Melbourne). This company also manufacturers the official rain gauges to the Bureau Of Meteorology, so, they are definitely good units.
I've got the official calibration tool of McVan that simulates a rainfall event of 200mm per hour (Delivered in 10 minutes, so basically about one tip per three seconds). According to the manufacturer the read out on the counter should be between 95 and 106 tips to be considered accurate and to be within spec. My unit reads 102 tips after the calibration test and delivers the same result over and over.
I've also got an official McVan calibration counter connected which is used for official calibration purposes.

So, when I do the calibration test I've got the official McVan counter connected and also an Arduino Mega256.
The official counter reads 102 tips. However the Arduino reads... 128 tips!
I got no idea why it behaves likes this. I have changed de-bounce times and I have also changed the terminal connections (Swapped over Arduino with official counter and vice versa) and have changed the ISR from FALLING to CHANGE but to no avail.

Maybe an interrupt routine is not suitable for this application and maybe I should attach a flip-flop to the rain gauge instead and read the status of the flip-flop every second or so. If a tip has occurred it then needs to re-set the flip-flop.

Anyway, I need to give it more thought. I thought it would be a simple process but it turned out differently.

Irregardless of the end-result I want to thank you wholeheartedly for all your time, effort and perseverance!
Cheers,

Luc

LRAT:
I think I understand the whole interrupt process but to alter the code to suit my needs is more complex then I expected :slight_smile:

Actually, using an interrupt to read a bouncy switch input is fraught with problems and you shouldn't call anything else which uses interrupt inside the interrupt routine (like delay() or Serial.print()).

Unless you have a really good reason to use interrupt, you should avoid it

Why not read the switch in the main loop() and properly debounce it?

#define DEBOUNCE_TIME 20  //20mS per debounce step
const int tbrgPin = 2;     // Rain gauge is connected to this pin

// debounce pin function
// - returns 1 when input switches low, otherwise returns 0
//
int debounceThePin( int pin )
{
  static uint32_t debounceTimer = 0;
  static uint8_t debounceShift = 0xff; // 8-bit shift to debounce 8 times
  static uint8_t debounceState = 1;    // current debounced state of the imput
  int returnValue = 0;

  if ( millis() > (debounceTimer + DEBOUNCE_TIME) )
  {
    debounceTimer = millis();

    if ( digitalRead( pin ) == HIGH )
    {
      debounceShift = (debounceShift << 1) | 0x01; // shift in a '1'
      if ( debounceShift == 0xff )
      {
        debounceState = 1; // input is really HIGH
      }
    }else
    {
      // input is low
      debounceShift = (debounceShift << 1); // shift in a '0'
      if ( (debounceShift == 0x00) && (debounceState == 1) )
      {
        returnValue = 1; // switch has just switched LOW
        debounceState = 0;
      }
    }
  }
  return returnValue;
}


void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);
  pinMode(tbrgPin, INPUT_PULLUP);// initialize the TBRG pin as an input:
}

void loop() {
  // put your main code here, to run repeatedly:
  if ( debounceThePin( tbrgPin ) )
  {
    // do something useful...
    Serial.println("Pin gone LOW");
  }
}

Notice that you have to debounce the pin LOW and debounce it HIGH again !
This example takes 20 * 8 = 160 mS to debounce the pin and really makes sure it's been in the same state 8 times before accepting it is as HIGH or LOW. Try it by scratching a grounded wire on the input pin.

... something which an interrupt routine is not really suited for.

Yours,
TonyWilk

Hi,
You said that your are using a bouncing switch for the interrupt. Do you have a pull up resistor and If not set the pin input with the pullup enable. Also disable the interrupt immediately you enter the routine and enable before you leave it. That will prevent in case the contact bouncing that will trigger another interrupt while your are servicing the first one. These are just some suggestion. Working with interrupt it is not an easy task. You must know what are you doing.

Well I was thinking interrupts WERE disabled in an ISR, but anyway, try this, also might try a stronger pullup, 10k from pin 2 to Vcc and a ceramic capacitor 22 to 47 nF from pin 2 to GND.

/*TBRG is connected between ground and pin 2 on UNO
Whenever the TBRG makes a tip an interrupt will override the main program
*/
uint32_t displayTimer;
uint16_t displayInterval = 5000;
volatile uint32_t dbTimer;
volatile uint16_t TBRGCOUNT;
volatile bool timing = false;
const int dbTime = 300;

const byte tbrgPin = 2;
//volatile byte state = LOW;
//volatile int val = 0;            //Current value of reed switch
//int old_val = 0;                //Old value of reed switch
//int TBRGCOUNT = 0;              //This is the variable that hold the count of switching

void dbISR ()
{
  noInterrupts();
  if (!timing)
  {
    timing = true;
    dbTimer = millis();
    ++TBRGCOUNT;
  }
  interrupts();
}

void setup() {
  Serial.begin(9600);
  pinMode(tbrgPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(tbrgPin), dbISR, FALLING);
  TBRGCOUNT = 0;
}

void loop() {
  if (millis() - dbTimer > dbTime)
    timing = false;
  if(millis() - displayTimer > displayInterval)
  {
    Serial.println(TBRGCOUNT);
    Serial.println("Just wasting time...");
    displayTimer += displayInterval;
  }
}

tauro0221:
Also disable the interrupt immediately you enter the routine and enable before you leave it. That will prevent in case the contact bouncing that will trigger another interrupt while your are servicing the first one. These are just some suggestion. Working with interrupt it is not an easy task. You must know what are you doing.

Indeed. Interrupts are automatically disabled when you enter an ISR and automatically enabled when you exit. You can add code to do it, but it is a canard.

A rain guage is way too slow to justify using an interrupt to measure it. If you want to debounce an interrupt, be aware that you can do it but you can not easily prevent the interrupt from being called on every switch transition (even though they would be ignored). It is inefficient, and could delay other processes that are occurring.

Dear enthusiasts,
Good news: I've got it working!

Thank you very much for all your suggestions. Initially I was using an interrupt to read the tips of a rain gauge. However after lots of frustration I decided to walk away from this sort of approach.
I thought an asynchronous D-flipflop would be a better solution. So I set up an SN7402 (Dual NOR-gates) chip and I got it working. The moment a tip occurs the flipflop holds the state until the Arduino polls the pin. After the micro has read the state it resets the flipflop. It worked great but I ended up with a breadboard full of wires and was looking for something cleaner.
It was then that I received a response from Ralph Bacon (Check out his YouTube channel) and he advised to watch his video about rotary encoders (#Video 19). This video dealt with the issues I had problems with.
I adopted his code, modified it to suit my needs and presto: It worked!
So, I'm now using an interrupt routine that monitors the rain gauge.
Here is my code:

/*TBRG (Tipping Bucket Rain Gauge) sketch
Connected to Ground and Signal on Pin 2 breakout board on MEGA2560 R3 board
Whenever the tipping bucket makes a tip, a reed switch closes momentarily
This pulse starts the isr routine and the value is then added to the total rain tips for the day
Sketch written with the help of Ralph Bacon, https://www.youtube.com/channel/UC8Ob-HnnmhlgSv5Vs_i32TQ */
const int tbrgPin = 2;
int tbrgVal = 0;

volatile int startPosition = 0;
/*TBRG INTERRUPT ROUTINE*/
void isr () {
  static unsigned long lastInterruptTime = 0;
  unsigned long interruptTime = millis();
  if (interruptTime - lastInterruptTime > 10) {
    if (digitalRead(tbrgPin) == HIGH){
      startPosition++ ;}
    lastInterruptTime = interruptTime;}
}
/*SETUP*/
void setup() {
  Serial.begin(9600);
  pinMode(tbrgPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(tbrgPin), isr, HIGH);
  Serial.println("Start"); 
}
/*MAIN LOOP*/
void loop() {
  Serial.println("Currently reading other non-related sensors...");
  Serial.println((String)"TBRG tips recorded so far = "+(tbrgVal));
  delay(3000);
  tbrg();
  }
 void tbrg() {
if (startPosition != tbrgVal) {
    Serial.println("xxxxxxxxxxxxx");
    Serial.println((String)"TBRG tips = "+(startPosition));
    Serial.println("xxxxxxxxxxxxx");
    tbrgVal = startPosition; 
 }
}

Once again thank you all for your input and help. I hope this code could be useful for other Arduino enthusiasts.
Cheers and have a nice day,

Luc

Ok, good, you got yours working.

But, sorry to rain on your parade, but your switch handling is completely unreliable and could be a real pain to other Arduino enthusiasts.

It may work now, it may work next week, but then it might just start counting the odd one too many. If you built another or used a different switch - it may count too many.

You have to debounce a switch from HIGH to LOW AND from LOW to HIGH.

Do yourself a favour and Google "how to debounce properly"

Yours,
TonyWilk

This is cause for concern:

  attachInterrupt(digitalPinToInterrupt(tbrgPin), isr, HIGH);

Doesn't that mean that as long as the pin is HIGH, this interrupt will fire over and over? The check if (digitalRead(tbrgPin) == HIGH) is redundant because the fact that you're running this code means the pin was HIGH.

I think this code might work, but not for the reason you think it works. INT0 is a very high priority interrupt. Which means while this code is running, no other interrupts are handled. Including the timer interrupts. Which means as long as this pin is HIGH, your millis() counter will not be incrementing. It's pretty much the same thing as disabling the interrupts and doing a spin wait checking for the pin to go LOW. Nothing else useful is being done.

I think this is what you really want:

...
volatile int startPosition = 0;
/*TBRG INTERRUPT ROUTINE*/
void isr () {
  static unsigned long lastInterruptTime = 0;
  unsigned long interruptTime = millis();
  if (interruptTime - lastInterruptTime > 10) {
    if (digitalRead(tbrgPin) == HIGH){
      startPosition++ ;}
  }
  lastInterruptTime = interruptTime;
}
/*SETUP*/
void setup() {
  Serial.begin(9600);
  pinMode(tbrgPin, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(tbrgPin), isr, CHANGE);
  Serial.println("Start"); 
}
...

Actually I was wrong. According to the datasheet, there is no HIGH level trigger mode for external interrupts. There is LOW, but not HIGH. The Arduino reference page for attachInterrupt is just plain wrong. From Arduino.h:

#define HIGH 0x1
#define LOW  0x0
...
#define CHANGE 1
#define FALLING 2
#define RISING 3

HIGH is defined as the same as CHANGE. The code that actually implements attachInterrupt doesn't even check the mode. It slams it into the ISC bits directly. So when you use the mode HIGH, you're really setting it up for CHANGE.

Thank you for the extra suggestions.
Although my code works great you're absolutely correct about the ISR. There is not such a thing as HIGH level trigger. I wasn't aware of this and it compiled without any issues. I will refine my code over the coming weekend.
In the meantime I have been improving the hardware de-bouncing of the reed switch.
I have attached a PDF-file showing my setup. I have incorporated a Schmitt trigger (74HC14) to clean up the signal.
The combination of a 1000 Ohm resistor and a 0.1 microFarad should create a 10 millisecond de-bounce delay. This in combination with the software de-bouncing delay should be sufficient for my project.
I'm also planning to put a resistor of around 100 Ohm in between the TBRG reed switch and ground. This would protect the circuit in case of inadvertently shorting out pins.
Cheers,

Luc

Reed switch debouncing circuit.pdf (169 KB)

There is LOW, but not HIGH. The Arduino reference page for attachInterrupt is just plain wrong.
HIGH is defined as the same as CHANGE. The code that actually implements attachInterrupt doesn't even check the mode. It slams it into the ISC bits directly. So when you use the mode HIGH, you're really setting it up for CHANGE.

The spacing of the reference material is confusing, but the HIGH mode is valid for the Due,Zero, MKR1000.

mode: defines when the interrupt should be triggered. Four constants are predefined as valid values:

LOW to trigger the interrupt whenever the pin is low,

CHANGE to trigger the interrupt whenever the pin changes value

RISING to trigger when the pin goes from low to high,

FALLING for when the pin goes from high to low.
The Due, Zero and MKR1000 boards allows also:

HIGH to trigger the interrupt whenever the pin is high.

I have attached a PDF-file showing my setup. I have incorporated a Schmitt trigger (74HC14) to clean up the signal.
The combination of a 1000 Ohm resistor and a 0.1 microFarad should create a 10 millisecond de-bounce delay.

Did you actually go and google "how to debounce properly" ?
did you look at the very first hit: A Guide to Debouncing - University of Utah ?

So you try to paste over some ineffective interrupt-based input software with unnecessary, poorly - conceived extra external hardware (at least do your maths again, you are a few powers of 10 out).

Just do effective debouncing HIGH to LOW and LOW to HIGH in software.

Yours,
TonyWilk