Simultaneous use of timer and input pin interrupts

While working on a project for school, my team has run into an interesting problem experienced only while simultaneously using the PWM-Output (Timer1 with AVR code) and digital input interrupt triggers (using Arduino's attachInterrupt()). We are using ArduinoMega2560.

Pins 20 & 21 are respectively connected to a rain gauge and an anemometer (wind gauge). Both devices work in the same manner: the user simply counts the HIGH to LOW transitions generated by a mechanical switch. Meanwhile, the PWM code is used to output a 31-khz PCM audio stream on pin 11.

The digital-input reading code (countrain, countwind, attachinterrupt(), etc) works great without the PWM-Output (Timer1), but if used in tandem with the PWM code, the variables c0 and w0 are incremented "too many" times, resulting in inaccurate measurements. The rain gauge counter appears count around twice as many transitions, while the rain gauge counts have increased by a factor ~= 8. Commenting out the line "initPWM()" in setup() stops audio output and returns the gauge-counting interrupts to normal operation.

thanks! Ross

volatile int c0;
volatile int w0;
int tmr;
float rain;
float wind;

void countrain()
{
 c0++;
}

void countwind()
{
 w0++;
}

void initPWM(void)
{
 pinMode(11, OUTPUT);  // Pin 11 PWM Audio output
 TCCR1A= _BV(COM1A1) | _BV(WGM10);  // 8-bit Fast PWM - non inverted PWM
 TCCR1B = TCCR1B & 0b11111000 | 0x01 ; // Set Prescaler (0x01)
 TIMSK1 = _BV(TOIE1);   // Enable overflow interrupt for OCR1A
}

ISR(TIMER1_OVF_vect)  // audio sample-rate Interrupt Handler ~31khz
{
 static uint16_t osc = 0;
 OCR1A = osc;  // output the last calculated sample 
 /* non-suspect code (limited number of operations) */
}

void setup(void)  
{
 initPWM(); // init PWM/interrupt handler
 sei(); // Enable global interrupts
 attachInterrupt(2, countrain, FALLING);   // pin 20
 attachInterrupt(3, countwind, FALLING);   // pin 21
 c0 = 0;
 w0 = 0;
 tmr = 0;
}

void loop()
{
 if (about one second has passed according to micros()) {
    
     rain  = c0*.02;
     tmr++;

     if (tmr >= 10) //average windspeed for ten seconds and reset
     {
      wind = (w0 * 2.5)/10; //2.5MPH per 1hz
      w0 = 0;
      tmr = 0;
     }
   }

  /* non-suspect code */
}

I would be interested to look at some of your "non-suspect" code. For example, what else is done in:

 ISR(TIMER1_OVF_vect)  // audio sample-rate Interrupt Handler ~31khz

Here's the omitted code from the audio-rate interrupt handler:

  phaseAccum1 += phaseIncrement1;
  phaseAccum2 += phaseIncrement2;
  phaseAccum3 += phaseIncrement3;
  phaseAccum4 += phaseIncrement4;
  phaseAccum5 += phaseIncrement5;
  phaseAccum6 += phaseIncrement6;
  index1 = phaseAccum1 >> 8;
  index2 = phaseAccum2 >> 8;
  index3 = phaseAccum3 >> 8;
  index4 = phaseAccum4 >> 8;
  index5 = phaseAccum5 >> 8;
  index6 = phaseAccum6 >> 8;
  osc =  pgm_read_byte(&oscTable_c[index1])*ampTable_c1[a1];
  osc += pgm_read_byte(&oscTable_c[index2])*ampTable_c1[a2];
  osc += pgm_read_byte(&oscTable_c[index3])*ampTable_c1[a3];
  osc += pgm_read_byte(&oscTable_c[index4])*ampTable_c2[a4];
  osc += pgm_read_byte(&oscTable_c[index5])*ampTable_c2[a5];
  osc += pgm_read_byte(&oscTable_c[index6])*ampTable_c2[a6];
  osc = osc >> 8;

Hmmm, so you aren't going to show us the whole function, right? We have to assemble that like a jigsaw?

You have two tables there (oscTable_c and ampTable_c1) which you haven't shown the declarations for, so we have to guess what they are like.

Ditto for the declarations for phaseAccum1 and the other things.

Can you estimate (or measure) how often TIMER1_OVF_vect is likely to be called? Because that seems to be a lot of stuff to do in an interrupt handler. That quite a few adds, shifts, accessing program memory, and dereferencing indexes. And the time taken is going to increase if they are longs rather than ints or bytes.

Doing many things in an interrupt handlers is likely to have the effect of missing interrupts in other handlers. Now while you say you seem to be getting too many counts rather than too few, I would be looking closely at the timing involved here.

sorry for the incomplete code, I was trying to just show the relevant/affected/suspect parts of code, but should have included the entire TIMER1 ISR in the first post! btw, thanks for looking at it! :)

oscTable_c is a PROGMEM array of uint8_t, length=256 byte, indexes are uint8_t, and ampTables are length=32 byte arrays of uint8_t, while the phase accumulators and and phase increment values are all uint16_t.

the code in TIMER1_OVF_vect is, as mentioned in the first post, is triggered around 31,000 times a second to create PCM audio samples at that sample rate -- so the ATMega has around 256 clock cycles to get into to TIMER1_OVR_vect ISR, complete at least

(12) 16-bit additions, (7) 16-bit bitshifts, (6) 1-byte length reads from PROGMEM, and (6) 8bit x 8bit -> 16-bit multiplies -- all while supporting the input pin interrupts and doing non-time sensitive background processing.

I don't know where to go to find a reference for the # of clock cycles used by each operation (without even considering the overhead). I suppose I'm asking too much of the poor thing? 8)

I would have guessed that was the case from the start, but I assumed the expected problem would manifest itself as MISSING some input pin interrupts, rather than "counting" too many input pin interrupts -- so I thought that perhaps I did something wrong in declaring and/or using the variables w0 / r0 and the input-pin interrupt (attachInterupt, etc).

rossfalk:
oscTable_c is a PROGMEM array of uint8_t, length=256 byte,
indexes are uint8_t, and
ampTables are length=32 byte arrays of uint8_t,
while the phase accumulators and and phase increment values are all uint16_t.

You are making me work to help you aren’t you? Instead of just copying and pasting your declarations, you describe them. So then I have to interpret them back into C code in order to do a timing test.

And you still haven’t posted the entire interrupt routine.

the code in TIMER1_OVF_vect is, as mentioned in the first post, is triggered around 31,000 times a second to create PCM audio samples at that sample rate

I wrote a small sketch to test the timing, here it is:

#include <avr/pgmspace.h>

volatile uint16_t phaseAccum1, phaseAccum2, phaseAccum3, phaseAccum4, phaseAccum5, phaseAccum6;
volatile uint16_t phaseIncrement1, phaseIncrement2, phaseIncrement3, phaseIncrement4, phaseIncrement5, phaseIncrement6;

volatile uint8_t a1, a2, a3, a4, a5, a6;
volatile uint8_t index1, index2, index3, index4, index5, index6;
volatile uint8_t ampTable_c1 [32], ampTable_c2 [32];
volatile uint8_t oscTable_c  [256] PROGMEM;

void setup ()
{
  pinMode (13, OUTPUT);
}

void loop ()
{
 static uint16_t osc = 0;
 
  digitalWrite (13, HIGH);
 
 phaseAccum1 += phaseIncrement1;
  phaseAccum2 += phaseIncrement2;
  phaseAccum3 += phaseIncrement3;
  phaseAccum4 += phaseIncrement4;
  phaseAccum5 += phaseIncrement5;
  phaseAccum6 += phaseIncrement6;
  index1 = phaseAccum1 >> 8;
  index2 = phaseAccum2 >> 8;
  index3 = phaseAccum3 >> 8;
  index4 = phaseAccum4 >> 8;
  index5 = phaseAccum5 >> 8;
  index6 = phaseAccum6 >> 8;
  osc =  pgm_read_byte(&oscTable_c[index1])*ampTable_c1[a1];
  osc += pgm_read_byte(&oscTable_c[index2])*ampTable_c1[a2];
  osc += pgm_read_byte(&oscTable_c[index3])*ampTable_c1[a3];
  osc += pgm_read_byte(&oscTable_c[index4])*ampTable_c2[a4];
  osc += pgm_read_byte(&oscTable_c[index5])*ampTable_c2[a5];
  osc += pgm_read_byte(&oscTable_c[index6])*ampTable_c2[a6];
  osc = osc >> 8;  
  digitalWrite (13, LOW);

  delay (100);
  
}

Now assuming I interpreted your “described” data declarations correctly, the pin 13 LED should be alight during the time it would take to do what your ISR does. And I time that as being 22.625 microseconds. According to my calculations you should be able to do 44,150 of those a second, so (excluding the overhead of entering and leaving the ISR) you might squeeze that into 31,000 times a second. However that isn’t giving the rest of your code much time to do stuff. In fact I calculate that to do something 31,000 times a second would take 32.26 microseconds. So you could do other stuff, provided you don’t take more than about 10 microseconds per loop iteration to do it.

In fact, a revamped test where I actually use your interrupt generating code reveals slightly different figures. Here is the code:

#include <avr/pgmspace.h>

volatile uint16_t phaseAccum1, phaseAccum2, phaseAccum3, phaseAccum4, phaseAccum5, phaseAccum6;
volatile uint16_t phaseIncrement1, phaseIncrement2, phaseIncrement3, phaseIncrement4, phaseIncrement5, phaseIncrement6;

volatile uint8_t a1, a2, a3, a4, a5, a6;
volatile uint8_t index1, index2, index3, index4, index5, index6;
volatile uint8_t ampTable_c1 [32], ampTable_c2 [32];
volatile uint8_t oscTable_c  [256] PROGMEM;

void initPWM(void)
{
 pinMode(11, OUTPUT);  // Pin 11 PWM Audio output
 TCCR1A= _BV(COM1A1) | _BV(WGM10);  // 8-bit Fast PWM - non inverted PWM
 TCCR1B = TCCR1B & 0b11111000 | 0x01 ; // Set Prescaler (0x01)
 TIMSK1 = _BV(TOIE1);   // Enable overflow interrupt for OCR1A
}

ISR(TIMER1_OVF_vect)  // audio sample-rate Interrupt Handler ~31khz
{
 digitalWrite (13, HIGH);

 static uint16_t osc = 0;
 OCR1A = osc;  // output the last calculated sample 
 
 
  phaseAccum1 += phaseIncrement1;
  phaseAccum2 += phaseIncrement2;
  phaseAccum3 += phaseIncrement3;
  phaseAccum4 += phaseIncrement4;
  phaseAccum5 += phaseIncrement5;
  phaseAccum6 += phaseIncrement6;
  index1 = phaseAccum1 >> 8;
  index2 = phaseAccum2 >> 8;
  index3 = phaseAccum3 >> 8;
  index4 = phaseAccum4 >> 8;
  index5 = phaseAccum5 >> 8;
  index6 = phaseAccum6 >> 8;
  osc =  pgm_read_byte(&oscTable_c[index1])*ampTable_c1[a1];
  osc += pgm_read_byte(&oscTable_c[index2])*ampTable_c1[a2];
  osc += pgm_read_byte(&oscTable_c[index3])*ampTable_c1[a3];
  osc += pgm_read_byte(&oscTable_c[index4])*ampTable_c2[a4];
  osc += pgm_read_byte(&oscTable_c[index5])*ampTable_c2[a5];
  osc += pgm_read_byte(&oscTable_c[index6])*ampTable_c2[a6];
  osc = osc >> 8;  
  digitalWrite (13, LOW);
} // end of ISR
 
void setup ()
{
  pinMode (13, OUTPUT);
  initPWM(); // init PWM/interrupt handler
   sei(); // Enable global interrupts
}

void loop ()
{
}

This time the interrupt routine took 23.125 us, and measuring the “off” gap gives 13.458 us (see screenshot). So you can see from the screenshot that the bulk of the time is in the ISR (certainly over 50%). Interestingly, the reported frequency is 27kHz not 31kHz.

This doesn’t really solve your problem, but gives you hints that maybe you need to make the ISR more efficient. It also doesn’t explain why the counts are high rather than low. Perhaps if we saw more code.

Timer_Interrupt_Issue_57407.0.png

Thanks for the tips! I should have thought to test the audio-rate ISR "spent time" by writing to a pin. I was able to roughly duplicate your test results using digitalWrite() and an O-scope. Next, our team will try enabling interrupts within the audio-rate ISR to see if that solves anything... 8)