Problem disabling timer interrupts during ISR call

Hello all, I have a slight issue. The project I'm working on uses an Arduino Uno to interface a data logging and processing program in Python with two sensors and a valve for an experiment I'm working on.

The problem I'm having is figuring out how to turn on the valve for a certain amount of time. Ideally, the python code sends a message over serial with the duration for the valve to be opened in milliseconds. Here is the program flow that I am trying to accomplish:

  1. timer1 is initialized in setup(), although it is not turned on and the number of clock cycles is not set.

  2. processPiData() is called from loop() when there is new serial data available

  3. processPiData() pulls out the number of ms sent from python and calls valveOpenTest(openTime) with the int value of milliseconds as the argument

  4. valveOpenTest() opens the valve, does the necessary math to set OCR1A with the proper number of clock cycles, and turns on the CTC interrupt (now set to the time specified by the python code)

  5. ISR(TIMER1_COMPA_vect) is called when the CTC returns a match. It closes the valve, and turns off the CTC interrupt (otherwise the valve would keep opening and closing every openTime seconds until valveOpenTest is called again).

The next time python wants to turn on the valve, it sends the duration again and the process repeats.


So, to the problem. To test, I'm just flashing the builtin led in the same place where I would be actuating the valve. As it stands, I can accomplish all of the above except turn off the interrupt at the end of the ISR. If I don't turn off the timer interrupt, the led keeps flashing with the period specified by openTime, but as soon as I add the line to disable further interrupts the led doesn't even flash anymore.

I'll stop talking and show the relevant code:

void setup() {

	pinMode(LED_PIN, OUTPUT);			// open LED for visual feedback
	Serial.begin(115200);					// initialize serial data stream
	
	// Configure interrupts
	cli();

	//... other interrupts here...

	// Setup Timer 1
	TCCR1A = 0;								// set settings register A to 0
	TCCR1B = 0;								// set settings register B to 0
	TCCR1B |= (1 << WGM12);					// turn on CTC mode (allows for variable overflow time)
	// Set 1024 prescaler					
	TCCR1B |= (1 << CS10);
	TCCR1B |= (1 << CS12);	
 	// TIMSK1 |= (1 << OCIE1A);  // would turn on timer interrupt, but leaving that disabled
	sei();									// enable global interrupts

}
void loop() {
	// ... irrelevant stuff...

        // Check if there is an incoming stream and if so, process it
	if (Serial.available()) {

		// Serial.print("data available\n");
		processPiData();

	}

}
boolean processPiData( void ) {

\\... heavily excerpted because there is much confusing stuff going on

 \\ checks if string in array matches keyword. if so, the next value in the char array is openTime 
\\ and thus we call the valve function with that argumentassigns it as an int
  if ((strcmp(strptr[numWords], "VAL") == 0)) {

	long openTime = strtol(token, &ptr, 10);
	if (openTime) {
		valveOpenTest(openTime);
		\\Serial.print("got ms\n");
	}

}
void valveOpenTest(int ms) {

	Serial.print("valveOpen\n");
	digitalWriteFast(LED_PIN, !digitalReadFast(LED_PIN));  // cycle LED
	cli();
	ms = ms / 1000;                    // convert ms to s
	OCR1A = ms * 15625 - 1;        // set the number of clock cycles
	TIMSK1 |= (1 << OCIE1A);       // turn on CTC interrupts
	sei();

}

And finally,

ISR(TIMER1_COMPA_vect) {

	digitalWriteFast(LED_PIN, !digitalReadFast(LED_PIN));
	TIMSK1 &= ~(1 << OCIE1A);             // turn off CTC interrupts

}

So, it almost works perfectly. If I comment out that last line, the led starts to flash after the initial message from python, and continues to flash at intervals of 2000ms (that's what I sent from Python) for the rest of the time.

Of course, I want it to flash just once (after the message was sent from python). Ideally, that last line would accomplish this, as it would turn off the timer until I called it again, but once I add that line the led doesn't flash at all.

It's weird, because due to strategically placed print statements, I can see that the program hits valveOpenTest() every time it's called from processPiData(), and when the last line is commented I can see that the ISR gets called every 2000 ms after each timer overflow. However, when I add that line, valveOpenTest() and the ISR only get called once for each new message python sends, which is exactly what should happen. Yet, the led doesn't flash.

Also interesting is the order in which things are called. Without the interrupt turned off (last line is commented), valveOpen() gets called, flashes the led, finishes, and goes back to processPiData(). Then 2000ms later, I see that the ISR was called. This is how it should be; of course the ISR is still called every 2000ms and that is undesirable. However, with the last line included, valveOpen() finishes, the ISR gets called, and then the program returns back to processPiData(). This happens every time a new message is sent from python, and that is of course not the order in which things are supposed to flow (the ISR should always get called 2000ms after valveOpen()).

I've tried this with digitalWrite() as well, but that doesn't work either.

So, does anyone have any helpful hints/suggestions/solutions for this issue? And yes, I'm fully aware that I could do some millis() interval checking, but if I wanted to do that I would have saved the time typing this post and finished my project by now. I'll use that method if there is no other fix.

Thanks in advance.

Er, why turn on and off the interrupts detection rather than the timer itself? As I read it, the timer is still running, so next time you turn the detection on it would fire.

I would be stopping running the timer, and turning it back on (so it starts from zero*) and then turn the timer off when you are done with it.

  • You probably need to write zero to the counter actually.

My page about timers:

You may also want to read up on:

Bit 1 – OCF1A: Timer/Counter1, Output Compare A Match Flag

Writing a one to that may help you, as it would reset the match (so it has to match a second time, not remember the last match).

void valveOpenTest(int ms) {

You realize this will only let you time up to 32 seconds? Maybe make that an unsigned long.

	ms = ms / 1000;                    // convert ms to s
OCR1A = ms * 15625 - 1;        // set the number of clock cycles

Massive loss of precision here? (ms will be 0, 1, 2 or 3 after the division) And won't this only work for ms being less than 3000? After that OCR1A will overflow. Can't you rework this to allow for (say) 1500 mS? Or if you only ever want to have the valve open for 1 or 2 seconds exactly, a simple switch statement might be easier.

Why do you need a timer for this? Open the valve. Record the time using millis(). Periodically, like every pass through loop(), see if it's time to close the valve. If it is, close it.

@PaulS beat me to it ....

Simple solutions are usually best.

...R

Or see blink without delay, use micros not mills if you require greater accuracy.

Mark

Massive loss of precision here?

This was simply a test mechanism to input time (I was having trouble because at first I didn't realize you can only write 16-bit numbers to OCR1A. I wanted to use a 64 prescaler so I wouldn't get floats after division, but this didn't work because any useful amount of time has a much larger value than can be supported by the 16-bit register).

You realize this will only let you time up to 32 seconds? Maybe make that an unsigned long.

Yeah I realize, but the valve will only ever be opened for less than a second (max).

I would be stopping running the timer, and turning it back on (so it starts from zero*) and then turn the timer off when you are done with it.

I tried resetting the counter in valveOpenTest() and turning off CTC in the ISR, but it still didn't work. This is an interesting problem as timers aren't really supposed to be used like this, but I have a deadline to meet and I should probably just do this another way. Thank you for the help and suggestions.

srofrano:
I should probably just do this another way

as has been suggested.

...R