Need help with basic code problem

I'm trying to build a basic program that allows me to run through a series of colors on an RGB led while holding down a push button. The program works fine...once. It will run through all the colors in the switch statement, and then just go black. I want it to loop back and run through the switch statements as long is the input pin reads HIGH. If someone would be willing to take the time to show me what I'm doing wrong, I would really appreciate it.

int redLED = 9;
int greenLED = 10;
int blueLED = 11;
int inputPin = 2;
int buttonCounter = 0;
int buttonState = 0;

void setup() {
  pinMode(inputPin, INPUT);
  pinMode(redLED, OUTPUT);
  pinMode(greenLED, OUTPUT);
  pinMode(blueLED, OUTPUT);
}

void loop() {
  buttonState = digitalRead(inputPin);

  if (buttonState == HIGH ) {
    buttonCounter++;
    switch (buttonCounter) {
    case 1:
      analogWrite(redLED, 255);
      analogWrite(greenLED, 255);
      analogWrite(blueLED, 255);
      break;
    case 2:
      analogWrite(redLED, 255);
      analogWrite(greenLED, 0);
      analogWrite(blueLED, 0);
      break;
    case 3:
      analogWrite(redLED, 0);
      analogWrite(greenLED, 255);
      analogWrite(blueLED, 0);
      break;
    case 4:
      analogWrite(redLED, 0);
      analogWrite(greenLED, 0);
      analogWrite(blueLED, 255);
      break;
    case 5:
      analogWrite(redLED, 255);
      analogWrite(greenLED, 255);
      analogWrite(blueLED, 0);
      break;
    case 6:
      analogWrite(redLED, 255);
      analogWrite(greenLED, 0);
      analogWrite(blueLED, 255);
      break;
    case 7:
      analogWrite(redLED, 0);
      analogWrite(greenLED, 255);
      analogWrite(blueLED, 255);
      break;
    default:
      analogWrite(redLED, 0);
      analogWrite(greenLED, 0);
      analogWrite(blueLED, 0);
      buttonCounter == 0;
      break;
    }
  }
  delay(300);
}

PS. I took out the break at the end of default with no change.

After you increment buttonCounter, you might want to check to see if it's greater than 7 and set it back to 0 so it goes through your cases again.

You probably want code to handle the 0 case, too.

-br

Im only new here.. but

Can you use 'while' for this instead of your 'if' statement?

eg

while (buttonstate == high) ;
buttoncounter++ ;

Well… you can use a while, but it will act a little strange (the state will increment extremely fast, so the effect will be to choose a random state.)

Couple issues with the code as presented, though:

  1. As written that code won't compile because HIGH has to be uppercase.

  2. If you don't digitalRead the button pin inside the loop, how is the value of buttonState ever going to change?

-br

I will cop the HIGH stuffup.

But I wasnt going to retype the whole code. hence why I didnt have the Digital read bit. Just the line I was referring to, plus the next so it made sense.

I didnt know about the problems with the while statement. Ill add while to my 'must learn' list.

Thanks for all the feedback, guys.

Thruan:
Im only new here.. but

Can you use 'while' for this instead of your 'if' statement?

eg

while (buttonstate == high) ;
buttoncounter++ ;

Will give this a try later, and I know what you meant with "high". Thanks for weighing in.

billroy:
If you don't digitalRead the button pin inside the loop, how is the value of buttonState ever going to change?

Isn't that the purpose of "buttonState = digitalRead(inputPin);" (first line inside the loop). If not, please let me know. Obviously I'm a newbster.

billroy:
After you increment buttonCounter, you might want to check to see if it's greater than 7 and set it back to 0 so it goes through your cases again.

You probably want code to handle the 0 case, too.

-br

It was my understanding that the "default" at the end of the switch statement handled both of these problems. Perhaps my understanding of switch statements is flawed. If you don't mind, please take a look at the default section of the code and let me know why it wouldn't handle the two problems you listed. And thanks for weighing in.

@Thuran: I tried your suggestion of using a while over an if loop, and for reasons beyond me, it did not work. I would have thought that by moving the delay up into the loop, it would have solved the issue Billroy pointed out, but apparently not. Regardless, thanks for the suggestion.

First a suggestion: put a Serial.println(buttonCounter); in the loop. Then using the serial monitor you would quickly see that the counter is not being reset and after the first pass the switch value is beyond the possible case choices. This is because your counter is a global and is set outside of your loop, never to be reset in your code, after you increment it. You could move the counter into your loop and reset it each pass or leave it global and reset it each pass. As it is it unless you reset it the counter would have to go until it wrapped around before you would get another series of colors, a very long time. Serial.print is your friend.

turbosnail:
First a suggestion: put a Serial.println(buttonCounter); in the loop. Then using the serial monitor you would quickly see that the counter is not being reset and after the first pass the switch value is beyond the possible case choices. This is because your counter is a global and is set outside of your loop, never to be reset in your code, after you increment it. You could move the counter into your loop and reset it each pass or leave it global and reset it each pass. As it is it unless you reset it the counter would have to go until it wrapped around before you would get another series of colors, a very long time. Serial.print is your friend.

Thanks, Turbo. I was unaware of the Serial.println();, and can definitely see how it will be helpful in the future.

Though I think I understand what you're saying the problem is, after much tinkering I have been unable to fix it. Would you mind showing an example of how to reset the global counter?

You never set buttonCounter back to zero.

It was my understanding that the "default" at the end of the switch statement handled both of these problems. Perhaps my understanding of switch statements is flawed. If you don't mind, please take a look at the default section of the code and let me know why it wouldn't handle the two problems you listed. And thanks for weighing in.

Umm no.

In your "default" case at the end of the switch statement, you appear to have buttonCounter == 0.

You use double equals in an if statement for testing equality.

You use single equals in an assignment statement, which is what you should be doing here.

buttonCounter = 0 ;

NOT buttonCounter == 0 ; !

michinyon:
You never set buttonCounter back to zero.

It was my understanding that the "default" at the end of the switch statement handled both of these problems. Perhaps my understanding of switch statements is flawed. If you don't mind, please take a look at the default section of the code and let me know why it wouldn't handle the two problems you listed. And thanks for weighing in.

Umm no.

In your "default" case at the end of the switch statement, you appear to have buttonCounter == 0.

You use double equals in an if statement for testing equality.

You use single equals in an assignment statement, which is what you should be doing here.

buttonCounter = 0 ;

NOT buttonCounter == 0 ; !

Ha! Right, you are. That fixed it. I figured it had to be something stupid like that, but for the life of me I couldn't see it. Thanks very much for your time, sir.

Thanks to everyone for their time and suggestions.