Button Controlled Up/Down Counter issue

Projects Programming
1

So I'm making an electronic scoreboard, and I have a Uno hooked up to 3 shift registers to give a 1-999 count,

it's working fine except for a minor issue - when pressing the increase count button it increases then pressing the decrease count button increases the count by 1 and then the next press lowers it.

So if the count was 0 it would go as follows
up button pressed = 1
up = 2
down = 3
down = 2
down = 1
up = 0
up = 1

It is a minor issue I could live with, but ideally I'd like to solve it if it is a simple fix. Any idea's?
Thanks.

// Pin connected to SH_CP of 74HC595 = Pin 11
int clockPin = 4;
// Pin connected to ST_CP of 74HC595 = Pin 12
int latchPin = 5;
// Pin connected to DS of 74HC595 = Pin 14
int dataPin = 6;

byte dataArray[10];
const int buttonpin1 = 2; // +1 button
const int buttonpin2 = 3; // -1 button

// variable
int button1state = 0;
int button2state = 0;
int button3state = 0;
int count = 0;

void setup() {
  //set pins to output so you can control the shift register
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
  pinMode(dataPin, OUTPUT);
  // initialize the pushbutton pin as an input:
  pinMode(buttonpin1, INPUT);
  pinMode(buttonpin2, INPUT);

  
  dataArray[0] = 0x03; // 0
  dataArray[1] = 0x9F; // 1
  dataArray[2] = 0x25; // 2
  dataArray[3] = 0x0D; // 3
  dataArray[4] = 0x99; // 4
  dataArray[5] = 0x49; // 5
  dataArray[6] = 0xC1; // 6
  dataArray[7] = 0x1F; // 7
  dataArray[8] = 0x01; // 8
  dataArray[9] = 0x19; // 9
  
  digitalWrite(latchPin, 0);
  shiftOut(dataPin, clockPin, LSBFIRST, dataArray[0]);
    shiftOut(dataPin, clockPin, LSBFIRST, dataArray[0]);
      shiftOut(dataPin, clockPin, LSBFIRST, dataArray[0]);
  digitalWrite(latchPin, 1);
}

void loop() {
int val = digitalRead(buttonpin1); // plus button
int val2 = digitalRead(buttonpin2); // minus button
int num1 = count / 100; // hundreds
int num2 = (count / 10) - (num1*10); //tens
int num3 = count - num1*100 - num2*10; // units
 
 // main loop ###################
  if (val == HIGH && val2 == LOW) 
  {
    count++;
    if (count >= 1000) count = 0;
    delay(100);
      //ground latchPin and hold low for as long as you are transmitting
      digitalWrite(latchPin, 0);
      shiftOut(dataPin, clockPin, LSBFIRST, dataArray[num3]); 
      shiftOut(dataPin, clockPin, LSBFIRST, dataArray[num2]);
      shiftOut(dataPin, clockPin, LSBFIRST, dataArray[num1]);
      digitalWrite(latchPin, 1);
  }
  else if (val2 == HIGH && val == LOW)
  {
    count--;
    if (count <= -1) count = 999;
    delay(100);
      //ground latchPin and hold low for as long as you are transmitting
      digitalWrite(latchPin, 0);
      shiftOut(dataPin, clockPin, LSBFIRST, dataArray[num3]); 
      shiftOut(dataPin, clockPin, LSBFIRST, dataArray[num2]);
      shiftOut(dataPin, clockPin, LSBFIRST, dataArray[num1]);
      digitalWrite(latchPin, 1);
  }
  else if (val == HIGH && val2 == HIGH)
  {
    count = 0;
    delay(100);
      //ground latchPin and hold low for as long as you are transmitting
      digitalWrite(latchPin, 0);
      shiftOut(dataPin, clockPin, LSBFIRST, dataArray[num3]);
      shiftOut(dataPin, clockPin, LSBFIRST, dataArray[num2]);
      shiftOut(dataPin, clockPin, LSBFIRST, dataArray[num1]);
      digitalWrite(latchPin, 1);
  }
}
2

At the top of the loop num1, num2, num3 are calculated, but after a button is pressed (either up or down), count is incremented or decremented accordingly, but then num1,2,3 need to be recalculated before sending the data to update the display. As it is the previous value of count is sent to the display. Whenever count changes, so should num1,2,3 ... might be a good candidate for a function that could be called since it has to happen in a couple places.

Or maybe better yet, have a function that accepts an integer as input (e.g. count), then internal to the function, calculates the three digits and updates the display. Moves redundant code out of loop().

3

Doh, can't believe I didn't spot that - thanks very much, once you pointed it out it was a quick fix.

just moved all of that code to the bottom of the loop, now it works perfectly.

// Pin connected to SH_CP of 74HC595 = Pin 11
int clockPin = 4;
// Pin connected to ST_CP of 74HC595 = Pin 12
int latchPin = 5;
// Pin connected to DS of 74HC595 = Pin 14
int dataPin = 6;

byte dataArray[10];

const int buttonPin1 = 2; // +1 button
const int buttonPin2 = 3; // -1 button

// variable
int count = 0;

void setup() {
  //set pins to output so you can control the shift register
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
  pinMode(dataPin, OUTPUT);
  // initialize the pushbutton pin as an input:
  pinMode(buttonPin1, INPUT);
  pinMode(buttonPin2, INPUT);

  dataArray[0] = 0x03; // 0 
  dataArray[1] = 0x9F; // 1
  dataArray[2] = 0x25; // 2
  dataArray[3] = 0x0D; // 3
  dataArray[4] = 0x99; // 4
  dataArray[5] = 0x49; // 5
  dataArray[6] = 0xC1; // 6
  dataArray[7] = 0x1F; // 7
  dataArray[8] = 0x01; // 8
  dataArray[9] = 0x19; // 9

}

void loop() {
int val = digitalRead(buttonPin1); // plus button
int val2 = digitalRead(buttonPin2); // minus button

 // Counter
  if (val == HIGH && val2 == LOW) // up
  {
    count++;
    if (count >= 1000) count = 0;
    delay(100);
  }
  else if (val2 == HIGH && val == LOW) // down
  {
    count--;
    if (count <= -1) count = 999;
    delay(100);
  }
  else if (val == HIGH && val2 == HIGH) // reset
  {
    count = 0;
    delay(100);
  }

// Output of number

  int num1 = count / 100; // hundreds
  int num2 = (count / 10) - (num1*10); //tens
  int num3 = count - num1*100 - num2*10; // units
//ground latchPin and hold low for as long as you are transmitting
  digitalWrite(latchPin, 0);
  shiftOut(dataPin, clockPin, LSBFIRST, dataArray[num3]);
  shiftOut(dataPin, clockPin, LSBFIRST, dataArray[num2]);
  shiftOut(dataPin, clockPin, LSBFIRST, dataArray[num1]);
  digitalWrite(latchPin, 1);

}
4 
int val = digitalRead(buttonpin1); // plus button
int val2 = digitalRead(buttonpin2); // minus button

Meaningful names, like valUp, valDn, upPin, dnPin, etc. would make this code a lot more readable.

Proper indentation would help, too.

5

Yeah I really should get into the habit of doing that.

// Pin connected to SH_CP of 74HC595 = Pin 11
  int clockPin = 4;
// Pin connected to ST_CP of 74HC595 = Pin 12
  int latchPin = 5;
// Pin connected to DS of 74HC595 = Pin 14
  int dataPin = 6;
// Buttons  
  const int upPin1 = 2; // +1 button
  const int downPin1 = 3; // -1 button

// 7 seg numbers for dataArray
  byte dataArray[10]; 
// variable
  int count1 = 0;

void setup() {
//set pins to output so you can control the shift register
 pinMode(latchPin, OUTPUT);
 pinMode(clockPin, OUTPUT);
 pinMode(dataPin, OUTPUT);
// initialize the pushbutton pin as an input:
 pinMode(upPin1, INPUT);
 pinMode(downPin1, INPUT);

// For RL-S5620
  dataArray[0] = 0x03; // 0 
  dataArray[1] = 0x9F; // 1
  dataArray[2] = 0x25; // 2
  dataArray[3] = 0x0D; // 3
  dataArray[4] = 0x99; // 4
  dataArray[5] = 0x49; // 5
  dataArray[6] = 0xC1; // 6
  dataArray[7] = 0x1F; // 7
  dataArray[8] = 0x01; // 8
  dataArray[9] = 0x19; // 9
}

void loop() {
  int valUp = digitalRead(upPin1); // plus button
  int valDown = digitalRead(downPin1); // minus button

// Counter
  if (valUp == HIGH && valDown == LOW) // up
   {
     count1++;
     if (count1 >= 1000) count1 = 0;
     delay(100);
   }
  else if (valDown == HIGH && valUp == LOW) // down
   {
     count1--;
     if (count1 <= -1) count1 = 999;
     delay(100);
   }
  else if (valUp == HIGH && valDown == HIGH) // reset
   {
     count1 = 0;
     delay(100);
   }

// split count into units
  int numHun1 = count1 / 100; // hundreds
  int numTen1 = (count1 / 10) - (numHun1*10); //tens
  int numUni1 = count1 - numHun1*100 - numTen1*10; // units

//ground latchPin and hold low for as long as you are transmitting
  digitalWrite(latchPin, 0);
    shiftOut(dataPin, clockPin, LSBFIRST, dataArray[numUni1]);
    shiftOut(dataPin, clockPin, LSBFIRST, dataArray[numTen1]);
    shiftOut(dataPin, clockPin, LSBFIRST, dataArray[numHun1]);
  digitalWrite(latchPin, 1);

}
6 
  int valUp = digitalRead(upPin1); // plus button
  int valDown = digitalRead(downPin1); // minus button

You got more than 1 upPin and more than one downPin?

  int count1 = 0;

You got more than 1 count?