/* 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
const unsigned int signalMax = 0;
const 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);
}
/* 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
const unsigned int signalMax = 0;
const 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);
}
/* 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);
}
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
const unsigned int signalMax = 0;
const 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;
lang-cpp