Arduino uno R3 timer 1 in CTC mode pulses once after reset, will not re-trigger

Hi! The program executes correctly after reload or after pressing of the reset button.

This program should generate precision pulses on pins D9 and D10 of Arduino uno R3. The pins D9 and D10 should be normally high and go low only during the pulse. The pulses should start simultaneously but stop after individually set timeouts. The individual pulse duration should be up to 32 msecs setable in 0.5 microsecond increments. The current program has 200 to 300 nanoseconds setup delay added to the total pulse duration.

All of the program is in the Loop() but after producing first pulse, it loops forever doing nothing. I have printed out the status of all registers immediately after reset and have attempted to recreate them at the end of the execution. I was successful except for the ICF1 interrupt flag which will not reset (i.e. TIFR1 remains 0x20). I am not using ICF1 interrupt, I only set ICF1 register to 0xFFFF as a TOP value for the timer.

I have included the debugging observations in the code comments.

boolean running = false;
volatile boolean doneA = false;
volatile boolean doneB = false;
 ISR (TIMER1_COMPA_vect)  {
  doneA = true;
}
 ISR (TIMER1_COMPB_vect)  {
  doneB = true;
}
void setup() {                
  sei();                 // enable global interrupts
}
void loop() {
   /* Only one set of pulses per trigger is required, but for test purposes, the loop should
   produce a free running pulse trains with the repetition rate set by the 'delay()' statement
   at the end of the loop. */
  if (running == false) {
// Setup 'if' statement flags
    doneA = false;
    doneB = false;
    running = true;
//  The following code initializes Timer1 
    GTCCR = 0x83;          //Stop and reset timers during setup
    /* At this point the OC1A & OC1B pins are inputs and will be set as outputs later.
    Presetting them high or low has no effect until the DDRB is initialized.
    I want them to go low only during the pulse so I am just making sure...*/
    PORTB |= 0x06;         // preset OC1A & OC1B high
//  PORTB &= 0xF9;         // preset OC1A & OC1B low
    ICR1 |= -1;            //Set TOP to 0xFFFF (i.e. to MAX)
    TCNT1 = 0;             // initialize counter
    TIMSK1 = 0x06;         // enable compare interrupts
    /* In order to make sure there are no pending interrupts on exiting the timer setup
    I prefer to clear all interrupt flags by writing logic '1' into their locations. First time through,
    the ICF1 flag is '0' */
    TIFR1 = 0x27;          // reset all interrupt flags - should but does not seem to reset ICF1 next time around
    TCCR1B = 0x1A;         // set up mode = 12 and prescaler = 8
   /* Setting TCCR1A must occur before setting of the OCR1A and OCR1B registers or the first
   compare will be missed and the output will be set at TCNT1 == 0xFFFF or thereabout - my
   oscilloscope is not precise enough */
    TCCR1A = 0xF0;         // set OC1A & OC1B on compare
   /* The OCR1A and OCR1B registers may be set to any value >= 1 
   I did not check for values close to 0xFFFF yet */
    OCR1A = 0x0005;        // Set comparator A - here testing with 2.5 microseconds
    OCR1B = 0x0001;        // Set comparator B - here testing with 0.5 microseconds
    GTCCR &= 0x03;         // Start timer after setup
   /* The OC1A and OC1B pins (D9 and D10) will go low as soon as set as outputs. In order to
   reduce initial dead time to minimum, this is done last */
    DDRB |= 0x06;          // Set OC1A & OC1B as outputs
  }
  // The timers now run until both interrupt routines have completed  setting both doneA and doneB to true
  if  ((doneA & doneB) == true)  {
// The following section attempts to return the ATmega328 into the initial state
    GTCCR = 0x83;          //Stop and reset timers during setup
    // clear 'if' statement flags
    doneA = false;         
    doneB = false;
    running = false;
    /* The OC1A & OC1B pins are set as inputs again. Presetting them low has no effect
    (i.e. they remain high) if done before TCCR1A and TCCR1B are reprogrammed.*/
    DDRB = 0;              // Set OC1A & OC1B as inputs
    PORTB &= 0xF9;         // Preset OC1A & OC1B low
    /* preset mode 1 (8-bit PWM) and prescaler of 64
    Presetting these or any other timer 1 related registers has no effect on ultimate outcome.
    The OC1A and OC1B (D9 and D10) will now remain high until reset button is pressed
    or program reloaded */
    TCCR1A = 1;
    TCCR1B = 3;
    OCR1A = 0;
    OCR1B = 0;
    TCNT1 = 0;
    TIMSK1 = 0;
    ICR1 = 0;
    GTCCR &= 0x03;         // restart timers
    delay(2000);           // wait for two seconds then repeat
  }    
}

Once debugged it will be converted into procedure and triggered by hardware interrupt on an input pin.

I don't actually see anything wrong. But I'm not an expert manipulating the AVR timers...

I'd be a lot happier if you separated the timer initialization (modes/etc) from the code that you expect to repeat. "Restoring to initial state" seems like a poor idea. (Hmm. Are you restoring to the AVR "reset" state, or to the state that the arduino init() code sets the timers to?)

How about some symbolic constants for those timer register bits?

It doesn't seem like a good idea to me, to reset the port pins to inputs immediately after the OCC matches both occur. don't you want the pulses to be "active" ?

You want something like this, right?

  initialize_timer();
   while (1) {
     start_timer(); // start both pulses.
     while (!(doneA && doneB))
        ;  // spin, waiting for both pulses to complete.
     stop_timer();
     delay(2000);
   }  // repeat the pulses

Why don't you start by writing it just like that? if it's too slow AFTER it is working, you can work on optimizing it by glopping all the code together. (but the compiler will probably do that for you anyway...)

In general, I think we don't emphasize breaking code into modular pieces enough in these forums... :-(

Thanks for your reply.

I have found the cause:

The states of OC1A and OC1B are the states of internal hardware latches which are set or cleared on compare match. The latches override the port setting when selected (in TCCR1A register) thus writing to port had no effect. The latches are also cleared on system reset, and since I was never resetting them in the program, the program worked only once after system reset.

My current solution is to set D9 and D10 as inputs (once the pulse is done), preset compare registers again (to some small value), and preset compare output mode to clear on compare match. Following that, I clear the timer registers (probably not needed).

So the real question is: is there a simpler or more elegant way to clear the OC1A and OC1B hardware latches?

You are right, the program is messy, but it was written only as a proof of concept. I will clean it up now that I have found the problem. I find that writing the whole register in HEX is easier to debug because I print their content as HEX as well (i.e.: Serial.println( TIFR1, HEX); ) which makes it easy to compare and spot changes. Clearly, such code is not portable to different CPU.

So here is a working code (messy as it is)

boolean running = false;
boolean cleaning = false;
volatile boolean doneA = false;
volatile boolean doneB = false;

 ISR (TIMER1_COMPA_vect)  {
  doneA = true;
}
 ISR (TIMER1_COMPB_vect)  {
  doneB = true;
}
void setup() {                
}
void loop() {
  if (running == false) {
    doneA = false;
    doneB = false;
    running = true;
// The following code initialises Timer1 for pulse generation
    GTCCR = 0x83;          //Stop and reset timers during setup
    ICR1 |= -1;            //Set TOP to 0xFFFF (i.e. to MAX)
    TCNT1 = 0;             // initialize counter
    TIMSK1 = 0x06;         // enable compare interrupts
    TIFR1 = 0x27;          // reset all interrupt flags - should but does not seem to reset ICF1
    sei();                 // enable global interrupts
    TCCR1B = 0x1A;         // set up mode = 12 and prescaler = 8
    TCCR1A = 0xF0;         // set OC1A & OC1B on compare
    OCR1A = 0x0005;        // Set comparator A - here testing with 2.5 microseconds
    OCR1B = 0x0001;        // Set comparator B - here testing with 0.5 microseconds
    GTCCR &= 0x03;         // Start timer after setup
    DDRB |= 0x06;          // Set OC1A & OC1B as outputs
  }
  if  ((doneA & doneB) == true)  {
    doneA = false;
    doneB = false;
    if (cleaning == false) { //setup compare to clear OC1A & OC1B
      cleaning = true;
// The following code initialises Timer1 to clear OC1A & OC1B
      DDRB = 1;              // Set OC1A & OC1B as intputs
      GTCCR = 0x83;          //Stop and reset timers during setup
      ICR1 = 0x000F;         // Set TOP to 0x000F
      TCNT1 = 0;             // initialize counter
      TIFR1 = 0x27;          // reset all interrupt flags - should but does not seem to reset ICF1
      TCCR1A = 0xA0;         // reset OC1A & OC1B on compare
      OCR1A = 0x0001;        // Set comparator A - here testing with 0.5 microseconds
      OCR1B = 0x0001;        // Set comparator B - here testing with 0.5 microseconds
      GTCCR &= 0x03;         // Start timer after setup
    }
    else  {                  // Leave the camping site in state in which you have found it
      cleaning = false;
      running = false;
      cli();
      // cleaning up the timer registers is probably not needed
      GTCCR = 0x83;          //Stop and reset timers during cleanup
      TCCR1A = 1;
      TCCR1B = 3;
      OCR1A = 0;
      OCR1B = 0;
      TCNT1 = 0;
      TIMSK1 = 0;
      ICR1 = 0;
      GTCCR &= 0x03;         // Start timer after cleanup
      delay(2000);           // wait for two seconds then repeat
    }
  }    
}

OK everybody, I have found correct way to reset the OC1A and OC1B latches and have cleaned up my code.

Now for some serious weirdness:

  1. The program works as designed for time-out values between 1 and 0x8210 inclusive.
  2. When D10 timeout (timeOutB) is set to 0x8211, the D10 drops low and stays low forever
  3. When D10 timeout (timeOutB) is set to 0x8210, but D9 timeout (timeOutA) is set to 0x8211, the D10 pulses once, then stays high forever
  4. When statement TIFR1 = intFlags; is commented out, the maximum value increases from 0x8210 to 0xF0E0, beyond that is as per 2) and 3) above.
  5. The ultimate insult : including Serial.begin( 9600); into setup() section makes everything work up to 0xFFFF without me ever opening the serial monitor itself!!!

Here is the code:

unsigned int timeOutA = 0x8210;
unsigned int timeOutB = 0x8210;
volatile boolean doneA = false;
volatile boolean doneB = false;

// The following presets are used for timer 1 control

   // set OC1A & OC1B on compare
unsigned int controlA1 = 0|(1 << COM1A1)|(1 << COM1A0)|(1 << COM1B1)|(1 << COM1B0);

   // reset OC1A & OC1B on compare
unsigned int controlA2 = 0|(1 << COM1A1)|(1 << COM1B1);

   // set up mode = 12 and prescaler = 8
unsigned int controlB = (1 << WGM13)|(1 << WGM12)|(1 << CS11);

   // reset OC1A & OC1B on compare
unsigned int controlC = 0|(1 << FOC1A)|(1 << FOC1B);

   // set interrupt mask
unsigned int intMask = 0|(1 << OCIE1A)|(1 << OCIE1B);

   // reset all interrupt flags
unsigned int intFlags = (1 << ICF1)|(1 << OCF1B)|(1 << OCF1A)|(1 << TOV1);

   //Set timer 1 TOP to 0xFFFF (i.e. to MAX)
unsigned int Top = 0xFFFF; 


// compare match interrupt routines
 ISR (TIMER1_COMPA_vect)  {
  doneA = true;              // end of pulse on D9
}
 ISR (TIMER1_COMPB_vect)  {
  doneB = true;              // end of pulse on D10
}

/* This routine generates pulses on D9 and D10 (one pulse each)
The variable timeOutA sets duration of the pulse on D9 in 0.5 microsecond increments
The variable timeOutB sets duration of the pulse on D10 in 0.5 microsecond increments
*/
void pulse_D9_D10()  {

    // The following code resets OC1A & OC1B to low
    DDRB = 0;                // Set OC1A & OC1B as intputs
    TCCR1A = controlA2;      // select 'reset OC1A & OC1B on compare'
    TCCR1C = controlC;       // force compare to reset OC1A & OC1B
    TCCR1C = 0;              // clear force compare

    // The following code initializes Timer1 for pulse generation
    GTCCR |= 0x83;           //Stop and reset timers during setup
    ICR1 = Top;              //Set TOP to 0xFFFF (i.e. to MAX)
    TCNT1 = 0;               // initialize counter
    TIMSK1 = intMask;        // enable compare interrupts
    TIFR1 = intFlags;        // reset all interrupt flags - not really needed and may be commented out
    TCCR1B = controlB;       // set up mode = 12 and prescaler = 8
    TCCR1A = controlA1;      // set OC1A & OC1B on compare
    OCR1A = timeOutA;        // Set comparator A
    OCR1B = timeOutB;        // Set comparator B
    GTCCR &= 0x03;           // Start timer after setup
    doneA = false;
    doneB = false;
    DDRB |= 0x06;            // Set OC1A & OC1B as outputs

 // wait for both pulses to end, then exit
   while  (!(doneA & doneB));
}

void setup() {                
//  Serial.begin( 9600);
    sei();                   // enable global interrupts
 }

void loop() {
  pulse_D9_D10();              // generate pulses on D9 and D10 (one pulse each)
  delay(1000);                  // wait, then repeat (this is for tests only)
}

Any help would be greatly appreciated.