Timer1_CompA interrupt handler fires too early

I’m trying to make a PID based water bath with a configurable temperature. I want a simple display and two buttons, one for up, one for down. When a button is pressed, i’d like it to alter the temperature by 1 degree, when it is pressed and held, i’d like it to alter the temperature by 10 degrees.

I’ve distilled as much out of the code as I can to keep things simple. I have a single button attached to pin 2 of my arduino uno board, which is debounced with a 10K resistor a 10uF cap and a schmitt trigger. I’m setting off Timer1 when the button is pressed, then disabling it when the button is lifted again.

Here is my code:

#include <avr/io.h>
#include <avr/interrupt.h>

int ledPin = 13;
int sensePin = 2;

volatile int value = 0;

void startTimerOne() {
   cli();
   TCCR1A = 0;
   TCCR1B = 0;
   

   OCR1A = 6000; // run to roughly half a second in 1024 mode
   
   TCCR1B |= (1 << WGM12); // turn on CTC mode
   TCCR1B |= (1 << CS10) | (1 << CS12); // enable 1024 prescaler 
   TCNT1 = 0; // set counter to zero
   
   TIMSK1 |= (1 << OCIE1A); // enable timer1 compare interrupt  
   sei(); //enable global interrupts   
}

void stopTimerOne() {
  cli();
  TIMSK1 = (0 << OCIE1A); //disable timer1 compare interrupt
  sei();
  }

void buttonEvent() {
  value = digitalRead(sensePin);
  if (value == HIGH) { // button pressed
    startTimerOne();
    Serial.println("Button Down");
  } else { // button released
    stopTimerOne();
    Serial.println("Button Up");
  }
}

ISR(TIMER1_COMPA_vect) {
  Serial.println("Button Held");
}

void setup() {
   Serial.begin(9600);
   Serial.println("Initializing Interrupt Handler");
   pinMode(ledPin, OUTPUT);
   pinMode(sensePin, INPUT);
   attachInterrupt(0, buttonEvent, CHANGE);
}

void loop() {
}

If i press, hold, and then release I get this output:

Initializing Interrupt Handler
Button Held
Button Down
Button Held
Button Held
Button Held
Button Up

and if I press and release without holding i get:

Button Held
Button Down
Button Up

I’m really confused by the “button held” bit right at the start of both of these outputs. It looks like the compare interrupt is being thrown when the timer is initialised. I can easily sort this out by testing to see if its the first time the interrupt is thrown and ignoring the run, but that seems messy, and I’d really like to understand what is going on first. Can someone help please?

Hi,
Try clearing the timer capture interrupt flag before or after this line -

TIMSK1 |= (1 << OCIE1A); // enable timer1 compare interrupt

It might be that the flag is already set and so you immediately trigger an interrupt as soon as you enable interrupts (the flag indicates that an interrupt should be fired) - also not that some register bits are cleared by writting a 1 to them - very confusing - check the data sheet.

Duane B

rcarduino.blogspot.com

http://www.gammon.com.au/interrupts

As DuaneB says, interrupts can be remembered if they happened to occur before you enable them.

ISR(TIMER1_COMPA_vect) {
  Serial.println("Button Held");
}

Preferably don't do serial prints inside an ISR.

I'm trying to make a PID based water bath with a configurable temperature.

Involving how much water? What kind of heater? How fast does the heater cause a change in temperature of the water? How are you measuring that temperature? How well insulated is the vessel that contains the water?

PID may be complete overkill if the volume of water is large, as in a hot tub.

I'm mystified why you need timers and interrupts to measure how long a switch is pressed. If it is a human pressing the switch, the time, as far as the Arduino is concerned, could be measured in eons. No need for interrupts to measure things that happen eons apart.

DuaneB:
Try clearing the timer capture interrupt flag before or after this line -

TIMSK1 |= (1 << OCIE1A); // enable timer1 compare interrupt

Absolutely bang on the money. I added the lines:

       TIFR1 = OCF1A; // clear timer1 compare interrupt
       TIFR1 = OCF1B; //

before the line you quoted and it all works perfectly now. Thanks! Interestingly just clearing the A channel interrupt was insufficient.

Yeah - this was just for debugging. The “long” execution time of the ISR wont matter here as there won’t be any other interrupts and it will be much shorter than the half second compA fire. I do absolutely agree though - there will be no serial stuff in the ISRs in the production code.

PaulS:
Involving how much water? What kind of heater? How fast does the heater cause a change in temperature of the water? How are you measuring that temperature? How well insulated is the vessel that contains the water?

PID may be complete overkill if the volume of water is large, as in a hot tub.

I’m mystified why you need timers and interrupts to measure how long a switch is pressed. If it is a human pressing the switch, the time, as far as the Arduino is concerned, could be measured in eons. No need for interrupts to measure things that happen eons apart.

Its a 7L laboratory water bath using a combination of a 1Kw kettle heating element and a 150W aquarium heater. I’m using a PRT in a three lead configuration with an external SPI ADC I can measure to within a twentieth of a degree or so. I’m looking for a tenth of a degree stability, so I need to improve on that a bit, but this is good enough for testing. I’m achieving about a fifth of a degree stability at the moment (measuring with a laboratory thermometer).

What would you suggest other than interrupts? More than happy to think about doing it another way, but you just say “don’t do interrupts” - what would you do instead?

The "long" execution time of the ISR wont matter here as there won't be any other interrupts

How can you say this? You can't know that there won't be serial input, that the clock won't tick, that the chip won't overheat, that the battery won't go dead, etc.

Since serial output relies on interrupts happening, it seems silly to even try serial output when they are disabled, as in an ISR.

Its a 7L laboratory water bath using a combination of a 1Kw kettle heating element and a 150W aquarium heater.

This assumes that we know what a 7L laboratory water bath is. Seven liters?

What would you suggest other than interrupts?

That you answer all the questions asked. If it is a human pressing the switch, then simply detect when the transition occurs (from pressed to released or from released to pressed). Each time a transition occurs, record the time, using millis() or micros(), depending on the interval you are trying to measure. Each time the transition is to released, you can determine how long the switch was held down, and react accordingly.

You can also, on each pass through loop, determine if the switch is still pressed, and, if so, how long it was since the last time you did something. You can then determine if it is time to do something again, and, if so, adjust the "last time we did something" value.

7L is 7 liters.

It is a human pressing the switch.

I also realized that I didn't say what the container was insulated with. Its a modified cooler box - I've no idea what's in the lag, but with the lid on it looses about half a degree in 10 minutes.

If I just measure the time the switch has been down on transition, then it won't increment by 10 degrees more than once. The user would have to do repeated "long" button presses - using my method they just press and hold and it will increment by 10 degrees every (roughly) half second. You're right - I could stick a loop in and poll the switch, but that feels really messy. I'm not using the timer for anything else at the moment, so it may as well get use d for this. Its working great now :-)

rvodden: Interestingly just clearing the A channel interrupt was insufficient.

Both channels share the same interrupt. Once the timer is started both channels count away.

I don't particularly mind the interrupt for the switch, but debouncing could be done just by looking at elapsed time. Waiting for 10 mS to elapse after the last bounce should be sufficient. In other words, note the time of the switch press. If you get another press before 10 mS, reset the time noted to the new time. Repeat until more than 10 mS has passed. Consider the switch debounced.

Oh sorry. ThIs isn't about debouncing, that's all done in hardware. This is about detecting the difference between a momentary press and a press-and-hold. I did say in my first post that I was debouncing with an RC and a Schmitt trigger...That's why I'm waiting half a second with the timer, not 10ms.

OK, but still all the ISR has to do is note the time the interrupt occurred. Then the main loop can see if the button is still down after x mS.

I could stick a loop in and poll the switch, but that feels really messy.

Yeah, perhaps. For an app that basically does nothing else it isn't too bad.