PWM, Changing OCRA1 is not changing the Puls width

Go To Last Post
12 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

I am trying to make a variable pulse with on a pulse period of +-65ms (15Hz)

I use PWM on Timer1 in ATMEGA16.

PWM-Mode 5, Fast Pwm 8-Bit, Top = 0x00FF

Atmega16 is running on 4Mhz internal clock. The Timer1 prescallers are set to clk/1024. Timer1 will increment every 4k clock cycles.

When Top value of 255 is reached i will have the period of +-65ms.

It is the intention to change OCR1A register between 0-255 to have a variable pulse width.

The problem is that the changements of OCR1A are not reflected in the Pulse width!!!!
The pulse width stays the same, corresponding to the value filled in OCR1A during initialization.

I have the TOIE1, Timer-Overflow interrupt enabled.
I update OCR1A in this interrupt routine, with interrupts disabled.

Datasheet extract

Quote:

The OCR1A Register however, is double buffered.
/* This feature allows the OCR1A I/O location to be written anytime.
/* When the OCR1A I/O location is written the value written will be put into the OCR1A
/* Buffer Register. The OCR1A Compare Register will then be updated with the value in
/* the Buffer Register at the next timer clock cycle the TCNT1 matches TOP. The update is
/* done at the same timer clock cycle as the TCNT1 is cleared and the TOV1 Flag is set */

This i don't understand.

OCR1A is double buffered. In my code i update OCR1A in the Timer Overflow interrupt routine. Should i update another register?

Thanks for help,
Fabrizio

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

no, you are updating the right register. What they are describing is what happens behind the scenes with that register.

Show us your timer setup, and ISR code. Also how are you verifying that the duty is not changing?

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Here is my complete code.

OCR1A is updated with the Analog To Digital converted value of an potentiometer connnected to AD0.
I use only the Lower byte of the converted value for testing at this moment.

Timer0 has nothing to do with the PWM mechanism. He is generating an 50ms interrupt. I use this to create internal timers. At this moment only an heartbeat led is toggled every 50ms. Then i know the processor is alive.

How is see the Pulse width is not changing? I have a scoop hooked on PD5.
I should see a clear difference in pulse width when OCR1A is changing from 00->FFH

When i place a breakpoint in the ADC_Interrupt_Handler, i can see the Digital value of the potentiometer input changing.

Thanks for looking into this.

/* Ventilatorsturing  ATmega16 project */
/* Version      : V01
/* Latest Update: 15-Aug(08)-2009 */
/*
/* Ventilator sturing with the ATmega16 Processor. */

/* The processor is running on the internal 4Mhz oscilator */

#include 
#include "Ventilator.h" 

/* External Data declarations. */



/* Input and output sensor definition */
/**************************************/
/* PB0  Out   Heardbeat_Led              */
/* PD5  Out   OC1A     Output compare registor of the PWM steered by Timer_1            */


/* Input Sensor Names and Number */

/* Output Sensor Names and Number */


/* Definition Interrupt handlers */ 
/*********************************/
#pragma interrupt_handler Timer0_Comp_Interrupt_Handler: iv_TIMER0_COMP
#pragma interrupt_handler ADC_Interrupt_Handler: iv_ADC 
#pragma interrupt_handler PWM_Timer1_Overflow: iv_TIMER1_OVF

/* Definition Global Data */
/**************************/
unsigned int Z_PWM_Value;
   
  		   
/* Procedures*/
/*===========*/	  
void Initialisation(void)
  {
  /* Init Timer0, Interrupt every 50ms
  /* Timer0 is used to drive the System clock. */
  TCCR0 = (1<<WGM01) | (5<<CS00);                                 
  OCR0 = 0xC3;                       
  TIMSK = TIMSK | (1<<OCIE0);

  /* Timer1, will be used has For PWM Timer */
  TCCR1A = (1<<COM1A1) | (1<<COM1A0) | (1<<WGM10);
  OCR1A = 0x0050;
  TCCR1B = (1<<WGM12) | (5<<CS10);                 
  TIMSK = TIMSK | (1<<TOIE1);                     
  
  /* A/D convertor */
  ADMUX = (1<<REFS0);                                         
  ADMUX |= ((0<<MUX4) | (0<<MUX3) | (0<<MUX2) | (0<<MUX1));    
  ADCSRA = (1<<ADIE) | (1<<ADATE);                             
  ADCSRA |= ((1<<ADPS0) | (1<<ADPS2)); 
  
  PORTB = 0xFE;  
  DDRB |= 0x01;             
  /* PB0  Out   Heardbeat_Led              */
  
  PORTD = 0xDF;  
  DDRD |= 0x20;            
  /* PD5  Out   OC1A     Output compare registor of the PWM steered by Timer_1            */
  
  SREG |= 0x80;             /* Set global interrupt flag */
  }

  

/* Interrupt handlers */
/* ================== */
void Timer0_Comp_Interrupt_Handler(void)
  {
  SREG |= 0x80;        /* Set global interrupt flag */
  Toggle_Hearbeat_Led;
  }   /* void Timer0_Comp_Interrupt_Handler(void) */


  
void ADC_Interrupt_Handler(void)
  {
  unsigned int L_ADC_Result, L_ADC_Low, L_Test; 
  
  /* The A/D convertor completed the conversion. */
  SREG |= 0x80;
  
  /* Disable the A/D onvertor */
  /* ADCSRA = (0<<ADEN); */
  L_ADC_Result = ADC;
  L_ADC_Low = ADCL;
  Z_PWM_Value = L_ADC_Low;
  L_Test = 0x00;
  }   /* void ADC_Interrupt_Handler(void) */
    


void PWM_Timer1_Overflow(void)
  {
  /*  SREG |= 0x80;*/
  OCR1A = Z_PWM_Value;
  }   /* void PWM_Timer1_Overflow(void) */
  
  
  
/* MAIN Program Entry */
/**********************/  
void main()
  {
  Initialisation();
  
  /* Enable the A/D onvertor */
  ADCSRA |= ((1<<ADEN) | (1<<ADSC));
  
  while (1)
    {
    
  
  
  
  
    }   /* while (1) */
  }   /* void main() */
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
  L_ADC_Result = ADC; 
  L_ADC_Low = ADCL; 

That looks a bit suspicious to me. You read ADC which is a composite combination of ADCH and ADCL then you read ADCL again? What's the thinking there?

Not sure about this compiler but in some Z_PWM_Value would need to be 'volatile'.

If you only want to use 8 bits of the ADC conversion then why not use ADLAR and just read ADCH?

Cliff

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

do NOT play with SREG in your ISR's. Interrupts will automatically be re-enabled on exit from the ISR. Enabling interrupts by setting the global interrupt bit in SREG can cause you all sorts of troubles if you don't know what you are doing.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
  L_ADC_Low = ADCL; 
  Z_PWM_Value = L_ADC_Low; 

If you only want to use 8 bits of the ADC result, then you want the upper 8 bits, not the lower 8 bits. Set the ADLAR bit and just read ADCH. Also, I'm not sure why you thought that you needed an intermediate local variable, though it will likely be optimized out anyways. And I would just set OCR1A in the ADC interrupt. There is really no reason for doing it in the timer overflow ISR.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

glitch wrote:
do NOT play with SREG in your ISR's. Interrupts will automatically be re-enabled on exit from the ISR. Enabling interrupts by setting the global interrupt bit in SREG can cause you all sorts of troubles if you don't know what you are doing.

I switch on the interrupts has first statement of an interrupt procedure to allow nesteled interrupts.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

  L_ADC_Result = ADC; 
  L_ADC_Low = ADCL; 

That looks a bit suspicious to me. You read ADC which is a composite combination of ADCH and ADCL then you read ADCL again? What's the thinking there?

The thinking behind this is that i find it easyser when stepping true the code with the debugger to see what is happening. When it works has it should i will simplify the code.

clawson wrote:

Not sure about this compiler but in some Z_PWM_Value would need to be 'volatile'.

Z_PWM_Value is defined global. All procedures can use this variable.

clawson wrote:

If you only want to use 8 bits of the ADC conversion then why not use ADLAR and just read ADCH?

I will try this.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

fabrizio wrote:
glitch wrote:
do NOT play with SREG in your ISR's. Interrupts will automatically be re-enabled on exit from the ISR. Enabling interrupts by setting the global interrupt bit in SREG can cause you all sorts of troubles if you don't know what you are doing.

I switch on the interrupts has first statement of an interrupt procedure to allow nesteled interrupts.

I understand what it is you are doing, what I'm telling you is that it's a bad idea. Besides your interrupts are short enough that you don't need nesting.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

fabrizio wrote:

clawson wrote:

Not sure about this compiler but in some Z_PWM_Value would need to be 'volatile'.

Z_PWM_Value is defined global. All procedures can use this variable.

it is not that simple. "volatile" adds a special attribute that prevents the compiler from optimizing away access to the variable. This is VERY important when the global is shared between an ISR and the main-line code.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

glitch wrote:
fabrizio wrote:
glitch wrote:
do NOT play with SREG in your ISR's. Interrupts will automatically be re-enabled on exit from the ISR. Enabling interrupts by setting the global interrupt bit in SREG can cause you all sorts of troubles if you don't know what you are doing.

I switch on the interrupts has first statement of an interrupt procedure to allow nesteled interrupts.

I understand what it is you are doing, what I'm telling you is that it's a bad idea. Besides your interrupts are short enough that you don't need nesting.

Ok, thanks for remark.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It is working when i set ADLAR=1 and use
ADCH to update OCR1A in the ADC complete
interrupt procedure.
I thinks i understand. We loose two bits of resolution and ADCH will go from 00->FF

Thanks for all reply's and advice.

Best Regards,
Fabrizio