Using Switch Case and Cleaning up Code

So I have been working on a project with my friend to get his drum hit to light up to the hits of his drums on the high tom, low tom, and kick. I started with napkin code and ended up making this mess of a code. It works but looks UGLY. Apologies for the lack of comments, I was doing this pretty quickly.

One of my friends told me to I could use “switch case” to do this instead, but when I did the code broke. I have since gone back and forked the code to be useful again.

After I got to about 80 lines Arduino told me that my colors weren’t declared in scope when I put the in a separate tab in a new .ino so I ended up throwing all the “voids” for colors into the main code. I would have placed them below but I still get the same “ColorName is not declared in scope”. So I moved them up behind the void loop to declare them before the code runs.

Like I said before everything works, it’s just all a mess and I know it can be way simpler and cleaner, any help is much appreciated.

int hit = 100;
int t =  1;
int c = 100;
int x = 0;
int randomColor = 1;
int P1 = A1;
int P2 = A2;
int P3 = A3;
int Rpin = 10;
int Gpin = 11;
int Bpin = 9;
int piezo = 0;

void setup() {
  // put your setup code here, to run once:
  pinMode(P1, INPUT); //1023 is max possible value
  pinMode(12, OUTPUT);
  pinMode(11, OUTPUT);
  pinMode(10, OUTPUT);
  //Serial.begin(9600);
}

void off() {
  analogWrite(Rpin, 0);
  analogWrite(Gpin, 0);
  analogWrite(Bpin, 0);
}

void red() {
  if (piezo > hit) {
    analogWrite(Rpin, piezo);
    analogWrite(Gpin, 0);
    analogWrite(Bpin, 0);
  }
  if (piezo < hit) {
    off();
  }
  piezo = map((analogRead(P1) + analogRead(P2) + (analogRead(P3))) / 3, 0, 1000, 0, 255);
}

void blue() {
  analogWrite(Rpin, 0);
  analogWrite(Gpin, piezo);
  analogWrite(Bpin, 0);
  if (piezo < hit) {
    off();
  }
  piezo = map((analogRead(P1) + analogRead(P2) + (analogRead(P3))) / 3, 0, 1000, 0, 255);
}
void green() {
  analogWrite(Rpin, 0);
  analogWrite(Gpin, 0);
  analogWrite(Bpin, piezo);
  if (piezo < hit) {
    off();
  }
  piezo = map((analogRead(P1) + analogRead(P2) + (analogRead(P3))) / 3, 0, 1000, 0, 255);
}

void purple() {
  analogWrite(Rpin, piezo);
  analogWrite(Gpin, piezo);
  analogWrite(Bpin, 0);
  if (piezo < hit) {
    off();
  }
  piezo = map((analogRead(P1) + analogRead(P2) + (analogRead(P3))) / 3, 0, 1000, 0, 255);
}
void yellow() {
  analogWrite(Rpin, piezo);
  analogWrite(Gpin, 0);
  analogWrite(Bpin, piezo);
  if (piezo < hit) {
    off();
  }
  piezo = map((analogRead(P1) + analogRead(P2) + (analogRead(P3))) / 3, 0, 1000, 0, 255);
}

void cyan() {
  analogWrite(Rpin, 0);
  analogWrite(Gpin, piezo);
  analogWrite(Bpin, piezo);
  if (piezo < hit) {
    off();
  }
  piezo = map((analogRead(P1) + analogRead(P2) + (analogRead(P3))) / 3, 0, 1000, 0, 255);
}
void white() {
  analogWrite(Rpin, piezo);
  analogWrite(Gpin, piezo);
  analogWrite(Bpin, piezo);
  if (piezo < hit) {
    off();
  }
  piezo = map((analogRead(P1) + analogRead(P2) + (analogRead(P3))) / 3, 0, 1000, 0, 255);
}

void loop() {
  // put your main code here, to run repeatedly:
  piezo = map((analogRead(P1) + analogRead(P2) + (analogRead(P3))) / 3, 0, 1000, 0, 255);
  randomColor = random(1, 7);
  if (piezo > hit && randomColor == 1) {
    while (x < c) {
      red();
      delay(t);
      if (piezo > hit) {
        x++;
      }
    }
    randomColor++;
    x = 0;
  }
  if (piezo > hit && randomColor == 2) {
    while (x < c) {
      green();
      delay(t);
      if (piezo > hit) {
        x++;
      }
    }
    randomColor++;
    x = 0;
  }
  if (piezo > hit && randomColor == 3) {
    while (x < c) {
      blue();
      delay(t);
      if (piezo > hit) {
        x++;
      }
    }
    randomColor++;
    x = 0;
  }
  if (piezo > hit && randomColor == 4) {
    while (x < c) {
      cyan();
      delay(t);
      if (piezo > hit) {
        x++;
      }
    }
    randomColor++;
    x = 0;
  }
  if (piezo > hit && randomColor == 5) {
    while (x < c) {
      purple();
      delay(t);
      if (piezo > hit) {
        x++;
      }
    }
    randomColor++;
    x = 0;
  }
  if (piezo > hit && randomColor == 6) {
    while (x < c) {
      yellow();
      delay(t);
      if (piezo > hit) {
        x++;
      }
    }
    randomColor++;
    x = 0;
  }
  if (piezo > hit && randomColor == 7) {
    while (x < c) {
      white();
      delay(t);
      if (piezo > hit) {
        x++;
      }
    }
    randomColor = 1;
    x = 0;
  }
  if (piezo < hit) {
    off();
  }
}

You could get rid of some repetition by creating a function which contains the repeating parts of your colour handling routines. Here is an example which doesn’t change the structure of your code too much:

void colourBase(int rValue, int gValue, int bValue) {
  analogWrite(Rpin, rValue);
  analogWrite(Gpin, gValue);
  analogWrite(Bpin, bValue);
  if (piezo < hit) {
    off();
  }
  piezo = map((analogRead(P1) + analogRead(P2) + (analogRead(P3))) / 3, 0, 1000, 0, 255);
}

void white() {
   colourBase( piezo, piezo, piezo ) ;
}

void cyan() {
   colourBase( 0 , piezo, piezo ) ;
}

// etc. for all colours

yes in progamming anytime you find yourself rewriting code there is generally a more profficient way to write it.

if you think about color there are 3 channels you are working with. this is 8 possibilitys of on/off. but you dont want black so that is 7 possabilitys.

red,green,blue,yellow,cyan,magenta and white.

so generate a random number between 0 and 7 plus one. adding one eliminates black when you break it dowm.

now you can use a little binary math (powers of 2) to break your random number into the three on/offs to represent your individual channels and apply them to your color pins.

lookup binary conversion if you dont understand the math

here is a loop example that sets all 3 of your pins A9 and up. and also does the binary math at the same time to set each pins value to produce the random color.

anyways this bit of math will make your code much more efficient

void randomcolor(){
  
  
    byte power = 4;
    byte r = random(7)+1;
    byte i = 3;
    while(i>0){i--;
    if(r>=power){r-=power;
    analogWrite(A9+i,peizo);
    }else{ analogWrite(A9+i,0);}
    power=byte(power*.5);
   
    }}