Delay in main loop causes crash

Hi all, and Merry Christmas! I'm (still) working on getting a zytemp tn9 infrared sensor working with an Arduino 2560. The sensor sends a 5-byte packet of data every half a second or so and each packet contains either a infrared or ambient temperature reading. The sensor follows an SPI sort of protocol; when the sensors clock line falls, one bit is available to be read. To accomplish this, I set up an interrupt as seen below:

volatile byte n = 0;						// Interrupt Bit Count				
volatile byte pos = 0;						// Values Position Count
volatile byte values[5] = {0,0,0,0,0};		        // Values to be stored by sensor
volatile byte cbit = 0;						// Current bit read in

boolean irFlag = false;						// Flag to indicate IR reading has been made
boolean ambFlag = false;					// Flag to indicate ambient temp reading has been made


byte irValues[5] = {0,0,0,0,0};				         // Variable to store IR readings
byte ambValues[5] = {0,0,0,0,0};			         // Variable to store Ambient readings

const int len = 5;							// Length of values array
const int clkPin = 3;						// Pins
const int dataPin = 2;
const int actionPin = 4;

void setup(){
  Serial.begin(9600);						

  pinMode(clkPin, INPUT);					   // Initialize pins
  pinMode(dataPin, INPUT);
  pinMode(actionPin, OUTPUT);
  digitalWrite(clkPin, HIGH);
  digitalWrite(dataPin, HIGH);
  digitalWrite(actionPin, HIGH);

  Serial.println("Type to Start...");		                  // Wait for input to start
  while(!Serial.available());
  Serial.println("Starting...");

  attachInterrupt(1,tn9Data,FALLING);  		         // Interrupt
}

void loop(){
  digitalWrite(actionPin,LOW);	                        // Make sensor start sending data


  if(pos == len && values[0] == 0x4C){		        // If sensor has sent IR packet...
    for(int i = 0; i < len; i++){			                // Store values to irValues
      irValues[i] = values[i];
    }
    irFlag = true;							// Indicate IR reading
    pos = 0;
  }

  if(pos == len && values[0] == 0x66){		        // If sensor has sent ambient packet...
    for(int i = 0; i < len; i++){			                // Store values to ambValues
      ambValues[i] = values[i];
    }
    ambFlag = true;						// Indicate Ambient reading
    pos = 0;
  }

  if(pos == len && values[0] == 0x53){		        // If sensor has sent junk packet
    pos = 0; 
  }

  if(irFlag && ambFlag){					        // If successful IR and Ambient reading...
    word tempword = 0;  					// Next 4 lines isolate temperature component of values
    tempword = tempword | irValues[1];
    tempword = tempword << 8;
    tempword = tempword | irValues[2];
    if(tn9Check(irValues)){					// If checksum is valid, print IR temperature
      Serial.print("IR = ");
      Serial.print(int(tempword)/16.0 - 273.15);
      Serial.print(" :: ");       
    }
    else{								// If checksum isn't valid, print impossible temp
      Serial.print("IR = ");
      Serial.print("-273.15");
      Serial.print(" :: ");  
    }

    tempword = 0;						// Isolate temperature component again for ambient
    tempword = tempword | ambValues[1];
    tempword = tempword << 8;
    tempword = tempword | ambValues[2];
    if(tn9Check(ambValues)){				         // If checksum is valid, print ambient temperature
      Serial.print("Amb = ");
      Serial.println(int(tempword)/16.0 - 273.15);        
    }
    else{						                // If checksum isn't valid, print impossible temp
      Serial.print("Amb = ");
      Serial.println("-273.15"); 
    }
    irFlag = false;							// Reset flags
    ambFlag = false;
    
    //delay(3000);							// !!PROBLEM LINE!! Simulates other sensors, code
  }									    // doesn't work with this line here.
  


}


void tn9Data(){							// Interrupt Function
  cbit =  digitalRead(dataPin);				// Read bit
  values[pos] = (values[pos] << 1) | cbit;	                // Store to values
  n++;							                // Increment bit count
  if(n == 8){								// Increment position count based on bits read in
    pos++;
    n = 0; 
  }
  if(pos == len){ 							// If complete "packet" sent, stop sensor from sending 
    digitalWrite(actionPin,HIGH);  			        // again until main loop allows it.
  }
}


boolean tn9Check(byte tn9Values[]){			                             // Checksum calculating function
  int mcheck = (int)tn9Values[0] + (int)tn9Values[1] + (int)tn9Values[2];    // Checksum calculation
  int scheck = (int)tn9Values[3];			                                     // Checksum sent by sensor
  boolean crc = false;						                             // Initialize return value
  if(mcheck > 510) mcheck = mcheck - 512;	                                     // Handle sensor byte rollover
  if(mcheck > 255) mcheck = mcheck - 256;	                                     // Handle sensor byte rollover
  if(mcheck == scheck) crc = true;			                                     // Check checksum
  return(crc);								                     // Return
}

The problem has to do with the delay in the last if statement. When that delay is there, the arduino reads one or two "packets" and then restarts. If the delay isn't there the program works fine. I know that you are not supposed to use delays inside interrupts but this one is in the main loop. I have tried detaching the interrupt right before the delay and reattaching right after but that doesn't work either. Finally, the reason I need the delay is because the code for other sensors will be going there requires delays. Thank you guys for all the help!

Perhaps try a delay without using delay();
look at the blink without delay example and have it just do nothing during the delay time

I think you've been a bit liberal with the use of "volatile".
Some of those variables are not shared between interrupt context and normal context.

I was hoping to avoid the millis() delay since I would have to change the entire flow of my code... I was also wondering why the delay was not working with what I assume is the interrupts. Does it have something to do with the internal timers?

@AWOL: Can you expand on the volatile part please? I was under the impression that anything that gets modified in an interrupt should be declared volatile. Also, the only thing I'm doing with a volatile variable out of the interrupt is checking a condition and copying values from it.

I don't think you're using cbit outside of the interrupt, so no need for it to be declared volatile.
The reason for declaring variables as volatile is if they could otherwise be reasonably optimized out by the compiler.

I see, thank you for your help! I've been trying a lot of different things about delay() problem. It turns out that delays below about 500 ms don't crash the Arduino but are also ignored. I tested this by printing the millis() at the delay point with and without the delay. I might try making my own delay function next...

Your problem is here:

void tn9Data(){							// Interrupt Function
...
  values[pos] = (values[pos] << 1) | cbit;	                // Store to values  n++;							                // Increment bit count
  if(n == 8){								// Increment position count based on bits read in
    pos++;
    n = 0; 
  }
...
}

There is no test for pos exceeding the array size for 'values'. So it could potentially go over the end of 'values' and overwrite memory.

The delay of more than about 500 mS gives it time to do this. With a shorter delay loop has a chance to set pos back to zero.