Can't get this switch / case sketch to work. Any ideas?

[PlasticOne]

The program was suppose to run an RGB led with a lot of different functions off of one button but it was to many to cycle throw so i thought i could add a second button to brake down the functions making it easier to use so one button cycle the colours whilst the other dealt with the colours functions but iv made a but of a mess any help would be greatly appreciated. A small sample of the code

Code: [Select]

const int redledPin = 2;
const int greenledPin = 1;
const int blueledPin = 0;
const int buttonPin = 3;
const int buttonPin2 = 4;
int RGB=1;
int B2=2;

int prevUp = HIGH;
int prevDn = HIGH;

void setup ()
{
  pinMode (redledPin, OUTPUT);
  pinMode (greenledPin, OUTPUT);
  pinMode (blueledPin, OUTPUT);
  pinMode (buttonPin, INPUT);
  }

void loop ()
{
int currUp = digitalRead(buttonPin2);

   if(currUp != prevUp)
   {
      delay(5);

      currUp = digitalRead(buttonPin2);

      if(currUp != prevUp)
      {
         if(currUp == LOW)
            B2++;
      }
   }
 
   switch (B2)
     case 1:{
  {
int currUp = digitalRead(buttonPin);

   if(currUp != prevUp)
   {
      delay(5);

      currUp = digitalRead(buttonPin);

      if(currUp != prevUp)
      {
         if(currUp == LOW)
            RGB++;
      }
   }
 
   switch (RGB){
    case 1:
    analogWrite(redledPin, 0);
    analogWrite(blueledPin, 250);
    analogWrite(greenledPin, 0);
    delay(200);
    analogWrite(redledPin, 0);
    analogWrite(blueledPin, 250);
    analogWrite(greenledPin, 0);
    delay(100);
    break;
   
    case 2:
    analogWrite(redledPin, 0);
    analogWrite(blueledPin, 250);
    analogWrite(greenledPin, 0);
    break;
   
   }
    prevUp = currUp;
}
break;

  case 2:{
int currUp = digitalRead(buttonPin);

   if(currUp != prevUp)
   {
      delay(5);

      currUp = digitalRead(buttonPin);

      if(currUp != prevUp)
      {
         if(currUp == LOW)
            RGB++;
      }
   }
 
   switch (RGB){
    case 1:
    analogWrite(redledPin, 0);
    analogWrite(blueledPin, 0);
    analogWrite(greenledPin, 250);
    delay(200);
    analogWrite(redledPin, 0);
    analogWrite(blueledPin, 0);
    analogWrite(greenledPin, 0);
    delay(100);
    break;
   
    case 2:
    analogWrite(redledPin, 0);
    analogWrite(blueledPin, 0);
    analogWrite(greenledPin, 250);
   
   }
    prevUp = currUp;
  }break;
   }
}

[Arrch]

Start by putting any curly brace on its own line and using the auto format tool (In the Arduino IDE, under Tools) to make the code more readable. The seemingly random indentation is distracting.

[MaJiG]

You have a curly brace out of place, methinks

Code: [Select]

   
   switch (B2)
     case 1:{
  {
int currUp = digitalRead(buttonPin);


should be

Code: [Select]


   switch (B2)
   {
     case 1:
     {
         int currUp = digitalRead(buttonPin);


you may need to match closing brackets.

The curly brackets after the "case 1:" isn't necessary, btw.

[UKHeliBob]

Code: [Select]

  switch (B2)
  case 1:
  {
    {
should be

Code: [Select]

  switch (B2)
  {
  case 1:
    {

[Grumpy_Mike]

You can't define a variable inside a switch statement. Define that int outside first and then assign it a value inside the switch.

[PaulS]

You can't define a variable inside a switch statement. Define that int outside first and then assign it a value inside the switch.

Actually, you can, if the variable is defined in a block. That is why each case starts and ends with { and },

[PlasticOne]

Still no joy : / thanks for the help anyway guys : ) is there an easier way to achieve a switch case within a switch case only using two inputs?

[PeterH]

is there an easier way to achieve a switch case within a switch case only using two inputs?

There's nothing difficult about using one switch statement inside another. I have no idea whether it's a sensible solution for whatever your problem is, but it's straight forward to code. However, you do need to get the syntax right and put your curly brackets in the right place and so on - just as with any other code.

[nickgammon]

Still no joy : / thanks for the help anyway guys : ) is there an easier way to achieve a switch case within a switch case only using two inputs?

Show your amended code, thanks.

You might want to put stuff into functions, I can't make out what you are trying to do.

[nickgammon]

Code: [Select]

int B2=2;

...

      if(currUp == LOW)
        B2++;

...
  switch (B2)

B2 is going to keep going up and up, right?

[PlasticOne]

Ok so i scraped the first code redid a lot of it and came up with this layout still not working but i think its in beter shape than the first

Code: [Select]

const int redledPin = 2;
const int greenledPin = 1;
const int blueledPin = 0;
const int buttonPin = 3;
const int buttonPin2 = 4;
int RGB=1;
int RGB2=2;

int prevUp = HIGH;
int prevDn = HIGH;

void setup ()
{
  pinMode (redledPin, OUTPUT);
  pinMode (greenledPin, OUTPUT);
  pinMode (blueledPin, OUTPUT);
  pinMode (buttonPin, INPUT);
  pinMode (buttonPin2, INPUT);
  }

void loop ()
{
int currUp = digitalRead(buttonPin && buttonPin2);

   if(currUp != prevUp)
   {
      delay(25);

      currUp = digitalRead(buttonPin && buttonPin2);


      if(currUp != prevUp)
      {
         if(currUp == LOW)
            RGB++ && RGB2;

      }
   }
   
  switch (RGB){
    case 1:
   switch (RGB2){
    case 1:
    analogWrite(redledPin, 0);
    analogWrite(blueledPin, 250);
    analogWrite(greenledPin, 0);
    break;
     
    case 2:
    analogWrite(redledPin, 0);
    analogWrite(blueledPin, 0);
    analogWrite(greenledPin, 0);
    break;   
   
    case 3:
    analogWrite(redledPin, 250);
    analogWrite(blueledPin, 250);
    analogWrite(greenledPin, 250);
    break;
   
    case 4:
    analogWrite(redledPin, 250);
    analogWrite(blueledPin, 0);
    analogWrite(greenledPin, 250);
    break;
   
    case 5:
    analogWrite(redledPin, 0);
    analogWrite(blueledPin, 250);
    analogWrite(greenledPin, 250);
    break;
  }
    prevUp = currUp;
   
    case 2:
    switch (RGB2){
    case 1:
    analogWrite(redledPin, 0);
    analogWrite(blueledPin, 250);
    analogWrite(greenledPin, 0);
    break;
     
    case 2:
    analogWrite(redledPin, 0);
    analogWrite(blueledPin, 0);
    analogWrite(greenledPin, 0);
    break;   
   
    case 3:
    analogWrite(redledPin, 250);
    analogWrite(blueledPin, 0);
    analogWrite(greenledPin, 0);
    break;
   
    case 4:
    analogWrite(redledPin, 0);
    analogWrite(blueledPin, 0);
    analogWrite(greenledPin, 250);
    break;
   
    case 5:
    analogWrite(redledPin, 0);
    analogWrite(blueledPin, 250);
    analogWrite(greenledPin, 0);
    break;
  }
    prevUp = currUp;
  }
}

[PaulS]

Code: [Select]

     currUp = digitalRead(buttonPin && buttonPin2);
In English, what is this supposed to be doing? It isn't, but I haven't any idea how to change it, because I have no idea what you think it does.

Code: [Select]

            RGB++ && RGB2;
And this?