HW: Arduino Micro. Nothing else. Serial output used.
Well..... Trying to practice with timer (Timer1) interrupts I found a strange behavior.
Code is attached.
-in setup, Timer1 is set at 1 kHz
-the servicing ISR is quite simple: just increment the volatile var "timer1"
-led on pin13 is lit shortly to mark every second passed
It looked good
BUT
suprisingly, sometimes I noticed that the led tuned on irregularly
So after trying to put some code to debug it,
yes, I saw that sometimes the loop ends after 762 ms (??)
after checking if(timer>TIMERMAX) // 1000 pulses passed
I added an extra check (now between /* */)
Later I was able to have it running correctly only after an extra test
Any ideas? Did I run into a compiler undesired optimization
related to checking the "timer" volatile variable?
// defines ------------------------------------------------------------
#define TIMERMAX 1000 // 1 kHz (1000 cycles actually)
#define LEDLIT 20000
#define ledPin 13
// vars ----------------------------------------------------------
volatile int timer; // the only var affected by the ISR
int ledtimer;
unsigned long prevval;
// code ----------------------------------------------------------
ISR (TIMER1_COMPA_vect)
{
timer++;
}
void setup()
{
pinMode(ledPin,OUTPUT); // led pin
cli(); //stop all interrupts
// Version with Timer 1
// turn on CTC mode
TCCR1A = 0;// set entire TCCR1A register to 0
TCCR1B = 0;// same for TCCR1B
TCCR1B |= (1 << WGM12);
// Set CS11 bit for prescaler of 256
TCCR1B |= (1 << CS12);
// initialize counter value to 0;
TCNT1 = 0;
// set timer count for 1 kHz
OCR1A = 61; // = (16*10^6) / (256*1000) - 1 (1000=1kHz)
// enable timer compare interrupt
TIMSK1 |= (1 << OCIE1A);
sei(); // allow interrupts
Serial.begin(9600);
}
void loop()
{
if(timer>TIMERMAX) // 1000 pulses passed
{
// uncomment the section below to see how the previously good test (timer>TIMERMAX) fails just after (sometimes "timer < 800 " is true)
/* if(timer<800)
Serial.write("????????????????\n");
else*/
{
timer=0;
digitalWrite(ledPin,1); // turn the led on each second (h.beat)
// digitalWrite(ledPin,state); // turn the led on each second (h.beat)
ledtimer=LEDLIT;
}
}
// if active, decrease led timer counter
if(ledtimer)
{
ledtimer--;
if(!ledtimer)
{
char ctext[64];
unsigned long mss;
digitalWrite(ledPin,0);
mss=millis();
sprintf(ctext,"%4ld\n",mss-prevval);
Serial.write(ctext);
prevval=mss;
}
}
}
The sketch writes on the serial console the number of ms passed after each set of 1000 interrupts.
That volatile variable deserves protected access (interrupts disabled) outside the ISR. The compiler is not clever enough to protect the variable for you. See also: atomic access.
What DrDiettrich is getting at is that the Arduino you are using is an 8 bit device and an int is 16 bits, so multiple 8 bit operations are required. When doing the 16 bit compare the value may be changing as you are doing the operation. There are a variety of ways to solve this. Probably the easiest is to make a local copy with interrupts disabled and then use the local copy.
That was so helpful.
Thanks to both! Worked immediately with an ATOMIC_BLOCK(ATOMIC_RESTORESTATE).
I am now checking how this macro expands, i.e. if the solution ATOMIC_BLOCK or noInterrupts - Interrupts is more appropriate.
Both work fine. ATOMIC_BLOCK is simple on the inside, go ahead and look.
I like ATOMIC_BLOCK even if the extra step it takes is unnecessary - rather than turn off, then later turn on interrupts, it turns them off, then restores whatever they were (on or off) prior to the
BLOCK.
In some circumstances that bit of extra care wou,d be polite if not critical.
How are you sure that TC1 (Timer/Counter 1) is running at 1 kHz?
You have set CTC Mode-4 operation, OCR1 = 61, and division factor = 256. The frequency of the wave is:
f = 16 MHz/(2xNx(1+OCR1A)) // N = division factor
==> f = 16000000/(2x256x((1+61))
==> f ~= 504 Hz
The compare match flag will become active at the elapse of every 3.968 ms.
Fix the frequency first and then learn the method of accessing critical section based on post #2@DrDiettrich.
Start with a simple case of operating TC1 in Normal Mode with N = 1024; where, overflow event will occur after each 4.194304 sec (Fig-1). Load suitable value into OCR1A register so that "Compare Match" occurs at 1 sec interval. This kind of setup, from my point of view, will make your experiment more interesting and enjoyable.
Hi,
thanks for your reply.
Actually the sketch worked 100% after the hints I got about the atomic block.
About the formula, I found in the literature that:
OCR1A=16E6 / (256*1000) - 1 being 1000 = 1 kHz and 256 the prescaler.
That gave me 61 (61.5 actually) and it worked fine giving me 1 kHz.
Although I was just experimenting with the timers+interrupt+ISR techniques, rather than with the frequency, it looked indeed good, as isolating f gives:
f = 16E6 / ((OCR1A+1) * Prescaler)
which gives 1008 (considering that we lost 0.5 truncating 61.5 to 61.
In the formula you wrote there is a 2 factor that I can't find in the literature.
