Advice on case or if then or statement

Can someone please recommend the best way to code for a setup I am trying to create.

I have an Arduino mega that I am going to connect a heap of push buttons to, which will then pass serial code to an MP3 trigger board and play a specific track. I have got the Arduino working for one input, but I am looking to make it work from about 20 different inputs.

#define LED 13   // the pin for the LED
#define BUTTON 22 // the input pin where the
                 // pushbutton is connected
int val = 0;     // val will be used to store the state
                 // of the input pin 
int track= 10;

void setup() {
  Serial.begin(38400);    //Configure Serial port
  pinMode(LED, OUTPUT);   // tell Arduino LED is an output
  pinMode(BUTTON, INPUT); // and BUTTON is an input
}

void loop(){
  val = digitalRead(BUTTON); // read input value and store it

  // check whether the input is HIGH (button pressed)
  if (val == HIGH) { 
    digitalWrite(LED, HIGH); // turn LED ON
    Serial.write('t');    //Send Input to MP3 trigger
    Serial.write(10);

     delay(1000);
    
  } else {
    digitalWrite(LED, LOW);
  }
}

Is it better to code with a heap of digital read statements then if val1 = high then, or if val2....., val3.... etc
or is it possible to do this neater with a case satement?

Appreciate the help.

Maxx

A switch statement is useful if one variable can take on a variety of values (1, 2, 3, 47, 93, -119, etc.). It is of no use if a variety of variables can take on one of two values.

You will need to use a multitude of if/else if statements.

maxx:
I have an Arduino mega that I am going to connect a heap of push buttons to, which will then pass serial code to an MP3 trigger board and play a specific track. I have got the Arduino working for one input, but I am looking to make it work from about 20 different inputs.

You don't want a cascade of if/then or a switch/case statement. You want arrays and loops.

static const unsigned char buttons[] = {2, 3, 7, 8, 9, 15, 23 (etc)...];  // List of input buttons
static const unsigned char lights[] = [4, 5, 6, 10, 11, 12, 13 (etc)...];  // List of associated lights
for (ing i=0; i< (sizeof(buttons)/sizeof(buttons[0])); i++)
    {
    if (digitalRead(buttons[i]) == HIGH)
        {
        digitalWrite(lights[i], HIGH);
        }
    else
        {
        digitalWrite(lights[i], LOW);
        }
//  actually digitalWrite(lights[i],digitalRead(buttons[i]); does the same as that whole IF statement
    }

So, for everything that is different between different buttons (in this example, the button pin and the pin of the matching light) make an array. Fill the arrays. Use the FOR loop to go through the array and act on each button. In the example the action is "light the corresponding LED as long as the button is pressed". If you want the action "play an MP3 trigger", make a list of the triggers to play for each button. Then play "trigger*" if the button is down.*

johnwasser:
You don't want a cascade of if/then or a switch/case statement. You want arrays and loops.

This is true as long as you generally want to do the same kind of thing with each button. In the OP's case, he wanted to trigger MP3 tracks, so that would work. You'd have an array of tracks for each button, and then queue that one. You'd just have to deal with the case of multiple buttons being pressed.

If you're doing DIFFERENT KINDS of things with each button, then a series of if/then's is preferrable just for readability. You could stuff them into uint16_t and then switch based on that, but I wouldn't recommend it.

Thanks gents for your replies. I have it (kinda) working with a case statement but will try with the arrays and loop method, if nothing else to improve my skills with the Arduino. Appreciate the feedback

Had a go at doing an array but I have tripped up and I am sure it will be something obvious to the educated.
When compiling, I am getting an error message that sizeOf cannot be used as a function In function'void loop ()':

I have read through the array example but to no avail. Would appreciate some guidence.

Code below.

int buttons [] = {22, 23, 24, 25, 26, 27, 28, 29, 30, 31 ,32, 33, 32, 33, 34, 35 ,36};  // List of input buttons 
int sizeOf = 17; // number of buttons - length of the array;
int val;
#define LED 13   // the pin for the LED

void setup () {

Serial.begin(38400);    //Configure Serial port
for (int val=0; val< (sizeOf(buttons)/sizeOf(buttons[0])); val++);
  pinMode(LED, OUTPUT);   // tell Arduino LED is an output
}

void loop() {
  
        if (digitalRead(buttons[val]) == HIGH) //Check for button press
        {
          digitalWrite(LED, HIGH); // turn LED ON to confirm input
        Serial.write('t');    //Send Input to MP3 trigger
        Serial.write(buttons[val]); //Send MP3 number to trigger

     delay(1000);

        }
        else {
    digitalWrite(LED, LOW);
   
}

sizeOf => sizeof

OMG I see you actually have a variable called sizeOf

Make that something like N_BUTTONS

const int N_BUTTONS = (sizeof(buttons)/sizeof(buttons[0]));

then

for (int val=0; val < N_BUTTONS; val++);


Rob

Graynomad:
OMG I see you actually have a variable called sizeOf

I think the words you are looking for are...."Noob!" lol
Yeah a bit more than green with regards to coding. Done a lot of electronics but coding is totally new to me. Thanks will give it a go.

maxx:
Had a go at doing an array but I have tripped up and I am sure it will be something obvious to the educated.
When compiling, I am getting an error message that sizeOf cannot be used as a function In function'void loop ()':

And then when you get through that, you may notice another problem... I wouldn't expect the sketch as-written to provide the results you want. By the time you get to loop(), 'val' is still zero. So it'll work for your first button, but not the others. You need a loop in your loop() also, e.g.

for (int val=0; val< N_BUTTONS; val++)
{
        if (digitalRead(buttons[val]) == HIGH) //Check for button press
        {
                digitalWrite(LED, HIGH); // turn LED ON to confirm input
                Serial.write('t');    //Send Input to MP3 trigger
                Serial.write(buttons[val]); //Send MP3 number to trigger
                delay(1000);
                break;
        }
        else 
        {
                digitalWrite(LED, LOW);
        }
}

You'll notice I also added a 'break', so the first button you get will be the last one you process, which I think is consistent with what you want.

for (int val=0; val< (sizeOf(buttons)/sizeOf(buttons[0])); val++);

Remove the semicolon at the end of that line. As written, this means "Loop through times, and do nothing whatsoever.

int val;

No need for this at the top of the sketch. You declare val inline in your 'for' loop, and I also do it later in my example.

maniacbug:

maxx:
Had a go at doing an array but I have tripped up and I am sure it will be something obvious to the educated.
When compiling, I am getting an error message that sizeOf cannot be used as a function In function'void loop ()':

And then when you get through that, you may notice another problem... I wouldn't expect the sketch as-written to provide the results you want. By the time you get to loop(), 'val' is still zero. So it'll work for your first button, but not the others. You need a loop in your loop() also, e.g.

for (int val=0; val< N_BUTTONS; val++)

{
        if (digitalRead(buttons[val]) == HIGH) //Check for button press
        {
                digitalWrite(LED, HIGH); // turn LED ON to confirm input
                Serial.write('t');    //Send Input to MP3 trigger
                Serial.write(buttons[val]); //Send MP3 number to trigger
                delay(1000);
                break;
        }
        else
        {
                digitalWrite(LED, LOW);
        }
}




You'll notice I also added a 'break', so the first button you get will be the last one you process, which I think is consistent with what you want.



for (int val=0; val< (sizeOf(buttons)/sizeOf(buttons[0])); val++);




Remove the semicolon at the end of that line. As written, this means "Loop through <some number of> times, and do nothing whatsoever.



int val;




No need for this at the top of the sketch. You declare val inline in your 'for' loop, and I also do it later in my example.

Bit lost here maniacbug. I dont want someone to write the sketch as I will learn nothing, but I am unsure what you mean with reagards to loop in a loop. I agree that the buttons are latching on the first input recieved. I think what you are saying is that the value 'val' cannot change again after the first input? So I need a function to ensure that the value is able to be updated on the next button press. Apologies for my 'Noobness' and I appreciate the patience I am getting.

It looks pretty good.

What is the MP3 Trigger looking for for input? Is it the letter 't' followed by a number (one or more digit characters)?

Serial.write(buttons[val]); sends a binary byte to the trigger.

If it's expecting digits you probably want:

Serial.print(buttons[val], DEC); //Send MP3 number to trigger

If the MP3 Trigger is looking for an 'end of line' so it knows it has all the digits of the number you may want:

Serial.println(buttons[val], DEC); //Send MP3 number to trigger

Thanks all. Got it going.

The trigger is looking for Binary for any number past 9 which is perfect for me as I am using another set of external triggers for tracks 1-16.
I was having trouble with the line

for (int val=0; val< (sizeOf(buttons)/sizeOf(buttons[0])); val++);

due to my inexperience.

Once again appreciate the feedback and paitence from all.