change PWM duty cycle from within ISR

Question

I have little problems implementing a dither function to upgrade the PWM resolution of my ATmega88 and the leds it's controlling. My thought was, to use a "kind of" floating number, consisting of two uint8_t as "decimal places" so that e.g. "255.31" would be represented by "0xff.0x1f" (BYTE_HIGH.BYTE_LOW).

Now I want the value in the OCR to dither between BYTE_HIGH and BYTE_HIGH+1 and for this I need a second counter (besides the timer itself) to generate an overlayed dutycycle (the duty cycle of the timer has to be incremented after (int) 255-BYTE_LOW cycles). But managing this stuff in the main() results in flicker and so I'd like to do this in an ISR called on timer overflow.

My avr is running at 20 MHz and to get a sufficient dither frequency I cannot use a prescaler higher than 8. Do you think, I might be calling the ISR too frequently? But there is not too much code in the ISR, though.

Another problem I could think of is, that the ISR has to write the OCR which might be read by the timer at the same time, but isn't this kind of "thread save" as the timer does never write to OCR?

I have not found anything concerning my problem appropriately and so I hope to get some hints. See my code below

#include <avr/interrupt.h>
#include <avr/io.h>

#define compare_register OCR2B  //duty cycle

volatile uint8_t dither_count=0;
volatile uint8_t BYTE_HIGH=0;
volatile uint8_t BYTE_LOW=0;

void init_pwm(void){

//some standard pwm initializations
TCCR2A |= (1 << COM2A1);
TCCR2A |= (1 << COM2B1);
// non inverting, fast PWM mode and prescaler to
TCCR2A |= (0 << WGM21) | (1 << WGM20);
TCCR2B |= (1 << CS21);
// set desired pin as an output (not sure, if it's the correct pin, because I left out other pins in use)
DDRD = (1 << DDD3)|(1<<DDD4);

// enable interrupt on timer overflow
TIMSK2 |= (1<<TOIE2);

// enable global interrupts
  sei();
}

ISR(TIMER2_OVF_vect){
  dither_count++;
  if(dither_count<=BYTE_LOW){
    compare_register=BYTE_HIGH+1;
  }else{
    compare_register=BYTE_HIGH;
  }
}

int main(){
init_pwm();
//this or any function dimming the leds
  BYTE_HIGH=10;
  BYTE_LOW=1;
}

This theoretically should work, but unfortunately it doesn't work in real. The ISR gets called, but does not act as desired. Do I call the variables correctly inside of the ISR? I would be glad if someone would see a fundamental misstake I did, especially because I do not have any serial interface working for debugging...

Edit: meanwhile I found out, that obiously the ISR gets stuck and will block any code executed in my main function...

Answer 1

I tried your program on an Arduino Uno. This board is based on a ATmega328P which, save for some extra memory, is basically the same as your Mega88 (same datasheet). I first added the following lines at the end of main():

init_pwm();

// Blink the LED on PB5.
DDRB |= _BV(PB5);
for (;;) {
    PINB |= _BV(PB5);
    _delay_ms(400);
}

The first line is obviously needed for the PWM to work. The LED blinking is just a test to see whether the program gets stuck in the ISR or not. The result is: it does not. The LED blinks just as expected. The PWM output looks also OK on the scope. Notice that, contrary to what the comment in the code says, you are configuring the timer in phase correct PWM mode, not in fast PWM. The timer period is then 2×255 timer clock cycles instead of 256.

I disassembled the binary and tried to count the number of CPU cycles needed to service the interrupt. With the program compiled at the -Os optimization level, I got about 50. Given that the interrupt fires every 2×255×8 = 4080 cycles, that's about 1.2% of the CPU power spent servicing the interrupt.