Timer1 is not creating PWM outputs

Hi,
I'm using a Mega2560 and was wanting to program the PWM functions. I know there are lots of libraries already for tones and PWM, but it seems as difficult to learn to use/debug them as it does to learn to program the PWM myself. So, I set about trying to make it work.

My goal is to use Timer1 in the phase and frequency correct mode. I'll eventually want to vary frequency and pulse width so things like updating OCR1A and OCR1B at the BOTTOM as well as being able to select the inverting output appealed to me. Here is my code:

int led1A = 11;                 // The pin that the LED is attached to for Timer1A
int led1B = 34;                 // OC1B and OC1C disconnected in phase and freq correct mode
volatile int led1A_Value;
volatile int led1B_Value;
unsigned int TopCount=62500;    // With 256 prescale sets freq to 0.5 Hz
unsigned int PWMCount=31250;    // With OCR1B = half TopCount duty cycle should be 50%

void setup(){ 
  Serial.begin(115200);   
  Serial.println("Start Setup");    
  noInterrupts ();
  TCCR1A = 0;                    // Clear Timer control registers
  TCCR1B = 0;                    // Clear Timer control registers
  TCCR1C = 0;                    // Clear Timer control registers
  TCNT1 = 0;                     // Set counter to zero
  TIFR1 = 0;                     // Clear Flags
  TIMSK1 = 0;                    // Clear Interrupt mask
  TIMSK1 = bit(OCIE1B); // Enable Output Compare Interrupt for Timer1B in interrupt mask
  TCCR1A = bit(COM1A1) | bit(COM1A0) | bit(COM1B1) | bit(COM1B0) | bit(WGM10);  //  Setting 
                                 // COMnx1:0=3 uses the inverting output
                                 // Per datasheet page 151 this looks more like the waveform I want.
                                 // Setting WGM11:0=1 toggles OC1A on match 
  TCCR1B = bit(WGM13) | bit(CS12);   // Set WGM13 to get the MSB of phase and freq correct mode set
                                 // Also set Clock Select Bit SC12 to set pre-scaler to 256
  interrupts();
  pinMode(led1A, OUTPUT);
  pinMode(led1B, OUTPUT);
  OCR1A = TopCount;              // f = 16,000,000 / (2 * 256 * OCR1A) = 0.5 Hz when OCR1A = 62500
  OCR1B = PWMCount;              // f = 1.0 when OCR1B = 31250  I.E. 50% duty cycle
  Serial.println("End Setup");
}

ISR (TIMER1_COMPB_vector) {
  led1A_Value = digitalRead(led1A);      // Capture current state of led1A
  if (led1A_Value == 0) led1B_Value = 1; // If OCR1B=TCNT1 while led1A_Value=0 - counting up - set led1B_Value
  else led1B_Value =0;                   // Otherwise OCR1B=TCNT1 while led1A_Value=1 - counting down - clear led1B_Value
}
void loop(){
  digitalWrite(led1B, led1B_Value); // Output displays what should be OC1B
  delay(10);
  Serial.print("TCNT = ");
  Serial.print(TCNT1);
  Serial.print("  led1A_Value = ");
  Serial.print(led1A_Value);
  Serial.print("  led1B_Value = ");
  Serial.println(led1B_Value);
  }

From the serial output, it looks like TCNT1 counts up to my PWMCount value and then the whole thing gets a reset. It goes back to setup and starts over. Neither of the LED's light up. This sketch seems so simple - and yet it doesn't work like I think it is supposed to.

Please let me know what I'm doing wrong.
Thanks.

What error/warning messages are you getting when you try to compile? Anything to do with ISR (TIMER1_COMPB_vector)?

You may also want to review your understanding of the bit() function.

Hi,
No errors. Just that the program seems to reset. It counts up to the point that TCNT1- OCR1B and it starts over. It goes back to the beginning of setup (according to the Serial.print statements. Plus, neither of the LED's that I've got hooked up to the output ports light up.

My guess would be that Pin 34 has too high a load on it so when that output is turned on it drags the power low and causes a reset. Does the counting continue if you disconnect Pin 34?

Great suggestion. I’ve got the external power supply plugged in to the board. But, I tried it anyway. No difference. Here’s what I’m seeing (I changed the delay to 50 to limit the amount number of displayed counter cycles):

Start Setup
End Setup
TCNT = 634  led1A_Value = 0  led1B_Value = 0
TCNT = 1298  led1A_Value = 0  led1B_Value = 0
TCNT = 1965  led1A_Value = 0  led1B_Value = 0
TCNT = 2632  led1A_Value = 0  led1B_Value = 0
TCNT = 3298  led1A_Value = 0  led1B_Value = 0
TCNT = 3965  led1A_Value = 0  led1B_Value = 0
TCNT = 4632  led1A_Value = 0  led1B_Value = 0
TCNT = 5299  led1A_Value = 0  led1B_Value = 0
TCNT = 5966  led1A_Value = 0  led1B_Value = 0
TCNT = 6633  led1A_Value = 0  led1B_Value = 0
TCNT = 7300  led1A_Value = 0  led1B_Value = 0
TCNT = 7966  led1A_Value = 0  led1B_Value = 0
TCNT = 8634  led1A_Value = 0  led1B_Value = 0
TCNT = 9301  led1A_Value = 0  led1B_Value = 0
TCNT = 9967  led1A_Value = 0  led1B_Value = 0
TCNT = 10634  led1A_Value = 0  led1B_Value = 0
TCNT = 11304  led1A_Value = 0  led1B_Value = 0
TCNT = 11974  led1A_Value = 0  led1B_Value = 0
TCNT = 12644  led1A_Value = 0  led1B_Value = 0
TCNT = 13315  led1A_Value = 0  led1B_Value = 0
TCNT = 13985  led1A_Value = 0  led1B_Value = 0
TCNT = 14655  led1A_Value = 0  led1B_Value = 0
TCNT = 15325  led1A_Value = 0  led1B_Value = 0
TCNT = 15995  led1A_Value = 0  led1B_Value = 0
TCNT = 16666  led1A_Value = 0  led1B_Value = 0
TCNT = 17336  led1A_Value = 0  led1B_Value = 0
TCNT = 18006  led1A_Value = 0  led1B_Value = 0
TCNT = 18676  led1A_Value = 0  led1B_Value = 0
TCNT = 19347  led1A_Value = 0  led1B_Value = 0
TCNT = 20017  led1A_Value = 0  led1B_Value = 0
TCNT = 20687  led1A_Value = 0  led1B_Value = 0
TCNT = 21357  led1A_Value = 0  led1B_Value = 0
TCNT = 22028  led1A_Value = 0  led1B_Value = 0
TCNT = 22697  led1A_Value = 0  led1B_Value = 0
TCNT = 23368  led1A_Value = 0  led1B_Value = 0
TCNT = 24038  led1A_Value = 0  led1B_Value = 0
TCNT = 24708  led1A_Value = 0  led1B_Value = 0
TCNT = 25378  led1A_Value = 0  led1B_Value = 0
TCNT = 26048  led1A_Value = 0  led1B_Value = 0
TCNT = 26718  led1A_Value = 0  led1B_Value = 0
TCNT = 27388  led1A_Value = 0  led1B_Value = 0
TCNT = 28058  led1A_Value = 0  led1B_Value = 0
TCNT = 28729  led1A_Value = 0  led1B_Value = 0
TCNT = 29399  led1A_Value = 0  led1B_Value = 0
TCNT = 30069  led1A_Value = 0  led1B_Value = 0
TCNT = 30740  led1A_Value = 0  led1B_Value = 0
Start Setup
End Setup
TCNT = 635  led1A_Value = 0  led1B_Value = 0
TCNT = 1299  led1A_Value = 0  led1B_Value = 0
TCNT = 1966  led1A_Value = 0  led1B_Value = 0
TCNT = 2632  led1A_Value = 0  led1B_Value = 0
TCNT = 3299  led1A_Value = 0  led1B_Value = 0
TCNT = 3966  led1A_Value = 0  led1B_Value = 0

Try changing the ISR name from "ISR (TIMER1_COMPB_vector) {" to "ISR (TIMER1_COMPB_vect) {". When I compiled your code I got a warning message that the vector name appeared to be misspelled. If the name wasn't recognized and the wrong vector was initialized, that would cause the interrupt to go "off into the weeds" and crash the Arduino. That would lead to the symptom you see.

P.S.: Set you compiler warnings to "All" in Preferences to catch more programming mistakes that aren't TECHNICALLY illegal. :slight_smile:

Okay. John gave you the first one. Secondly, the bit function returns the bit number of a bit definition. So, bit(WGM13) will return 4 or something like that. Then you go on to assign this value to TCCR1B (or whatever) thinking that you are setting the WGM13 bit when in fact you are doing this → TCCR1B = 0B00001000, which is not correct. You either need to specifiy TCCR1B |= 1 << WGM13 or you can use the macro bv and write TCCR1b |= bv(WGM13).

Thanks guys! johnwasser is exactly correct. I needed to set the compiler warnings to All. Plus, changing to TIMER1_COMPB_vect cleared up the compiler warning I wasn't seeing.

DKWatson, you've given me things to think about. I initially wrote the code mimicking things I'd seen on Nick Gammon's timer and interrupt pages. I see that there have been exchanges about the relative merits of bitwise OR'ing with and without macros: bit() versus _BV() - Programming Questions - Arduino Forum

I've followed your advise and used the _BV. Lights are working. You've saved me lots of time and frustration. Thanks again.

Hi,
Sorry to pester, but this counter still has some unexpected results. I set OCR1A to 62500, and expected that OC1A would toggle when that TCNT1 value was reached. But, it doesn't do that. Here's where it switches:

Start Setup
End Setup
TCNT = 3134  led1A_Value = 0  led1B_Value = 0
TCNT = 6301  led1A_Value = 0  led1B_Value = 0
TCNT = 9468  led1A_Value = 0  led1B_Value = 0
TCNT = 12635  led1A_Value = 0  led1B_Value = 0
TCNT = 15806  led1A_Value = 0  led1B_Value = 0
TCNT = 18976  led1A_Value = 0  led1B_Value = 0
TCNT = 22146  led1A_Value = 0  led1B_Value = 0
TCNT = 25316  led1A_Value = 0  led1B_Value = 0
TCNT = 28486  led1A_Value = 0  led1B_Value = 0
TCNT = 31656  led1A_Value = 0  led1B_Value = 1
TCNT = 34827  led1A_Value = 0  led1B_Value = 1
TCNT = 37997  led1A_Value = 0  led1B_Value = 1
TCNT = 41168  led1A_Value = 0  led1B_Value = 1
TCNT = 44338  led1A_Value = 0  led1B_Value = 1
TCNT = 47508  led1A_Value = 0  led1B_Value = 1
TCNT = 50679  led1A_Value = 0  led1B_Value = 1
TCNT = 53849  led1A_Value = 0  led1B_Value = 1
TCNT = 57020  led1A_Value = 0  led1B_Value = 1
TCNT = 60190  led1A_Value = 0  led1B_Value = 1
TCNT = 61640  led1A_Value = 0  led1B_Value = 1
TCNT = 58469  led1A_Value = 0  led1B_Value = 1
TCNT = 55299  led1A_Value = 0  led1B_Value = 1
TCNT = 52129  led1A_Value = 0  led1B_Value = 1
TCNT = 48958  led1A_Value = 0  led1B_Value = 1
TCNT = 45788  led1A_Value = 0  led1B_Value = 1
TCNT = 42618  led1A_Value = 0  led1B_Value = 1
TCNT = 39447  led1A_Value = 0  led1B_Value = 1
TCNT = 36277  led1A_Value = 0  led1B_Value = 1
TCNT = 33107  led1A_Value = 0  led1B_Value = 1
TCNT = 29936  led1A_Value = 1  led1B_Value = 0
TCNT = 26765  led1A_Value = 1  led1B_Value = 0
TCNT = 23595  led1A_Value = 1  led1B_Value = 0
TCNT = 20425  led1A_Value = 1  led1B_Value = 0
TCNT = 17255  led1A_Value = 1  led1B_Value = 0

So, it looks like the counter goes up to the TOP, starts counting down, and then OC1A changes at the same point as OC1B? That is strangeness.

Time to post your revised code.

Here is the revised code. Thanks for responding so quickly.

int led1A = 11;                 // The pin that the LED is attached to for Timer1A
int led1B = 34;                 // OC1B and OC1C disconnected in phase and freq correct mode
volatile int led1A_Value;
volatile int led1B_Value;
unsigned int TopCount=62500;    // With 256 prescale sets freq to 0.5 Hz
unsigned int PWMCount=31250;    // With OCR1B = half TopCount duty cycle should be 50%

void setup(){ 
  Serial.begin(115200);   
  Serial.println("Start Setup");    
  noInterrupts ();
  TCCR1A = 0;                    // Clear Timer control registers
  TCCR1B = 0;                    // Clear Timer control registers
  TCCR1C = 0;                    // Clear Timer control registers
  TCNT1 = 0;                     // Set counter to zero
  TIFR1 = 0;                     // Clear Flags
  TIMSK1 = 0;                    // Clear Interrupt mask
  TIMSK1 |= _BV(OCIE1B);         // Enable Output Compare Interrupt for Timer1B in interrupt mask.  
  TCCR1A |= _BV(COM1A1) | _BV(COM1A0) | _BV(COM1B1) | _BV(COM1B0) | _BV(WGM10); // Setting 
                                 // COMnx1:0=3 uses the inverting output
                                 // Per datasheet page 151 this looks more like the waveform I want.
                                 // Setting WGM11:0=1 toggles OC1A on match 
  TCCR1B |= _BV(WGM13) | _BV(CS12); // Set WGM13 to get the MSB of phase and freq correct mode set
                                 // Also set Clock Select Bit SC12 to set pre-scaler to 256
  interrupts();
  pinMode(led1A, OUTPUT);
  pinMode(led1B, OUTPUT);
  OCR1A = TopCount;              // f = 16,000,000 / (2 * 256 * OCR1A) = 0.5 Hz when OCR1A = 62500
  OCR1B = PWMCount;              // f = 1.0 when OCR1B = 31250  I.E. 50% duty cycle
  Serial.println("End Setup");
}

ISR (TIMER1_COMPB_vect) {
  led1A_Value = digitalRead(led1A);      // Capture current state of led1A
  if (led1A_Value == 0) led1B_Value = 1; // If OCR1B=TCNT1 while led1A_Value=0 - counting up - set led1B_Value
  else led1B_Value =0;                   // Otherwise OCR1B=TCNT1 while led1A_Value=1 - counting down - clear led1B_Value
}

void loop(){
  digitalWrite(led1B, led1B_Value); // Output displays what should be OC1B
  delay(50);
  Serial.print("TCNT = ");
  Serial.print(TCNT1);
  Serial.print("  led1A_Value = ");
  Serial.print(led1A_Value);
  Serial.print("  led1B_Value = ");
  Serial.println(led1B_Value);
}

Okay. Explain again what the problem is. I've got an LED hooked up to pin11 (OC1A) and it's flashing at 0.5Hz.

And here's a scan
scan.jpg

scan.jpg

Note that your interrupt is compare match with OCR1B. Funny thing about computers - they do exactly what you tell them to do, not necessarily what you want them to do.