Any clever ideas for excluding aberrant values produced by an ISR?

I have a simple ISR to record the value of micros() everytime a QRE1113 reflective optical sensor detects the white blob on a disc on the shaft of a small motor.

void revDetectorISR() {
 isrMicros = micros();
 newIsrMicros = true;
}

The code in loop() figures out the time interval between pulses.

Most of the time this works fine but occasionally (perhaps 1 in 20) a very short interval is logged - usually less than half the correct interval. The correct intervals can range between about 100,000 µsecs and 4,000 µsecs depending on the motor speed.

I'm just wondering if anyone has a clever (i.e. simple and few CPU cycles) method to identify and ignore those incorrect intervals. A moving average would not be appropriate because that would give credence to the incorrect values.

...R

In loop(), are you reading or using the global variable isrMicros without turning off the interrupts?

If so, it will occasionally be corrupted during that process by the ISR.

If you know the expected value then it sounds like you just need an if statement to see if it is within some range. If you currently have some running average going or something then you could just compare to the current average.

if(abs(newValue - currentAverage) <= someThreshold){
  // record the value
}
else {
  // don't
}

jremington:
In loop(), are you reading or using the global variable isrMicros without turning off the interrupts?

If so, it will occasionally be corrupted during that process by the ISR.

Yes, @Robin should have posted all complete code. Or at least a minimal example that compiles and exhibits the issue. Of all people I would expect @Robin to know this. :wink:

Calculating the time difference in the ISR should consume too much time, so it might be an option. Just ensure all variables used in the ISR and the loop() code are declared volatile.

Delta_G:
Yes, @Robin should have posted all complete code. Or at least a minimal example that compiles and exhibits the issue. Of all people I would expect @Robin to know this. :wink:

I don't expect anyone to set up a similar experiment to see if they get the same problem. I believe I have stated the case with sufficient clarity. Maybe another way to present it is like this

8123
7990
7957
8011
3876
8001
7997

What is a clever way to detect and ignore the 3876?

pylon:
Calculating the time difference in the ISR should consume too much time, so it might be an option. Just ensure all variables used in the ISR and the loop() code are declared volatile.

I have tried that. It does not make any difference. The problem seems to be that something spurious triggers the interrupt. And I have spent too much time trying to figure out the cause.

...R

I believe I have stated the case with sufficient clarity.

Nope.

And I have spent too much time trying to figure out the cause.

And so have we.

Can you change to a different interrupt pin and see if the problem follows?

Paul

And I have spent too much time trying to figure out the cause.

I don't think so. It's definitely better to find that error than to simply throw away value you don't like.

If you want to go that way, how about throwing all values away that aren't inside a +/-20% range of the previous value? You have to decide if that fits your setup.

Robin2:
I don't expect anyone to set up a similar experiment to see if they get the same problem. I believe I have stated the case with sufficient clarity. Maybe another way to present it is like this

Come on man, you've answered enough questions to know how this goes.

So the question remains, are you reading the variables in a critical section? If we can eliminate the bad readings, isn't that in many ways better than figuring out how to ignore them?

And the code in my reply still stands. That's the only way I see you getting it done. You have to compare the new reading to the old readings to see if it fits.

Similar to the manner in which PWM duty cycles are derived and assuming you have a spare timer, when servicing your ISR, you know roughly when the next interrupt will arrive, say X microseconds. You also have some idea as to the rate of change that may occur, call it dX. The next interrupt then, is unlikely to occur before X - dX.

After setting newIsrMicros, set the OCR value on the available timer to OCR + (X - dX) and then turn your ISR off. The only instruction in the timerX_COMP_vect will then be to turn your ISR back on.

Robin2:
What is a clever way to detect and ignore the 3876?

The correct way is with statistics (calculate the likelihood the current value is a member of the set).

A simple way is a (small) window average ignoring the high and low values.

Are you averaging or are the individual values important?

Are those values "real"? Or a simplified example for illustration purposes? (This question is very important. You may have an overrun problem.)

But, it would be comforting to see the rest of the code that touches those two variables.

I'm wondering what the purpose of "newIsrMicros" is? If it is used to tell the "loop()" that there is new data to process, that data should be protected in the ISR by the same variable.

void loop()
{
  if (newIsrMicros)
  {

    //Do something with isrMicros

    newIsrMicros = false; //Done playing with isrMicros
  }
}

void revDetectorISR() {
 if (newIsrMicros) return; //loop() has not handled isrMicros yet
 isrMicros = micros();
 newIsrMicros = true;
}

But for this to work properly, you would probably need to add a counter which counts how many times "revDetectISR" has been invoked without updating "isrMicros" and this would make the ISR a bit more bulky.

Danois90:
If it is used to tell the "loop()" that there is new data to process, that data should be protected in the ISR by the same variable.

Both variables have to be protected by a critical section. Please add that to your example.

What would be the purpose of that? As long as the same variable is not written to in two places "at once", there should be no problem.

Ugh. I am just so tired of answering questions that are trivial to answer with Google. @Danois90, do some research. Nick Gammon's website is a great place to start.

And yet you answer anyway.. Please gimme some links instead of referring to Google as a reference :slight_smile:

OP's ISR is text book, timestamp, set flag, get out. Test the flag and deal with it elsewhere. The flag is only set in the ISR and only cleared by whatever function processes it.

DKWatson:
OP's ISR is text book, timestamp, set flag, get out. Test the flag and deal with it elsewhere. The flag is only set in the ISR and only cleared by whatever function processes it.

The ISR does not respect the flag it sets, that ain't "text book" coding to me since the timestamp and flag may be changed during "processing elsewhere".

Don't know where you learned your coding but the function of an ISR is to acknowledge an event and get out, full stop. By disrupting global interrupts any longer than that you negatively impact the deterministic behaviour of a bare metal, real-time processor.

Try matt black paint on the shaft!

I can't see why you are playing around with the code !

Mark