Hi guys
I'm developing a kiln controller. I'm experimenting with using an interrupt driven routine for reading the temperatures of the kiln using an Ocean Controls thermocouple shield. I've fallen into and climbed back out of several ISR traps already, and I'm wary that there may still be some lurking in the code. If you're expert in ISRs, can you please take a look over it ?
#include <SPI.h>
#include <EEPROM.h>
#include <TimerOne.h>
void setup(){
// other stuff
Timer1.initialize(1000000);
Timer1.attachInterrupt( readTemperatures ); // attach the service routine here
}
// the ISR
void readTemperatures(){
// pick up last temperature asked for, and request the next one
// divides the standard MUX read in two parts: initiate, and read
// this way eliminates the delay(220) in the standard Ocean readfunction, leaving maximum time for other activity,
// while maintaining readings for each channel every tempRedaingPeriod
// declare, initialise temperature working variables
static int st_temp[mcp1];
static int st_slope[mcp1];
static byte n_ramp[mcp1];
static int rp=-1; // data byte record pointer
static unsigned long reading_count=0;
int temperature;
int slope; // change from previous temp
int slope_difference_limit=2;
int slope_difference=0; // difference from initial slope
static int channelToRead=1;
unsigned long time; // to calculate delays (in microseconds)
if (readTemperaturesSuspended) return;
//temperature=Read_Temperature_take_reading(); // inserted code in-line to avoid function call - necesssary?
//int Read_Temperature_take_reading(void){ // old declaration
// unsigned int temp_reading; // using temperature instead
// read result (by virtue of the timer, it has been over 250ms since the read was initiated (time for 'conversion' on the chip)
digitalWrite(CS_TEMP,LOW); // Set MAX7765 /CS Low
//delay(1); // replaced with loop
time=Timer1.read(); while (Timer1.read()-time>1*1000) {};
temperature = SPI.transfer(0xff) << 8;
temperature += SPI.transfer(0xff);
digitalWrite(CS_TEMP,HIGH); // Set MAX7765 /CS High
//delay(1); // replaced with loop
time=Timer1.read(); while (Timer1.read()-time>1*1000) {};
// check result
if(bitRead(temperature,2) == 1) temperature=-1; // Failed / No Channel Error (KD presumably no thermocouple attached, broken lead wire, etc)
else temperature=(int)(temperature >> 5); //Convert to Degc
slope=temperature-temp[channelToRead]; // during last interval
slope_difference=slope-st_slope[channelToRead]; // difference from starting slope, not from last slope reading
if ((abs(slope_difference)>slope_difference_limit) || // if the slope is too different from the original, or
(n_ramp[channelToRead]==255) || //if we've run out of space in intervals byte, or
(forceTempLog)) { // if we've been forced to log an interval, as when closing a session, or doing a data upload
recordCount+=1; // recordCount increments for each reading for each channel
// store the latest interval data
EEPROM.write(rp+1,n_ramp[channelToRead]); // byte 1 = number of intervals
if (temperature < 0) { // usually a broken channel
EEPROM.write(rp+2,0);
EEPROM.write(rp+3,0);
}
else {
// channel goes in left most 3 bits, high bits of temp in balance
EEPROM.write(rp+2,(channelToRead<<5)+(st_temp[channelToRead]>>8));
EEPROM.write(rp+3,st_temp[channelToRead] & 255); // least significant byte
}
rp += 3; // update record buffer pointer
// record the new starting points
st_temp[channelToRead]=temperature;
st_slope[channelToRead]=slope;
n_ramp[channelToRead]=0; // will be incremented to 1
} // if a new record is required
forceTempLog=OFF; // reset in case it was used
temp[channelToRead]=temperature;
n_ramp[channelToRead] += 1; // number of intervals associated with this reading (roughly speaking the time at the current rate of temperature change)
// find next channel to read
do {
channelToRead=(((channelToRead-1)+1)% 8) +1; // -1 to return to zero base, +1 to increment to next channel, % to loop back to 0, +1 to return to 1-base
if (channelToRead==1) {
reading_count += 1; // reading count incremented only after all active channels have been read
}
} while (!(bitRead(channelsInUse,channelToRead-1))); // using 0-based bit reader
// now pointing at next channel requiring servicing
// Set_Mux_Channel(channelToRead);
byte chanZeroBased=channelToRead-1;
// set the MUX channel
// place address bits on pins
digitalWrite(MUX_A0,(chanZeroBased & B00000001)>>0);
digitalWrite(MUX_A1,(chanZeroBased & B00000010)>>1);
digitalWrite(MUX_A2,(chanZeroBased & B00000100)>>2);
// Initiate next read
digitalWrite(CS_TEMP,LOW); // Set MAX7765 /CS Low
//delay(5); // replaced with loop
time=Timer1.read(); while (Timer1.read()-time>5*1000) {};
digitalWrite(CS_TEMP,HIGH); // Set MAX7765 /CS High
}
Are EEPROM reads and writes ssafe in an ISR? (there will be EEPROM activity in the main programme: there will be reads from the EEPROM addresses written by the ISR, and there will be Writes, but these will never be to the addresses used in the ISR)
I have eliminated all user-defined functions from the ISR, but the following function calls remain:
digital.Write()
SPI.transfer()
bitRead()
Are these safe?
Was it necessary to avoid user-defined function calls? (I had problems when these contained Serial.prints, so decided to remove them entirely)
I have replaced both uses of delay() with a do-nothing loop, but the loop makes use of calls on Timer1 - is this safe?
The ISR updates a global array temp[] so I have made this volatile in the main programme .
How does the compiler know that this is an ISR - is it clever enough to determine this through the Timer1 class invocation?
Many thanks
Kenny