unwanted delay in photo interrupter circuit with LEDs and buzzer

Other Hardware LEDs and Multiplexing
1

I've got a circuit with two photo interrupters, a buzzer, an Arduino Nano, and two NeoPixel LEDs. Everything is powered by a 5V power supply feeding the breadboard. When one of the photo interrupter sensors is triggered, it lights one of the LEDs and triggers the buzzer, both for the duration of the sensor interruption. This all works great.

The problem:
When the sensor is NOT triggered, I'd like each of the LEDs to run a random sequence of colors, changing color ever 300 milliseconds. When I implement this random color cycle as the default setting, it introduces an unwanted delay time in how it responds to a sensor event: when the sensor is triggered, there is a delay before it shows the sensor color and tone.

I suspect the problem has to do with the "delay(300)" code in the random color generator, but don't know how to fix it. I think the delay is temporarily blocking the code from reading the sensor input. Any ideas on how to get around this?

Here's the code:

#include <Adafruit_NeoPixel.h>

// Which pin on the Arduino is connected to the NeoPixels?
#define LED_PIN 10

// How many NeoPixels are attached to the Arduino?
#define LED_COUNT 2

// Declare our NeoPixel strip object:
Adafruit_NeoPixel strip(LED_COUNT, LED_PIN, NEO_GRB + NEO_KHZ800);

// Time (in milliseconds) to pause between pixels
#define DELAYVAL 500

#define NOTE_GS4 415
#define NOTE_A4  440
#define NOTE_AS4 466
#define NOTE_B4  494
#define NOTE_C5  523
#define NOTE_CS5 554
#define NOTE_D5  587
#define NOTE_DS5 622
#define NOTE_E5  659
#define NOTE_F5  698
#define NOTE_FS5 740
#define NOTE_G5  784
#define NOTE_GS5 831
#define NOTE_A5  880
#define NOTE_AS5 932
#define NOTE_C6  1047

int val_1;
int val_2;
int speakerPin = 6;

void setup() {
  strip.begin();           // INITIALIZE NeoPixel strip object (REQUIRED)
  strip.show();            // Turn OFF all pixels ASAP
  strip.setBrightness(50); // Set BRIGHTNESS to about 1/5 (max = 255)
}

void loop() {  
  val_1 = (analogRead(A0));
  val_2 = (analogRead(A1));
  
if (val_1 < 100)
{    
  strip.setPixelColor(0, 200, 0, 200);
  strip.show();
  tone(speakerPin, NOTE_AS5);
  delay(500);
}

else
{
  strip.setPixelColor(random(0, 1), random(0, 255), random(0, 255), random(0, 255));
  strip.show();
  noTone(speakerPin);
  delay(500);
}

if (val_2 < 100)
{    
  strip.setPixelColor(1, 0, 200, 200);
  strip.show();
  tone(speakerPin, NOTE_C6);
  delay(500);
}

else
{
  strip.setPixelColor(random(1, 1), random(1, 255), random(1, 255), random(1, 255));
  strip.show();
  noTone(speakerPin);
  delay(500);
}

}
2

Have you considered using millis() instead of delay, for this project?

type into the search box "using millis". Your code lends its self to using millis()

3

I suspect the problem has to do with the "delay(300)" code in the random color generator

That and all the other delays!

These two tutorials should help you.
Using millis for timing
Demonstration for several things at the same time

++Karma; // For posting your code correctly on your first post and for realising that you need to get away from using delay();

4

Thanks guys. From my cursory research I can see that your suggestion to use millis could certainly solve this problem, as it unblocks the code. But I also see I have much to learn before I can implement successfully.

Any ideas what the if / else code might look like using millis instead of delay? It's clearly not just a question of replacing a few terms. Seems like it needs a different approach altogether.

I'm making my way through these tutorials, but in the meantime I'd be so grateful for any quick tips you care to offer.

Thomas

5

Any ideas what the if / else code might look like using millis instead of delay? It's clearly not just a question of replacing a few terms. Seems like it needs a different approach altogether.

Yes, it's a different approach. Once you get it then it will become natural to write all your code that way. Every instance of delay should be replaced by a millis state machine based approach, don't ask yourself if you can leave this or that delay in, they all have to go.

6

Think what a waiter does in a restaurant; s/he doesn't just serve you, s/he doesn't wait by your table in case you need something. A waiter does what one table needs, then checks to see if anyone else needs something and goes to do it if they do, otherwise they stand ready to go to any table that needs them. Pretty much any other job works like that, this is a human example of as state machine and I suspect you find it perfectly easy to understand. You just put similar ideas into code.

7

holdingpattern:
Timing issues aside, what's supposed to happen if both sensors are <100 and trying to play 2 different notes on the same buzzer? Or does the physical layout preclude that?

But then, apart from the fact it doesn't work :wink: (edit: or should I say, it has unintended consequences) is the purpose of this delay() here, when it is tripped:

if (val_1 < 100)

{ 
  strip.setPixelColor(0, 200, 0, 200);
  strip.show();
  tone(speakerPin, NOTE_AS5);
  delay(500); <<<<<<<<<<<<<<<<<<<
}




... to make sure the buzzer's on for at least a minimum time even if the sensor goes back above 100 very quickly?

You're absolutely right that both sensors being triggered simultaneously would be problematic for the buzzer, but fortunately the physical configuration of the circuit ensures that won't happen. This circuit is designed to translate the levers of a fully mechanical numeric keypad from an old calculator into digital signals. Only one button will be pushed at any given time.

Regarding the "delay(500)" line that you spotted: yes the idea was to make sure the LED and buzzer stay on long enough for the user to understand what's happening. My testing has shown that it's unnecessary, however, so I've since removed it from the code.

Thanks for your feedback!

8

holdingpattern:
... as below.

I wouldn't know a neopixel if I met one in my soup*, so the below is for 2 normal RGB leds, but I'm sure you'll see how and where to put your neopixel stuff. I didn't bother with the buzzer. I did though, assume that there's a requirement to stay triggered for a minimum time even if the the trigger is lost very quickly.

The whole thing is delay()-less, as evidenced by the blink-without-delay on the builtin led as a sort of proof-of-life. Both sensors respond immediately, independently of each other.

It will benefit from an array-based makeover, but that can be another discussion.

Thanks so much for this. In looking at the code I definitely feel that I've waded into the deep end of the pool without really knowing how to swim, but I suppose that's the fastest way to learn.

Your mention of an "array-based" makeover is interesting, as ultimately there will be 10 sensors and 10 LEDs (the circuit is designed to translate the movement of the physical levers from an old mechanical numeric keypad into digital signals). Where do you recommend I go to learn more about an array approach?

9

holdingpattern:
Tricky, with only 6 analog inputs...

The current circuit with Arduino Nano and 2 sensors is just for prototyping. I had planned to move to an Arduino Mega to accommodate my analog input needs, though I'm not thrilled about working with the large physical footprint of a Mega.

What other options are available that can accommodate 10 analog inputs?

10

What other options are available that can accommodate 10 analog inputs?

Putting a CD4051 multiplexer on one of the inputs you have and using the others directly.

tbird_forever:
In looking at the code I definitely feel that I've waded into the deep end of the pool without really knowing how to swim, but I suppose that's the fastest way to learn.

Or drown.

11

tbird_forever:
What other options are available that can accommodate 10 analog inputs?

The ProMicro (Leonardo/ATmega32U4), that is slightly smaller than a Nano has analog inputs on most pins. This means A0 to A11 is defined in Arduino.

12

Many thanks to all for the ideas and support. I implemented HoldingPattern's state machine example, adapting it for the neopixel configuration, and it worked beautifully.

holdingpattern:
Array version: so only one switch...case "engine" now.

This array code seems to be working for the LED portion of the circuit (I used "i" to address the individual LEDs in the LED strip—I hope that's not sloppy work), but having trouble making this produce different tones. I feel like I'm missing something easy:

// https://forum.arduino.cc/index.php?topic=668665
// 5 mar 2020

#include <Adafruit_NeoPixel.h>

// Which pin on the Arduino is connected to the NeoPixels?
#define LED_PIN 10

// How many NeoPixels are attached to the Arduino?
#define LED_COUNT 2


#define NOTE_1 415   //  GS4*
#define NOTE_2 466   //  AS4
#define NOTE_3 494   //  B4
#define NOTE_4 554   //  CS5
#define NOTE_5 622   //  DS5* 
#define NOTE_6 698   //  F5
#define NOTE_7 740   //  FS5  
#define NOTE_8 831   //  GS5*
#define NOTE_9 932   //  AS5*
#define NOTE_10 1047 //  NOTE_C6*


// Declare our NeoPixel strip object:
Adafruit_NeoPixel strip(LED_COUNT, LED_PIN, NEO_GRB + NEO_KHZ800);

//states
enum {ST1_idle, ST1_triggered} currentState1 = ST1_idle;
enum {ST2_idle, ST2_triggered} currentState2 = ST2_idle;
// ala https://www.gammon.com.au/statemachine

//pulse, to prove there's no blocking
unsigned long previousPulse;
bool pulseState;
int pulseInterval = 500;
int speakerPin = 6;


const byte sensors[] = {0, 1}; //A0 A1
const byte numberOfSensors = sizeof(sensors) / sizeof(sensors[0]);
const byte red[numberOfSensors] = {10};
const byte green[numberOfSensors] = {5, 10};
const byte blue[numberOfSensors] = {6, 11};

enum theStates {ST_idle, ST_triggered};
theStates currentState[numberOfSensors] = {ST_idle, ST_idle};

unsigned long lastBlink[numberOfSensors];
int blinkInterval[numberOfSensors] = {500, 500};
unsigned long triggeredAt[numberOfSensors];


   const byte redVals[numberOfSensors] = {255, 0}; //for when solid on
   const byte greenVals[numberOfSensors] = {0, 0};
   const byte blueVals[numberOfSensors] = {0, 255};

void setup()
{
  // initialize serial communication:
  Serial.begin(9600);
  Serial.print("setup() ... ");

  pinMode(LED_BUILTIN, OUTPUT);
  digitalWrite(LED_BUILTIN, pulseState);
  for (byte i = 0; i < numberOfSensors; i++)
  {
    pinMode(redVals[i], OUTPUT);
    pinMode(green[i], OUTPUT);
    pinMode(blue[i], OUTPUT);
  }

  Serial.println(" done");
  Serial.println(" ");
  for (byte i = 0; i < numberOfSensors; i++)
  {
    Serial.print(i);
    Serial.println(" is idle");
  }
}//setup


void loop()
{
  manageStates();
  doPulse();
} //loop



void manageStates()
{
  for (byte i = 0; i < numberOfSensors; i++)
  {
    int val = analogRead(i); //val's not need anywhere else, so local to manage states
    //if val *is* needed elsewhere, make an array val[numberOfSensors] as a global
    
    switch (currentState[i]) //ST_idle, ST_triggered
    {
      case ST_idle:
        if (millis() - lastBlink[i] >= blinkInterval[i])
        {
          lastBlink[i] = millis();
          strip.setPixelColor(random(i, 1), random(i, 155), random(i, 155), random(i, 155));
          strip.show();
          noTone(speakerPin);
        }

        if (val < 100)
        {
          currentState[i] = ST_triggered;
          Serial.print(i);
          Serial.println(" is triggered");
          triggeredAt[i] = millis();
        }

        break;

      case ST_triggered:
        strip.setPixelColor(i, 200, 0, 0);
        strip.show();
        tone(speakerPin, Note);

        if (val >= 100) //lost trigger
        {
          currentState[i] = ST_idle;
          Serial.print(i);
          Serial.println(" is idle");
        }
        break;

    }//switch i
  }//for
}//manageStates

void doPulse()
{
  if (millis() - previousPulse >= pulseInterval)
  {
    previousPulse = millis();
    pulseState = !pulseState;
    digitalWrite(LED_BUILTIN, pulseState);
  }
}//do pulse

Here's the line that's giving me trouble:

        tone(speakerPin, NOTE_(i));   <<<<<<<

Also worth mentioning that my serial monitor does not print "is triggered" when the sensor is triggered.

So regarding the serial monitor and the tone, what am I missing?

13

holdingpattern:
But your notes aren't in an array of NOTE_(i)'s... (would be[i]not (i) btw)

Your NOTE_1 and NOTE_4 etc are discrete items not in an array.

You need to mimic this:

const byte sensors[] = {0, 1}; //A0 A1

... for the notes.

I see what you mean. I've altered the code to include a 10 note array and 10 sensors. It doesn't work yet, and I'm sure it's connected to a persistent question: if "i" is effectively the number of a sensor, and we use "i" to trigger the corresponding LED, why can't we also use "i" to trigger the corresponding note? Heres the code:

// https://forum.arduino.cc/index.php?topic=668665
// 5 mar 2020

#include <Adafruit_NeoPixel.h>

// Which pin on the Arduino is connected to the NeoPixels?
#define LED_PIN 10

// How many NeoPixels are attached to the Arduino?
#define LED_COUNT 2


#define NOTE_0 415   //  GS4*
#define NOTE_1 466   //  AS4
#define NOTE_2 494   //  B4
#define NOTE_3 554   //  CS5
#define NOTE_4 622   //  DS5* 
#define NOTE_5 698   //  F5
#define NOTE_6 740   //  FS5  
#define NOTE_7 831   //  GS5*
#define NOTE_8 932   //  AS5*
#define NOTE_9 1047 //  NOTE_C6*


// Declare our NeoPixel strip object:
Adafruit_NeoPixel strip(LED_COUNT, LED_PIN, NEO_GRB + NEO_KHZ800);

//states
enum {ST1_idle, ST1_triggered} currentState1 = ST1_idle;
enum {ST2_idle, ST2_triggered} currentState2 = ST2_idle;
// ala https://www.gammon.com.au/statemachine

//pulse, to prove there's no blocking
unsigned long previousPulse;
bool pulseState;
int pulseInterval = 500;



const byte sensors[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; //A0 A1 et al.
const byte numberOfSensors = sizeof(sensors) / sizeof(sensors[0]);
const byte red[numberOfSensors] = {10};
const byte green[numberOfSensors] = {5, 10};
const byte blue[numberOfSensors] = {6, 11};

// tones
int notes[] = {NOTE_0, NOTE_1, NOTE_2, NOTE_3, NOTE_4, NOTE_5, NOTE_6, NOTE_7, NOTE_8, NOTE_9}; 
int speakerPin = 6;

enum theStates {ST_idle, ST_triggered};
theStates currentState[numberOfSensors] = {ST_idle, ST_idle};

unsigned long lastBlink[numberOfSensors];
int blinkInterval[numberOfSensors] = {500, 500};
unsigned long triggeredAt[numberOfSensors];


const byte redVals[numberOfSensors] = {255, 0}; //for when solid on


void setup()
{
  // initialize serial communication:
  Serial.begin(9600);
  Serial.print("setup() ... ");

  pinMode(LED_BUILTIN, OUTPUT);
  digitalWrite(LED_BUILTIN, pulseState);
  for (byte i = 0; i < numberOfSensors; i++)
  {
    pinMode(redVals[i], OUTPUT);
    pinMode(green[i], OUTPUT);
    pinMode(blue[i], OUTPUT);
    pinMode(speakerPin, OUTPUT);
  }


  Serial.println(" done");
  Serial.println(" ");
  for (byte i = 0; i < numberOfSensors; i++)
  {
    Serial.print(i);
    Serial.println(" is idle");
  }
}//setup


void loop()
{
  manageStates();
  doPulse();
} //loop



void manageStates()
{
  for (byte i = 0; i < numberOfSensors; i++)
  {
    int val = analogRead(i); //val's not need anywhere else, so local to manage states
    //if val *is* needed elsewhere, make an array val[numberOfSensors] as a global
    
    switch (currentState[i]) //ST_idle, ST_triggered
    {
      case ST_idle:
        if (millis() - lastBlink[i] >= blinkInterval[i])
        {
          lastBlink[i] = millis();
          strip.setPixelColor(random(i, 1), random(i, 155), random(i, 155), random(i, 155));
          strip.show();
          noTone(speakerPin);
        }

        if (val < 100)
        {
          currentState[i] = ST_triggered;
          Serial.print(i);
          Serial.println(" is triggered");
          triggeredAt[i] = millis();
        }

        break;

      case ST_triggered:
        strip.setPixelColor(i, 200, 0, 0);
        strip.show();
        tone(speakerPin, notes[i]);

        if (val >= 100) //lost trigger
        {
          currentState[i] = ST_idle;
          Serial.print(i);
          Serial.println(" is idle");
          
        }
        break;

    }//switch i
  }//for
}//manageStates

void doPulse()
{
  if (millis() - previousPulse >= pulseInterval)
  {
    previousPulse = millis();
    pulseState = !pulseState;
    digitalWrite(LED_BUILTIN, pulseState);
  }
}//do pulse
14

It seems to me that you never give the tone any chance to play anything. Code is quick and you race through all the tones in significant less time than it takes to play a fraction of a cycle.

15

Grumpy_Mike:
It seems to me that you never give the tone any chance to play anything. Code is quick and you race through all the tones in significant less time than it takes to play a fraction of a cycle.

There should only be one tone playing at a time, and only when a sensor is triggered. Each sensor should activate its own dedicated LED and unique tone frequency. Clearly I've missed the mark on this. :slight_smile:

16

The current circuit with Arduino Nano

But this code is written for a Mega, there aren't this Jenny analogue sensors on a nano.

const byte sensors[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; //A0 A1 et al.
17

Grumpy_Mike:
But this code is written for a Mega, there aren't this Jenny analogue sensors on a nano.

const byte sensors[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; //A0 A1 et al.

Yes that's correct. My prototype only has a few LEDs at the moment, but that should be enough to get a sense of whether the code is working. The LEDs are behaving well, but the buzzer/tone is a mess. Sounds like it's trying to play many different notes, continuously.

18

Sounds like it's trying to play many different notes, continuously.

That is because it is. To see put a delay of 300 mS after you set the tone going.

19

holdingpattern:
FWIW, the below is my code from a few days ago (with normal rgb leds, not fancy stuff), with the tones added.

Works brilliantly. Thank you!

Now on to a prototype with the full sensor array, and subsequent levels of code complexity as I attempt to scale functionality.