# 20Khz 50% duty cycle PWM generation Problem in atmega 328p

I wanted to create a PWM of 20Khz with 50 percent duty cycle(can be varied within while loop) in timer 0 of atmega 328p with MOODE 7:fast PWM mode with top value as OCROA.I used OCROA register to make 20khz and OCR0B to create 50 percent duty cycle(can be varied) by updating duty cycle in every compare match of OCR0B with TCNT0 register value.Datasheet says that output will be found at OC0B pin(11th pin).I compile the code without any error and sucessfully burn it to 328p.But led is not glowing at all.I am attaching may code:

• Could you clarify why it is that you're expecting to see a blinking LED? You can't see 20kHz, and you're never changing the duty-cycle value - so assuming your code is correct, all it'll do is output a 20kHz square wave with a constant duty-cycle. – brhans Nov 20 '17 at 18:15
• I can change the duty_cycle in While loop.For 1st checking I have taken 50% duty cycle.I know it is not possible to see the blinking but led will glow right?but it doesnt glow at all. – Zarzisur Nov 20 '17 at 18:22
• So fix your question so that it describes your actual problem. – brhans Nov 20 '17 at 18:26
• i have fixed the question – Zarzisur Nov 20 '17 at 18:39
• I have found out the problem.I had to initiate COM0B1 bit in TCCROA register but i did it in TTCR0B register. – Zarzisur Nov 20 '17 at 20:06

## Main problem

You are setting the Compare Match Output pin in the wrong register. Replace

TCCR0A |= (1<<WGM01) | (1<<WGM00);
TCCR0B |= (1<<COM0B1) | (1<<WGM02) | (1<<CS01);


with

TCCR0A = (1<<COM0B1) | (1<<WGM01) | (1<<WGM00);
TCCR0B = (1<<WGM02) | (1<<CS01);


Of course, this assumes you have a 16MHz CPU frequency with no clock divide as you are using a prescaler of 8 with a TOP value of 100.

F_OCRO = F_CPU / (Prescaler * (1 + OCR0A)) = 16MHz / (8 * 100) = 20kHz


## Other problems

This might be working, but it's not the best idea. There's lots I could go over, but let's just talk about this

double duty_cyle = 50;
OCR0B = (duty_cycle * 99) / 100;


AVR is 8-bit. Double is a 64-bit number with decimal places. Actually, (in GCC, at least), doubles are automatically replaced with Floats (single-precision). AVR doesn't have a floating-point unit (FPU) and any floating-point operations (which have 32-bit storage) will take an order of magnitude MORE clock cycles to complete than a typical 8-bit operation. Division is also not supported in AVR hardware, so the compiler emulates it in software. You are also doing all of this math in an interrupt service routine. That's ... not good.

OCR0B is an 8-bit register (max of 255, unsigned). Since you are using OCR0A as the timer TOP with a value of 99, you have a nice 0 - 99% duty cycle setup. So, just set OCR0B to whatever duty cycle you want. Why do (50 / 99 * 100) which equals 49.5 and will be chopped to 49 anyway? Just do this:

static volatile uint8_t duty_cycle = 50;   // unsigned 8-bit integer
OCR0B = duty_cycle;


The "static" keyword means this variable is only available in the current file - it's like a local global. You should also use it on any local functions that aren't accessed outside of this .c file. The "volatile" keywords tells the compiler not to optimize the variable because it could be updated somewhere not expected (like in an ISR). You don't need volatile if you aren't changing the duty_cycle variable in an ISR. Better yet, why use a global variable at all when you can access the register directly?

OCR0B = 50;


If you don't like that, use a macro or static inline function:

#define SET_DUTY(PERCENT)     OCR0B = (PERCENT)
// ...
SET_DUTY(50);
// OR
static inline void set_duty(const uint8_t percent) {
OCR0B = percent;
}
// ...
set_duty(50);
`

Also, unless you plan on re-initializing the timer at some point while keeping other changes you've made to those registers, you don't need to do "|=" which is a "read-modify-write" operation. Just use "=". The "|=" means to set only the specified bit while leaving everything else alone, which is pointless during initialization when you want everything not specifically set to be 0.