Sequencing timing using 1 timer and interrupts

I have a project where I want to accurately time several things in sequence using Timer1 or Timer3... and it all gets started when an interrupt enabled pin gets a rising edge.

Now, I have it basically working, but I can't seem to get it to change the values for each step.. I've tried many things now, here's my current code

#include "TimerThree\TimerThree.h"

#define TriggerPin 2 //interrupt enabled pin
#define OutputPin 13 //LED pin

#define SignalPin 4


uint16_t OnDelay = 500;
uint16_t OnTime = 50000;

unsigned long starttime = 0;

void setup()
{
    Serial.begin(921600);
    pinMode(TriggerPin, INPUT);
    pinMode(OutputPin, OUTPUT);
    pinMode(SignalPin, OUTPUT);

    attachInterrupt(digitalPinToInterrupt(TriggerPin), TriggerISR, RISING);
    Timer3.initialize();

    Serial.println("Setup finished");
}

void loop()
{   

starttime = micros(); //record the start time
digitalWrite(SignalPin, HIGH); //blip the signal to fire the interrupt enabled pin from
digitalWrite(SignalPin, LOW);  //signal pin is looped to TriggerPin
Serial.println("Fire  ");


delay(2000);

}
void TriggerISR() {
    Serial.println("  Trigger ISR  " + String(micros()-starttime));
    Timer3.stop();
    Timer3.detachInterrupt();
    Timer3.attachInterrupt(T1ISR);
    Timer3.setPeriod(OnDelay);
    Timer3.restart(); 
}

void T1ISR() {
    Serial.println("  T1 ISR  " + String(micros()-starttime));
    Timer3.stop();
    Timer3.detachInterrupt();
    Timer3.attachInterrupt(T2ISR);
    Timer3.setPeriod(OnTime);
    digitalWrite(OutputPin, HIGH);
    Timer3.start();
}

void T2ISR() {
    Timer3.stop();
    Timer3.detachInterrupt();
    Serial.println("  T2 ISR  " + String(micros() - starttime));
    digitalWrite(OutputPin, LOW);
}

No matter what I try to do, I just get ~300uS between steps... What am I doing wrong?

While I haven't used TimerThree.h I am familiar with interrupts During an interrupt Weird thing happen with variables especially when the are changed outside the interrupt and the value is expected to be global inside. (I'm sure there is a better person to explain this in detail) what I have discovered is that the volatile Keyword is necessary to keep the variable up to date each time it is used. With that said try this simple change:

volatile unsigned long starttime = 0;

Did it work? Z

void TriggerISR() {
    Serial.println("  Trigger ISR  " + String(micros()-starttime));
    Timer3.stop();
    Timer3.detachInterrupt();
    Timer3.attachInterrupt(T1ISR);
    Timer3.setPeriod(OnDelay);
    Timer3.restart(); 
}

Bad things can happen when you try to use interrupt driven services like Serial inside an ISR. It is one of those tough bugs where the code appears to work occasionally and then occasionally locks up. So the Serial.print line needs to be taken out of the ISR.

The volatile keyword is needed for values that change inside an ISR. Without it the compiler is free to optimize it out of other code thinking that it won't change when in fact the ISR will change it. The compiler just sees that it doesn't change in loop and doesn't change in anything called from loop so it just throws the variable away and puts in the initial value. It won't notice that this ISR can change the value unless you tell it that the variable is volatile.

Grrr.. lost my post

Normally I wouldn't have serial printing happening in the ISR.. I don't see how that would cause it not work though.. I could see it causing inaccuracy, but I'm just unable to set the period at all.

I've tried with volatile globals, but (correct me if I'm wrong), that's usually for when you're modifying the variable within the ISR, I'm only reading it at this point.. nevertheless, volatile or not, nothing changes

Here's the serial output

Fire  
  Trigger ISR  12
  T1 ISR  344
  T2 ISR  636
Fire  
  Trigger ISR  16
  T1 ISR  304
  T2 ISR  600
Fire  
  Trigger ISR  12
  T1 ISR  304
  T2 ISR  604
Fire  
  Trigger ISR  12
  T1 ISR  312
  T2 ISR  604

no matter what I set things to, the time from the trigger happening to T1 is 300uS, and same for T1-T2..

I have tried using literals instead of globals for troubleshooting, it doesn't matter if I use 5uS or 5000uS, the timing stays the same.. I've tried calling "initialize(time)", I've tried calling restart vs start, I've detached one interrupt before adding the other.. nothing is changing anything, I've tried using TimerOne vs TimerThree, both act the same.

I must be doing something wrong, and I do usually like figuring it out myself, but this is just getting too frustrating!

I fiddled more… It seems that the Timer.start and Timer.restart are counterintuitive, or buggy, I can’t say…

If I call Timer.stop, then do some changes to the period, then call .start or .restart, it doesn’t work
If I just change the period or change the ISR, it works. Also, in the last one I call .stop, I do NOT need to call .start to get it going again, it seems as though either changing the ISR handler or setting the period restarts it anyhow…

Very counter intuitive, but I seem to have things under control now… I still have some hair left

Figure I might as well post the working code here

#include "TimerThree\TimerThree.h"

#define TriggerPin 2 //interrupt enabled pin
#define OutputPin 13 //LED pin

#define SignalPin 4


uint16_t OnDelay = 10000; //10ms
uint16_t OnTime = 50000;  //50ms


unsigned long starttime = 0;

void setup()
{
    Serial.begin(921600);
    pinMode(TriggerPin, INPUT);
    pinMode(OutputPin, OUTPUT);
    pinMode(SignalPin, OUTPUT);

    attachInterrupt(digitalPinToInterrupt(TriggerPin), TriggerISR, FALLING);
    Timer3.initialize(0);
    Timer3.attachInterrupt(T1ISR);

    Serial.println("Setup finished");
}

void loop()
{   
      Serial.println("Fire");
    digitalWrite(SignalPin, HIGH);
    starttime = micros();
    digitalWrite(SignalPin, LOW);

    delay(4000);


}
void TriggerISR() {
    Serial.println(" TriggerISR " + String(micros()- starttime));
    Timer3.setPeriod(OnDelay);
    Timer3.attachInterrupt(T1ISR);
    //apparently I don't need to call Timer3.start() or restart(), that's breaks things
}

void T1ISR() {
    Serial.println("  T1ISR " + String(micros() - starttime));
    digitalWrite(OutputPin, HIGH);    
    Timer3.setPeriod(OnTime);
    Timer3.attachInterrupt(T2ISR);
}


void T2ISR() {
    Serial.println("   T2ISR " + String(micros() - starttime));
    digitalWrite(OutputPin, LOW);
    Timer3.stop();
}

Here's the output

Fire
 TriggerISR 16
  T1ISR 9936
   T2ISR 59936
Fire
 TriggerISR 12
  T1ISR 9936
   T2ISR 59936
Fire
 TriggerISR 12
  T1ISR 9932
   T2ISR 59936
Fire
 TriggerISR 16
  T1ISR 9936
   T2ISR 59936
Fire
 TriggerISR 16
  T1ISR 9936
   T2ISR 59936

It's pretty darned close, good enough for what I need to do, and considering there's serial data getting transferred, that might screw with the timing a little.

Rx7man: Normally I wouldn't have serial printing happening in the ISR.. I don't see how that would cause it not work though..

Allow me to explain what can happen. Serial going out is interrupt driven. When you call Serial.print is loads the characters into the serial buffer and then an interrupt sends them out one at a time. But if the serial buffer happens to be full when you call print, then it has to wait until there is room in the buffer, so it blocks for a short time while it sends out enough characters to make room for what you printed.

BUT, in an interrupt routine those other interrupts are temporarily disabled. So you call print and it starts to block and wait for this interrupt to clear out the buffer but the interrupt is disabled so you're waiting on something that isn't going to happen. So you are waiting forever, stuck in a blocking call. The code is locked up at the ISR.

Delta_G: Allow me to explain what can happen. Serial going out is interrupt driven. When you call Serial.print is loads the characters into the serial buffer and then an interrupt sends them out one at a time. But if the serial buffer happens to be full when you call print, then it has to wait until there is room in the buffer, so it blocks for a short time while it sends out enough characters to make room for what you printed.

BUT, in an interrupt routine those other interrupts are temporarily disabled. So you call print and it starts to block and wait for this interrupt to clear out the buffer but the interrupt is disabled so you're waiting on something that isn't going to happen. So you are waiting forever, stuck in a blocking call. The code is locked up at the ISR.

So why does the second version I posted work fine, with serial calls in the ISR as well.. The problem is for some reason when I call Timer.start().

I do agree that if nothing else it would be risky behavior to rely on serial comms within an ISR, this is just for debugging

Rx7man: So why does the second version I posted work fine, with serial calls in the ISR as well.. The problem is for some reason when I call Timer.start().

That's the thing with that bug, it works great until that one time the ISR gets called with the Serial buffer almost full and then the whole thing locks up.

things don't lock up though.. the timer just runs at 300us instead of what I set it at.. regardless of if my setting was higher or lower.. I'm going to eliminate the serial write and blink an LED and see if it still does it when I use the start method

Confirmed.. eliminating the serial print from the ISR's does not fix the problem if start() is used..

Rx7man: Confirmed.. eliminating the serial print from the ISR's does not fix the problem if start() is used..

I never said it would. Perhaps you misunderstood. I was trying to point out a bug that was going to bite you later. I guess that's not appreciated. Good luck with your code.

Delta_G: I never said it would. Perhaps you misunderstood. I was trying to point out a bug that was going to bite you later. I guess that's not appreciated. Good luck with your code.

I did.. Sorry about that. I did want to make sure of it. I do fully realize that serial within an ISR is asking for trouble, I just really didn't think that was causing my particular flavour of trouble I was having, and had to be sure... If I get courageous I'll try using the registers directly instead of relying on the library... for another day though