Jukebox Retrofit State Change Issues

Hey guys
Retrofitting an old jukebox to send state change buttons serial messages via Arduino into Maxuino to trigger a playlist in Max. I am fine on the Max end as that is a program I am much more familiar with. I am having a hard time getting the state change code to work in testing. These are jukebox buttons so they hold when you press them, hence the state change necessity.

This works fine when I upload the State change detection example patch, but once there is more than one input the state change holds and I get constant "1" or "2" messages until I disengage the button.

This seemed like an easy project but I guess I'm missing a fundamental understanding of the problem in this.

Thanks for the help...

  State change detection (edge detection)
 	
 Often, you don't need to know the state of a digital input all the time,
 but you just need to know when the input changes from one state to another.
 For example, you want to know when a button goes from OFF to ON.  This is called
 state change detection, or edge detection.
 
 This example shows how to detect when a button or button changes from off to on
 and on to off.
 	
 The circuit:
 * pushbutton attached to pin 2 from +5V
 * 10K resistor attached to pin 2 from ground
 * LED attached from pin 13 to ground (or use the built-in LED on
   most Arduino boards)
 
 created  27 Sep 2005
 modified 30 Aug 2011
 by Tom Igoe

This example code is in the public domain.
 	
 http://arduino.cc/en/Tutorial/ButtonStateChange
 
 */

// this constant won't change:
const int  buttonPin1 = 22; 
const int  buttonPin2 = 23; 
const int  buttonPin3 = 24; 
const int  buttonPin4 = 25; // the pin that the pushbutton is attached to
const int ledPin = 13;       // the pin that the LED is attached to

// Variables will change:
int buttonPushCounter = 0;   // counter for the number of button presses
int buttonState = 0;         // current state of the button
int lastButtonState = 0;     // previous state of the button

void setup() {
  // initialize the button pin as a input:
  pinMode(buttonPin1, INPUT);

    pinMode(buttonPin2, INPUT);

  pinMode(buttonPin3, INPUT);

  pinMode(buttonPin4, INPUT);

  // initialize the LED as an output:
  pinMode(ledPin, OUTPUT);
  // initialize serial communication:
  Serial.begin(9600);
}


void loop() {
  // read the pushbutton input pin:
  buttonState = digitalRead(buttonPin1);

  // compare the buttonState to its previous state
  if (buttonState != lastButtonState) {
    // if the state has changed, increment the counter
    if (buttonState == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter++;
      Serial.println("1");
      Serial.print("number of button pushes:  ");
      Serial.println(buttonPushCounter);
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
    }
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState = buttonState;

   // read the pushbutton input pin:
  buttonState = digitalRead(buttonPin2);

  // compare the buttonState to its previous state
  if (buttonState != lastButtonState) {
    // if the state has changed, increment the counter
    if (buttonState == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter++;
      Serial.println("2");
      Serial.print("number of button pushes:  ");
      Serial.println(buttonPushCounter);
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
    }
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState = buttonState;

   // read the pushbutton input pin:
  buttonState = digitalRead(buttonPin3);

  // compare the buttonState to its previous state
  if (buttonState != lastButtonState) {
    // if the state has changed, increment the counter
    if (buttonState == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter++;
      Serial.println("3");
      Serial.print("number of button pushes:  ");
      Serial.println(buttonPushCounter);
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
    }
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState = buttonState;

   // read the pushbutton input pin:
  buttonState = digitalRead(buttonPin4);

  // compare the buttonState to its previous state
  if (buttonState != lastButtonState) {
    // if the state has changed, increment the counter
    if (buttonState == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      buttonPushCounter++;
      Serial.println("4");
      Serial.print("number of button pushes:  ");
      Serial.println(buttonPushCounter);
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
    }
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState = buttonState;

}

It looks like you copied and pasted an example for a single button to deal with your four buttons and this has given you a problem because you're using only using one LastButtonState variable. You need one for each button, as it is, the four buttons are interfering with each other. There may well be other, similar, issues but fixing this should get you closer.

Note though that this isn't an ideal approach - any time you find yourself with a lot of similar code as in this case, it's likely a sign you need to refactor. Arrays and a function to read any button would help here, but I assume you'd just like to get it working first.

Great! thanks!

So here is the final code that is up and working fine with Max.
Now just for future projects, or if I want to make this more efficient...How would I implement this with an array? All of the online array demos are for outputs, like LEDs, and the only one I could find on Adafruit with input buttons and bounce control did the exact same thing, kept sending the on signal over and over.

Here is the code that works...its super messy and gross I know...but it works for now.
I had to cut alot out to post under the 9500 characters allowed here...it normally has 33 buttons

what are the risks of using such terrible coding if it works correctly? will it eventually fail?
super new to C
thanks again.

/*


const int buttonPin1 = 2;     // the number of the pushbutton pin
const int buttonPin2 = 3; 
const int buttonPin3 = 4; 
const int buttonPin4 = 5; 
const  int buttonPin5 = 6; 
const  int buttonPin6 = 7; 
const int buttonPin7 = 8; 
const int buttonPin8 = 9; 
const int buttonPin9 = 10; 


// variables will change:
int buttonState1 = 0; 
int buttonState2 = 0; 
int buttonState3 = 0; 
int buttonState4 = 0; 
int buttonState5 = 0; 
int buttonState6 = 0; 
int buttonState7 = 0; 
int buttonState8 = 0; 
int buttonState9 = 0; 
int buttonState10 = 0; 



// variable for reading the pushbutton status
int lastButtonState1 = 0; 
int lastButtonState2 = 0; 
int lastButtonState3 = 0; 
int lastButtonState4 = 0; 
int lastButtonState5 = 0; 
int lastButtonState6 = 0; 
int lastButtonState7 = 0; 
int lastButtonState8 = 0; 
int lastButtonState9 = 0; 
int lastButtonState10 = 0; 

void setup() {

  // initialize the pushbutton pin as an input:
  pinMode(buttonPin1, INPUT);     
  pinMode(buttonPin2, INPUT);   
  pinMode(buttonPin3, INPUT);     
  pinMode(buttonPin4, INPUT); 
  pinMode(buttonPin5, INPUT);     
  pinMode(buttonPin6, INPUT);   
  pinMode(buttonPin7, INPUT);     
  pinMode(buttonPin8, INPUT); 
  pinMode(buttonPin9, INPUT);     
 
  
  Serial.begin(9600);  

  
}

void loop(){
  // read the state of the pushbutton value:
  buttonState1 = digitalRead(buttonPin1);
  if (buttonState1 != lastButtonState1) {
    // if the state has changed, increment the counter
    if (buttonState1 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      Serial.println("1");
      if (buttonState1 == LOW); 
 Serial.println("off"); 
      delay (100);
   
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
     }
  
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState1 = buttonState1;
   
 
 
 
  // read the state of the pushbutton value:
  buttonState2 = digitalRead(buttonPin2);
  if (buttonState2 != lastButtonState2) {
    // if the state has changed, increment the counter
    if (buttonState2 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      Serial.println("2");
       if (buttonState2 == LOW); 
 Serial.println("off"); 
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
    }
   
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState2 = buttonState2;
  
  
  // read the state of the pushbutton value:
  buttonState3 = digitalRead(buttonPin3);
  if (buttonState3 != lastButtonState3) {
    // if the state has changed, increment the counter
    if (buttonState3 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      Serial.println("3");
       if (buttonState3 == LOW); 
 Serial.println("off"); 
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
      
    
    }
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState3 = buttonState3;   
 
 
  // read the state of the pushbutton value:
  buttonState4 = digitalRead(buttonPin4);
   if (buttonState4 != lastButtonState4) {
    // if the state has changed, increment the counter
    if (buttonState4 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      Serial.println("4");
         if (buttonState4 == LOW); 
 Serial.println("off"); 
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
      
   
    }
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState4 = buttonState4;  
 
 
  // read the state of the pushbutton value:
  buttonState5 = digitalRead(buttonPin5);
   if (buttonState5 != lastButtonState5) {
    // if the state has changed, increment the counter
    if (buttonState5 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      Serial.println("5");
         if (buttonState5 == LOW); 
 Serial.println("off"); 
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
      
      if (buttonState5 == LOW); 
 Serial.println("off"); 
    }
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState5 = buttonState5;   
 
 
  // read the state of the pushbutton value:
  buttonState6 = digitalRead(buttonPin6);
   if (buttonState6 != lastButtonState6) {
    // if the state has changed, increment the counter
    if (buttonState6 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      Serial.println("6");
         if (buttonState6 == LOW); 
 Serial.println("off"); 
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
      if (buttonState6 == LOW); 
 Serial.println("off"); 
    }
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState6 = buttonState6;   
 
 
  // read the state of the pushbutton value:
  buttonState7 = digitalRead(buttonPin7);
   if (buttonState7 != lastButtonState7) {
    // if the state has changed, increment the counter
    if (buttonState7 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      Serial.println("7");
         if (buttonState7 == LOW); 
 Serial.println("off"); 
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
      if (buttonState7 == LOW); 
 Serial.println("off"); 
    }
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState7 = buttonState7;  
 
 
  // read the state of the pushbutton value:
  buttonState8 = digitalRead(buttonPin8);
   if (buttonState8 != lastButtonState8) {
    // if the state has changed, increment the counter
    if (buttonState8 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      Serial.println("8");
         if (buttonState8 == LOW); 
 Serial.println("off"); 
     
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
      if (buttonState8 == LOW); 
 Serial.println("off"); 
    }
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState8 = buttonState8;   
 
 
  // read the state of the pushbutton value:
  buttonState9 = digitalRead(buttonPin9);
   if (buttonState9 != lastButtonState9) {
    // if the state has changed, increment the counter
    if (buttonState9 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      Serial.println("9");
         if (buttonState9 == LOW); 
 Serial.println("off"); 
    
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
      if (buttonState9 == LOW); 
 Serial.println("off"); 
    }
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState9 = buttonState9;    
 
 
  // read the state of the pushbutton value:
  buttonState10 = digitalRead(buttonPinA);
  if (buttonState10 != lastButtonState10) {
    // if the state has changed, increment the counter
    if (buttonState10 == HIGH) {
      // if the current state is HIGH then the button
      // wend from off to on:
      Serial.println("A");
         if (buttonState10 == LOW); 
 Serial.println("off"); 
      
    } 
    else {
      // if the current state is LOW then the button
      // wend from on to off:
      Serial.println("off"); 
      if (buttonState10 == LOW); 
 Serial.println("off"); 
    }
  }
  // save the current state as the last state, 
  //for next time through the loop
  lastButtonState10 = buttonState10;
 

  
delay (1000);
 
}

what are the risks of using such terrible coding if it works correctly? will it eventually fail?

No, if it works, it works. Unless you're using dynamic memory allocation, in which case it may fail after a while. Not relevant in your case though. The issue is simply that if you discover a bug in your code, such as this semicolon for example:

 if (buttonState1 == LOW); // This one should not be here
 Serial.println("off");

Then you have to fix the code in 38 places. Sooner or later, on some bug fix you'll miss one and be baffled as to why some buttons work and others don't.