IntervalTimer::update and begin documentation improvement

petesoper

Member
The current doc says "To immediately begin a new interval, without completion of the current interval as scheduled, use begin()." I suggest it say one more thing after this sentence: something like "If the same object involved with an interval in progress needs to override it the end function must be called first".

Also the begin function doc says "False is returned if all hardware resources are busy, used by other IntervalTimer objects." This is misleading (for the T4/4.1 at least: I don't know about earlier chips). The source code clearly shows that the test for resources in use is object-agnostic. Object X that calls begin and then calls begin again without having called the end function will get the same error return as object Y if all the timers are in use. I suggest, if this is portable across all the Teensy implementations, simply deleting ", used by other IntervalTimer objects".
 
The current doc says "To immediately begin a new interval, without completion of the current interval as scheduled, use begin()." I suggest it say one more thing after this sentence: something like "If the same object involved with an interval in progress needs to override it the end function must be called first".
.

I wonder if calling mytimer.end(), then mytimer.begin(newinterval) is not guaranteed to result in one longer interval between calls to the timer's interrupt handler. Will the sequence look like this:
Code:
usecs     action
0000      mytimer.begin(1000)
1000      call to timerchore IRQ handler
2000      call to timerchore IRQ handler
. . .
8000      call to timerchore IRQ handler
8500      call to mytimer.end()
8501      call to mytimer.begin(900)
9401      call to timerchore IRQ handler
10301    call to timerchore IRQ handler

In this case there is no call to the IRQ handler between 8000 and 9401 microseconds. In many cases, that may not be an issue, but if the timer is controlling a motor, or being used to generate a waveform, you're going to see a glitch.

NOTE: I haven't actually observed this behavior, as I've not recently had reason to change a timer interval on the fly. I'll see if I can put together a code example that illustrates the problem (or non-problem if I've misunderstood the interval timer code).
 
My simple test program seems to confirm that using mytimer.end(), mytimer.begin(newinterval) can cause a long interval between interrupts:

Output results are intervals between timer interrupt handler callse:
Code:
Intervals with end/begin
  1000   1000   1000   1000    990    990    990    990    990    990 

Intervals with end/begin
  1000   1000   1000   1296    990    990    990    990    990    990 

Intervals with end/begin
  1000   1000   1000   1081    990    990    990    990    990    990 

Intervals with end/begin
  1000   1000   1000   1985    990    990    990    990    990    990 

Intervals with update
  1000   1000   1000   1000    990    990    990    990    990    990

As you can see, using the end/begin pair can cause an interval up to twice the original interval, depending on when in the cycle, the change is made. With the normal update, the change always occurs just after the fourth output. With end/begin, the change occurs after the third output, but the interval can be much longer.

Here's the demo code:
Code:
/*************************************
 * A test to illustrate the perils of
 * changing an interval timer on the fly.
 * 
 * mborgerson  9/3/2020
 * 
 ********************************************/
#define MAXVALS 10

#define STDINTERVAL 1000
#define NEWINTERVAL 990

IntervalTimer mytimer;

const char compileTime [] = "\n\nInterval Timer Update Test compiled on " __DATE__ " " __TIME__;
volatile uint32_t deltavals[MAXVALS];
volatile uint16_t didx;

// when the interrupt occurs, save the micros() difference from last call
void timerChore(void){
static uint32_t lastmicro, thismicro;
  thismicro = micros();  // get microseconds at interrup handler start
  if(didx < MAXVALS){
    deltavals[didx] = thismicro-lastmicro;
    didx++;  
  }
  lastmicro = thismicro;
}

void setup() {
  Serial.begin(9600);
  delay(1000);
  didx = MAXVALS;
  Serial.println(compileTime);  
  mytimer.begin(timerChore, STDINTERVAL);
}

void loop() {
  char ch;
  if(Serial.available()){
    ch = Serial.read();
    if(ch == 'n' ) ShowNormal();
    if(ch == 'u' ) ShowUpdate();
    if(ch == 'e' ) ShowEndUpdate();
  }

}

// Show interval timer results with no update
void ShowNormal(void){
  didx = 0;  // starts the collection
  while(didx < MAXVALS){}; // wait until collection done
  Serial.println("\nIntervals with no change");
  DisplayResults();
  delay(10);
  mytimer.update(STDINTERVAL); // reset to standard microsecond interval
}

// Show interval timer results with standard update
void ShowUpdate(void){
  didx = 0;  // starts the collection
  delay(3);  // collect 'front porch' data
  mytimer.update(NEWINTERVAL);
  while(didx < MAXVALS){}; // wait until collection done
  Serial.println("\nIntervals with update");
  DisplayResults();
  delay(10);
  mytimer.update(STDINTERVAL); // reset to standard  interval
}

// Show interval timer results with end/begin update
void ShowEndUpdate(void){
  didx = 0;  // starts the collection
  delay(3);  // collect 'front porch' data
  mytimer.end();
  mytimer.begin(timerChore, NEWINTERVAL);
  Serial.println("\nIntervals with end/begin");
  DisplayResults();
  delay(10);
  mytimer.update(STDINTERVAL); // reset to standard  interval
}

void DisplayResults(void){
  uint16_t i;
  for(i= 0; i<MAXVALS; i++) Serial.printf("%6lu ", deltavals[i]);
  Serial.println();
  
}
 
OK, submitted the report at the top based purely on the behavior of the implementation, before looking at the source code for the library. Now clear there should be no reason why begin can't be immediately called after another begin. But for me it is failing because *something* other than a call to the end function is setting the object's private "channel" field to zero. This is causing the begin code to search for a channel where TCTRL is zero, but there isn't one (because I've tied all four up with begin calls), causing my system to fail.

I've instrumented IntervalTimer and am hoping to discover how this can be happening.This might be self-inflicted, but it's difficult to imagine how I'm clearing this private field in the object. I've searched around and don't see any other use of IntervalTimer by other parts of the Teensy runtime except for tone (that I'm not using). But I've also confirmed that my sketch is the only thing calling beginCycles (and I'm no longer calling end()). Working on a simple test case now.

The big difference in my use case is I'm calling begin() within the completion interrupt handler, but am also calling begin() from the non-interrupt level sometimes while the channel in question is in use and between interrupts. This is aimed at generating a PWM-like waveform that can potentially have its rate and duty cycle changed at an arbitrary time.

Should also mention I'm being religious about disabling interrupts around calls into the IntervalTimer functions from the non-interrupt level since of course an interrupt could otherwise happen while the function is executing.

Wait, should have said "nothing to prevent begin being called" but end is needed to clear out the state of the timer and release its channel. So when the dust has settled I'm prepared to find out end *should* be called before begin to get expected behavior. (sorry for the late update of the notes)
 
Last edited:
Here is the test case. It had some surprises in store for me, starting with the fact that this has nothing to do with the private IntervalTimer::channel field. My workaround is trivial: if begin returns false, try it until it returns true. Whatever causes the false return from the begin method "heals itself", or at least it does with this sketch. The test case starts putting out errors for me in under a minute with the Teensy 4.0. I'll be standing by to answer any questions.
 

Attachments

  • intervaltimerbug.ino
    5.1 KB · Views: 95
I'll taka a look at your example. In the meantime, it seems very risky to me to call timer.begin inside an interrupt handler for the timer. The begin() function messes about with the internal function pointer table and other private data. If some other process happens to call begin() then gets interrupted by the existing setup, there could be a messy collision in the internal data.




Changing things inside the interrupt routine does have the advantage that you know the counter value has just been reset. However, it would seem to be simpler to just call timer.update(newinterval) inside the interrupt handler. That call doesn't muck about with the private variables--it just plugs a new value into the counter hardware register. The question will be whether that new value takes effect in the current timer cycle. If not, a few calls to set various hardware flags may be needed. Looks like I have a new test program to try tomorrow.

Your example code shows:
Code:
uint32_t interval[2][NUM_TIMERS] = {
  49500, 49600, 49700, 49800,
  500, 400, 300, 200
};

Are those counts representative of your real world program? In the example the two intervals add up to 50,00 microseconds and you decrement the larger part by 50 and increment the smaller part by 50. That's fine until your initial count of 49500 decrements to zero and wraps around to A VERY LARGE VALUE! That happens in 1000 times around your loop function--which has no obvious constraints on how fast it runs. It doesn't surprise me at all that this program goes south in a hurry (and even further south than North Carolina!)

If your real-world problem involves a PWM output with a 50,000 microsecond period and 50-microsecond resolution there are a lot of solutions that don't involve mucking about with resetting the interval timer. I managed a lot of variable PWM outputs in that range (20Hz, one Part-per-thousand resolution) to drive RC servos some years ago and never had to mess about with anything near the complexity of your code.

BTW, putting your source code as an attachment involves a lot more work for the reader than just copying your code and pasting it between
Code:
markers (the '#' at the right of the top line when you post). That way we can scroll through the code and take a look at it without having to download the attachment and open it in an external editor.
 
TLDR; I can use update and the test case above is flawed, failing every time the interval parameter passed to begin() is zero.

@mborgerson I can use update, should use update, and update is the cure for what ailed me. After all the dust has settled I now believe I was always only getting begin failures part of the time by calling it with a zero wait parameter. Then I found myself intensely curious about how it was failing (thinking that it had a valid parameter, of course) and didn't want to end up with you and others wishing I'd submit my application code again (I'm under an NDA). And I'm sorry for not using cut/paste. I was intending to include my hacked IntervalTimer.{h,cpp} and the callback in the sketch (but not have the sketch actually use that library or callback: just including it to share) and then decided it would just be noise.

Here's an example that hopefully shows correct usage and that others might find useful.

Code:
/*
 This sketch exercises four instances of IntervalTimer to generate 
 squarewaves with different duty cycles, such as for confirming hardware 
 operation with a logic analyzer or oscilloscope. It drives pins two through 
 five.
*/

// *** Start user configurable declarations ***

/*
 The waveform period microseconds
 */

static const uint32_t PERIOD = 50000;

/*
 Microseconds between duty cycle mutations
 */

static const uint32_t USEC_BETWEEN_MUTATIONS = 10000;

/*
 Next dutycycle adjustment time.
 */

static uint32_t report_micros;

/*
 The amount to change the timing of the two "halves" of the waveform
 */

static const uint32_t INCR = 50;

/*
 Name and version. Change the number when you change the code
 */

#define NAME_AND_VERSION "IntervalTimer exerciser 1.00"

// *** End user configurable declarations ***

#define NUM_TIMERS (4)

#include <IntervalTimer.h>

/*
 The type of a generic function with no return value and no parameters
 */

typedef void (*voidFuncPtr)();

/*
 The timer objects
 */

IntervalTimer itimer0, itimer1, itimer2, itimer3;

IntervalTimer timer_object[NUM_TIMERS] = {
  itimer0, itimer1, itimer2, itimer3
};

/*
 Square wave output pins
 */

static const uint32_t pin[NUM_TIMERS] = {
  2, 3, 4, 5
};

/*
 The (variable) microseconds for each of the two parts of an output waveform for
 each timer. 
 */

static uint32_t interval[2][NUM_TIMERS];

/*
 The starting microseconds for each of the two parts of an output waveform for
 each timer
 */

static const uint32_t interval_initial[2][NUM_TIMERS] = {
  PERIOD-(INCR*10), PERIOD-(INCR*8), PERIOD-(INCR*6), PERIOD-(INCR*5),
  INCR*10, INCR*8, INCR*6, INCR*5
};

/*
 The microseconds to use as lower (first timing state) and upper 
 (second timing state) limits
 */

static const uint32_t interval_limit[2][NUM_TIMERS] = {
  INCR*10, INCR*8, INCR*6, INCR*5,
  PERIOD-(INCR*10), PERIOD-(INCR*8), PERIOD-(INCR*6), PERIOD-(INCR*5)
};

/*
 The state of each timer: 0 means "first portion of the waveform" and
 "output off". A value of 1 means "second portion of the waveform" and
 "output on". 
 */

static uint32_t state[NUM_TIMERS] = {
  0, 0, 0, 0
};

/*
 Common interrupt handler. Flip the state and output pin and update the next 
 time interval.
 */

void tickCommon(uint32_t timer) {
  // Flip to the next part of the waveform

  if (state[timer]) {
    state[timer] = 0;
  } else {
    state[timer] = 1;
  }

  // Have the pin reflect the state

  digitalWrite(pin[timer], state[timer]);

  // Update the timer with the new interval

  timer_object[timer].update(interval[state[timer]][timer]);
};

void tick0() {
  tickCommon(0);
}

void tick1() {
  tickCommon(1);
}

void tick2() {
  tickCommon(2);
}

void tick3() {
  tickCommon(3);
}

/*
 Interrupt handlers
 */

voidFuncPtr handler[NUM_TIMERS] = {
  tick0, tick1, tick2, tick3
};

/*
 Like common interrupt handler, but using begin to get the timers started
 */

void setupTimer(uint32_t timer) {
  // Have the pin reflect the state

  digitalWrite(pin[timer], state[timer]);

  // Start the timer with the new interval

  if (! timer_object[timer].begin(handler[timer], 
			interval[state[timer]][timer])) {
    Serial.print("timer: ");
    Serial.print(timer);
    Serial.print(" interval: ");
    Serial.print(interval[state[timer]][timer]);
    Serial.println("usec IntervalTimer::begin returned false: HANGING");
    while(true);
  }
};

// Set up I/O, announce name and version and start the timers.

void setup() {

  // Serial monitor window output speed
  Serial.begin(115200);

  // Have to do this with for Teensy boards
  while(!Serial) {
  }

  Serial.println(NAME_AND_VERSION);

  for (uint32_t timer = 0; timer < NUM_TIMERS; timer++) {
    interval[0][timer] = interval_initial[0][timer];
    interval[1][timer] = interval_initial[1][timer];
    pinMode(pin[timer], OUTPUT);
    setupTimer(timer);
  }
} // setup

/*
 Every USEC_BETWEEN_MUTATIONS change the waveform timing of all four
 channels. With a logic analyzer or oscilloscope a set of squarewaves will
 be seen that shrink their low periods and grow their high periods until
 limits are reached, then the changes begin again.
 */

void loop() {
  if (micros() > report_micros) {
    report_micros = micros() + USEC_BETWEEN_MUTATIONS;

    for (uint32_t timer = 0; timer < NUM_TIMERS; timer++) {
      /*
       Disabling interrupts might not be necessary, but this prevents us
       from the possible pain of discovering why it is, and this application
       isn't sensitive to the slight glitch caused by the calls. The compiler
       cannot "hide" updates here and the data manipulation matches the
       word size of the machine, so probalby don't need volatile decls either.
      */
      noInterrupts();
      interval[0][timer] = interval[0][timer] - INCR; 
      if (interval[0][timer] < interval_limit[0][timer]) {
	interval[0][timer] = interval_initial[0][timer];
      }
      interval[1][timer] = interval[1][timer] + INCR; 
      if (interval[1][timer] > interval_limit[1][timer]) {
	interval[1][timer] = interval_initial[1][timer];;
      }
      interrupts();
    }
  }
} // loop
 
It's pretty simple to do the same thing with just one interval timer and about 50 lines for the PWM and another 50 for setup and generating the ramp up and down part. Since it doesn't involve any bug reports, I moved my demo code to "General discussion" forum.
 
While it is of course possible to change the period of an Intervaltimer on the fly it might be more direct to use one shot timers instead. In case you are interested, here a version using 4 of the 16 TMR(QUAD) based one shot timers from the the TeensyTimerTool.

Code:
#include "TeensyTimerTool.h"
using namespace TeensyTimerTool;

constexpr unsigned nrOfTimers = 4;

// Array of 4 one-shot-timers based on the TMR3 module
OneShotTimer timer[nrOfTimers]{{TMR3}, {TMR3}, {TMR3}, {TMR3}};

constexpr float frequency = 20;                     // Hz
constexpr float period = 1E6 / frequency;           // micros
constexpr uint32_t pins[nrOfTimers] = {2, 3, 4, 5};

float duty[4] = {0.5, 0.75, 0.1, 0.9};              // start duty cycles

void callback(int tnr)
{
    uint32_t pin = pins[tnr];                                                             // pin for calling timer
    digitalToggleFast(pin);                                                               // just toggle the pin
    uint32_t nextPeriod = period * (digitalReadFast(pin) ? duty[tnr] : 1.0f - duty[tnr]); // get next 'half' period from the duty cycle
    timer[tnr].trigger(nextPeriod);                                                       // retrigger the timer
}

void setup()
{
    // initialize pins periods and timers
    for (unsigned i = 0; i < nrOfTimers; i++)
    {
        pinMode(pins[i], OUTPUT);
        timer[i].begin([i] { callback(i); }); // attach the same callback to all timers and pass the timer number to it. (same as you achieved with your tick1.. tick3 functions)
        callback(i);                          // invoke the callback to start the trigger chain;
    }
}

elapsedMillis stopwatch = 0;

void loop()
{
    if (stopwatch > 100)
    {
        stopwatch -= 100;
        for (unsigned i = 0; i < nrOfTimers; i++)
        {
            float newDuty = (duty[i] < 0.95f) ? duty[i] + 0.05f : 0.05;

            noInterrupts();
            duty[i] = newDuty;
            interrupts();
        }
    }
}

Anmerkung 2020-09-04 213006.jpg
 
Thanks for sharing that. If my circumstances make a more efficient solution necessary I'll keep this in mind. I went with the four IntervalTimers for convenience. Using a Teensy 4.0 is extreme overkill, but we got tired of running low on memory, having to think about performance instead of what we're aiming to demonstrate, needing more buses, etc.

But if you have a straight forward way for me to plug a JTAG debugger into the Teensy 4.0, I'm all ears. :)
 
But if you have a straight forward way for me to plug a JTAG debugger into the Teensy 4.0, I'm all ears. :)

I once did it for a T3.5 which worked good: Here some info, a video of a debugging session and links to the required hardware changes https://github.com/luni64/VisualTeensy/releases/tag/v0.9.7.0. (Scroll down to the debugger section). Actually, I was more interested in getting it running than really use it. Stepping through the source code is nice but I never stumbled about something which wasn't debuggable with a scope and some print outs :).
 
Back
Top