OCR1A problem

Hi all !

My program works and compiles correctly. I use an Uno card, the IDE rev 1.6.10.

I define the TIMER1 registers values in the setup.
The ISR(TIMER1_COMPA_vect) routine works correctly.
It seems the compiler sets the OCR1A to 0 (ZERO) rather than the value I want ! The interrupt toggles a pin at 122 HZ, but a need an other frequency !

// Initialisation des timers pour LECTURE DES TOUCHES =================== 122 Hz sur (A0)

TCCR1A = 0;
TCCR1B = 0;

// Timer1 en mode CTC (pas de PWM) - ports de sortie déconnectés
TCCR1A = (0<<COM1A1)|(1<<COM1A0)|(0<<COM1B1)|(0<<COM1B0);

// Timer 1 en Mode 4: CTC top = OCR1A
TCCR1B = (0<<WGM13)|(1<<WGM12); TCCR1A = (0<<WGM11)|(0<<WGM10);

// Pas de noise canceler, trigger pas utilisé
TCCR1B = (0<<ICNC1)|(0<<ICES1); // pas utilisé
TCCR1B = (0<<CS12)|(0<<CS11)|(1<<CS10); // prescaler = 1

TCCR1C = 0; // pas de comparaison forcee
TCCR1C = (0<<FOC1A);

TCNT1H = 0xFF; // valeur initiale du compteur
TCNT1L = 0xFF;

OCR1A = 0x0050;

TIMSK1 = 0x00;
TIMSK1 = (1<<OCIE1A);
TIFR1 = (1<<OCF1A);

// . . . .

How do you know that OCR1A is being set to 0?

Post the whole sketch - that code should definitely end with OCR1A being set to the value you gave it; the problem is elsewhere in your code (assuming you've verified that OCR1A is set to 0 - and it's not some other aspect of timer behavior that is not what you expect).

Also, why are you using CTC mode with an ISR to toggle a pin? You can do that with the PWM modes and then you don't need the ISR...

With prescale set to 1 and TOP set to 0 the interrupt would be triggering at 8 MHz!

Sorry !

The Serial.print (OCRA1) outputs the defined value: 0x0050, but my oscillo says the output frequency is 122Hz (= 244/2Hz is the frequency displayed, due to the toggle of the ISR).

This means the OCRA1 value seems to be "internally" = 0xFFFF !

The program is very long (about 29 kbytes !, carefull to the bootloader . . . ), in 8 tabs.

The only important parts before the previous code is the following lines : the other lines are variable definitions. I use the Ucg Library to control an ILI9341 TFT screen, the DS1302 RTC library, and 3 keys that I control.
I will try with the OCR1A at the very end of the Timer1 parameters. Thanks for your help, and I say you nextly how it runs !

#include <DS1302.h>
#include "Ucglib.h"
#include <SPI.h> // for the Ucglib use

// Hardware definitions : components pins mapping
// garder les ports 0 et 1 pour Rx et Tx
// keys are on the ports 5, 6, and 7, normally open
// Init the DS1302
// DS1302 CE IO SCLK
DS1302 rtc(2, 3, 4);

// Init the TFT screen sclk data dc cs reset
Ucglib_ILI9341_18x240x320_SWSPI ucg( 13, 12, 9, 10, 8);

I just read over that code again (in the first post). It closely resembles code that might work, but it averages more than one problem per line.

You're not configuring the timer the way you think you are.

You're assigning values to TCCR1A and TCCR1B in several steps, and it looks like you think you're doing &= or |= - but you have straight up assignment.

You also need to review your bitwise operators. Whenever you're doing 0<<(anything), you're doing something wrong.

Oh, and you clear the interrupt flags by writing 1 to them, not 0, as stated in the relevant section of the datasheet.

Did you know that when you use the assignment (=) operator, it overwrites the entire register, not just the bits that you specify?

EDIT: GAH!!! I've been ninja'ed!

Hi croiset. I'm fairly sure that your problem is in the following three lines. Specifically, in the unusual way that 16 bit registers are written with this timer.

   TCNT1H = 0xFF;
   TCNT1L = 0xFF; 
   
   OCR1A  = 0x0050;

I suspect that the line "OCR1A = 0x0050" is only doing an 8 bit write. This is by design. To ensure correct synchronization the 16 bit registers have to be written in a single cycle, and this is just not possible with a one instruction 16 bit write.

To get around this what happens is that you write the high byte first, but it doesn't actually get written to the target register (not yet anyway). It is instead just held internally in a temporary register until you do the subsequent low byte write, at which time the entire 16 bits is written in one cycle.

That internal temporary register is shared between all of the timers 16 bit registers!
So when you previously did "TCNT1H = 0xFF" it set the high byte temporary register to 0xFF. The subsequent write to OCR1A (which is really equiv to a write to OCR1AL, because the register cannot be accessed as a word) then wrote 0xFF50 to this compare register.

So your operating frequency should be 16MHz / (2* 0xFF51), about 122.4 Hz.

You need to change it to this:

   TCNT1H = 0xFF;
   TCNT1L = 0xFF; 
   
   OCR1AH = 0x00;
   OCR1AL = 0x50;

Stuart0, I think you're missing the forest through the trees (like I did the first time reading it). - look at what he does to the TCCR1A/1B there!

TCCR1A ends up as 0, TCCR1B ends up with only CS10 set. That is not what he intends - mode is now 0 (normal) mode with top as 0xFFFF!

DrAzzy:
Stuart0, I think you're missing the forest through the trees (like I did the first time reading it). - look at what he does to the TCCR1A/1B there!

TCCR1A ends up as 0, TCCR1B ends up with only CS10 set. That is not what he intends - mode is now 0 (normal) mode with top as 0xFFFF!

Ok I'm not seeing it.

Here's the code for TCCR1A for example.

  TCCR1A = 0;
  TCCR1A = (0<<COM1A1)|(1<<COM1A0)|(0<<COM1B1)|(0<<COM1B0);

It's a bit goofy, the "TCCR1A=0" part is redundant unless he actually meant to use |= instead of = for the other part. But in the end it still assigns TCCR1A to 0x40 which is compatible with what he wants. :slight_smile:

DrAzzy:
Stuart0, I think you're missing the forest through the trees (like I did the first time reading it).

Arh yes, I see it now. You're right. There is another assignment to TCCR1A after that (hidden in a two instruction per line part of the code) that clears TCCR1A.

Looks like croiset needs to fix all "=" that should be "|=" problems first. :slight_smile:

Fixed !

I changed the TCCR1A and TCCR1B lines in :

TCCR1A = (0<<COM1A1)|(0<<COM1A0)|(0<<COM1B1)|(0<<COM1B0)|(0<<WGM11)|(0<<WGM10);

TCCR1B = (0<<ICNC1)|(0<<ICES1)|(0<<WGM13)|(1<<WGM12)|(0<<CS12)|(0<<CS11)|(1<<CS10);

It seems all the bits of the registers must be written in 1 instruction, else the previous written values are erased/changed.

Thanks to all.

I will nextly post my sketch (. . . when complete . . .) on the Elektor webpage : a 3 displays clock on a TFT screen. The kind of display changes each hour : 7 segments, hour and minute hands or numbers (defined by myself) , and 1 screen for the hour/date/clock settings.

I use an ISR routine for 2 functions : 1) the 3 keys reading, and 2) for the beep/sound generation. It's the reason why I don't use the integrated PWM circuit.

Thanks to all

It seems all the bits of the registers must be written in 1 instruction, else the previous written values are erased/changed.

If you're using normal assignment (=), yes.

If you use And-assignment (&=) you can selectively clear bits, and Or-assign (|=) you can selectively set bits.

1 Like