Slotted optical switch triggering twice on falling edge

Novice, trying to setup an Arduino Nano with a slotted optical switch so that it tells me when a blade (in a disk) has passed through. The disk will be spinning at varying speeds.
The switch has an analogue and digital output. I am using the digital output (connected to pin D2), I think it uses a Schmitt trigger to give 0=open, 1=blocked.
I have used interrupts with a Falling edge and as you can see from the attached image of the digital output, it gives a very clean square wave as I put a card in/out the slot.
My code prints the trigger time "millis()" from within the ISR.
Even though I have a "Falling" trigger, the ISR gets triggered when I put the card in and triggered again when I take it out.
The debounce that I have added should be adequate for my needs, but I would still like to know why my Falling trigger is triggering on both rising (0-1) and falling (1-0) edges?
Also I thought millis() wasn't supposed to work within an ISR. If I can't use millis() within the ISR I don't know how I will do the debouncing?

/*
  Optical counter
  Reads state of optical switch and reports to serial port
  It looks like the digital output is the analogue output with a schmidt trigger applied for hysteresis (or debouncing)
*/

// PIN CONNECTIONS (Connects to Digital pin and analogue "sensitivity" pin)
const byte              Digital_in = 2;
const byte              Analogue_in = A0; 

// CONSTANTS AND VARIABLES
volatile long lastDebounceTime_Optical_switch_ISR = 0;
const long debounceDelay_Optical_switch_ISR = 200;

void setup() {
  Serial.begin(9600);

  // SET PIN MODES
  pinMode(Digital_in, INPUT);        // make pin input
  pinMode(Analogue_in, INPUT);       // make pin input
  attachInterrupt(digitalPinToInterrupt(Digital_in), Optical_switch_ISR, FALLING);

  Serial.println("Start");
}

void loop() {
  int Analogue_val = analogRead( Analogue_in);
}

void Optical_switch_ISR() {
  long current_time = millis();
  if ((current_time - lastDebounceTime_Optical_switch_ISR) > debounceDelay_Optical_switch_ISR) Serial.println(current_time);
  lastDebounceTime_Optical_switch_ISR = current_time;
}

First off, you need to format your code so it isn't impossible to read. 1) change all your comment lines to /***** text ***/ instead of slashes. 2) remove all the redundant comment lines. 3) if you want to divide your code into sections, format it like this

[code]/*** Code to do stuff ********/
   int = Code goes here;
/***

Millis should be stored in a volatile unsigned long.
Every time loop() runs you are declaring the variable Analogue_val which resets it. Look up how variable scope works and move it outside of the loop().
serial.print() inside and ISR is a bad idea. It takes far too long to execute. All the ISR should do is grab the current millis then exit as quickly as possible. You can check the debounce time and print once your back in the main loop. [/code]

Also I thought millis() wasn't supposed to work within an ISR.

millis() does not increment in an ISR, but it returns the current value. delay() does not work in an ISR and should never be used in one.

ISRs should generally be written to run as fast as possible, which means less than one millisecond. As mentioned, they should NEVER do serial I/O.

it gives a very clean square wave

Maybe. The edges could be quite rough, with multiple bounces on a microsecond to millisecond time scale, but that plot won't show it.

You did not show a picture of the slotted disk. Is it at all reflective of IR light? Are the slots coated with material to stop IR fringing and reflections?

Paul

The problem with an optical sensor is that the signal is slowly varying and noisy, causing false
transitions when naively converted to a logic signal.

A comparator with hysteresis would normally be used to fix this, but if the hysteresis isn't adequate,
or the object passing through the slot gives reflections off its edge, you can still get multiple transitions.

The cure for this is two slot sensors used in quadrature, as with a mechanical mouse mechanism.

Cheers for the replies all.

I have removed the redundant comments in the code above. I will apply the other fixes and share the new code. I haven't made the wheel yet, its just me with a piece of card.

The consensus appears to be that I am seeing a noisy bouncing signal as the card goes in or out.

I ran this speed test and it appears to suggest I can run at a baud rate of 2000000: -

So I ran the code below (at baud rate 2000000) and manually inspected the serial monitor output.
To the best of my ability it looks like it has clean edges (looks like 0000000111111110000000).

I guess I am not recognising the limitations of the serial monitor. Can I assume that my serial monitor is missing the noisy event?
Made up example: -
000111000 2MHz Serial output sees this
000000000000000000000000101100111111111111111111010011010000000000000000 16MHz Arduino sees this

I was confused because even with debounce delay set to 0 (in the code above), I get a single trigger as the card goes in and a single trigger when the card comes out. I would have thought that I would get multiple triggers as the signal bounces around at the edge. Can I assume that I am not seeing this because the noise event is over (in a steady state) before the ISR has completed?

Is there a convenient way of recording the full digital output from a pin for a few ms either side of when an ISR gets triggered?

const byte      Digital_in = 2;

void setup() {
  Serial.begin(2000000);
  pinMode(Digital_in, INPUT);
  Serial.println("Start");
}

void loop() {
  int Digital_val = digitalRead(Digital_in);      // Read in the value. It will be 0 when open and 1 when blocked
  Serial.println(Digital_val);
}

Printing is very slow. One approach is to store a bunch of digital values in an array as fast as possible, then print the array.

For example, this code will examine the falling edge in steps of a couple of microseconds.

byte bits[1000];
while(digitalRead(input) == HIGH); //wait for first falling edge
for (int i=0; i<1000; i++) bits[i]=digitalRead(input);  //read input bit as fast as possible
for (int i=0; i<1000; i++) Serial.print(i);  //print the data

For even faster port reads, use direct port access. For pin 2 (PORTD, bit 2 on Arduino Uno):

for (int i=0; i<1000; i++) bits[i]=(PIND&(1<<2))>>2; //bit read, port D.2

But you can safely assume that the falling edge is not clean, and write code to debounce it. There are many examples on line.

It could be the opto device and its ability to respond to a signal. How about a link to the opto device?

And a picture of your slotted wheel.

Paul

I haven't built the slotted wheel yet, I am just using a piece of card and doing it by hand.
Can't remember where I purchased the optical switch , there is a picture up above and it says "MH-Sensor-Series", that's all I know. I found this manual, looks like the same board to me: -
[IR Speed sensor.pdf (e-gizmo.net)](https://www.e-gizmo.net/oc/kits documents/IR Speed Sensor/IR Speed sensor.pdf)

lrussell101:
I haven't built the slotted wheel yet, I am just using a piece of card and doing it by hand.
Can't remember where I purchased the optical switch , there is a picture up above and it says "MH-Sensor-Series", that's all I know. I found this manual, looks like the same board to me: -
[IR Speed sensor.pdf (e-gizmo.net)](https://www.e-gizmo.net/oc/kits documents/IR Speed Sensor/IR Speed sensor.pdf)

The I submit you have been wasting our time.
Paul

I already said up above I haven't made the wheel yet. I am trying to get all of the components working before I complete the physical design/build. It will use a slotted wheel 3D printed in black PLA. For the moment I am simulating chopping the beam with a piece of card, I don't see how that can be considered time wasting.

See below for my implementation of JRemington's code (not sure if I got it right). I inspected the output in excel and found that I got a clean (sums to 1000) output every time I put the card in. The only way I could get a noisy edge was if I put the card in very slowly.

I also now have some other questions :-[
This code works but I don't understand why. The for loops are nested inside a while loop. shouldn't it immediately exit the for loop when Digital_in==0 ?
I was expecting that it would test the while loop conditions on every iteration of the for loop, or does it just test as follows? : -

while (condition) {
The condition was met so it will perform the first loop
For Loop Runs without interruption
Test While Loop condition and exit if not met
For Loop Runs without interruption
Test While Loop condition and exit if not met
}

I assume this code allows me to see everything that the Arduino see's on that pin for 1000 clock cycles, please let me know if I am misunderstanding?
Does digital read take up one full (1/16000000) clock cycle, or multiple clock cycles or small fractions of it?
I assume the16MHz clock is for the underlying electronic logic of the Atmel chip, so most calculations, loop iterations, functions will take up multiple clock cycles before the code moves on to the next step?

const byte Digital_in = 2;
byte bit_array[1000];

void setup() {
  Serial.begin(9600);
  pinMode(Digital_in, INPUT);
  Serial.println("Start");
}

void loop() {
  while (digitalRead(Digital_in) == HIGH) { //wait for first rising edge
    for (int i = 0; i < 1000; i++) bit_array[i] = digitalRead(Digital_in); //read input bit as fast as possible
    for (int i = 0; i < 1000; i++) Serial.println(bit_array[i]); //print the data
  }
}

lrussell101:
I already said up above I haven't made the wheel yet. I am trying to get all of the components working before I complete the physical design/build. It will use a slotted wheel 3D printed in black PLA. For the moment I am simulating chopping the beam with a piece of card, I don't see how that can be considered time wasting.

I know what you wrote, but that did not mean you were not using a different slotted wheel. Can I now guess you do not have a slotted wheel of any kind? Most are metal with etched or laser cut slots. Others are photographically produced on glass wheels and others are laser printed on clear plastic wheels. I doubt very much that you will be able to 3D print a satisfactory slotted wheel.
Paul

I assume this code allows me to see everything that the Arduino see's on that pin for 1000 clock cyclesNo. If coded correctly, the loop samples the pin once every 1-2 microseconds, for a few nanoseconds per loop cycle.

I got a clean (sums to 1000) output every time

That was not useful, because you did not copy the code correctly. You forgot the semicolon at the end of the while statement and instead used "{", in this line. Your loop samples ONLY when the pin is HIGH. That is why you got 1000 "1" bits.

 while (digitalRead(Digital_in) == HIGH) { //wait for first rising edge

The for loops are nested inside a while loop.

No. This single line of the original code is an empty loop that waits for a LOW, then proceeds.

while(digitalRead(input) == HIGH); //wait for first falling edge[/quit]

Cheers JRemington

I know there are some other solutions I need to follow above and I understand that we have basically already concluded beyond reasonable doubt that I have a noisy edge. But I would like to make sure I can run your code because it looks like a useful tool.

I think I understand now, the while loop will hold the processor in a loop until the first 0 (LOW).

I have had another go (code below). Now it gives me a constant stream of what looks like random digits (1-9) and I can't see anything changing as I put the card in. Can you give me a pointer to what I am doing wrong?

const byte input = 2;
byte bits[1000];

void setup() {
  Serial.begin(9600);
  pinMode(input, INPUT);
}

void loop() {
  byte bits[1000];
  while (digitalRead(input) == HIGH); //wait for first falling edge
  for (int i = 0; i < 1000; i++) bits[i] = digitalRead(input); //read input bit as fast as possible
  for (int i = 0; i < 1000; i++) Serial.print(i); //print the data
}

Cheers Paul

I had assumed I might need to consider something with a sharper edge than 3D printing, but I wanted to scope out the problem/solution space first. Also I wanted to make sure my double triggering wasn't a code error, I think I am now satisfied that it is a physical issue. I assume that those other slotted disk options might reduce the problem (a shorter duration noisy edge) but I guess I am likely to need some kind of debouncing whichever one I use.

Sorry, this time the mistake in the code was mine.

This line just prints the value of "i".

 for (int i = 0; i < 1000; i++) Serial.print(i); //print the data

Change it to

 for (int i = 0; i < 1000; i++) Serial.print(bits[i]); //print the data

Or for better readability, break the 1000 bit string into lines using something like this:

   int counter = 0;
   for (int i = 0; i < 1000; i++) {
      Serial.print(bits[i]); //print the data
      counter++;
     if (counter >= 50) {
         Serial.println();
         counter = 0;
        }
     }

OK, I should have noticed that. Now I can see why I thought it was random numbers, I expected it would cycle through 0-9 but it kept counting and concatenating (as it should): -
012345678910111213141516171819202122232425262728293031323334353637383940414243444546

I have made the alterations (code below for others benefit), it works now, cheers for that, I have a new tool.

I guess this is now showing me all of the digital reads on that pin (after the while loop "trigger") but I assume the only thing I need to be careful of is that this will be sampling at a frequency set by the for loop. Other bits of code will sample at different rates (most likely slower, so not an issue for this exercise).

Results: I could see noise after the rising edge but only if I go very slow with the card. So I am still a bit surprised that my Falling trigger ISR was triggering on the rising edge (card going in). I will go back and address the other issues and use this new tool.

const byte input = 2;

void setup() {
  Serial.begin(9600);
  pinMode(input, INPUT);
}

void loop() {
  byte bits[1000];
  while (digitalRead(input) == LOW); //wait for first rising edge. This while loop will keep the prcessor here until a 1 (HIGH) is received then the following for loops will run
  for (int i = 0; i < 1000; i++) bits[i] = digitalRead(input); //read input bit as fast as possible
  int counter = 0;
  for (int i = 0; i < 1000; i++) {
    Serial.print(bits[i]); //print the data
    counter++;
    if (counter >= 50) {
      Serial.println();
      counter = 0;
    }
  }
}

Here is my updated code (below), just in case anybody is using it as an example of de-bouncing an optical switch. I am also hoping that if anybody sees any errors or potential improvements they will highlight/edit.

/*
  Optical counter
  Reads state of optical switch and reports trigger time to serial port
  It looks like the digital output is the analogue output with a schmidt trigger applied for hysteresis (or debouncing)
*/

// PIN CONNECTIONS
// Connects to Digital pin and analogue "sensitivity" pin
const byte              Digital_in = 2;
const byte              Analogue_in = A0;


// CONSTANTS AND VARIABLES
unsigned long           lastDebounce_Time_Optical_switch_ISR = 0; 
volatile unsigned long  Trigger_Time_Optical_switch_ISR = 0; 
const unsigned long     debounceDelay_Optical_switch_ISR = 200;
int Analogue_val = 0; 


// SETUP LOOP
void setup() {
  // Setup serial coms
  Serial.begin(9600);

  // SET PIN MODES
  pinMode(Digital_in, INPUT);   
  pinMode(Analogue_in, INPUT);   
  attachInterrupt(digitalPinToInterrupt(Digital_in), Optical_switch_ISR, FALLING); 

}


// MAIN LOOP
void loop() {
  // Read the analogue signal from the optical switch
  Analogue_val = analogRead( Analogue_in);

  // Debounce the digital output from the optical switch
  if ((Trigger_Time_Optical_switch_ISR - lastDebounce_Time_Optical_switch_ISR) > debounceDelay_Optical_switch_ISR) {
    Serial.println(Trigger_Time_Optical_switch_ISR);
    lastDebounce_Time_Optical_switch_ISR = Trigger_Time_Optical_switch_ISR;
  }
}


//ISR
//Note: The ISR code should be as brief as possible to allow a speedy return to the main loop. Grab values in the ISR and perform calculations in the main loop
void Optical_switch_ISR() {
  Trigger_Time_Optical_switch_ISR = millis();   // The trigger time when the optical switch ISR was called. Note: millis() does not increment inside an ISR, so the value reported will be when the ISR was triggered
}