Multiplexed 7 Seg counter by push button

I have been trying unsuccessfully to write/modify/beg/borrow or steal some code XD to make this 7 seg code count up and down between zero and eight by pressing either an up or down push button. I can get it going up or down singularly but not together in the same piece of code.Here is what I have that will count up and back to zero
I realise that I have to incorporate another push button switch to handle to counting down aspect of the code, and hold and subsequently update switch states as the count variable goes up and down, but as for actually implementing this I am lost. Would someone be kind enough to give me a few clues as to how to move forward with this. I have looked at numerous examples of code that counts up and down, usually in a menu situation but I think I am more confused than ever. Thanks Pedro.

  //  7-Segment LED counter, multiplexing using 74HC595 8-bit shift register, increment counter zero to nine to zero via push button switch
  // Code mangled together from these sources - thanks fellas
  //  http://www.sweeting.org/mark/blog/2011/11/27/arduino-74hc595-shift-register-and-a-7-segment-led-display
  //  http://thecustomgeek.com/2011/06/29/multiplexing-for-a-7-year-old/

  
  const int latchPin = 5;  // Pin connected to Pin 12 of 74HC595 (Latch)
  const int dataPin  = 6;  // Pin connected to Pin 14 of 74HC595 (Data)
  const int clockPin = 7;  // Pin connected to Pin 11 of 74HC595 (Clock)
  int switchPin = 12;      // pushbutton attached to pin 12
  int buttonState = HIGH;   // initialise buttonState as HIGH
                            // pushbutton switch pin / pullup resistor to +5v
                            // same side of sw to pin 12
                            // other side of sw to Gnd
                            
  int counter = 0;          // initialise counter as zero
  
  
  
  const byte numbers[10] =  // Describe each digit in terms of display segments  0, 1, 2, 3, 4, 5, 6, 7, 8, 9

                     {
                      B11111100,
                      B01100000,
                      B11011010,
                      B11110010,
                      B01100110,
                      B10110110,
                      B10111110,
                      B11100000,
                      B11111110,
                      B11100110,
                     };
  
  void setup()
  {
  
     //Serial.begin(9600);
   
    pinMode(latchPin, OUTPUT);   // set SR pins to output 
    pinMode(clockPin, OUTPUT);
    pinMode(dataPin, OUTPUT);
    pinMode(switchPin, INPUT);   // sets pin 12 as pushbutton INPUT
  }
  
  void loop()
  {
      // Serial.println(counter);
      int newstate = digitalRead(switchPin);
      if (buttonState != newstate)                // has the state changed from  
      {                                           // HIGH to LOW or vice versa
        buttonState = newstate;
        if (newstate == HIGH)                      // IF the button was pressed
          ++counter;                              // increment the counter by one
         
      }
        if(counter > 8)
        counter = 0;
        show(numbers[counter]); // display the current digit
    }
  
  void show( byte number)
  {
    // Use a loop and a bitwise AND to move over each bit that makes up
    // the seven segment display (from left to right, A => G), and check
    // to see if it should be on or not
    for(int j = 0; j <= 7; j++)
    {
      byte toWrite = number & (B10000000 >> j); 
  
       if(!toWrite) { continue; }  // If all bits are 0 then no point writing it to the shift register,so break out and move on to next segment.
  
       shiftIt(toWrite); // Otherwise shift it into the register
    }
  }
  
  void shiftIt (byte data)
  {
    digitalWrite(latchPin, LOW); // Set latchPin LOW while clocking these 8 bits in to the register
  
      for (int k=0; k <= 7; k++)
      {
        digitalWrite(clockPin, LOW); // clockPin LOW prior to sending a bit 
  
        // Note that in our case, we need to set pinState to 0 (LOW) for
        // “On” as the 74HC595 is sinking current when using a common
        // anode display. If you want to use a common cathode display then
        // switch this around.
        if ( data & (1 << k) )
        {
          digitalWrite(dataPin, HIGH); // turn “On”
        }
        else
        {
          digitalWrite(dataPin, LOW); // turn “Off”
        }
          digitalWrite(clockPin, HIGH); // and clock the bit in
      }
          digitalWrite(clockPin, LOW);  //stop shifting out data
          digitalWrite(latchPin, HIGH); //set latchPin to high to lock and send data
  }
      int newstate = digitalRead(switchPin);
      if (buttonState != newstate)                // has the state changed from

I find it far easier to understand code like this with matching names, like currState and prevState for the now and then states.

I find it far easier to read (and write) code is all variables use camelCase. No need to remember which do and which don't.

I find it far easier to follow code that uses meaningful names. switchPin implies that there is a switch attached to that pin. But, is that the up switch or the down switch? upPin, currUpState, prevUpState, downPin, currDownState, and prevDownState would, in my opinion, remove all ambiguity from your code.

The current state of the pin should be saved as the previous state on every pass through loop, not just when the state changes.

I realise that I have to incorporate another push button switch to handle to counting down aspect of the code

Yes, along with three more variables, for the pin number, current state, and previous state of that switch/pin.

Would someone be kind enough to give me a few clues as to how to move forward with this

After renaming the variables as I suggest above, the new code for dealing with the down switch looks just like the code for dealing with the up switch, except that you decrement the counter, and test that the counter has not gone below 0.

if(counter > 8)

Will 9 ever appear on the display ?

Will 9 ever appear on the display ?

Only when the hero cuts the right wire, cutting all power to the bomb.

Thanks for your suggestions PaulS. This is what I concocted after your excellent tutelage

  //  7-Segment LED counter, multiplexing using 74HC595 8-bit shift register, increment counter zero to nine to zero via push button switch
  // Code mangled together from these sources - thanks fellas
  //  http://www.sweeting.org/mark/blog/2011/11/27/arduino-74hc595-shift-register-and-a-7-segment-led-display
  //  http://thecustomgeek.com/2011/06/29/multiplexing-for-a-7-year-old/
  //  http://nootropicdesign.com/projectlab/2009/10/25/led-clock/
  
  const int latchPin = 5;  // Pin connected to Pin 12 of 74HC595 (Latch)
  const int dataPin  = 6;  // Pin connected to Pin 14 of 74HC595 (Data)
  const int clockPin = 7;  // Pin connected to Pin 11 of 74HC595 (Clock)
  int upPin = 12;      // pushbutton attached to pin 12
  int downPin = 13;      // pushbutton attached to pin 12
  int currUpState = 1;   // initialise currUpState as HIGH
  int currDownState = 1;   // initialise currDownState as HIGH
  int prevUpState = 0; 
  int prevDownState = 0;  
  int counter = 0;          // initialise counter as zero
  
  
  
  const byte numbers[10] =  // Describe each digit in terms of display segments  0, 1, 2, 3, 4, 5, 6, 7, 8, 9
  
  {
    B11111100,
    B01100000,
    B11011010,
    B11110010,
    B01100110,
    B10110110,
    B10111110,
    B11100000,
    B11111110,
    B11100110,
  };
  
  void setup()
  {
    pinMode(latchPin, OUTPUT);   // set SR pins to output 
    pinMode(clockPin, OUTPUT);
    pinMode(dataPin, OUTPUT);
    pinMode(upPin, INPUT);   // sets pin 12 as pushbutton INPUT
    pinMode(downPin, INPUT);   // sets pin 13 as pushbutton INPUT
  }
  
  void loop()
  {
  
    currUpState = digitalRead(upPin);
    if (prevUpState != currUpState)             // has the state changed from  
    {                                           // HIGH to LOW or vice versa
      prevUpState = currUpState;
      if (currUpState == HIGH)                  // If the button was pressed
        ++counter;                              // increment the counter by one
    }  
  
    if(counter > 8)
      counter--;
    show(numbers[counter]); // display the current digit
  
  
  
  
    currDownState = digitalRead(downPin);
    if (prevDownState != currDownState)         // has the state changed from  
    {                                           // HIGH to LOW or vice versa
      prevDownState = currDownState;
      if (currDownState == HIGH)                // If the button was pressed
        --counter;                              // decrement the counter by one
  
    }
    if(counter < 0)
      counter++;
    show(numbers[counter]); // display the current digit
  }
  
  void show( byte number)
  {
    // Use a loop and a bitwise AND to move over each bit that makes up
    // the seven segment display (from left to right, A => G), and check
    // to see if it should be on or not
    for(int j = 0; j <= 7; j++)
    {
      byte toWrite = number & (B10000000 >> j); 
  
      if(!toWrite) { 
        continue; 
      }  // If all bits are 0 then no point writing it to the shift register,so break out and move on to next segment.
  
      shiftIt(toWrite); // Otherwise shift it into the register
    }
  }
  
  void shiftIt (byte data)
  {
    digitalWrite(latchPin, LOW); // Set latchPin LOW while clocking these 8 bits in to the register
  
    for (int k=0; k <= 7; k++)
    {
      digitalWrite(clockPin, LOW); // clockPin LOW prior to sending a bit 
  
      
      if ( data & (1 << k) )
      {
        digitalWrite(dataPin, HIGH); // turn “On”
      }
      else
      {
        digitalWrite(dataPin, LOW); // turn “Off”
      }
      digitalWrite(clockPin, HIGH); // and clock the bit in
    }
    digitalWrite(clockPin, LOW);  //stop shifting out data
    digitalWrite(latchPin, HIGH); //set latchPin to high to lock and send data
  }

It seems to be working as expected albeit some minor switch bounce issues which I should be able to remove with judicious use of the devils function 8)
Thanks for your excellent advice, that’s what got me over the line, Pedro.

It seems to be working as expected albeit some minor switch bounce issues which I should be able to remove with judicious use of the devils function

Or recording when the transition of interest occurred, and ignoring any transitions that occur less than some minimum time later.

No delay()s needed. Really.

It makes me nervous when you knights of the superior coding intelligentsia offer these morsels to us mere mortals. Might this be along the lines of something pertaining to

if(currentMillis - previousMillis > interval)

If so getting my head around millis is my next quest :fearful:

Breaking news - So this is how you became the forums first Brattain member. Well done.

Might this be along the lines of something pertaining to

Exactly.

If so getting my head around millis is my next quest

You've got a wrist watch, right? millis() is simply a function that returns a time value relative to some arbitrary event, just like your watch does. In once case, it's midnight or noon, whatever they mean, and in the other it's when the Arduino was reset.

Your watch "rolls over" every 12 or 24 hours, depending on the kind you wear. millis() rolls over every 49 days.

Nothing really complex there.

Arduino has a shiftOut() function built in, so you don’t have to bother with your shiftit.

Also, your count sequence is weird. If I’m reading it properly, it will go:

0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 8, 9, 8, 9, 8, 9, 8, 9, 8…

If you don’t have external pullup resistors attached to your buttons, the pinMode should be INPUT_PULLUP.

Thanks again PaulS