CTC Output Compare Interupt: Problems Calling Certain Functions Inside the ISR

Projects Programming
1

Hello All,

I am trying to use the CTC Output Compare Interrupt to update sensor values for a robot that I am building. I am trying to troubleshoot an issue that I am having where the ISR will not work if the functions pulseIn() and compass.ReadRawAxis() are called within the ISR (see attached code). I have functions heading() and ping_ultrasonic_sensors() being called inside the ISR. These functions in turn call the functions pulseIn() and compass.ReadRawAxis(). If I don't comment these two functions out, the program will not run and get stuck in the ISR. If I comment the two functions out, the program will work just fine. Of course, I have the option to move the functions heading() and ping_ultrasonic_sensors() inside the loop function, but I would like to get these guys working within an ISR. Any suggestions?

Note: I apologize the half commented code. However, most of it should be pretty easy to interpret.

[#include <HMC5883L.h>
#include <Wire.h>
#include<Serial.h>

HMC5883L compass;

float initial_heading=0;
float heading=0;
float headingDegrees=0;
unsigned long Ave1 = 0;
unsigned long Ave2 = 0;
unsigned long Ave3 = 0;
unsigned long Time_L = 0;                             //Variable for the time reading of the left sonic sensor
unsigned long Time_R = 0;                             //Variable for the time reading of the right sonic sensor

void ping_ultrasonic_sensors()
{
  unsigned long sum1 = 0;
  unsigned long sum2 = 0;
  unsigned long sum3 = 0;
  int test = 1;
  
  do{
  DDRD|=_BV(0);                           //Sets pin 6 of PORTD to output
  PORTD&=~_BV(0);                         //Sets the voltage on pin 6 of PORTD to low
  delayMicroseconds(100);                   //Delays for 2 microseconds
  PORTD|=_BV(0);                          //Sets the voltage on pin 6 of PORTD to high
  delayMicroseconds(20);                   //Delays for 5 microseconds
  PORTD&=~_BV(0);                         //Sets the voltage on pin 6 of PORTD to low
  DDRD&=~_BV(0);                          //Sets pin 6 of PORTD to input
  Time_L = pulseIn(0, HIGH);              //Waits for the returning pulse, stores time
  sum1 = sum1 + Time_L;
  Ave1 = sum1/test;
  
  DDRD|=_BV(1);                           //Sets pin 6 of PORTD to output
  PORTD&=~_BV(1);                         //Sets the voltage on pin 6 of PORTD to low
  delayMicroseconds(100);                   //Delays for 2 microseconds
  PORTD|=_BV(1);                          //Sets the voltage on pin 6 of PORTD to high
  delayMicroseconds(20);                   //Delays for 5 microseconds
  PORTD&=~_BV(1);                         //Sets the voltage on pin 6 of PORTD to low
  DDRD&=~_BV(1);                          //Sets pin 6 of PORTD to input
  Time_R = pulseIn(1, HIGH);              //Waits for the returning pulse, stores time
  sum2 = sum2 + Time_R;
  Ave2 = sum2/test;
  
  DDRD|=_BV(2);                           //Sets pin 6 of PORTD to output
  PORTD&=~_BV(2);                         //Sets the voltage on pin 6 of PORTD to low
  delayMicroseconds(100);                   //Delays for 2 microseconds
  PORTD|=_BV(2);                          //Sets the voltage on pin 6 of PORTD to high
  delayMicroseconds(20);                   //Delays for 5 microseconds
  PORTD&=~_BV(2);                         //Sets the voltage on pin 6 of PORTD to low
  DDRD&=~_BV(2);                          //Sets pin 6 of PORTD to input
  Time_L = pulseIn(2, HIGH);              //Waits for the returning pulse, stores time
  sum3 = sum3 + Time_L;
  Ave3 = sum3/test;
  
  test = test + 1;
  }while(test < 5);
}

void get_heading()
{
  MagnetometerRaw raw = compass.ReadRawAxis();
  MagnetometerScaled scaled = compass.ReadScaledAxis();
  heading = atan2(raw.YAxis, raw.XAxis);
  if(heading < 0)
    heading += 2*PI;
  headingDegrees = heading * 180/M_PI;
}

void cancle_PWM()
{
 analogWrite(5,0);           //Cancles all motor PWM's
 analogWrite(6,0);
 analogWrite(9,0);
 analogWrite(10,0); 
}

void cancle_signals()
{
  PORTB&=~(_BV(4) | _BV(5)); //Cancles all transistor signals
  PORTD&=~(_BV(4) | _BV(7));
}

void turn_left()
{
  cancle_signals();
  PORTB |= _BV(4);
  PORTD |= _BV(7);
  cancle_PWM();
  analogWrite(11,255);     //Enables right side motor
  analogWrite(5,150);      //Enables left side motor
}

void turn_right()
{
  cancle_signals();
  PORTB |= _BV(4);
  PORTD |= _BV(7);
  cancle_PWM();
  analogWrite(11,150);     //Enables right side motor
  analogWrite(5,255);      //Enables left side motor
}

void go_back()
{
  cancle_signals();
  PORTB |= _BV(5);
  PORTD |= _BV(4);
  cancle_PWM();
  analogWrite(9,255);      //Enables right side motor
  analogWrite(6,255);      //Enables left side motor
}
void go_straight()
{
  cancle_signals();
  PORTB |= _BV(4);
  PORTD |= _BV(7);
  cancle_PWM();
  analogWrite(11,255);     //Enables right side motor
  analogWrite(5,255);      //Enables left side motor
}

ISR(TIMER1_COMPA_vect)
{
  //Serial.println("A");
  ping_ultrasonic_sensors();
  get_heading();
}

void AD_setup(void)
{
  ADMUX=_BV(5) | _BV(6);                                   //left aligned conv., 5V ref
  ADCSRA=_BV(0) | _BV(1) | _BV(2) | _BV(4) | _BV(7);                //128 Prescalar, clear ADC flag, enables AD conversions
}

void Timer1_ISR_setup(void)
{
    cli();
    TCCR1A= 0; 
    TCCR1B=_BV(1) | _BV(3);                                         //Sets a prescalar of 8 and puts timer in CTC mode
    OCR1A=20000;                                                    //Sets OCR1A register to give 10 ms delay
    TIFR1=_BV(1);                                                   //Clears compare flag
    TIMSK1|=_BV(1);                                                  //Sets output compare interrupt
    sei();
}

void setup()
{
  delay(5000);
  
  Serial.begin(9600);
  DDRB = 0xFF;                              //Sets all ports in PORTB to outputs
  DDRD = 0xFF;                              //Sets all ports in PORTD to outputs
  PORTB|=_BV(0);
  
  Timer1_ISR_setup();
  AD_setup();
  
  Wire.begin();
  compass = HMC5883L();
  compass.SetScale(1.3);
  
  MagnetometerRaw raw = compass.ReadRawAxis();
  MagnetometerScaled scaled = compass.ReadScaledAxis();
  heading = atan2(raw.YAxis, raw.XAxis);
  if(heading < 0)
    heading += 2*PI;
  initial_heading = heading * 180/M_PI;
}

void loop()
{
  Serial.println(initial_heading);
  Serial.println(headingDegrees);
  Serial.println("  ");
  /*  
  if((heading - initial_heading) < -5){
    turn_right();
  }else if((heading - initial_heading > 5)){
    turn_left();
  }else{
    go_straight();
  }
  
  if(Ave2 < 800){
     cancle_PWM(); 
  }*/
}]
2

ISRs are supposed to be short. That doesn't look short.

ReadRawAxis looks to me like it is doing I2C reads. I2C requires interrupts which are off in an ISR.

I suggest you rework to not do all this stuff inside your ISR.

3

In addition to the previous answer, you cannot use delay() or Serial() inside code running in or from an ISR.

4 
void cancle_signals()
{
  PORTB&=~(_BV(4) | _BV(5)); //Cancles all transistor signals
  PORTD&=~(_BV(4) | _BV(7));
}

Why not do digitalWrite here? Speed is hardly of the essence is it? Readability almost always is, in my book.

void turn_left()
{
  cancle_signals();
  PORTB |= _BV(4);
  PORTD |= _BV(7);
  cancle_PWM();
  analogWrite(11,255);     //Enables right side motor
  analogWrite(5,150);      //Enables left side motor
}

It seems to me to be throwing away future maintainability here, especially when you use analogWrite rather than directly manipulating the registers. If you are going to use the libraries, make the whole project readable. You'll thank me in a year's time when you go back and wonder what the hell those PORTB things are doing. Especially in the absence of explaining comments.

5

Suggestion... it seems to me that you are using the ISR for timing only. I would use the ISR to set a flag, then check it in loop(), and call your functions if it's set, clearing it after the job is done.

6

Nick and Iar3ry,

Thank you for your suggestions. I think I understand where my problem might be. Sorry for not using digitalWrite, I will start using it if it from now on if it helps at all. Using PORTB assignments was just the way I was taught and its the most comfortable way for me to assign pin values. But I will start using digitalwrite() if it makes the code easier to read.

7

I was an AVR coder for many years, virtually all of it in assembler, and it was a little difficult for me to switch to a high level language, because it always seemed to me that the code 'behind the scenes' was probably inefficient, or that there were things I just couldn't do. I didn't really understand that the library code (the standard libraries), was actually pretty darned good. I have since changed my mind. I no longer care about inefficient code, because I have seen the results in disassemblies, and though I could probably save a fair number of bytes, or speed things up a little, the effort to do so seems not worth it. If I find myself running short of memory, well, I can just get a bigger AVR. If I REALLY need to save a few cycles in a time-critical application, I can always revert to assembler and take the time-hit while writing the code.

In my opinion, the advantages gained in not having to comment so much, and in maintainability, are well worth pursuing by writing in the higher level language whenever possible.

It's a LOT easier to read and maintain when you use something like digitalWrite ( pingPin, HIGH );