Spot the dumb mistake

Using Arduino Programming Questions
1

Looking into the idea of having a 16 bit PWM using timer1 and I knocked up the below test code but it just keeps resetting the UNO and I don't know why (and hope someone can tell me why).

Ignore the commented out stuff I used to test different ideas.

#define ledPin 13
#define PB5 5

volatile byte Compare = 0;
volatile byte Overflow = 0;
byte OverflowOld;

void setup(){
  Serial.begin(115200);
  Serial.println("Setup...");
  
  pinMode(ledPin, OUTPUT);
  
  cli();                                // Disable all interrupts
  TCCR1A = 0;                           // Clear all flags in control register A
  TCCR1B = 0;                           // Clear all flags in control register B
  //TCCR1C = 0;                           // Clear all flags in control register C
  TCNT1 = 0;                            // Zero timer 1 count
  
  OCR1A = 32768;                        // Preload compare match register (50% duty cycle)
  
  // prescaler   
  TCCR1B |= (1 << CS12);    
  //TCCR1B |= (1 << CS11);    
  TCCR1B |= (1 << CS10);    
  
  TIMSK1 |= (1 << OCIE1A);              // Enable timer compare interrupt
  TIMSK1 |= (1 << TOIE1);               // Enable timer overflow interrupt
  sei();                                // enable all interrupts
}

void loop(){
  /*   if (Overflow != OverflowOld){
    Serial.print("Overflow = ");
    Serial.println(Overflow);
    Serial.print("Compare = ");
    Serial.println(Compare);
    Serial.println(OverflowOld);
    OverflowOld = Overflow;
    }
  */
}

ISR(TIMER1_OVF_vect){                   // Timer1 overflow interrupt service routine
  digitalWrite(ledPin,HIGH);
  //PORTB |= _BV(PB5);                    // Turn LED on
  //Overflow++;
}

ISR(Timer1_COMPA_vect){                 // Timer1 compare interrupt service routine
  digitalWrite(ledPin,LOW);
  //PORTB &= ~_BV(PB5);                   // Turn LED off
  //Compare++;
}
2

This may be stupid but does the capitalisation of "Timer1_COMPA_Vect" matter? I have "TIMER1_" rather than "Timer1_".

If it did matter then you would have an unhandled interrupt which could cause no end of problems, but I would expect that it wouldn't compile.

Ugi

3

Here is how I did it:

void setup() {
  Serial.begin(9600);
  PWM16Begin(true, false);  // Enable PWM on UNO Pin 9 and not UNO Pin 10.
}

void PWM16Begin(bool EnableA, bool EnableB) {
  // Stop Timer/Counter1
  TCCR1A = 0;  // Timer/Counter1 Control Register A
  TCCR1B = 0;  // Timer/Counter1 Control Register B
  TIMSK1 = 0;   // Timer/Counter1 Interrupt Mask Register
  TIFR1 = 0;   // Timer/Counter1 Interrupt Flag Register

  // Set 'TOP' for PWM resolution.  Assumes 16 MHz clock.
  ICR1 = 0xFFFF;  // 16-bit resolution. 244 Hz PWM
  // ICR1 = 0x7FFF; // 15-bit resolution.  488 Hz PWM
  // ICR1 = 0x3FFF; // 14-bit resolution.  976 Hz PWM
  // ICR1 = 0x1FFF; // 13-bit resolution.  1953 Hz PWM
  // ICR1 = 0x0FFF; // 12-bit resolution.  3906 Hz PWM
  // ICR1 = 0x07FF; // 11-bit resolution.  7812 Hz PWM

  // Set clock prescale to 1 for maximum PWM frequency
  TCCR1B |= (1 << CS10);

  // Set to Timer/Counter1 to Waveform Generation Mode 14: Fast PWM with TOP set by ICR1
  TCCR1A |= (1 << WGM11);
  TCCR1B |= (1 << WGM13) | (1 << WGM12) ;

  if (EnableA)
    PWM16EnableA();

  if (EnableB)
    PWM16EnableB();
}

void PWM16EnableA() {
  // Enable Fast PWM on Pin 9: Set OC1A at BOTTOM and clear OC1A on OCR1A compare
  TCCR1A |= (1 << COM1A1);
  OCR1A = ICR1 / 2;  // Default to 50% PWM
  pinMode(9, OUTPUT);
}

void PWM16EnableB() {
  // Enable Fast PWM on Pin 10: Set OC1B at BOTTOM and clear OC1B on OCR1B compare
  TCCR1A |= (1 << COM1B1);
  OCR1B = ICR1 / 2;  // Default to 50% PWM
  pinMode(10, OUTPUT);
}

inline void PWM16A(unsigned int PWMValue) {
  PWMValue = constrain(PWMValue, 0, ICR1);
  OCR1A = PWMValue;
}

inline void PWM16B(unsigned int PWMValue) {
  PWMValue = constrain(PWMValue, 0, ICR1);
  OCR1B = PWMValue;
}


void loop() {
  const float increment = 1.01;
  
  unsigned long time = millis();

  for (float pwm = 1; (long)pwm < 65536; pwm *= increment) {
    PWM16A((long)pwm);
    delay(269);
  }
  
  for (float pwm = 65535; (long)pwm > 0; pwm /= increment) {
    PWM16A((long)pwm);
    delay(269);
  }
  Serial.println(millis() - time);
}
4

Dr_Ugi:
This may be stupid but does the capitalisation of "Timer1_COMPA_Vect" matter? I have "TIMER1_" rather than "Timer1_".

If it did matter then you would have an unhandled interrupt which could cause no end of problems, but I would expect that it wouldn't compile.

Spot on Ugi, changed to upper case (how did I miss that) and now works as expected. Strange it compiled without error though?

Finished example without all the crap I had used to figure out what was going wrong. Fades built in LED on UNO.

// 16 bit PWM on any pin
// Example uses built in LED on pin 13 (PORTB bit 5)

void setup(){
  pinMode(13, OUTPUT);
  
  cli();                                // Disable all interrupts
  TCCR1A = 0;                           // Clear all flags in control register A
  TCCR1B = 0;                           // Clear all flags in control register B
  TCNT1 = 0;                            // Zero timer 1 count
  
  OCR1A = 32768;                        // Preload compare match register (50% duty cycle)
  
   // No prescaler   
  //TCCR1B |= _BV(CS12);    
  //TCCR1B |= _BV(CS11);    
  TCCR1B |= _BV(CS10);    
  
  TIMSK1 |= _BV(OCIE1A);                // Enable timer compare interrupt
  TIMSK1 |= _BV(TOIE1);                 // Enable timer overflow interrupt

  sei();                                // enable all interrupts
}

void loop(){
  for(unsigned int x = 1; x < 65535; x++){
    OCR1A = x;
    delayMicroseconds(50);
  }
}

ISR(TIMER1_OVF_vect){                   // Timer1 overflow interrupt service routine
  PORTB |= _BV(PORTB5);                 // Turn LED (pin 13) on
}

ISR(TIMER1_COMPA_vect){                 // Timer1 compare interrupt service routine
  PORTB &= ~_BV(PORTB5);                // Turn LED off
}
5

Yay! :smiley:

One of these days someone will invent a compiler that can work out what we mean rather than what we say. Unfortunately, that will probably be the day before the SkyNet nuclear apocalypse!

It's close to impossible to see your own mistakes - if I had a penny for every "=" that should have been "=="...