Timer interrupt firing late only for certain values

Hi!
I’ve been programming flight controllers for quadcopters lately and I found a weird bug when generating the signals for the ESCs to drive the motors and was wondering if someone could help.

When I try to set the Output Compare Register B (OCR1B) to the actual time form timer 1 (TCNT1) plus 235 (clock prescaler is 8, each timer count is 0.5 us, so that is 117.5 us later), raise the pin and then lower it when the interrupt fires, the actual pulse lenght is 122 us. This is not normal delay since my other 3 motor outputs come at 118.5 us, 1 us longer than programmed (235/2 = 117.5) which is reasonable taking ISR overheads in account and stuff.

Where it gets really weird is that if I lower the time amount from 235 to 232, the pulse length comes at 117 us, which is the expected (232/2 us + 1 us overhead). If I try with 233 I get alternating values 117-122-117-122, and if I turn it up to 234, i get the steady 122 us pulse length again.

122 us pulse length would be equivalent to 242 ((122 us - 1 us overhead)*2) timer counts, so it look like OCR1B doesn´t like to be set to fire between 233 and 242 timer counts in the future, so it defaults to 242 whenever it is set between these values. After 242, it works as expected.

I use both OCR1A and OCR1B (for two output compare interrupts), each manages two motors. Perhaps it has something to do with the way this is done, but I don´t think so since both ISRs are excatly the same, and only in one motor pulse of one ISR does this bug happens.

To help understand how this is implemented it goes something like this:

  • You have the duration for the 4 pulses to be generated
  • Set ISRs flags to 0. Set OCR1A to actual timer time plus the duration of first motor pulse (in timer counts), then raise pin
  • Do the same for OCR1B and second motor
  • When OCR1A fires after the duration of the pulse, the ISR checks the flag, if it´s 0 it lowers the raised pin, raises the flag, sets OCR1A for the duration of the next motor pulse (timer time+motor time), and raises the next motor’s pin. If the flag is 1, it just lowers the second motor´s pin
  • Same happens for OCR1B (not necessarily in this order, since that will depend on motor pulse duration)

And even though both the ISRs A and B do exactly the same thing just on different pins, I end up with one motor’s pulse length at 122 us and the other three at 118.5 us.

The offending one is the motor on pin 3 (BR), as marked in the comments in the code.

Here´s the code, if anyone notices something please tell:

#define MOTOR_FR  2                  //Motor output pins
#define MOTOR_FL  10           
#define MOTOR_BR  3
#define MOTOR_BL  11

bool actualChannelA;            //variables for OneShot and its timer ISRs, currently only supports four
bool actualChannelB;            //outputs but could easily be modded for more
uint16_t channelATicks[2];
uint16_t channelBTicks[2];

void setup() {
  pinMode(MOTOR_FR, OUTPUT);          //Motor outputs
  pinMode(MOTOR_FL, OUTPUT);
  pinMode(MOTOR_BR, OUTPUT);
  pinMode(MOTOR_BL, OUTPUT);

  oneShotSetup();                     //Timer setup
}

void loop() {
  oneShotMotors();
  delayMicroseconds(1500);
}

void oneShotSetup() {
  TCNT1 = 0;                  // clear the timer count
  TCCR1A = 0;                 // set entire TCCR1A register (timer config 1) to 0
  TCCR1B = 0;                 // set entire TCCR1B register (timer config 2) to 0 
  TCCR1B |= (1<<CS11);        // set timer prescaler to 8
  TIFR1 |= (1<<OCF1A);        // clear any pending interrupts A (cleared by writing a logic 1 to its location)
  TIFR1 |= (1<<OCF1B);        // clear any pending interrupts B
  TIMSK1 |= (1<<OCIE1A);      // enable the output compare interrupt A
  TIMSK1 |= (1<<OCIE1B);      // enable the output compare interrupt B
}

void oneShotMotors(){
  channelATicks[0] = 235;                   //Will output between 117.5-250 us, will take at max           
  channelATicks[1] = 235;                   //500us to update all motors, while loop time is 
  channelBTicks[0] = 233;                  //longer than this there will be no problem
  channelBTicks[1] = 235;
  
  actualChannelA = 0;
  actualChannelB = 0;                                                     //range from 250 to 500
  OCR1A = TCNT1 + channelATicks[0];                                       //since they are timer counts
  PORTD |= _BV (2);   //rise pin (2) for motor FR                         //(0.5us) not microseconds
  OCR1B = TCNT1 + channelBTicks[0];
  PORTD |= _BV (3);   //rise pin (3) for motor BR (faster i/o by port manipulation)
}

ISR(TIMER1_COMPA_vect){                              //ISR for Output Compare Register A
  if(actualChannelA == 0){
    PORTD &= ~_BV (2);        //lower pin (2) for motor FR
    actualChannelA = 1;
    OCR1A = TCNT1 + channelATicks[1];
    PORTB |= _BV (2);         //raise pin (10) for motor FL
  }
  else
    PORTB &= ~_BV (2);        //lower pin (10) for motor FL
}

ISR(TIMER1_COMPB_vect){                              //ISR for Output Compare Register B
  if(actualChannelB == 0){
    PORTD &= ~_BV (3);          //lower pin (3) for motor BR
    actualChannelB = 1;
    OCR1B = TCNT1 + channelBTicks[1];
    PORTB |= _BV (3);           //raise pin (11) for motor BL
  }
  else
    PORTB &= ~_BV (3);          //lower pin (11) for motor BL
}

Thank you!

After more exhaustive testing, it happened to be that those particular values weren´t the issue, but that the other values were the same. Both OCR1A and OCR1B were set too close and when OCR1As interrupt was being executed, OCR1B fired and had to wait till the other interrupt finished. Added a delay of around 4 microseconds using a for loop with a no intruction instruction inside, so it waits for 50 clock cycles plus the for overhead to dephase the signals, so interrupts don´t fire at the same time if pulses are the same lenght.

Like this:

void oneShotMotors(){ 
 
 channelATicks[0] = 235;                   //Will output between 117.5-250 us, will take at max           
 channelATicks[1] = 235;                   //500us to update all motors, while loop time is 
 channelBTicks[0] = 233;                  //longer than this there will be no problem
 channelBTicks[1] = 235;

 actualChannelA = 0;
 actualChannelB = 0;                                                     //range from 250 to 500
 OCR1A = TCNT1 + channelATicks[0];                                       //since they are timer counts
 PORTD |= _BV (2);   //rise pin (2) for motor FR                         //(0.5us) not microseconds
 for(int i=0; i<50; i++){                                                //To dephase pulse generation so ISRs don´t clash when the
   __asm__("nop\n\t");                                                   //value is the same (ie. both try to execute after 125us, one has to wait)
 }
 OCR1B = TCNT1 + channelBTicks[0];
 PORTD |= _BV (3);   //rise pin (3) for motor BR (faster i/o by port manipulation, according to the documentation below)
}