Go Down

Topic: [SOLVED] Dealing with a noisy interrupt signal (Read 12884 times) previous topic - next topic

Coding Badly

Here's the code cleaned up a bit (a little more to do yet):
You have to put the digitalRead inside the interrupt service routine.  Separating the interrupt handling from the digitalRead creates a race condition that will leave you pulling your hair out.  (Technically, the race condition is always present.  Moving the digitalRead to the top of the interrupt service routine dramatically reduces the chances of trouble.)

Code: [Select]
void processSignal()
{
  if ( interruptFlag )
  {
    inputState = digitalRead(inputPin);
    interruptFlag = 0;


I commend you for trying to handle the interrupt the "right way".  However, in this situation, doing that opens up a second race condition.  (There is actually a third but the third one is trivial to fix.)  I strongly suggest, for this application, your processSignal code belongs in the interrupt service routine as you had it originally.


dlloyd

#16
Apr 07, 2016, 02:53 am Last Edit: Apr 07, 2016, 06:17 am by dlloyd
Quote from: GoForSmoke
One thing about global variables is that they only get allocated once.
Those local constants in the pattern generator, maybe the optimizer catches them otherwise they get initialized on the stack every time the function runs.
Didn't know that ... thank you.

Quote from: GoForSmoke
Also timeShiftNow is only used once after it gets set to micros(). Maybe the optimizer catches that otherwise you don't need the variable added to the stack, just use micros() in the time check if().
Done ... thanks again.

Quote from: Coding Badly
You have to put the digitalRead inside the interrupt service routine.  Separating the interrupt handling from the digitalRead creates a race condition that will leave you pulling your hair out.  (Technically, the race condition is always present.  Moving the digitalRead to the top of the interrupt service routine dramatically reduces the chances of trouble.)
I seriously appreciate this (I've already lost enough hair). Later, I may end up doing a direct register read as a speedup measure.

Quote from: Coding Badly
I commend you for trying to handle the interrupt the "right way".  However, in this situation, doing that opens up a second race condition.  (There is actually a third but the third one is trivial to fix.)  I strongly suggest, for this application, your processSignal code belongs in the interrupt service routine as you had it originally.
Done. I've found a workaround to set the plot's timebase. Now we can see the transitions more clearly. The first set of traces were with the signal processing in the loop, the second set is with the processing in the ISR. There is a difference.



I'll be adding a "glitch" signal circled in red ... this one is going to be difficult to eliminate without affecting timing, but I have some ideas (my next challenge).

Some notes on the Serial Plotter: It seems to work quite well for me at this stage. I'm able to stretch the timebase by x2 or higher by repeating the print action.

Latest code:
Code: [Select]
byte ledPin = 13;
byte inputPin = 2;
byte inputState;

//-------pattern generator---------------
unsigned long pattern;
unsigned long timeShiftPrev;
byte shiftCount, signalSelect;

//-------ISR constants and variables-----
const unsigned long minMicros = 120000;
const unsigned long maxMicros = 2500000;
volatile unsigned long nowMicros, prevMicros, elapsedMicros;
volatile byte cleanOutput, timeout;

void setup() {
  pinMode(ledPin, OUTPUT);
  Serial.begin(115200);
  attachInterrupt(0, onPulse, CHANGE);
}

void loop() {
  patternGenerator(100000, 4);  // shiftInterval (┬Ás), plotTimebase (x4)
}

void onPulse()
{
  nowMicros = micros();
  inputState = digitalRead(inputPin);
  elapsedMicros = nowMicros - prevMicros;
  if (elapsedMicros >= minMicros)  // ALL cases >= min time
  {
    prevMicros = nowMicros;
    (elapsedMicros >= maxMicros) ? timeout = 1 : timeout = 0;  // timeout flag
    if (inputState)      // Rising
    {
      cleanOutput = 1;
    }
    else                 // Falling
    {
      cleanOutput = 0;
    }
  }
}

void patternGenerator(unsigned long shiftInterval, byte plotTimebase)
{
  if (micros() - timeShiftPrev > shiftInterval)
  {
    timeShiftPrev += shiftInterval;
    pattern = pattern >> 1;
    shiftCount++;
    if (shiftCount >= 32)
    {
      shiftCount = 0;
      signalSelect++;
      //                             Normally High Signal   Normally Low Signal
      // (signalSelect & 2) ? pattern = 0xFBF50AFF : pattern = 0x040AF500; // bounce with glitch
      (signalSelect & 2) ? pattern = 0xFFF50AFF : pattern = 0x000AF501; // bounce
    }
    digitalWrite(ledPin, (pattern & 1));

    for (int i = 0; i < plotTimebase; i++) {
      Serial.print((pattern & 1) + 1);
      Serial.print(" ");
      Serial.print(cleanOutput);
      Serial.print(" ");
      Serial.println(timeout - 1);
    }
  }
}

Results (debouncing completed):


aarg

#17
Apr 07, 2016, 03:33 am Last Edit: Apr 07, 2016, 03:41 am by aarg
Yes, high speed pulse debouncing is a completely different animal than low speed switch debouncing. The premise, in it's simplest form, is that the bounce impulses are higher in frequency than the signal. I think it is hard to refute this. So the idea of low pass filtering and applying a threshold makes good sense.

However, in order to properly implement the low pass function, the digital filter must be sampling at precise, periodic intervals. This seems incompatible with the mechanism of a pin state interrupt. Furthermore, it will be difficult to run the filter at extremely high frequencies without incurring significant demand on the processor.

There isn't much doubt that it can be made to work well. A good question is, what is a useful upper limit on the signal frequency?

A corresponding question would be, can a simple hardware RC outperform it by a large margin?
  ... with a transistor and a large sum of money to spend ...
Please don't PM me with technical questions. Post them in the forum.

dlloyd

There isn't much doubt that it can be made to work well. A good question is, what is a useful upper limit on the signal frequency?
Yes, good question. Well, I'm hoping for at least 10kHz for a noisy signal, but there'll be a point where jumping into the ISR with needless false interrupts will consume too many cycles. I'm trying just the timing approach for now ... at least its very fast with the least demand on the processor.

dlloyd

@Robin2

I think something like this could work for your sensor because (at this stage) it should debounce both edges of the pulses, so it won't matter which side of the pulse the false transitions occur. (I assume you'll be using an interupt for this). Just use CHANGE mode for your interupt. Then, for example, you could use  minMicros = 4000 (4ms, 250Hz) and maxMicros = 4000000 (4sec, 0.25Hz) to set the input timing range.

GoForSmoke

#20
Apr 07, 2016, 04:30 am Last Edit: Apr 07, 2016, 04:37 am by GoForSmoke
The interrupt occurs because of pin change so yup read the pin in the ISR. I missed that.
When timing is (was) in millis I don't worry about taking < 100 micros to read the pin.

Should it do the time checks in the IRQ as well?

How fast are these spikes? The original code measured millis. When the goalposts move to spikes less than 50 micros long then that's a different ballpark however...

const unsigned long minMicros = 120000;
const unsigned long maxMicros = 2500000;

Those are the goalposts right there. 120 to 2500 milliseconds.

Quote
Yes, high speed pulse debouncing is a completely different animal than low speed switch debouncing.
My debounce switch tester/calibration catches bounces 12 to 20 micros in length while reporting them to look for N micros with no change. Grounding a jumper on the USB port box to make dirty switching, I only need 2000 micros to debounce that and I don't use interrupts to do it. Want the sketch?

Are you debouncing or making an oscilloscope? Both?
1) http://gammon.com.au/blink  <-- tasking Arduino 1-2-3
2) http://gammon.com.au/serial <-- techniques howto
3) http://gammon.com.au/interrupts
Your sketch can sense ongoing process events in time.
Your sketch can make events to control it over time.

aarg

Yes, good question. Well, I'm hoping for at least 10kHz for a noisy signal, but there'll be a point where jumping into the ISR with needless false interrupts will consume too many cycles. I'm trying just the timing approach for now ... at least its very fast with the least demand on the processor.
Sure, but I'm still wondering how it can possibly work with an ISR. Or, I guess, what would be the point of using an ISR when you have to keep sampling going with a timing process anyway. In that case, you could just as easily read the input port.
  ... with a transistor and a large sum of money to spend ...
Please don't PM me with technical questions. Post them in the forum.

dlloyd

Sure, but I'm still wondering how it can possibly work with an ISR. Or, I guess, what would be the point of using an ISR when you have to keep sampling going with a timing process anyway. In that case, you could just as easily read the input port.
The interrupt is in CHANGE mode ... there's no sampling, just checking elapsed time, edge to edge of input signal transitions.

GoForSmoke

You can knock a few micros off those digital reads and writes using port manipulation.

Uno pin 2 is PINSD pin 2 for read and PORTD pin 2 for write or write 1 to PINSD pin 2 to change whatever state pin 2 is.

pinTwoState = ( PINSD & 4 ) >> 2; // fast read pin 2

PORTD = PINSD & 0xFB; // fast set pin 2 to 0
PORTD = PINSD | 4; // fast set pin 2 to 1

PINSD |= 4; // fast invert pin 2


 
1) http://gammon.com.au/blink  <-- tasking Arduino 1-2-3
2) http://gammon.com.au/serial <-- techniques howto
3) http://gammon.com.au/interrupts
Your sketch can sense ongoing process events in time.
Your sketch can make events to control it over time.

GoForSmoke

#24
Apr 07, 2016, 07:54 am Last Edit: Apr 07, 2016, 07:56 am by GoForSmoke

void patternGenerator(unsigned long shiftInterval, byte plotTimebase)
{
  if (micros() - timeShiftPrev > shiftInterval)
  {
    timeShiftPrev += shiftInterval;
    pattern = pattern >> 1;
    shiftCount++;
    if (shiftCount >= 32)
    {
      shiftCount = 0;
      signalSelect++;
      //                             Normally High Signal   Normally Low Signal
      // (signalSelect & 2) ? pattern = 0xFBF50AFF : pattern = 0x040AF500; // bounce with glitch
      (signalSelect & 2) ? pattern = 0xFFF50AFF : pattern = 0x000AF501; // bounce
    }
    digitalWrite(ledPin, (pattern & 1));

    for (int i = 0; i < plotTimebase; i++) {
      Serial.print((pattern & 1) + 1);
      Serial.print(" ");
      Serial.print(cleanOutput);
      Serial.print(" ");
      Serial.println(timeout - 1);
    }

  }
}


Is this really about writing to the led pin (jumpered to the interrupt pin) and having the IRQ change CleanOutput and timeout before they are printed on all 4 lines? A race between serial print and the IRQ?
1) http://gammon.com.au/blink  <-- tasking Arduino 1-2-3
2) http://gammon.com.au/serial <-- techniques howto
3) http://gammon.com.au/interrupts
Your sketch can sense ongoing process events in time.
Your sketch can make events to control it over time.

Coding Badly

#25
Apr 07, 2016, 07:59 am Last Edit: Apr 07, 2016, 08:00 am by Coding Badly
PINSD |= 4; // fast invert pin 2
Oops.

Code: [Select]
PORTD = PINSD | 4; // fast set pin 2 to 1

That one is trouble as well.


GoForSmoke

#26
Apr 07, 2016, 08:56 am Last Edit: Apr 07, 2016, 09:10 am by GoForSmoke
Oops.

Code: [Select]
PORTD = PINSD | 4; // fast set pin 2 to 1

That one is trouble as well.

PORTD |= 4; // right, PIND is inputs, no need to use that... fast set pin 2 to 1
PORTD &= 0xFB; // much better

Well the real name is PIND and AVR's have this feature you can say oops about all over again.

Quote
Three I/O memory address locations are allocated for each port, one each for the Data Register - PORTx, Data
Direction Register - DDRx, and the Port Input Pins - PINx. The Port Input Pins I/O location is read only, while the
Data Register and the Data Direction Register are read/write. However, writing a logic one to a bit in the PINx Register,
will result in a toggle in the corresponding bit in the Data Register.
In addition, the Pull-up Disable - PUD bit
in MCUCR disables the pull-up function for all pins in all ports when set.
PIND |= 4; // does toggle PORTD bit 2.

First time I saw this, Nick Gammon used it. It's a neat trick, TY ATMEL!
IIRC he used it in a video-signal generator.
1) http://gammon.com.au/blink  <-- tasking Arduino 1-2-3
2) http://gammon.com.au/serial <-- techniques howto
3) http://gammon.com.au/interrupts
Your sketch can sense ongoing process events in time.
Your sketch can make events to control it over time.

Coding Badly

PIND |= 4; // does toggle PORTD bit 2.
I agree.  But your version does more than that.  You have too many operators.


aarg

#28
Apr 07, 2016, 10:27 am Last Edit: Apr 07, 2016, 10:29 am by aarg
The interrupt is in CHANGE mode ... there's no sampling, just checking elapsed time, edge to edge of input signal transitions.
Sorry, I got was getting your code mixed up with the code linked to in reply #3. So everything I said applies to that, not what you have done (except for general theoretical comments). Oops.

Doesn't letting the ISR handle the noise get a little CPU intensive? Because the ISR gets called on every transition?
  ... with a transistor and a large sum of money to spend ...
Please don't PM me with technical questions. Post them in the forum.

GoForSmoke

Quote
from: GoForSmoke on Today at 02:56 am

    PIND |= 4; // does toggle PORTD bit 2.
I agree.  But your version does more than that.  You have too many operators.


http://www.arduino.cc/en/Reference/BitwiseCompoundOr

Syntax:

x |= y;   // equivalent to x = x | y;

Parameters

x: a char, int or long variable
y: an integer constant or char, int, or long

PIND is a register that is read/write, just like PORTD and DDRD


Tested & working sketch to blink led13 using PINB |= 0x20;

Code: [Select]

void setup()
{
  Serial.begin( 115200 );
  pinMode( 13, OUTPUT );
}

byte run20;
unsigned int tStart;


void loop()
{
  if (( millis() & 0xFFFF ) - tStart >= 0x1FF )
  {
    PINB |= 0x20;
    tStart += 0x1FF;
    Serial.print( tStart, HEX );
    Serial.print( "    " );
    Serial.println( PORTB, HEX );

    if ( ++run20 >= 20 )    while (1);
  }

}


Quote
1FF    20
3FE    0
5FD    20
7FC    0
9FB    20
BFA    0
DF9    20
FF8    0
11F7    20
13F6    0
15F5    20
17F4    0
19F3    20
1BF2    0
1DF1    20
1FF0    0
21EF    20
23EE    0
25ED    20
27EC    0
I could have used PINB5 |= 1; but where's the fun in that?
1) http://gammon.com.au/blink  <-- tasking Arduino 1-2-3
2) http://gammon.com.au/serial <-- techniques howto
3) http://gammon.com.au/interrupts
Your sketch can sense ongoing process events in time.
Your sketch can make events to control it over time.

Go Up