External interrupt queuing ? 2 Interrupts at exactly the same time, differ

Hi everyone,

I'm trying to get the arduino mega to read multiple (!) S0 power meter outputs by external interrupts (2 and 3, pin 21 and 20 at the moment - will become more) and display the counters on lcd and print to serial.

In my code I attached the interrupts to the pins and activated the pull-up resistors. My (copied) code basically seems to work ok, I did no debouncing in the interrupt routine as to not artificially lengthen the time wasted in the interrupt routine.

Now worse case would be 2 pulses at exactly the same time, although iirc the arduino does some interrupt queuing. So I connected both pins and as a test connect them to ground. Pulses are counted (remember no debouncing), but both counters differ quite a lot (36 to 48 e.g.).

Why ?

My code based on Openenergymonitor:

/*
The code below detects the falling edge of each pulse and increment pulseCount
It calculated the power by the calculating the time elapsed between pulses.
Read more about pulse counting here:
http://openenergymonitor.org/emon/buildingblocks/introduction-to-pulse-counting

-----------------------------------------
Part of the openenergymonitor.org project
Licence: GNU GPL V3

Changed for my personal use
*/

// Library for LCD
#include <LiquidCrystal.h>

// select the pins used on the LCD panel
LiquidCrystal lcd(8, 9, 4, 5, 6, 7);

unsigned long pulseCount3 = 0;
unsigned long pulseCount2 = 0;
unsigned long pulseTime,lastTime; // Used to measure time between pulses
double power;
int ppwh = 1; // pulses per watt hour - found or set on the meter.

void setup()
{
  // Initialize Serial USB
  Serial.begin(9600);
  
  // Start LCD Library
  lcd.begin(16, 2);     
  lcd.setCursor(0,0);
  // Print title
  lcd.print("SO Stromzaehler ");
  // Cursor to pos1 line 2
  lcd.setCursor(0,1);
 
 // pinmode auf INPUT setzen
  pinMode (20, INPUT);
  pinMode (21, INPUT);
 // Pull-up auf pin 20 aktivieren
  digitalWrite(20, HIGH); 
  digitalWrite(21, HIGH); 
 // pulse detection interrupt (pulse channel - IRQ3 pin20)
  attachInterrupt(3, onPulse3, FALLING);
  attachInterrupt(2, onPulse2, FALLING);
  
  pulseTime=micros();
}

void loop()
{
//  lastTime = pulseTime;
//  pulseTime = micros();
//  power = int((3600000000.0 / (pulseTime - lastTime))/ppwh); // calculate power
  
  //Set Cursor to line 2
  lcd.setCursor(0,1);
  // Print counters on LCD
  lcd.print  ("P1 ");
  lcd.print (pulseCount3);
  lcd.setCursor(8,1);
  lcd.print  ("P2 ");
  lcd.print (pulseCount2);
  
//  Serial.print(power);
//  Serial.print(' ');
//  Serial.println(pulseCount2 * ppwh); // watt hour elapsed
Serial.print("P1");
Serial.print(' ');
Serial.print("P2");

  delay(1000);
}

// The interrupt routine - runs each time a falling edge of a pulse is detected
void onPulse3()
{
  pulseCount3++; // count pulse
}
void onPulse2()
{
  pulseCount2++; // count pulse
}

Any time variables are going to be shared by different functions in a sketch ( like in loop() and inside ISR functions ) you must declare them 'volatile' so that the compiler will not store them into registers for faster access, but rather be forced to get them from SRAM when reading or writing to them. Failure to do so can lead to very strange and improper operation of the code.

http://arduino.cc/en/Reference/Volatile
So:

unsigned long pulseCount3 = 0;
unsigned long pulseCount2 = 0;

//should become

volatile unsigned long pulseCount3 = 0;
volatile unsigned long pulseCount2 = 0;

Also be aware of the so called 'atomic problem' when you are accessing a variable in your main loop function that can be changed at any machine cycle by an ISR. So while your main loop is doing this:

lcd.print (pulseCount3);
// and
lcd.print (pulseCount2);

As those variables are 4 bytes long it takes several machine instruction cycles to read them from SRAM and it's quite possible an interrupt ISR can be triggered in between the reading of each byte and therefore the value can be changed and thus invalid as read by the loop() function. Typical protection from this behavior is to either bracket the reference to the variable in the loop function like this:

noInterrupts();
lcd.print (pulseCount3);
lcd.print (pulseCount2);
interrupts();

The disadvantage of that method is that if the interrupts are coming in fast enough it's possible to miss some while all interrupts are disabled, thus missing data.

Another method is to make the global variables shared by ISR functions and your loop() function of the type byte, as such a read or write is a single cycle and can't be corrupted by an ISR triggering.

Using interrupts are a very useful and powerful tool, but one should be aware of the pitfalls, rules, and recommended practices when using them.

Lefty

Thank you VERY much for this information - I wil change the code and see if it changes the behaviour.

Never a good idea to block interrupts around I/O calls like this:

  noInterrupts();
  lcd.print (pulseCount3);
  lcd.print (pulseCount2);
  interrupts();

always copy the variables, then enable interrupts before the (potentially glacially slow I/O).

  noInterrupts();
  long c3 = pulseCount3 ;
  long c2 = pulseCount2 ;
  interrupts();
  lcd.print (c3) ;
  lcd.print (c2) ;

Good suggestion Mark. Also I wasn't sure if lcd.print library uses interrupts or not, but certainly turning off interrupts and then attempting to do a serial print would fail.

Lefty