Long interrupts and I2C

Projects General Guidance
1

I have hit a snag in my project regarding long interrupt routines crashing when the program includes I2C communications. I understand that it's bad practice to have long interrupts in the first place and I might have to do things differently anyway, but the whole issue interests me so I thought I'd ask about it.

My project is basically a crude autopilot for a RC parkflyer. It includes an Arduino nano, a GPS, the usual gyroscope/accelerometer, a barometer, differential barometer and so on. I had all of these in my sketch and working easily fast enough (about 30 loops per second) using mostly Adafruit libraries, but the GPS parsing function takes long enough for there to be a noticeable pause, which isn't really good.

So, instead of breaking up the GPS parsing into small state machine steps as I just might have to do anyway, I got the bright idea of reading the gyroscope/accelerometer and update the control servos accordingly in a timer interrupt, with all the other less time-critical stuff like the GPS running along in the "backround". Turns out, it works magnificently and smoothly until the sketch crashes at a random point in time. It only happens if the running loop uses I2C, so I reasoned that if the long interrupt fires during communications, it will not only garble the data (which sometimes happens) but crash the sketch as well.

I got the idea to fix this by manually disabling the timer interrupt just before I2C communication starts and enabling it right afterwards, testing it using only the I2C-connected barometer without which the sketch doesn't crash. Although the program's stability increased a lot leading me to believe I'm on the right track, it didn't completely fix the issue so here I am now.

This is basically what I'm doing now, turning off timer interrupts on timer 1 which triggers my periodic "control" loop whenever I2C is used, modified part of library is attached...

  TIMSK1 |= (0 << OCIE1A);
  Wire.beginTransmission(MPL3115A2_ADDRESS); // start transmission to device 
  Wire.write(a); // sends register address to read from
  Wire.endTransmission(false); // end transmission
  Wire.requestFrom((uint8_t)MPL3115A2_ADDRESS, (uint8_t)1);// send data n-bytes read
  byte info = Wire.read();
  TIMSK1 |= (1 << OCIE1A);

This is the offending interrupt itself, the whole code is also attached.

ISR(TIMER1_COMPA_vect){
  if(controlOn){
  
  //if(interruptDiv >= 10){
    sei();
    event = BnoUpdate();
    
    cli();
    BnoCursorHandle();
    if(digitalRead(11)== LOW){
      DrawGPSData(GPS);
    }
    else{
      DrawOrientationData(event);
    }
    SetContrast();
    if (toggle2){
      digitalWrite(boardLed,HIGH);
      toggle2 = 0;
    }
    else{
      digitalWrite(boardLed,LOW);
      toggle2 = 1;
    }
  }
}

I realize this sort of solution might be a dead end, but I am curious what causes these interrupt-related crashes. The test program has a lot of stuff on it, but in this cut-down state it's completely stable without the barometer, and crashes with it so the long interrupt by itself works fine.

Adafruit_MPL3115A2.cpp (5.44 KB)

displaytest.ino (11.5 KB)

2

If the ISR is needs attention only put a flag to TRUE, and return.
do not wait until the gps answers, just wait in the loop until it answers and then put the result in an array, if result is complete, then do the calcs.

3

shooter:
If the ISR is needs attention only put a flag to TRUE, and return.
do not wait until the gps answers, just wait in the loop until it answers and then put the result in an array, if result is complete, then do the calcs.

The gps works like that already, it only parses when new data is recieved. It is the parsing itself that causes the hitchup.

4

There should be no such thing as a long interrupt.

Do not do anything in the ISR apart from record the fact that it happened. For example

void myISR() {
   isrTriggered = true;
}

Do the rest of the processing elsewhere.

...R
Edit to change == to = after the error was pointed out in Reply #4 apologies

5 
isrTriggered == true;

Ooops.
It should be:

isrTriggered = true;

Pete

6

In your long interrupt routine you temporarily enable interrupts and access the I2C bus. If while doing that the GPS starts sending data it will screw up your I2C communication. The GPS code monopolizes the processor while the data is being received so it may be possible that another timer 1 interrupt occurs before you've disabled interrupts. Then you'd jump to the ISR while you're still in the ISR. Or maybe not, but you've opened yourself to these kinds of misadventures by your unorthodox programming style.

Frankly, the Adafruit GPS code isn't very good. Switch to NeoGPS. Or if not, then TinyGPS. They are both designed to parse each character as it arrives. And if you can use AltSoftSerial instead of SoftwareSerial that would help too.

But get rid of the long complicated interrupt. It's just not a good idea.

7

el_supremo:
Ooops.

Thanks for spotting that. I have corrected it.

...R

8

Guess I'll get rid of the interrupt.

jboyton:
In your long interrupt routine you temporarily enable interrupts and access the I2C bus. If while doing that the GPS starts sending data it will screw up your I2C communication. The GPS code monopolizes the processor while the data is being received so it may be possible that another timer 1 interrupt occurs before you've disabled interrupts. Then you'd jump to the ISR while you're still in the ISR. Or maybe not, but you've opened yourself to these kinds of misadventures by your unorthodox programming style.

Frankly, the Adafruit GPS code isn't very good. Switch to NeoGPS. Or if not, then TinyGPS. They are both designed to parse each character as it arrives. And if you can use AltSoftSerial instead of SoftwareSerial that would help too.

But get rid of the long complicated interrupt. It's just not a good idea.

I will try the other GPS and serial options, but the funny thing is that the GPS itself worked great with this setup. Only the addition of I2C communications crashes the sketch. It might be that when interrupts need to be enabled for a bit within the interrupt routine to read the gyro, enough millis will eventually be incremented that will cause the interrupt routine to fire recursively within itself. Perhaps if I selectively disable that interrupt at the same time... or just get rid of the whole thing :slight_smile:

Thanks for the suggestions.