multiple relays not triggering correctly

Question

quick description of the sketch: its a clap on/off relay that turns on only when it's dark.

Cannot seem to get the relays to stay off. What happens is they are off by default then when I do the double clap to turn them on that works, after this I double clap to turn them off and then go off then immediately go on again. I can get a single relay to work perfectly but when I uncomment the rest there seems to be a problem.

I am assuming it has something to do with the delay for the clap sensor, but I am not entirely sure-- I have tried quite a few things to try to resolve it but am really lost as to why this might be happening. ideas?

int soundSensor = A2;
int relay = A0;
int relay2 = A3;
int relay3 = A4;
int relay4 = A5;
int claps = 0;
long detectionSpanInitial = 0;
long detectionSpan = 0;
boolean clapState = false;
int doubleClap = 0;


//Light sensor 
int photocellPin = A1;     
int photocellReading;     
int LEDbrightness;       

//int delayValue = 100;
int photoSensor = 0;

void setup() {
  pinMode(soundSensor, INPUT);
  pinMode(relay, OUTPUT);
  pinMode(relay2, OUTPUT);
  pinMode(relay3, OUTPUT);
  pinMode(relay4, OUTPUT);
  Serial.begin(9600);
}

void loop() {

  /////////////
  // photo sensor
  //////////////////// 
  photocellReading = analogRead(photocellPin);  
  photocellReading = 1023 - photocellReading;
  LEDbrightness = map(photocellReading, 0, 1023, 0, 255);

  if(photocellReading > 500){
      photoSensor = 0;
  }
  if(photocellReading < 500){
    photoSensor = 1;
  }

  /////////////
  // clap sensor
  /////////////////////
  int sensorState = digitalRead(soundSensor);
  //erial.println(sensorState);
  if (sensorState == 1)
  {
    if (claps == 0)
    {
      detectionSpanInitial = detectionSpan = millis();
      claps++;
    }
    else if (claps > 0 && millis()-detectionSpan >= 50)
    {
      detectionSpan = millis();
      claps++;
    }
  }

  if (millis()-detectionSpanInitial >= 500)
  {
    if (claps == 2)
    {
      if (doubleClap == 0)
        {
          doubleClap = 1;
        }
        else if (doubleClap == 1)
        {
          doubleClap = 0;
        }
    }
    claps = 0;
  }

  if(doubleClap == 1 && photoSensor == 0){ 
    digitalWrite(relay, LOW);
    digitalWrite(relay2, LOW);
    digitalWrite(relay3, LOW);
    digitalWrite(relay4, LOW);
    Serial.println("off");

  }
  else{ 
    digitalWrite(relay, HIGH);
    digitalWrite(relay2, HIGH);
    digitalWrite(relay3, HIGH);
    digitalWrite(relay4, HIGH);
    Serial.println("on");
  };


}

Answer 1

As it happens, you are debugging in the dark. You have half a dozen devices attached to (presumably) some Arduino, driven by some hard-to-read code that might not work properly even if all the devices work ok, and definitely won't work properly if any of them fail or are incorrectly attached.

For example, you can't tell from that code whether your photoSensor works, whether your clap sensor works, or whether your relays work. Before messing around further trying to test all at once, run some definitive tests on each device individually. For example, run the following sketch and see if your system is able to turn on all of the relays at the same time.

// Test if relays work ok: First individually for a second each,
// then all together for 2.5 seconds
enum {relay=A0, relay2=A3, relay3=A4, relay4=A5, nRelays=4};
byte rePins[] = { relay, relay2, relay3, relay4};

void setup() {
  for (int r=0; r<nRelays; ++r) {
    pinMode(rePins[r], OUTPUT);    // Set pin to outputs
    digitalWrite(rePins[r], LOW);  // Turn off relay
  }
  Serial.begin(115200);
}

void loop() {
  byte r;
  // Turn relays on individually for a second each, with half-second gaps
  for (r=0; r<nRelays; ++r) {
    delay(500);         // Wait half a second
    Serial.print("Turning on relay #");
    Serial.print(r+1);
    Serial.print(" at ");
    Serial.println(millis());
    digitalWrite(rePins[r], HIGH); // Turn on relay
    delay(1000);           // One on for a second
    digitalWrite(rePins[r], LOW);  // Turn off relay
  }

  // Turn all the relays on
  Serial.print("Turning all relays on at ");
  Serial.println(millis());
  for (r=0; r<nRelays; ++r)
    digitalWrite(rePins[r], HIGH); // Turn on relay

  delay(2500);          // Wait 2.5 seconds

  // Turn all the relays off
  Serial.print("Turning all relays off at ");
  Serial.println(millis());
  for (r=0; r<nRelays; ++r)
    digitalWrite(rePins[r], LOW); // Turn off relay
}

Next, write a sketch to report results from photosensor and clap sensor readings. Once you know if your hardware is working, try to get your double-clap logic to work without involving the photosensor, or vice versa. Also, try to make the double-clap logic work just using a button in place of the clap sensor.

Note, to shorten your code and to get rid of some of the features that make it hard to read, do some of the following changes. With the code slightly shorter, it may be easier to comprehend and debug.

(a) Instead of introducing program constants as integer variables, introduce them as program constants. For example, instead of the verbose

int relay = A0;
int relay2 = A3;
int relay3 = A4;
int relay4 = A5;

say something like

enum {relay=A0, relay2=A3, relay3=A4, relay4=A5};

which tells the compiler that relay is a constant with the value A0, etc.

(b) Since you don't use variable LEDbrightness for anything, take it out. With that gone, change the verbose

photocellReading = analogRead(photocellPin);  
photocellReading = 1023 - photocellReading;
LEDbrightness = map(photocellReading, 0, 1023, 0, 255);

if(photocellReading > 500){
    photoSensor = 0;
}
if(photocellReading < 500){
  photoSensor = 1;
}

to

photoSensor = analogRead(photocellPin) > 523;

which will set photoSensor to 0 or 1 much as before. (It is slightly different: It handles a photocellReading==500 case that your code ignores.)

(c) Change the verbose and badly formatted

  if (doubleClap == 0)
    {
      doubleClap = 1;
    }
    else if (doubleClap == 1)
    {
      doubleClap = 0;
    }

to

 doubleClap = 1 - doubleClap;

(d) Refactor to cut about half the code involving detectionSpan.