creating an array from interrupts

Using Arduino Programming Questions
#1

i have two motors that i’m driving with PWM outputs. using optical sensors, i feedback sampled frequency information to my arduino in an attempt to synchronize the motors. this means i need both frequency and phase information. i had originally used an fft to get that information, but i decided that it might be much more efficient to use interrupts. i borrowed some code posted elsewhere to the forum ( http://arduino.cc/forum/index.php/topic,45295.0.html ) and it’s working pretty well as far as i can tell. i’m able to get both frequency and phase information this way, and it seems pretty reliable. sadly, i’m not very good at the programming side of things, and i’m having a hard time creating an array so that i can gather a few statistics by averaging some number of samples. it seems like it should be really easy, but i just can’t quite come up with the right code to do it. if i use a for loop in my my void loop() bit, i get repetitive entries because my loop is iterating faster than my interrupts are triggering. i also tried moving my array to my interrupt function, but i wasn’t able to get that to work either. (i get an entire array filled with the one value.) how do i tell it “hey, when you get your trigger, pass that value to an array?” am i making this way harder than it is? (i’m going to try to simplify the code below, so if something doesn’t make sense, it’s because i haven’t done a very good job simplifying it. forgive the fact that some of my variable names don’t make sense right now (i.e. “freqM1” is not an array of frequencies) - it’s because i’m simplifying things to try to get my array to work. :confused: )

int CH1_PIN = 2;
int CH3_PIN = 3;
unsigned long ch1_pulseLength;
unsigned long ch3_pulseLength;
unsigned long ch1_startTime;
unsigned long ch3_startTime;
const uint16_t samples = 9;
float freqM1[samples];
float freqM2[samples];

void setup() {
     pinMode(CH1_PIN, INPUT);
     pinMode(CH3_PIN, INPUT);
     // when pin2 goes high, call function to set T0:
     attachInterrupt(0, ch1_begin, RISING);
     // when pin 3 goes high, call function to determine elapsed time from T0:
     attachInterrupt(1, ch3_begin, RISING);
     Serial.begin(9600);
} 

//interrupt function to mark rising edge of first motor:
void ch1_begin() {
  ch1_startTime = millis();
 /* here's the second place i tried, where i get a full array of one value:
  for(i=0; i < samples; i++) {
  freqM1[i] = ch1_startTime;
 Serial.println(i);
 Serial.println(freqM1[i]);
 }*/
  detachInterrupt(0);
  attachInterrupt(0, ch1_end, FALLING);
  
}

void ch1_end() {
  ch1_pulseLength = millis() - ch1_startTime;
  detachInterrupt(0);
  attachInterrupt(0, ch1_begin, RISING);
}

void ch3_begin() {
  ch3_startTime = millis(); 
  detachInterrupt(1);
  attachInterrupt(1, ch3_end, FALLING);  
}

void ch3_end() {
  ch3_pulseLength = millis() - ch3_startTime;
  detachInterrupt(1);
  attachInterrupt(1, ch3_begin, RISING);
}

void loop()  { 
    /* here's where i first tried to make the array, where i get multiple entries of each value:
    for (int i = 0; i < samples; i++) 
    freqM1[i] = ch1_startTime;
    Serial.println(i);
    Serial.println(freqM1[i]); */

  }

so when i do this without the array, i get nice, meaningful numbers, and each occurs exactly once. when i try to put the array in the ch1_begin() function, i just get an entire array filled with the one ch1_startTime value. when i move it down to the void loop(), i get multiples. i’ve tried introducing another variable to save “ch1_startTime” to, and then pass it to the array, but that didn’t work either. please help me get this straightened out. it’s making me crazy.

#2

Note: Irrelevant code not included.
Warning: Untested. May not compile.

const int CH1_PIN = 2;
const int CH3_PIN = 3;
volatile unsigned long ch1_pulseLength;
volatile unsigned long ch3_pulseLength;
volatile boolean ch1_HaveValue;
volatile boolean ch3_HaveValue;

void ch1_end() {
ch1_pulseLength = millis() - ch1_startTime;
ch1_HaveValue = true;
detachInterrupt(0);
attachInterrupt(0, ch1_begin, RISING);
}

void ch3_end() {
ch3_pulseLength = millis() - ch3_startTime;
ch3_HaveValue = true;
detachInterrupt(1);
attachInterrupt(1, ch3_begin, RISING);
}

void loop() {
unsigned long ch1_pulseLengthSaved;
unsigned long ch3_pulseLengthSaved;
if ( ch1_HaveValue )
{
ch1_HaveValue = false;
noInterrupts();
ch1_pulseLengthSaved = ch1_pulseLength;
interrupts();
// YOUR CODE GOES HERE. “ch1_pulseLengthSaved” has a fresh value.
}
if ( ch3_HaveValue )
{
ch3_HaveValue = false;
noInterrupts();
ch3_pulseLengthSaved = ch3_pulseLength;
interrupts();
// YOUR CODE GOES HERE. “ch3_pulseLengthSaved” has a fresh value.
}
}

#3

… i get an entire array filled with the one value …

 for(i=0; i < samples; i++) {
  freqM1[i] = ch1_startTime;

You are filling the array from start to end with ch1_startTime so that’s exactly what I would expect.

 detachInterrupt(0);

attachInterrupt(0, ch1_end, FALLING);

Don’t do this. You are attaching and detaching interrupts like mad, and inside an interrupt service routine. I doubt that will work well. If you must catch leading and trailing edges set the interrupt to CHANGE in setup, and then test the pin state in the interrupt.

Serial.println(i);

Serial.println(freqM1[i]);

Don’t try to do debugging prints in an interrupt routine. It either won’t work or you will miss dozens of interrupts while it is doing it.

am i making this way harder than it is?

Yes. I can’t quite work out what you want but I would think carefully about the values in variables and what you want to do with them when the interrupt occurs.

#4

detachInterrupt(0); attachInterrupt(0, ch1_end, FALLING);

Don't do this. You are attaching and detaching interrupts like mad, and inside an interrupt service routine. I doubt that will work well.

So long as the subsequent calls to detachInterrupt and attachInterrupt are performed in an interrupt service routine -OR- detachInterrupt is performed before another attachInterrupt (which is all true in this case), there are not any problems. Both functions are simple and fast.

#5

If you want to catch both edges of the signal, surely a CHANGE interrupt is a more natural way of doing it? The technique of attaching and detaching could lead to timing issues. Here:

detachInterrupt(0);

// no interrupt handler right now

attachInterrupt(0, ch1_end, FALLING);

Now probably the signals won’t come in fast enough for this to be an issue, but personally I am uncomfortable with the idea of frequently detaching interrupts in a program that is relying upon them.

#6

nick gammon:
thanks for your comments. it is helpful to get input on what i should and shouldn’t do, because i don’t know these things.

You are filling the array from start to end with ch1_startTime so that’s exactly what I would expect.

yeah, i know that. my problem is that i can’t figure out how to write the code so that i’m NOT doing that… i can easily look at it and say, “yep, that’s what i’m doing, and it’s working just like it should for what i’ve written…”

also, my signals are really slow, typically around 30-50Hz. i wouldn’t try to use the interrupts or look at what i’m getting in the manner that i am if any of this involved high rep rates.

coding badly:
i’m sorry, i’m a little thick. i still can’t figure out how to go from this to an array.

if ( ch1_HaveValue )
{
ch1_HaveValue = false;
noInterrupts();

i think this is saying that if i haven’t gotten a value from my ch1_end() function, to disable the interrupts.

ch1_pulseLengthSaved = ch1_pulseLength;
interrupts();
// YOUR CODE GOES HERE. “ch1_pulseLengthSaved” has a fresh value.
}

and when i do have a value from my ch1_end function, to store that value in the ch1_pulseLengthSaved variable and re-enable the interrupts, which seems very straightforward.

assuming that ch1_pulseLengthSaved gave me values that made sense (and it doesn’t right now, but that’s a separate problem that i can figure out later), how would i go about passing those values to an array? i tried a bunch of stuff similar to the stuff i tried before that didn’t work, and i got similar results. :confused:

#7

It’s a question of the (hardware) details and personal preference.

The technique of attaching and detaching could lead to timing issues.

There is a similar timing issue with CHANGE interrupts. In the interrupt service routine, for this application, it is necessary to read the state of the pin to determine if a rising edge occurred or a falling edge. In the Arduino world, it takes about 30 machine instructions from when the interrupt fires to when the pin can be read. During this time, the hardware may have changed states. We could read a falling edge when a rising edge is what originally triggered the interrupt. We would miss a transition. The beauty of girlvslaser’s technique is that the hardware tells us that something important occurred AND what it was.

Now probably the signals won’t come in fast enough for this to be an issue, but personally I am uncomfortable with the idea of frequently detaching interrupts in a program that is relying upon them.

The risk can be minimized, but not eliminated, by shuffling the code…

void ch1_end() {
detachInterrupt(0);
attachInterrupt(0, ch1_begin, RISING);
ch1_pulseLength = millis() - ch1_startTime;
ch1_HaveValue = true;
}

The biggest problem with this technique is the side-effect of missing an edge. The measured time (pulseLength) would be a full period plus a pulse (high - low (missed) - high (skipped) - low). But that can be alleviated with a sanity check (if value is way too big, ignore it).

#8

coding badly: i still can't figure out how to go from this to an array.

Yikes! Sorry about wondering off. Do you still need help?

#9

coding badly:

oops, you're not the only one who wandered off. :)

i have gone through many iterations at this point; however, i am still having some problems, although i'm getting a lot closer. i think most of them relate to the interrupts themselves and the fact that i was trying to do a bunch of the processing outside the interrupt, which meant that my timing was off. i'm working through that now.

i'm trying to do a simple moving average, and let me just say, i recognize this is likely not the most efficient way. i started off with pointers but got stuck, and so i decided to try array manipulation first. i feel like it's really cumbersome and not at all nimble and elegant, which makes me not like it, but if i can get to work, it'll be a step in the right direction. tell me your thoughts on this, which begins with what you gave me a few weeks ago:

void PeriodM1Calc(){
   if ( M1_HaveValue )
  {
    M1_HaveValue = false;
    noInterrupts();
    //store the current period to the array (which begins with index = 0 and has array size of 10)
    PeriodM1[index] = M1_startTime - M1_oldTime;
    //make the current start time the old start time for the next time through the loop
    M1_oldTime = M1_startTime;
    interrupts();
    //sum the array
    sumPeriodM1 = sumPeriodM1 + PeriodM1[index];
    //increment the index
    index = index + 1;
    //once the array is full, calculate the average and do some array trickery, where samples is a const equal to 10
    if (index >= samples) {
      averageM1 = sumPeriodM1/samples;
      //subtract the first value in the array from the sum
      sumPeriodM1 = sumPeriodM1 - PeriodM1[0];
      //reset the index to 9 so that the next interrupt will re-enter this loop
      index = 9;
      //shift my array one position
      for (int j = 0; j > (samples-1); j++) {
         PeriodM1[j]=PeriodM1[j+1];
      }
      // reset the last value of the array to 0 - not sure if this is necessary
      PeriodM1[samples]=0;
    }
    
  }  
}

it seems like an awful lot to have in a function involving interrupts to me. any thoughts on this or guidance to accomplish this via pointers would be quite welcome! thanks.

#10

girlvslaser: i have two motors that i'm driving with PWM outputs. using optical sensors, i feedback sampled frequency information to my arduino in an attempt to synchronize the motors.

I wonder if there is an easier way of doing this? For one thing, perhaps drive both motors from the same PWM output (eg. via a transistor or two)?

Failing that, I would have thought that simply detecting whether motor A or B triggers an interrupt first would be enough to work out whether to slow A down (or speed B up). You could have some sort of feedback calculation. If A is much sooner than B then you slow A down a lot. If slightly sooner you slow it down a small amount. You need to be careful you don't get jitter (ie. constantly slowing down and speeding up by a small amount).

Try Googling: 

synchronize motors pwm

I found quite a few interesting posts about the same sort of thing.

#11

alas, i have found no simpler way. it's much more complicated than what i described in that i am presently superimposing two motors to get a desired output. they won't always be at the same frequency. i need to be able to control their frequency and phase independently. and once i've accomplished this, i'll scale it up to an entire array of motors - it's my propensity for all things complicated.

i actually have it working pretty well using PID controls with single samples. if i can just wrangle this array, i can compute the moving average and reduce the impact of subtle frequency variations, which will make for smoother adjustments.

#12

OK, well then something like this:

#define MAX_SAMPLES 100

volatile long ch1Samples [MAX_SAMPLES];
volatile byte ch1CurrentSample = 0 ;

void ch1_end() {
  detachInterrupt(0);
  attachInterrupt(0, ch1_begin, RISING);
  ch1_pulseLength = millis() - ch1_startTime;
 
  if (ch1CurrentSample < MAX_SAMPLES)
     ch1Samples [ch1CurrentSample++] = ch1_pulseLength;
 }

That will gradually put samples into a buffer, adding 1 each time to the number in it.

Then in “loop” you see if the array has filled up, and then work on it:

void loop ()
{

if (ch1CurrentSample >= MAX_SAMPLES)
  {

  // calculate frequencies here and do whatever


  ch1CurrentSample = 0;  // now we can gather another batch
  }

// other stuff

} // end of loop
#13

thanks. i've been able to accomplish something really similar to this wherein i acquire x number of samples, average, then reset to take the next x samples. it resulted in somewhat mediocre function, which is what led me to try a rolling average, so that i'm constantly buffering in each new value and recomputing the average over the last x samples. that's what all the array shifting is about.

#14

In that case you want a circular buffer:

void ch1_end() {
  detachInterrupt(0);
  attachInterrupt(0, ch1_begin, RISING);
  ch1_pulseLength = millis() - ch1_startTime;

  // wrap around at the end
  if (ch1CurrentSample >= MAX_SAMPLES)
     ch1CurrentSample = 0;

  ch1Samples [ch1CurrentSample++] = ch1_pulseLength;
 }

So now ch1CurrentSample is the first free spot (which might have wrapped).

In loop, you now subtract x from that (where x is the number of samples you want). If that becomes less than zero you make it equal to MAX_SAMPLES - 1 (taking care with byte types which cannot become negative).

If a particular sample is zero, it means you never got it yet (eg. at the start of the program). Or you could have a counter, and start doing calculations once the count of samples exceeds your value of x.

So now you average from the (ch1CurrentSample - x) up to (ch1CurrentSample - 1) allowing for the wrapping.

#15