Interrupts and timers

zopffa · May 25, 2018, 7:48am

Hi Guys,

I am trying to make myself familiar with timers and interrupts. For this purpose I created this piece of code which doesn`t behave as expected sometimes. The ISR function blink() turns the timer on and off. The ISR of the timer callback() turns a LED ON and OFF. So in the end I can control if the LED blinks by a push button. In most of the cases this works fine. But sometimes the LED stays solid ON without blinking, which is as far as I understand this code not possible. Maybe some of you guys can explain this strange behaviour to me?

Greetings from Austria
leo

#include "TimerOne.h"

volatile bool isBlinking = false;

void setup()
{
  //***LED***
  pinMode(13, OUTPUT);

  //***Timer***
  Timer1.initialize(100000);
  Timer1.stop();
  Timer1.attachInterrupt(callback);

  //***Interrupt***
  pinMode(2, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(2), blink, FALLING);
}

void callback()
{
  digitalWrite(13, !digitalRead(13));
}

void blink()
{
  cli();
  if (isBlinking == false) {
    Timer1.start();
    isBlinking = true;
  }
  else { //true
    isBlinking = false;
    Timer1.stop();
    digitalWrite(13, LOW);
  }
  sei();
}

void loop()
{
}

septillion · May 25, 2018, 8:18am

I'm not sure (you have to dive into TimerOne library) but I'm getting the feeling .stop() might not remove the timer interrupt flag. So if the interrupt fires before interrupts are disabled it is queued and executed after interrupts resume. Or even (but not 100% sure), if a specific interrupt is off but global interrupts on, it only means the ISR isn't called, not that the flag isn't set. Aka, the the flag is still set when an interrupt fires only no code is run. But once you turn on the interrupt and the flag is still set it will run the code.

But there are more then one library called TimerOne so which do you use?

Also, quick note, when entering a ISR, interrupts are disabled by default. No need to do it yourself

septillion · May 25, 2018, 8:26am

To see if it fixes it, change to

Timer1.start();
TIFR1 = 0; //clear all Timer 1 Interrupt Flags

zopffa · May 25, 2018, 9:00am

Thanks for the fast answer. Your explanation sounds understandable to me. The callback() function fires one more time and turns on the LED forever.

I tried your fix with the flag register but it didn`t change the situation at all. I also included your statement after Timer.stop() but nothing changed.

#include "TimerOne.h"

volatile bool isBlinking = false;

void setup()
{
  //***LED***
  pinMode(13, OUTPUT);

  //***Timer***
  Timer1.initialize(100000);
  Timer1.stop();
  Timer1.attachInterrupt(callback);

  //***Interrupt***
  pinMode(2, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(2), blink, FALLING);
}

void callback()
{
  digitalWrite(13, !digitalRead(13));
}

void blink()
{
  cli();
  if (isBlinking == false) {
    Timer1.start();
    TIFR1 = 0; //clear all Timer 1 Interrupt Flags
    isBlinking = true;
  }
  else { //true
    isBlinking = false;
    Timer1.stop();
    TIFR1 = 0; //clear all Timer 1 Interrupt Flags
    digitalWrite(13, LOW);
  }
  sei();
}

void loop()
{
}

To answer your first question: I use this lib

zopffa · May 25, 2018, 10:05am

Just changed the code as below:

Timer1.stop();
TIFR1 |= (1 << TOV1); //clear all Timer 1 Interrupt Flags

Got this from the data sheet:

• Bit 0 – TOV1: Timer/Counter1, Overflow Flag
The setting of this flag is dependent of the WGM13:0 bits setting. In Normal and CTC modes, the TOV1 Flag is
set when the timer overflows. Refer to Table 16-4 on page 132 for the TOV1 Flag behavior when using another
WGM13:0 bit setting.
TOV1 is automatically cleared when the Timer/Counter1 Overflow Interrupt Vector is executed. Alternatively,
TOV1 can be cleared by writing a logic one to its bit location.

It seems to me this solved the problem.

septillion · May 25, 2018, 10:52am

Ahh, yeah. Missed the "write 1 for clear" part Not messing with it daily.

zopffa · May 25, 2018, 11:21am

No problem, thanks for putting me into the right direction anyway.

As far as I am reading many people suggest to use a timer an poll the button state within its ISR.
Unfortunately I can`t find some arduino related examples for this method. Maybe someone who already used this method can post a code snipet of it here please?

septillion · May 25, 2018, 11:27am

A IRS for a button usually is overkill. Unless your program is very busy or time critical, polling is much easier.

Same actually for simply blinking a led, timer is a bi overkill. Using millis is much simpler.

zopffa · May 25, 2018, 11:30am

Okay, I got it but if my loop lasts for a very long time there is no alternative to external interrupts, or polling inside the ISR of a timer?

system · May 25, 2018, 11:36am

but if my loop lasts for a very long time

There is no reason, generally, that that should be the case.

septillion · May 25, 2018, 12:14pm

If you're loop is slow you probably have a bigger problem. Aka, don't make the loop (to) slow.

zopffa · May 26, 2018, 8:25am

Finally found a solution that polls in the ISR of Timer2, wchich fires in a ~20ms rate. Seems to work fine and the loop stays free for other stuff.

volatile int pin_LED = 13;
volatile int pin_button = 2;

volatile boolean oldSwitchState = HIGH;
volatile boolean newSwitchState1 = HIGH;
volatile boolean newSwitchState2 = HIGH;
volatile boolean newSwitchState3 = HIGH;

volatile boolean LEDstatus = LOW;

void setup()
{
  //***Timer2***
  pinMode(13, OUTPUT);
  TCCR2A = 0;
  TCCR2B = 0;
  TCCR2A |= (1<<WGM21);                         //CTC Mode                                          (Timer zählt bis auf OCR2A und wird dann automatisch 0 gesetzt)
  TIMSK2 |= (1<<OCIE2A);                        //enable OCR1A Interrupt
  TCCR2B |= (1<<CS22) | (1<<CS21) | (1<<CS20);  //prescaler 1024                                    (Alle 1024 CPU Takte zählt der Timer einen Tick weiter)
  OCR2A = 200;                                  // OCR1A = f_clock / (prescaler * f_target) - 1     (Wert bis zu dem gezählt wird)
  
  //***LED***
  pinMode(pin_LED, OUTPUT);
  digitalWrite(pin_LED, LOW);

  //***Button***
  pinMode(pin_button, INPUT_PULLUP);
}

ISR(TIMER2_COMPA_vect)
{
  newSwitchState1 = digitalRead(pin_button);
  delay(1);
  newSwitchState2 = digitalRead(pin_button);
  delay(1);
  newSwitchState3 = digitalRead(pin_button);

  if (newSwitchState1 == newSwitchState2 && newSwitchState1 == newSwitchState3) {
    if ( newSwitchState1 != oldSwitchState ) { //hatt sich der zustand des schalter geändert?
      if ( newSwitchState1 == LOW ) {          //ist der schalter gedrückt (soll nur beim drücken nicht beim auslassen reagieren
        if ( LEDstatus == LOW ) {
          digitalWrite(pin_LED, HIGH);
          LEDstatus = HIGH;
        }
        else                    {
          digitalWrite(pin_LED, LOW);
          LEDstatus = LOW;
        }
      }
      oldSwitchState = newSwitchState1;
    }
  }
}

void loop()
{
}

Whandall · May 26, 2018, 8:43am

TIFR1 |= (1 << TOV1); //clear all Timer 1 Interrupt Flags

does not write a 1 to the TOV1 only, it will clear any other set flags in that register that happen to be set, I would use:

TIFR1 = (1 << TOV1);

zopffa · May 26, 2018, 11:17am

Are you sure? With my method I write 1 to the TOV1 Bit and leave the others untouched. With yours I overwrite all other Bits with zeros.

Or am I missing something?

Whandall · May 26, 2018, 11:51am

zopffa:
Or am I missing something?

Yes, the properties of a "write one to clear" register.

Do you expect the other bits in the register behave differently than the one you are writing?

By getting the current content of the flags register and writing it back you will clear all those bits
that happen to be set.

Don't you think that you should only reset the bit you use?

zopffa · May 26, 2018, 5:36pm

Now I get it!

When I use

TIFR1 |= (1 << TOV1)

and let's say another Bit in TIFR1 is 1, so for example TIFR1 is 0b00000011 (TOV1 is Bit0). Then I create (1 << TOV1) which is the same as 0b00000001.

Now I use the or operator on both Bytes:

0b00000011 TIFR1
|0b00000001 (1 << TOV1)
=0b00000011

so in fact I write Bit0 and Bit1 value = 1 to the register.

Is that what you mean?

Whandall · May 26, 2018, 5:52pm

zopffa:
so in fact I write Bit0 and Bit1 value = 1 to the register.

Is that what you mean?

Yes.

It may not hurt in this case, because all the bits belong to the same timer,
but it is inefficient (one read and the or are superfluous) and incorrect.

There are some other similar behaving bits, for example the PINx registers,
where writing a 1 to a bit toggles the output state, where it is more critical to get it right.

septillion · May 28, 2018, 8:32am

Using a timer for 20ms timing is borderline. I really hope your loop is faster than 20ms

Topic		Replies	Views
timerone - single execution Programming	11	2527	May 5, 2021
TimerOne interrupt not firing Programming	6	3431	May 5, 2021
Having Trouble with Timer Interrupts Programming	22	6141	May 5, 2021
Problem disabling timer interrupts during ISR call Programming	9	2736	May 5, 2021
Event-based timer interrupts Programming	15	4146	May 5, 2021

Interrupts and timers

Related topics