chess clock program not working correctly

I am slowly building my code up to have a fully functioning, adjustable, multi-mode chess clock; right now I have 2 led displays and 2 buttons hooked up.

I am just trying to make it so that when I press a button, one of the screens stops and the other starts; and this doesn't reverse until the other button is pressed.

Right now button1 only works if its pressed first and even then it stops working after button2 is pressed. Button2 responds normally according to the serial output. I'm not sure what part of the code is important to show so I will show it all.

#include <Metro.h>

Metro disp1Metro = Metro(1000);
Metro disp2Metro = Metro(1000);
int dataPin = 2;                   // connect to MOSI on both displays
int clockPin =3;                   // connect to SCK on both displays
int CS1 = 4;                       // connect to CS on first display
int CS2 = 5;                       // connect to CS on second display 
int num1 = 0;                      //number to be displayed on CS1
int num2 = 0;                      //number to be displayed on CS2
int inPin1 = 7;                 // switch connected to digital pin 7
int inPin2 = 8;                 // switch connected to digital pin 8
int button1 = 0;                   // variable to store the read value of button1
int button2 = 0;                   // variable to store the read value of button2
int hold1 = 0;                    // holds compounded button value
int hold2 = 0;                    // holds compounded button value

void setup (void) {
  pinMode (dataPin, OUTPUT);
  pinMode (clockPin, OUTPUT);
  pinMode (CS1, OUTPUT);
  pinMode (CS2, OUTPUT);
  digitalWrite (CS2, HIGH);        // deselect second display
  digitalWrite (CS1, LOW);       // select first display
  shiftOut(dataPin, clockPin, MSBFIRST, num1);
  shiftOut(dataPin, clockPin, MSBFIRST, num1);
  shiftOut(dataPin, clockPin, MSBFIRST, num1);
  shiftOut(dataPin, clockPin, MSBFIRST, num1);
  //shiftOut(dataPin, clockPin, MSBFIRST, 0x7A); special character for setting brightness
  //shiftOut(dataPin, clockPin, MSBFIRST, 0x00); full brightness (0x00) and dimmest (0xFF)
  digitalWrite (CS1, HIGH);        // deselect first display
  digitalWrite (CS2, LOW);       // select second display
  shiftOut(dataPin, clockPin, MSBFIRST, num2);
  shiftOut(dataPin, clockPin, MSBFIRST, num2);
  shiftOut(dataPin, clockPin, MSBFIRST, num2);
  shiftOut(dataPin, clockPin, MSBFIRST, num2);
  //shiftOut(dataPin, clockPin, MSBFIRST, 0x7A); special character for setting brightness
  //shiftOut(dataPin, clockPin, MSBFIRST, 0x00); full brightness (0x00) and dimmest (0xFF)
  pinMode(inPin1, INPUT);      // sets digital pin 7 as input
  pinMode(inPin2, INPUT);       // sets digital pin 8 as input
  Serial.begin(9600);
}

void loop (void) {
  button1 = digitalRead(inPin1);   // read the input pin for button 1
  button2 = digitalRead(inPin2);  // read the input pin for button 2
  
  hold1 += button1;
  hold2 += button2;
  
   if(button1==1){
    Serial.println("Button 1 pressed");
    Serial.println(hold1);
  }
  
  if(button2==1){
    Serial.println("Button 2 pressed");
    Serial.println(hold2);
  }
  
  if(hold2 >= 1){
    hold1 = 0;
  }
  
  if(hold1 >= 1){
    hold2 = 0;
  }
  
  if(hold1 >= 1){
      if(disp1Metro.check() == 1) {
        digitalWrite (CS2, HIGH);        // deselect second display
        digitalWrite (CS1, LOW);       // select first display
        shiftOut(dataPin, clockPin, MSBFIRST, num1);
        shiftOut(dataPin, clockPin, MSBFIRST, num1);
        shiftOut(dataPin, clockPin, MSBFIRST, num1);
        shiftOut(dataPin, clockPin, MSBFIRST, num1);
        num1++;
        if(num1 == 10){
          num1 = 0;
        }
      }
  }
  
  if(hold2 >= 1){
    if(disp2Metro.check() == 1) {
      digitalWrite (CS1, HIGH);        // deselect first display
      digitalWrite (CS2, LOW);       // select second display
      shiftOut(dataPin, clockPin, MSBFIRST, num2);
      shiftOut(dataPin, clockPin, MSBFIRST, num2);
      shiftOut(dataPin, clockPin, MSBFIRST, num2);
      shiftOut(dataPin, clockPin, MSBFIRST, num2);
      num2++;
      if(num2 == 10){
        num2 = 0;
      }
    }
  }

}

The problem is that hold2 never gets reset to 0.

Consider this snippet from your code:

  if(hold2 >= 1){
    hold1 = 0;
  }

  if(hold1 >= 1){
    hold2 = 0;
  }

Note that in the entire sketch, the only line of code that resets hold2 to 0 is within the second test. But as soon as hold2 is >= 1, hold1 becomes 0. Therefore, the next test (hold1 >= 1) will always fail. Even though the next time through the loop you may increment hold1 by pressing button1, hold2 is still >=1, so hold1 is reset to 0 again by the first test in the code snippet above.

You can verify this by putting the following code just after that code snippet. Press some buttons to see what happens to the values in hold1 and hold2 after you add this code:

  Serial.print("hold1=");
  Serial.println(hold1);
  Serial.print("hold2=");
  Serial.println(hold2);
  delay(1000); // just to keep from printing too fast

Now, what to do about it?

There are often many correct solutions to problems. Here is but one: remove the code snippet above from your original code, and reset hold2 and hold1 to 0 when button1 and button2 are pressed respectively.

   if(button1==1){
    hold2 = 0;
    Serial.println("Button 1 pressed");
    Serial.println(hold1);
  }

  if(button2==1){
    hold1 = 0;
    Serial.println("Button 2 pressed");
    Serial.println(hold2);
  }

If any of this is unclear, let me know and I'll try to change the wording to help you understand.

And just for the record, it was important to have all the code to diagnose this problem. :slight_smile:

thanks a lot for the response, it was clear. i showed it to a programmer friend this morning and he caught it. i have removed the problem code and replaced it with this:

if(button1==1){
    hold=1;
  }
  else if(button2==1){
    hold=2;
  }

now there is only one hold variable. it works well.

thanks again for taking the time to look at the code and to respond so nicely.