LED fade sequences, 2 buttons

[system]

Hi everyone!
Im new to using Arduino and need to create a sequence of changing patterns in an LED array using 2 buttons, fading and timing.

I need to have pin 5 fade in on the 1st press, then pin 6 also fade in (both pins 5 & 6 high) on the 2nd press, then the both pins fade out on the 3rd button press. The program doesn't like my else statement, coming 'before' an if:

if (buttonPushCounter % 3 == 1) {
// fade in from min to max in increments of 5 points:
for(int fadeValue = 0 ; fadeValue <= 255; fadeValue +=5) {
// sets the value (range from 0 to 255):
analogWrite(ledPin_wht_dim, fadeValue);
// wait for 30 milliseconds to see the dimming effect
delay(30);
} else {
digitalWrite(ledPin_wht_dim, LOW);
digitalWrite(ledPin_wht_bright, LOW);
}

Im guessing its probably something quite simple, I'd be really grateful for any help you can offer me!

Best regards

[earldbjr]

Post the whole code please

And welcome to arduino.cc!

[system]

Also, as a beginner, it is helpful to put the { on the next line, as well as the else, and line up the braces:

if(buttonPushCounter % 3 == 1)
{
   // Do something
}
else
{
   // Do something different
}

This way, an else with an if is easy to spot.

[system]

Hey thanks for the welcome SimbaSpirit!

I took your advice PaulS and rearranged the else lines, thanx!
Here's the whole code:

// these constants won't change:
const int buttonPin_wht = 2; // the pin that the pushbutton is attached to
const int ledPin_wht_dim = 5; // the pin that the white (dim) LED is attached to
const int ledPin_wht_bright = 6; // the pin that the white (bright) LED is attached to

// Variables will change:
int buttonPushCounter = 0; // counter for the number of button presses
int buttonState = 0; // current state of the button
int lastButtonState = 0; // previous state of the button

void setup() {
// initialize the button pin as a input:
pinMode(buttonPin_wht, INPUT);
// initialize the digital pin as an output:
pinMode(ledPin_wht_dim, OUTPUT);
// initialize the digital pin as an output:
pinMode(ledPin_wht_bright, OUTPUT);
// initialize serial communication:
Serial.begin(9600);
}

void loop() {
// read the pushbutton input pin:
buttonState = digitalRead(buttonPin_wht);
// compare the buttonState to its previous state
if (buttonState != lastButtonState) {
// if the state has changed, increment the counter
if (buttonState == HIGH) {
// if the current state is HIGH then the button
// went from off to on:
buttonPushCounter++;
Serial.println("off");
Serial.print("number of button pushes: ");
Serial.println(buttonPushCounter, DEC);
}
else
{
// if the current state is LOW then the button
// went from on to off:
Serial.println("on");
}

// save the current state as the last state,
//for next time through the loop
lastButtonState = buttonState;

}

// turns on the LEDs on the 1st button push then reduces the array by
// checking the modulo of the button push counter.
// the modulo function gives you the remainder of
// the division of two numbers:
if (buttonPushCounter % 3 == 0) {
digitalWrite(ledPin_wht_dim, LOW);
digitalWrite(ledPin_wht_bright, LOW);
}
else
{
digitalWrite(ledPin_wht_dim, LOW);
digitalWrite(ledPin_wht_bright, LOW);
}

if (buttonPushCounter % 3 == 1) {
// fade in from min to max in increments of 5 points:
for(int fadeValue = 0 ; fadeValue <= 255; fadeValue +=5) {
// sets the value (range from 0 to 255):
analogWrite(ledPin_wht_dim, fadeValue);
// wait for 30 milliseconds to see the dimming effect
delay(30);
}
else
{
digitalWrite(ledPin_wht_dim, LOW);
digitalWrite(ledPin_wht_bright, LOW);
}

if (buttonPushCounter % 3 == 2) {
// fade in from min to max in increments of 5 points:
for(int fadeValue = 0 ; fadeValue <= 255; fadeValue +=5) {
// sets the value (range from 0 to 255):
analogWrite(ledPin_wht_bright, fadeValue);
// wait for 30 milliseconds to see the dimming effect
delay(30);
digitalWrite(ledPin_wht_dim, HIGH);
digitalWrite(ledPin_wht_bright, HIGH);
}
else
{
digitalWrite(ledPin_wht_dim, LOW);
digitalWrite(ledPin_wht_bright, LOW);
}

if (buttonPushCounter % 3 == 3) {
// fade out from max to min in increments of 5 points:
for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5) {
// sets the value (range from 0 to 255):
analogWrite(ledPin, fadeValue);
// wait for 30 milliseconds to see the dimming effect
delay(30);
analogWrite(ledPin2, fadeValue);
// wait for 30 milliseconds to see the dimming effect
delay(30);
}
else
{
digitalWrite(ledPin_wht_dim, LOW);
digitalWrite(ledPin_wht_bright, LOW);
}

}

Thanks guys!

[system]

if (buttonPushCounter % 3 == 3) {

This will never evaluate to true.

[system]

Good point, thanks. I changed the modulo to 4 instead. Any ideas on why my 'else' error keeps occurring? Also how do I add my code in a grey box like you did?

[system]

Also how do I add my code in a grey box like you did?

On the top row is a button with a # on it. Before pasting the code, click that button.

Or, after pasting it, select all of it, and then click the button.

It adds tags that tell the forum software to generate different output (the gray background and scroll bars) for the block of text.

Any ideas on why my 'else' error keeps occurring?

Yes, but I think you can find it yourself. Pretty easily, in fact. Do like I suggested earlier. Put the { after the if on a separate line. Indent all the code between the { and the } by the same amount. It will be very obvious where the problem is when the if's are not nested correctly:

if(this == that)
{
   if(here > there)
   {
   }
   else
   {
   }
}
else
{
}

See how easy it is to see the whole nested if? See how easy it is to see that blocks open and close correctly?

This is the same code, without the indenting and braces on the same line as the if:

if(this == that){
if(here > there){
}else{
}
}else{
}

Much harder to see which } goes with which {, and whether all block are complete and in the proper order.

The compiler doesn't care, but human readers appreciate the difference.

A hint, though. Look at this block of code in particular.

if (buttonPushCounter % 3 == 3) {
  // fade out from max to min in increments of 5 points:
 for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5) {
   // sets the value (range from 0 to 255):
   analogWrite(ledPin, fadeValue);        
   // wait for 30 milliseconds to see the dimming effect    
   delay(30);
   analogWrite(ledPin2, fadeValue);        
   // wait for 30 milliseconds to see the dimming effect    
   delay(30);
 }
 else
 {
  digitalWrite(ledPin_wht_dim, LOW);
  digitalWrite(ledPin_wht_bright, LOW);
 }
 
}

Match the braces...

[system]

Hey PaulS, thanks so much fort your help I can see that my code had some extra open braces, and I also forgot to change the variable names in the last section. The last section of code is not working though, it won't fade... Here's the amended code:

 */

// these constants won't change:
const int  buttonPin_wht = 2;    // the pin that the pushbutton is attached to
const int ledPin_wht_dim = 5;       // the pin that the white (dim) LED is attached to
const int ledPin_wht_bright = 6;       // the pin that the white (bright) LED is attached to

// Variables will change:
int buttonPushCounter = 0;   // counter for the number of button presses
int buttonState = 0;         // current state of the button
int lastButtonState = 0;     // previous state of the button

void setup() {
  // initialize the button pin as a input:
  pinMode(buttonPin_wht, INPUT); 
  // initialize the digital pin as an output:
  pinMode(ledPin_wht_dim, OUTPUT);
  // initialize the digital pin as an output:
  pinMode(ledPin_wht_bright, OUTPUT);  
  // initialize serial communication:
  Serial.begin(9600);
}

void loop() {  
  // read the pushbutton input pin:
  buttonState = digitalRead(buttonPin_wht);
  // compare the buttonState to its previous state
  if (buttonState != lastButtonState) 
  {
  // if the state has changed, increment the counter
  if (buttonState == HIGH) 
  {
  // if the current state is HIGH then the button
  // went from off to on:
      buttonPushCounter++;
      Serial.println("off");
      Serial.print("number of button pushes:  ");
      Serial.println(buttonPushCounter, DEC);
    } 
    else 
    {
      // if the current state is LOW then the button
      // went from on to off:
      Serial.println("on"); 
    }

    // save the current state as the last state, 
    //for next time through the loop
    lastButtonState = buttonState;
    
  }
  
  // turns on the LEDs on the 1st button push then reduces the array by 
  // checking the modulo of the button push counter.
  // the modulo function gives you the remainder of 
  // the division of two numbers:
  if (buttonPushCounter % 4 == 0) 
  {
   digitalWrite(ledPin_wht_dim, LOW);
   digitalWrite(ledPin_wht_bright, LOW);
  } 
  else 
  {
   digitalWrite(ledPin_wht_dim, LOW);
   digitalWrite(ledPin_wht_bright, LOW);
  }
  
  if (buttonPushCounter % 4 == 1) 
  {
  // fade in from min to max in increments of 5 points:
  for(int fadeValue = 0 ; fadeValue <= 255; fadeValue +=5) 
    // sets the value (range from 0 to 255):
    analogWrite(ledPin_wht_dim, fadeValue);         
    // wait for 30 milliseconds to see the dimming effect    
    delay(30); 
  } 
  else
  {
   digitalWrite(ledPin_wht_dim, LOW);
   digitalWrite(ledPin_wht_bright, LOW);
  }
 
  if (buttonPushCounter % 4 == 2) 
  {
  // fade in from min to max in increments of 5 points:
  for(int fadeValue = 0 ; fadeValue <= 255; fadeValue +=5)
    // sets the value (range from 0 to 255):
    analogWrite(ledPin_wht_bright, fadeValue);         
    // wait for 30 milliseconds to see the dimming effect    
    delay(30);
   digitalWrite(ledPin_wht_dim, HIGH);
   digitalWrite(ledPin_wht_bright, HIGH);
  } 
  else 
  {
   digitalWrite(ledPin_wht_dim, LOW);
   digitalWrite(ledPin_wht_bright, LOW);
  }

  if (buttonPushCounter % 4 == 3) 
  {
   // fade out from max to min in increments of 5 points:
  for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5)
    // sets the value (range from 0 to 255):
    analogWrite(ledPin_wht_dim, fadeValue);         
    // wait for 30 milliseconds to see the dimming effect    
    delay(30);
    // fade out from max to min in increments of 5 points:
  for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5)
    // sets the value (range from 0 to 255): 
    analogWrite(ledPin_wht_bright, fadeValue);         
    // wait for 30 milliseconds to see the dimming effect    
    delay(30);
  } 
  else 
  {
   digitalWrite(ledPin_wht_dim, LOW);
   digitalWrite(ledPin_wht_bright, LOW);
  }
  
}

Do you think its because I left some digital command in the script?

[system]

  if (buttonPushCounter % 4 == 0)
  {
   digitalWrite(ledPin_wht_dim, LOW);
   digitalWrite(ledPin_wht_bright, LOW);
  }
  else
  {
   digitalWrite(ledPin_wht_dim, LOW);
   digitalWrite(ledPin_wht_bright, LOW);
  }

If the button push counter value is 0, turn both lights off. Otherwise, turn both lights off.

I think I missed something there.

  if (buttonPushCounter % 4 == 0)
  if (buttonPushCounter % 4 == 1)
  if (buttonPushCounter % 4 == 2)
  if (buttonPushCounter % 4 == 3)

Only one of these conditions can be true. You could use a switch statement, instead.

switch(buttonPushCounter % 4)
{
   case 0:
     // Do something
     break;
   case 1:
     // Do something
     break;
   case 2:
     // Do something
     break;
   case 3:
     // Do something
     break;
}

I think, though, that the fundamental problem is that you don't know how to code.

Start with a simple sketch:

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

void loop()
{
   Serial.println("Hey, there...");
   delay(1000);
}

Upload this, open the serial monitor window, and verify that it works.

Then, add code to detect if the button was pushed. Just print "Pressed" or "Not pressed".

Upload, and verify that it works. Then, add the code to compare the current state to the previous state, and only print something when the state changes. Verify.

Then, add the button press counting code. Print the number of times the button was pressed, when the button is pressed. Verify.

Then, create a new sketch to test the operation of the LEDs. Write some code to fade the LEDs like you want to have happen for one button press. Verify that that works.

Then, write some code to fade the LEDs like you want to have happen for two button presses. Verify.

Then, for 3 and for 4. Verify after each addition.

When the LED fading is working, merge the two scripts. Since you know that each one works independently, and you know what each part does, merging should be pretty easy.

If you want to just skip to the chase, though, analyze this code, and explain what it is doing:

  for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5)
    // sets the value (range from 0 to 255):
    analogWrite(ledPin_wht_dim, fadeValue);
    // wait for 30 milliseconds to see the dimming effect
    delay(30);

[system]

Thanks, before you replied I had a look at my code and realised it didnt make sense ;D so I started cutting and changing bits of it (mainly removing the else loops) to try to fix it.

Thanks for your reply, yes I am new to coding and I've never used the serial monitor feature before.

I looked at the code in my script (same as you quoted) and i guess its fading the led on but in 30 millisecond loops. I'm guessing i need to delete the loop so it does the fade just once. I'll try your suggestions for restarting the code now and let you know how I get on. Thanks again!

[system]

I looked at the code in my script (same as you quoted) and i guess its fading the led on but in 30 millisecond loops.

The for loop does not have any curly braces, so it runs the LED down from full on to not on, with no delay (so fast that you can't see a change). Then, it pauses for 30 milliseconds. Oops, you blinked and missed the LED being off.

The next for loop does not have any curly braces, either, so, it runs the LED up from full off to full on, and again pauses for 30 milliseconds.

Try adding some curly braces to enclose the commands that are supposed to be part of the loop. Indenting alone does not make the statement (delay()) part of the for loop.

[system]

Hey, I'm still working in it, but here's where I am so far:

 */

// these constants won't change:
const int  buttonPin_wht = 2;    // the pin that the pushbutton is attached to
const int ledPin_wht_dim = 5;       // the pin that the white (dim) LED is attached to
const int ledPin_wht_bright = 6;       // the pin that the white (bright) LED is attached to

// Variables will change:
int buttonPushCounter = 0;   // counter for the number of button presses
int buttonState = 0;         // current state of the button
int lastButtonState = 0;     // previous state of the button

void setup() {
  // initialize the button pin as a input:
  pinMode(buttonPin_wht, INPUT); 
  // initialize the digital pin as an output:
  pinMode(ledPin_wht_dim, OUTPUT);
  // initialize the digital pin as an output:
  pinMode(ledPin_wht_bright, OUTPUT);  
  // initialize serial communication:
  Serial.begin(9600);
}

void loop() {  
  // read the pushbutton input pin:
  buttonState = digitalRead(buttonPin_wht);
  // compare the buttonState to its previous state
  if (buttonState != lastButtonState) 
  {
  // if the state has changed, increment the counter
  if (buttonState == HIGH) 
  {
  // if the current state is HIGH then the button
  // went from off to on:
      buttonPushCounter++;
      Serial.println("off");
      Serial.print("number of button pushes:  ");
      Serial.println(buttonPushCounter, DEC);
    } 
    else 
    {
      // if the current state is LOW then the button
      // went from on to off:
      Serial.println("on"); 
    }

    // save the current state as the last state, 
    //for next time through the loop
    lastButtonState = buttonState;
    
  }
  
  // turns on the LEDs on the 1st button push then reduces the array by 
  // checking the modulo of the button push counter.
  // the modulo function gives you the remainder of 
  // the division of two numbers:
  if (buttonPushCounter % 3 == 0) 
   {
    digitalWrite(ledPin_wht_dim, LOW);
    digitalWrite(ledPin_wht_bright, LOW);
  }
  
  if (buttonPushCounter % 4 == 1) 
  {
  // fade in from min to max in increments of 5 points:
  for(int fadeValue = 0 ; fadeValue <= 255; fadeValue +=5) 
    {
    // sets the value (range from 0 to 255):
    analogWrite(ledPin_wht_dim, fadeValue);
    // wait for 30 milliseconds to see the dimming effect    
    delay(30);
    }
  }
  
  if (buttonPushCounter % 4 == 2) 
  {
  // fade in from min to max in increments of 5 points:
  for(int fadeValue = 0 ; fadeValue <= 255; fadeValue +=5)
    {
    // sets the value (range from 0 to 255):
    analogWrite(ledPin_wht_bright, fadeValue);
    // wait for 30 milliseconds to see the dimming effect    
    delay(30);    
    }
  }  
  if (buttonPushCounter % 4 == 3) 
   {
  // fade out from max to min in increments of 5 points:
  for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5)
    // sets the value (range from 0 to 255):
    analogWrite(ledPin_wht_dim, fadeValue);         
    // wait for 30 milliseconds to see the dimming effect    
    delay(30);
    // fade out from max to min in increments of 5 points:
    for(int fadeValue = 255 ; fadeValue >= 0; fadeValue -=5)
    // sets the value (range from 0 to 255): 
    analogWrite(ledPin_wht_bright, fadeValue);         
    // wait for 30 milliseconds to see the dimming effect    
    delay(30);
  }

}

I did something before this and both buttons worked well, but the fade is looping still. However when I run this code (above) the button presses are non responsive, unless I'm in the serial monitor. I'm not sure why and wonder if I should start this whole code again, maybe with the switch statements? :-/
I'd be grateful for your thoughts.

[system]

I did something before this and both buttons worked well, but the fade is looping still.

Why don't you start be describing (to us) what you want to have happen. Something like this:

Each time the button is pushed, I want to...
The first time, I want to...
The second time, I want to...

Your code and your comments (and print statements) do not match:

  if (buttonState == HIGH)
  {
  // if the current state is HIGH then the button
  // went from off to on:
      buttonPushCounter++;
      Serial.println("off");

Does HIGH represent pressed? Or, does it represent released?

if (buttonPushCounter % 3 == 0)

All the other cases use 4...

[system]

Hey thanks for your reply, I just noticed my modulo 3 and changed it, cheers.
I'm trying to get the sequence to change each time the button is pressed rather than released, so it responds straight away. Here's what I'm trying to do:
0 presses = all leds remain off.
press 1 = fade ON ledPin_wht_dim
press 2 = fade ON ledPin_wht_bright, and keep ledPin_wht_dim on too.
press 3 = fade OFF both ledPin_wht_dim and ledPin_wht_bright.

Then restart the loop from press 1. I wasn't sure if I should have a modulo 4 or 3, because I didnt know if it would be OK to start the leds with the fade out statement, to ensure they are off at the start.

[system]

So, 4 presses - same as 1, 5 presses - same as 2, etc.?

If so, then:

if(buttonPressCount == 0)
{
   // Everybody off (this is the default, so this test could be omitted
}
else if((buttonPressCount % 3 + 1) == 1)
{
   // Do the 1, 4, 7... thing
}
else if((buttonPressCount % 3 + 1) == 2)
{
   // Do the 2, 5, 8... thing
}
else if((buttonPressCount % 3 + 1) == 3)
{
   // Do the 3, 6, 9... thing
}

Since you have delays in the fade block, you will not get an immediate response to a button press.

You could get around this in a couple of ways. One would be to use interrupts. When a button is pressed, an interrupt handler would be called. This could set a flag that the for loops would check on each iteration.

The other would involve the removal of the delay calls, but, for this application, that would get very complicated.

[system]

Hey Paul, thanks for the example, incrementing the modulo is a much better idea!
Ill give it a try and see what I can do with the interrupts tomorrow night. I realise delays may generally be a problem (but forgot in the case of fading the leds :S) because, my end goal is to have the whole sequence on a 15 minute timer, with audible alerts at 5 min intervals. Each button press would, start/restart the countdown timer.

I am aware this is tricky, so I just want to get the led light sequence right first. Thanks again for your help, ill post back tomorrow! Have a great day