Relay Control -- Simple code for a simple project what am I missing?

I need to control a set of relays via button presses as well as via serial commands. I only need to toggle the state of an output pin with a button press and am using a single byte to control the output pins via serial.

Am I missing anything? Is there any reason to make my code more complicated than this?

I am using this highly recommended button library https://github.com/JChristensen/Button

Thanks
JM

#include <Button.h>        //Documentation from button library https://github.com/JChristensen/Button
#define PULLUP true        //To keep things simple, we use the Arduino's internal pullup resistor.
#define INVERT true         
#define DEBOUNCE_MS 20     

//Button pins-- Momentary normally open switch connected from pin to ground
#define BUTTON_PIN2 2       //Connect a tactile button switch (or something similar)from Arduino pin 2 to ground.
#define BUTTON_PIN3 3       //switch is closed, this is negative logic, i.e. a high state
#define BUTTON_PIN4 4       //means the button is NOT pressed. (Assuming a normally open switch.)
#define BUTTON_PIN5 5 
#define BUTTON_PIN6 6 
#define BUTTON_PIN7 7 

//output pins connected to TTL controled relays output state LOW == Relay ON
#define OUT_PIN8 8          //Button press on BUTTON_PIN2 Toggles OUT_PIN8 state
#define OUT_PIN9 9          //Button press on BUTTON_PIN3 Toggles OUT_PIN9 state etc...
#define OUT_PIN10 10 
#define OUT_PIN11 11 
#define OUT_PIN12 12 
#define OUT_PIN13 13 

//Declare the buttons for the button library
Button button2(BUTTON_PIN2, PULLUP, INVERT, DEBOUNCE_MS);    
Button button3(BUTTON_PIN3, PULLUP, INVERT, DEBOUNCE_MS); 
Button button4(BUTTON_PIN4, PULLUP, INVERT, DEBOUNCE_MS); 
Button button5(BUTTON_PIN5, PULLUP, INVERT, DEBOUNCE_MS); 
Button button6(BUTTON_PIN6, PULLUP, INVERT, DEBOUNCE_MS); 
Button button7(BUTTON_PIN7, PULLUP, INVERT, DEBOUNCE_MS); 

void setup(void)
{
    //Start Serial   
    Serial.begin(9600); 
    
    //Set the pins as out puts and their state to HIGH
    pinMode(OUT_PIN8, OUTPUT); digitalWrite(OUT_PIN8, HIGH);    
    pinMode(OUT_PIN9, OUTPUT); digitalWrite(OUT_PIN9, HIGH);     
    pinMode(OUT_PIN10, OUTPUT); digitalWrite(OUT_PIN10, HIGH);     
    pinMode(OUT_PIN11, OUTPUT); digitalWrite(OUT_PIN11, HIGH);    
    pinMode(OUT_PIN12, OUTPUT); digitalWrite(OUT_PIN12, HIGH);   
    pinMode(OUT_PIN13, OUTPUT); digitalWrite(OUT_PIN13, HIGH);   
 
}

void loop(void)
{
//read the buttons pins if pressed toggle state of corresponding output pin
    button2.read(); if (button2.wasReleased()){digitalWrite(OUT_PIN8,  !digitalRead(OUT_PIN8));}       
    button3.read(); if (button3.wasReleased()){digitalWrite(OUT_PIN9,  !digitalRead(OUT_PIN9));}
    button4.read(); if (button4.wasReleased()){digitalWrite(OUT_PIN10, !digitalRead(OUT_PIN10));}    
    button5.read(); if (button5.wasReleased()){digitalWrite(OUT_PIN11, !digitalRead(OUT_PIN11));}
    button6.read(); if (button6.wasReleased()){digitalWrite(OUT_PIN12, !digitalRead(OUT_PIN12));}
    button7.read(); if (button6.wasReleased()){digitalWrite(OUT_PIN13, !digitalRead(OUT_PIN13));}
    
//check serial status 
serialControl();
 }
 
//Serial Control Turn outpins on/off with single byte serial commands also return status of all outpins
void serialControl()
{
 if (Serial.available() > 0) {
    int inByte = Serial.read();
    //int lastOn = 0;
    switch (inByte) {
    case 'A':    
      digitalWrite(OUT_PIN8, HIGH); 
      break;
    case 'a':
      digitalWrite(OUT_PIN8, LOW); 
      break;  
        case 'B':    
      digitalWrite(OUT_PIN9, HIGH);
      break;
    case 'b':
      digitalWrite(OUT_PIN9, LOW); 
      break;         
     case 'C':    
      digitalWrite(OUT_PIN10, HIGH); 
      break;
    case 'c':
      digitalWrite(OUT_PIN10, LOW); 
      break;  
        case 'D':    
      digitalWrite(OUT_PIN11, HIGH); 
      break;
    case 'd':
      digitalWrite(OUT_PIN11, LOW); 
      break; 
    case 'E':    
      digitalWrite(OUT_PIN12, HIGH); 
      break;
    case 'e':
      digitalWrite(OUT_PIN12, LOW); 
      break;  
        case 'F':    
      digitalWrite(OUT_PIN13, HIGH);
      break;
    case 'f':
      digitalWrite(OUT_PIN13, LOW); 
      break;
    case 'S'://print status
      Serial.print(digitalRead(OUT_PIN8)); 
      Serial.print(digitalRead(OUT_PIN9));
      Serial.print(digitalRead(OUT_PIN10));
      Serial.print(digitalRead(OUT_PIN11));
      Serial.print(digitalRead(OUT_PIN12));
      Serial.print(digitalRead(OUT_PIN13));
      break;
    }
 }
}

JMS: Am I missing anything? Is there any reason to make my code more complicated than this?

Have you tried it? Does it work?

If not what problems manifest themselves?

I suspect it could be a helluva lot simpler

I don't do button libraries so I can't comment on that.

...R

You could simplify a lot with arrays. I don’t have the Button library so I don’t know if this compiles but it should be close.

#include <Button.h>        //Documentation from button library https://github.com/JChristensen/Button
#define PULLUP true        //To keep things simple, we use the Arduino's internal pullup resistor.
#define INVERT true         
#define DEBOUNCE_MS 20

const int NUM_BUTTONS = 6;

const int ButtonPins[NUM_BUTTONS] = {2,3,4,5,6,7};
const int RelayPins[NUM_BUTTONS] = {8,9,10,11,12,13};

Button *Buttons[NUM_BUTTONS];

void setup(void) {
  //Start Serial   
  Serial.begin(9600); 

  for (int i = 0; i< NUM_BUTTONS; i++) {
    Buttons[i] = &Button(ButtonPins[i], PULLUP, INVERT, DEBOUNCE_MS); 
  }
}

void loop(void) {
  for (int i = 0; i<NUM_BUTTONS; i++) {
    Buttons[i]->read();
    if (Buttons[i]->wasReleased())
      digitalWrite(RelayPins[i],  !digitalRead(RelayPins[i]));
  }

  //check serial status 
  serialControl();
}

//Serial Control Turn outpins on/off with single byte serial commands also return status of all outpins
void serialControl() {
  int inByte = Serial.read();

  if (inByte >= 'A' && inByte <= 'F')
    digitalWrite(RelayPins[inByte - 'A'], HIGH); 

  if (inByte >= 'a' && inByte <= 'f')
    digitalWrite(RelayPins[inByte - 'a'], LOW); 

  if (inByte == 'S') {
    //print status
    for (int i=0; i<NUM_BUTTONS; i++)
      Serial.print(digitalRead(RelayPins[i]?"ON ":"OFF ")); 
  }
}