Weird arduino bug, using microphone and relay

Question

I made a clap light switch to control my room lights and ground LEDs, and it works perfectly but when ever I have both the LEDs and the lights on it kinda stops working. I use two claps for the lights and three for the LEDs. Does anyone know what could be wrong? here is my code.

/* DEFINE PINS */
const int buzzer = 2;
const int micPin = 0;
const int relayLED = 9;
const int relayLights = 8;
const int sampleWindow = 50;
const int N = 50;
unsigned int sample;
boolean lights = false; //lighs on or off
boolean leds = false; //leds on or off
double soundArray[N]; //array to check claps
int i = 0;
int clapsCount = 0;
void setup() {
 pinMode(relayLED, OUTPUT);
 pinMode(relayLights, OUTPUT);
 digitalWrite(relayLED, HIGH);
 digitalWrite(relayLights, HIGH);
}
void loop() {
 /* gets 50 constant sound sample and resets */
 if (i<N) {
 soundArray[i] = getVolts(); 
 i++;
 } else {
 i = 0;
 }
 if (i == N-1) {
 clapsCount = clapCount(soundArray);
 }
 /* if 2 claps detected do this */
 if (clapsCount == 2) {
 if (lights == false) {
 turnOnLights();
 lights = true;
 } else {
 turnOffLights();
 lights = false;
 }
 playSound();
 clapsCount = 0;
 }
 /* if 3 claps detected do this */
 if (clapsCount == 3) {
 if (leds == false) {
 turnOnLeds();
 leds = true;
 } else {
 turnOffLeds();
 leds = false;
 }
 playSound();
 clapsCount = 0;
 }
}
int clapCount(double soundArray[]) {
 int claps = 0;
 for (int j=0; j<N; j++) {
 if (soundArray[j] > 3.0 && soundArray[j] < 3.5) {
 claps++;
 }
 }
 return claps;
}
void turnOnLights() {
 digitalWrite(relayLights, LOW);
}
void turnOffLights() {
 digitalWrite(relayLights, HIGH);
}
void turnOnLeds() {
 digitalWrite(relayLED, LOW);
}
void turnOffLeds() {
 digitalWrite(relayLED, HIGH);
}
/* gets mic volts */
double getVolts() {
 unsigned long startMillis= millis(); // Start of sample window
 unsigned int peakToPeak = 0; // peak-to-peak level
 unsigned int signalMax = 0;
 unsigned int signalMin = 1024;
 // collect data for 50 mS
 while (millis() - startMillis < sampleWindow)
 {
 sample = analogRead(micPin);
 if (sample < 1024) {
 if (sample > signalMax) {
 signalMax = sample; // save just the max levels
 }
 else if (sample < signalMin) {
 signalMin = sample; // save just the min levels
 }
 }
 }
 peakToPeak = signalMax - signalMin; // max - min = peak-peak amplitude
 double volts = (peakToPeak * 5.0) / 1024; // convert to volts
 return volts;
}
void playSound() {
 tone(buzzer, 1000);
 delay(100);
 tone(buzzer, 500);
 delay(100);
 noTone(buzzer);
}

Edit:
When I do both two claps and three claps, both lights turn on and no matter how many times I clap none of them turn back off.

Edit 2:
Components I am using are, a simple buzzer, and microphone called MAX4466 and a 5v relay with two outputs.
enter image description here enter image description here

• per1234: Please edit your question to explain exactly what you mean by "kinda stops working".
• Matthew: @per1234 edited now, sorry about that.
• Jasmine: We need to know what these devices are and how they are connected. The code listing has some issues, but we can't do a good answer without the hardware info. Like, are you using an actual microphone, condenser, piezo, what? I suspect your method of identifying a clap isn't working how you think it is.
• user31481: I read your code and looks well written at first sight. Not having the hardware, I can elaborate on the physical aspects of your problems. It can be how your are sampling or counting. Put some Serial.print in getVolts and clapCount to test that they detect and count correctly. You can also use an hand filled soundArray to check the logic.
• Chris Stratton: This scheme of clap counting does not seem sufficiently though through to work at all - first, you evaluate in a buffer and then throw everything away, so you'd have to luck out and get all your claps in a buffer, and not some in an old one that gets throw away and the rest in the next. But more importantly, you don't seem to ever implement anything that looks for a sharp sound and then a gap. You basically need to start over with a plausible algorithm - you need to count sounds with intervening gaps, and then act only when theres been a long enough gap to mean no more will follow.

Answer 1

I've looked through your code and can't see anything obvious that is causing the problem, so I have refactored it a bit in the hope that the problem becomes more obvious.

/* DEFINE PINS */
const int PinBuzzer = 2;
const int PinMic = 0;
const int PinRelayLED = 9;
const int PinRelayLights = 8;
const int sampleWindow = 50;
const int MaxSamples = 50;
// Simple enumeration to clarify which state is on and off.
enum EOnOff { Off, On };
EOnOff lights = Off; //lights on or off
EOnOff leds = Off; //leds on or off
double soundArray[MaxSamples]; //array to check claps
void setup() 
{
 pinMode(PinRelayLED, OUTPUT);
 pinMode(PinRelayLights, OUTPUT);
 digitalWrite(PinRelayLED, ConvertOnOff(leds));
 digitalWrite(PinRelayLights, ConvertOnOff(lights));
}
void loop() 
{
 for (int sample = 0; sample < MaxSamples; ++sample)
 { // Get the samples
 soundArray[sample] = getVolts();
 }
 switch (clapCount())
 { // Analyse the samples
 case 2:
 lights = ChangeRelay(lights, PinRelayLights);
 playSound();
 break;
 case 3:
 leds = ChangeRelay(leds, PinRelayLEDs);
 playSound();
 break;
 }
}
/// Count the number of 'claps' in the sample data
int clapCount(/*clap array is global*/) 
{
 int claps = 0;
 for (int sample = 0; sample < MaxSample; ++sample) 
 {
 if (soundArray[sample] > 3.0 && soundArray[sample] < 3.5) /* Why 3.5?? */
 {
 ++claps;
 }
 }
 return claps;
}
/// Convert an EOnOff value to a HIGH or LOW value.
// OFF = HIGH
int ConvertOnOff(const EOnOff& state)
{
 return state == Off ? HIGH : LOW;
}
/// Change a relay to the given state.
/// Return the new state.
EOnOff ControlRelay(const EOnOff& state, const int& relayPin)
{
 EOnOff result = state == On ? Off : On;
 digitalWrite(relayPin, ConvertOnOff (result));
 return result;
}
/* gets mic volts */
double getVolts() 
{
 const unsigned long startMillis = millis(); // Start of sample window
 unsigned int signalMax = 0;
 unsigned int signalMin = 1024;
 unsigned int sample;
 // collect data for 50 mS
 while (millis() - startMillis < sampleWindow)
 {
 sample = analogRead(PinMic);
 if (sample < 1024) // ?? Should this be 1024 or signalMin?
 {
 if (sample > signalMax) 
 {
 signalMax = sample; // save just the max levels
 }
 else if (sample < signalMin) 
 {
 signalMin = sample; // save just the min levels
 }
 }
 }
 const unsigned int peakToPeak = signalMax - signalMin; // max - min = peak-peak amplitude
 const double volts = (peakToPeak * 5.0) / 1024.0; // convert to volts
 return volts;
}
void playSound() 
{
 tone(buzzer, 1000);
 delay(100);
 tone(buzzer, 500);
 delay(100);
 noTone(buzzer);
}

What I have tried to do is to reduce the number of lines of code. If there are less lines then there should be less bugs :)

You used a bool to record the on off state, I changed that to an enumeration because that adds a bit of clarity, and when working with mains voltages I like things being clear.

I changed your while loop into a for loop, I think it looks cleaner. This was also the reason I changed your if statements into a switch statement.

The guts of the if statements have been removed and placed in a function ChangeRelay(), it returns the new state of leds or lights. I could have gone with a reference to make state an in out parameter, but decided a return was simpler.

ClapCount() and GetVolts() are relatively unchanged apart from a bit of const'ing of variables to make things easier to read. Also you might have noticed I, j and N have all been removed, because single letter variable names are my nemesis, who knows what q means and why r and t are being added to it to make z?

ControlRelay lets you route all you relay switching through one function, if there was a copy and paste bug in your code (which I don't think there was) then they will either all work or not work now.

There is a big difference between "my" code and yours, you won't be able to switch both on at the same time because the switch statement won't let you. So in a way you problem is solved. If you really wanted to you could add:

case 5: 
 lights = ChangeRelay(lights, PinRelayLights);
 leds = ChangeRelay(leds, PinRelayLEDs);
 playSound();
 break;

• Matthew: Thank you for making the code better! I tried it and it works just like mine, but the bug still happens, I am not sure if it is a limitation of the relay, if both lights are on it does not turn off no matter what. And yeah I know the single letter variables are bad, I normally use only single variables like i, j, k for the for loops, but since I was just testing I got some weird ones with single letter too.
• Matthew: I tried using the serial read to type 2 or 3 as simulation of the claps and it works perfectly fine, got both on, turn off each at a time all perfect, but when ever it is claps it doesnt seem to work, maybe my algorithm to get the claps is bad.
• Code Gorilla: @Matthew - In getVolts the else if should be just an if. Otherwise you need two samples to adjust the max and min to the correct values.