Help - Short/Long press on button not working to increment counter

Hi All,

I’m new to arduino, and I am trying to make a counter that increments and decrements a 7 segment display if you have a short or long press of a button.

Most of my code (all!) is copied from other people and modified to suit, but I can’t integrate them together - my button code is not increasing the variable (i) . I suspect it the line " for (int i = 0; i <= 15; i)" but not sure what the correct syntax should be.

I can’t figure out what I am doing wrong. Is anyone able to help???

thanks!

My code below

// Globals
const int dataPin 	= 5;  // blue wire to 74HC595 pin 14 INPUT
const int latchPin 	= 6; // green to 74HC595 pin 12 Output register clock
const int clockPin 	= 7; // yellow to 74HC595 pin 11 Shift register clock
const int button 	= 2;

const char common = 'a';    // common anode
//const char common = 'c';    // common cathode

bool decPt = true;  // decimal point display flag
// button code
boolean buttonActive = false;
boolean longPressActive = false;
long buttonTimer = 0;
long longPressTime = 250; //button long press time

void setup() {
  // initialize I/O pins
  pinMode(dataPin, OUTPUT);
  pinMode(latchPin, OUTPUT);
  pinMode(clockPin, OUTPUT);
//button 1 inputs
  pinMode(button, INPUT);

}
int i = 0;
void loop() {

// button code
if (digitalRead(button) == HIGH) {

		if (buttonActive == false) {
			buttonActive = true;
			buttonTimer = millis();
		}
		if ((millis() - buttonTimer > longPressTime) && (longPressActive == false)) {
			longPressActive = true;
			i=i+1;
            delay(250);
		}
	} else {

		if (buttonActive == true) {
			if (longPressActive == true) {
				longPressActive = false;
                 
			} else {
			i=i-1;
            delay(250);
			}
			buttonActive = false;
		}
	}

  decPt = !decPt; // display decimal point every other pass through loop

  // generate characters to display for hexidecimal numbers 0 to F
  for (int i = 0; i <= 15; i) {
    byte bits = myfnNumToBits(i) ;
    if (decPt) {
      bits = bits | B00000001;  // add decimal point if needed
    }
   
    myfnUpdateDisplay(bits);    // display alphanumeric digit
    delay(500);                 // pause for 1/2 second
  }
}

void myfnUpdateDisplay(byte eightBits) {
  if (common == 'a') {                  // using a common anonde display?
    eightBits = eightBits ^ B11111111;  // then flip all bits using XOR 
  }
  digitalWrite(latchPin, LOW);  // prepare shift register for data
  shiftOut(dataPin, clockPin, LSBFIRST, eightBits); // send data
  digitalWrite(latchPin, HIGH); // update display
}

byte myfnNumToBits(int someNumber) {
  switch (someNumber) {
    case 0:
      return B11111100;
      break;
    case 1:
      return B01100000;
      break;
    case 2:
      return B11011010;
      break;
    case 3:
      return B11110010;
      break;
    case 4:
      return B01100110;
      break;
    case 5:
      return B10110110;
      break;
    case 6:
      return B10111110;
      break;
    case 7:
      return B11100000;
      break;
    case 8:
      return B11111110;
      break;
    case 9:
      return B11110110;
      break;
    case 10:
      return B11101110; // Hexidecimal A
      break;
    case 11:
      return B00111110; // Hexidecimal B
      break;
    case 12:
      return B10011100; // Hexidecimal C or use for Centigrade
      break;
    case 13:
      return B01111010; // Hexidecimal D
      break;
    case 14:
      return B10011110; // Hexidecimal E
      break;
    case 15:
      return B10001110; // Hexidecimal F or use for Fahrenheit
      break;  
    default:
      return B10010010; // Error condition, displays three vertical bars
      break;   
  }
}

for (int i = 0; i <= 15; i) {oops

Please note how code tags are used.

Thanks fixed now. This is the first time I have posted, so still learning the ins and outs of how to ask for help properly

Still oops.

for (int i = 0; i <= 15; i) {

What values of i are you expecting this to produce ?
What values of i does it actually produce ?

byte myfnNumToBits(int someNumber) {
  switch (someNumber) {
    case 0:
      return B11111100;
      break;
    case 1:
      return B01100000;
      break;
    case 2:
      return B11011010;
      break;
    case 3:
      return B11110010;
      break;
    case 4:
      return B01100110;
      break;
    case 5:
      return B10110110;
      break;
    case 6:
      return B10111110;
      break;
    case 7:
      return B11100000;
      break;
    case 8:
      return B11111110;
      break;
    case 9:
      return B11110110;
      break;
    case 10:
      return B11101110; // Hexidecimal A
      break;
    case 11:
      return B00111110; // Hexidecimal B
      break;
    case 12:
      return B10011100; // Hexidecimal C or use for Centigrade
      break;
    case 13:
      return B01111010; // Hexidecimal D
      break;
    case 14:
      return B10011110; // Hexidecimal E
      break;
    case 15:
      return B10001110; // Hexidecimal F or use for Fahrenheit
      break;  
    default:
      return B10010010; // Error condition, displays three vertical bars
      break;   
  }

That's one hell of a complicated way to create an array!

byte numberToBits[] = {B11111100, B01100000, ....etc, you fill in the rest};

Use if(someNumber > 15) to trap for the error condition.

ok, what I am trying to do is each time I do a short button press, the counter goes up 1, and if I do a long press, the counter goes down by 1.
I have tried to set "i" to be the variable that changes, and then using i to write it to the 7-segment display

@perrybebbington - that is a much cleaner way to do the array. Much appreciated, I'll try and do that.
So much to learn

Here’s a slight reworking of your code. See if it helps. (Compiles, not tested…)

// Globals
const int dataPin     = 5;  // blue wire to 74HC595 pin 14 INPUT
const int latchPin  = 6; // green to 74HC595 pin 12 Output register clock
const int clockPin  = 7; // yellow to 74HC595 pin 11 Shift register clock
const int button    = 2;

const char common = 'a';    // common anode
//const char common = 'c';    // common cathode

const byte grBitPatterns[] = 
{
    B11111100, B01100000, B11011010, B11110010, 
    B01100110, B10110110, B10111110, B11100000, 
    B11111110, B11110110, B11101110, B00111110, 
    B10011100, B01111010, B10011110, B10001110 
    
};


bool decPt = true;  // decimal point display flag

// button code
const unsigned long ulPressTime = 250; //button long press time

byte
    lastBits,
    lastButton;
int 
    i;

void setup() 
{
    Serial.begin(9600);
    
    // initialize I/O pins
    pinMode(dataPin, OUTPUT);
    pinMode(latchPin, OUTPUT);
    pinMode(clockPin, OUTPUT);
    //button 1 inputs
    pinMode(button, INPUT);
    lastButton = digitalRead( button );
    lastBits = 0;

    i=0;

}//setup

void loop() 
{
    static unsigned long
        timePress=0,
        timeDecimal=0,
        timeButton=0;
    unsigned long
        timeNow;
    byte
        temp;

    //read the button every 20mS
    timeNow = millis();
    if( (timeNow - timeButton) > 20ul )
    {
        timeButton = timeNow;
    
        temp = digitalRead( button );
        if( temp != lastButton )
        {
            lastButton = temp;
            if( temp == HIGH )
            {
                //button press
                Serial.println( "Press" );
                timePress = millis();
                
            }//if
            else
            {
                //button release
                Serial.print( "Release :" );
                if( (millis() - timePress) >= ulPressTime )
                    i = i + (( i<15 ) ? 1:0);
                else
                    i = i - (( i>0 ) ? 1:0);

                Serial.print( i ); Serial.println( "mS" );
                
            }//else
            
        }//if
        
    }//if

    //decimal point toggles every 500mS
    if( timeNow - timeDecimal >= 500ul )
    {
        timeDecimal = timeNow;
        decPt = !decPt; // display decimal point every other pass through loop
        
    }//if

    //set the bit pattern
    byte bits = grBitPatterns[i] | (decPt==true)?B00000001:B00000000;

    //see if the display needs to be updated
    if( lastBits != bits )
    {
        //yes
        lastBits = bits;
        myfnUpdateDisplay(bits);    // display alphanumeric digit
        
    }//if
    
}//loop

void myfnUpdateDisplay(byte eightBits) 
{
    if (common == 'a') 
    {                  // using a common anonde display?
        eightBits = eightBits ^ B11111111;  // then flip all bits using XOR
        
    }//if
    
    digitalWrite(latchPin, LOW);  // prepare shift register for data
    shiftOut(dataPin, clockPin, LSBFIRST, eightBits); // send data
    digitalWrite(latchPin, HIGH); // update display
    
}//myfnUpdateDisplay

Hi,
I tried the new code - when I tried it, it seems like the button is not incrementing the i counter.
Am I doing something wrong?

jtharion:
Hi,
I tried the new code - when I tried it, it seems like the button is not incrementing the i counter.
Am I doing something wrong?

It's probably me.

Is the decimal point blinking?

I edited the code in post 7 to add a couple of debug messages; can you try it again with the Serial Monitor open at 9600 and see if you see if the logic is recognizing the presses?

No, the dp isn't blinking.
Now, don't laugh as I am very new to this, but I debug my code by running it on an emulator in Tinkercad (it's the only way I know how atm). Is there a better way to be doing this?
Not sure how to get the presses debug working inside Tinkercad. What I have been doing, is testing the button code in a different board/project then splicing it in to the main project.

jtharion:
No, the sp isn't blinking.
Now, don't laugh as I am very new to this, but I debug my code by running it on an emulator in Tinkercad (it's the only way I know how atm). Is there a better way to be doing this?
Not sure how to get the presses debug working inside Tinkercad. What I have been doing, is testing the button code in a different board/project then splicing it in to the main project.

Well, I ran the code on my 2560 and it recognized the button presses and incremented and decremented 'i' properly.

I had to make two changes: I changed the pinMode() for the button to "INPUT_PULLUP" and looked for a "LOW" in the pressed button read logic (I use the internal pullup and ground the pin.)

Because you're looking for a "high" when pressed it means your pin idles low. Does your switch apply 5V to the pin when closed? Do you have a pull-down resistor on that pin?

I don't know anything about Tinkercad, sorry.

Thanks for your help - when I press the switch, the output goes high (5v).
But what you said got me thinking and after a bit of messing around (ie trial and error) I was able to figure out my button press code was being reset by my for loop. So I moved by button code into the loop and now it works!

The only issue I am having is a debounce on the button, which I think is fixable.
I also want to figure out a better way to do the array, but for now it works.
Thanks so much for helping me out

for (int i = 0; i <= 15; i) {
    
    if (digitalRead(button) == HIGH) {

		if (buttonActive == false) {
			buttonActive = true;
			buttonTimer = millis();
		}
		if ((millis() - buttonTimer > longPressTime) && (longPressActive == false)) {
			longPressActive = true;
			i=i+1;
            delay(250);
		}
	} else {

		if (buttonActive == true) {
			if (longPressActive == true) {
				longPressActive = false;
                 
			} else {
			i=i-1;
            delay(250);
			}
			buttonActive = false;
		}
	}

    
    
    byte bits = myfnNumToBits(i) ;