Counter skipping numbers

mrneedles · December 8, 2015, 7:47pm

Hi everyone,

I'm having some newb problems here. Part of my program needs to increment a counter between 1 and 5. I have an up and a down button that will control the incriminating.

My problem is that I cannot get this to work right. I tried using the constrain() command but that skips numbers. For example,

speedCounter = constrain(speedCounter,1,5);

gives me this result. Up button = 2345 Down button = 5321

Or if I use this code below, I get 6,5,4,3,2,1,0,-1. I just want to count from 1 to 5 and stop. Then 5 to 1 and stop.

if((speedCounter >= 5))
{
  speedCounter = 5; 
}

if((speedCounter <= 1))
{
  speedCounter = 1; 
}


speedUpState = digitalRead(speedUp);                     //Increment the Speed up Button

  // compare the buttonState to its previous state
  if (speedUpState != lastSpeedUpState) 
  {
    // if the state has changed, increment the counter
    if (speedUpState == LOW) 
    {
      // if the current state is HIGH then the button
      // wend from off to on:
      speedCounter++;
      lcd.setCursor(13,0);
      lcd.print(speedCounter);
      EEPROM.update(speedAddress, speedCounter);
    }
    else {
    }
    // Delay a little bit to avoid bouncing
    delay(20);
  }
    lastSpeedUpState = speedUpState;


speedDownState = digitalRead(speedDown);                //Increment the Speed up Button

  // compare the buttonState to its previous state
  if (speedDownState != lastSpeedDownState) {
    // if the state has changed, increment the counter
    if (speedDownState == LOW) {
      // if the current state is HIGH then the button
      // wend from off to on:
      speedCounter--;
      lcd.setCursor(13,0);
      lcd.print(speedCounter);
      EEPROM.update(speedAddress, speedCounter);
    }
    else {
    }
    // Delay a little bit to avoid bouncing
    delay(20);
  }
  // save the current state as the last state,
  //for next time through the loop
  lastSpeedDownState = speedDownState;

jremington · December 8, 2015, 8:12pm

Post all your code.

Robin2 · December 8, 2015, 8:24pm

mrneedles:
an up and a down button that will control the incriminating.

I hope you only use the down button.

...R

KeithRB · December 8, 2015, 8:25pm

Probably one of two things:
debouncing
a classic-every-programmer-mistake off-by-one error.

mrneedles · December 8, 2015, 9:22pm

Sorry guys, here's my entire code.

#include <LiquidCrystal.h>
#include <EEPROM.h>

int STEPS = 1600;
Stepper myStepper(STEPS, 9, 8);
long travLength = 0;    //Length of traverse

int address = 0;

int Speed = 0;
int Speed1 = 300;
int Speed2 = 400;
int Speed3 = 500;
int Speed4 = 600;
int Speed5 = 700;
int speedUp = 22;      //Increase speed button
int speedDown = 23;    //Decrease speed button
int travUp = 24;       //Increase trav length button
int travDown = 25;     //Decrease trav legnth button

int limitSwitch = 50;     //limit siwtch for zero position
int enableSwitch = 52;    //Trav on/off switch

int travCounter = 0;
int travUpState = 0;
int lastTravUpState = 0;
int travDownState = 0;
int lastTravDownState = 0;

int speedCounter = 0;
int speedUpState = 0;
int lastSpeedUpState = 0;
int speedDownState = 0;
int lastSpeedDownState = 0;

int lengthAddress = 0;
int speedAddress = 0;
byte lengthValue = 0;
byte speedValue = 0;


LiquidCrystal lcd(12, 11, 5, 4, 3, 2);


/*************************************************************************************************/

void setup()
{
  pinMode(speedUp, INPUT_PULLUP);
  pinMode(speedDown, INPUT_PULLUP);
  pinMode(travUp, INPUT_PULLUP);
  pinMode(travDown, INPUT_PULLUP);
  pinMode(limitSwitch, INPUT_PULLUP);
  pinMode(enableSwitch, INPUT_PULLUP);



  while (digitalRead(limitSwitch) == HIGH)  //Home the traverse arm
  {
    myStepper.setSpeed(100);
    myStepper.step(-1);
    if (digitalRead(limitSwitch) == LOW)
    {
      myStepper.setSpeed(100);
      myStepper.step(1000);
    }
  }


  lcd.begin(20, 4);
  lcd.setCursor(0, 0);
  lcd.print("Trav Speed:    ");
  lcd.setCursor(0, 1);
  lcd.print("Trav Length:   ");
  lcd.setCursor(0, 4);
  lcd.print("Status:   ");
}
/***************************************************************************************************/

void loop()
{

  speedCounter = EEPROM.read(speedAddress);

  speedCounter = constrain(speedCounter, 1, 4);

  /*if((speedCounter >= 5))
  {
    speedCounter = 5;
  }

  if((speedCounter <= 1))
  {
    speedCounter = 1;
  }*/


  speedUpState = digitalRead(speedUp);                     //Increment the Speed up Button

  // compare the buttonState to its previous state
  if (speedUpState != lastSpeedUpState)
  {
    // if the state has changed, increment the counter
    if (speedUpState == LOW)
    {
      // if the current state is HIGH then the button
      // wend from off to on:
      speedCounter++;
      lcd.setCursor(13, 0);
      lcd.print(speedCounter);
      EEPROM.update(speedAddress, speedCounter);
    }
    else {
    }
    // Delay a little bit to avoid bouncing
    delay(20);
  }
  lastSpeedUpState = speedUpState;


  speedDownState = digitalRead(speedDown);                //Increment the Speed up Button

  // compare the buttonState to its previous state
  if (speedDownState != lastSpeedDownState) {
    // if the state has changed, increment the counter
    if (speedDownState == LOW) {
      // if the current state is HIGH then the button
      // wend from off to on:
      speedCounter--;
      lcd.setCursor(13, 0);
      lcd.print(speedCounter);
      EEPROM.update(speedAddress, speedCounter);
    }
    else {
    }
    // Delay a little bit to avoid bouncing
    delay(20);
  }
  // save the current state as the last state,
  //for next time through the loop
  lastSpeedDownState = speedDownState;

  /*speedAddress = speedAddress + 1;
  if (speedAddress == EEPROM.length())
  {
    speedAddress = 0;
  }*/



  travUpState = digitalRead(travUp);

  // compare the buttonState to its previous state
  if (travUpState != lastTravUpState) {
    // if the state has changed, increment the counter
    if (travUpState == LOW) {
      // if the current state is HIGH then the button
      // wend from off to on:
      travCounter = travCounter + 5;
      lcd.setCursor(13, 1);
      lcd.print(travCounter);
    }
    else {
    }
    // Delay a little bit to avoid bouncing
    delay(10);
  }
  lastTravUpState = travUpState;



  travDownState = digitalRead(travDown);

  // compare the buttonState to its previous state
  if (travDownState != lastTravDownState) {
    // if the state has changed, increment the counter
    if (travDownState == LOW) {
      // if the current state is HIGH then the button
      // wend from off to on:
      travCounter = travCounter - 5;
      lcd.setCursor(13, 1);
      lcd.print(travCounter);
    }
    else {
    }
    // Delay a little bit to avoid bouncing
    delay(10);
  }

  if (travCounter > 1000 || travCounter < 0)
  {
    (travCounter = 0);
  }
  // save the current state as the last state,
  //for next time through the loop
  lastTravDownState = travDownState;

  lengthAddress = lengthAddress + 1;
  // if (lengthAddress == EEPROM.length())
  // {
  //   lengthAddress = 0;
  // }


  travLength = 3.183 * (travCounter); //3.183 steps/mm :  add 20mm for spacing

  if (speedCounter == 1)
  {
    Speed = Speed1;
  }

  if (speedCounter == 2)
  {
    Speed = Speed2;
  }

  if (speedCounter == 3)
  {
    Speed = Speed3;
  }

  if (speedCounter == 4)
  {
    Speed = Speed4;
  }

  if (speedCounter == 5)
  {
    Speed = Speed5;
  }



  if (digitalRead(enableSwitch) == LOW)
  {
    myStepper.setSpeed(Speed);
    myStepper.step(13000);
    myStepper.step(-13000);
    lcd.setCursor(8, 4);
    lcd.print("Traverse ON ");
  }

  if (digitalRead(enableSwitch) == HIGH)
  {
    while (digitalRead(limitSwitch) == HIGH)  //Home the traverse arm
    {
      lcd.setCursor(8, 4);
      lcd.print("HOMING      ");
      myStepper.setSpeed(100);
      myStepper.step(-100);
    }
  }

  if (digitalRead(limitSwitch) == LOW)
  {
    myStepper.setSpeed(100);
    myStepper.step(0);
    lcd.setCursor(8, 4);
    lcd.print("Traverse OFF");
  }


}

I tried a little debouncing and it still didn't help

Robin2 · December 8, 2015, 9:50pm

I suggest you reorganize your code so all the buttons or switches are read in one function and then the actions take place in other functions. That way each function can be tested on its own. The code in loop() could be reduced to this

void loop() {
   readButtonsAndUpdateCounters();
   updateLCD();
   moveStepper();
}

Have a look at Planning and Implementing a Program

Also, you seem to have lots of delay()s. A single interval should be sufficient to debounce all the switches. Look at how it is done in Several Things at a Time

...R

mrneedles · December 16, 2015, 4:56pm

Thanks for the help Robin. I'm working on restructuring the program now.

While I'm learning, does anyone have a suggestion as to why the numbers are being skipped? This is really puzzling me.

system · December 16, 2015, 5:32pm

Or if I use this code below, I get 6,5,4,3,2,1,0,-1. I just want to count from 1 to 5 and stop. Then 5 to 1 and stop.

The time to constrain the value, whether using constrain() or your (equivalent) method, is AFTER you increment/decrement the value, not before doing so.

jurs · December 16, 2015, 5:54pm

mrneedles:
For example,

speedCounter = constrain(speedCounter,1,5);

gives me this result. Up button = 2345 Down button = 5321

Or if I use this code below, I get 6,5,4,3,2,1,0,-1. I just want to count from 1 to 5 and stop. Then 5 to 1 and stop.

Your programming logic is weird and your anti-bouncing delay duration lasts much too long:

   // Delay a little bit to avoid bouncing
    delay(20);

Bouncing with mechanical buttons typically occur within one millisecond, so if you'd debounce for a 5 millisecond period should be enough.

If I were you I'd use a "buttonPressed()" function which returns true after a pressed button was detected and some logic like that:

void loop()
{
  static byte count=1;
  static boolean countingUp=true;
  if (buttonPressed())
  {
    if (countingUp) count++;
    else count--;
    if (count>=5 || count<=1) countingUp=!countingUp;
  }
}

BTW: It's a shame that the Arduino platform comes along with big libraries for controlling complicated hardware like Ethernet or LiquidCrystal LCD, but for reading mechanical buttons the only library support is the "digitalRead()" function included in the Arduino core libraries.

BulldogLowell · December 17, 2015, 12:07pm

jurs:
BTW: It's a shame that the Arduino platform comes along with big libraries for controlling complicated hardware like Ethernet or LiquidCrystal LCD, but for reading mechanical buttons the only library support is the "digitalRead()" function included in the Arduino core libraries.

What about the Bounce2 library? Its a small library for controlling simple buttons.

jurs · December 17, 2015, 12:37pm

BulldogLowell:
What about the Bounce2 library? Its a small library for controlling simple buttons.

That library is not included in the Arduino-IDE, it needs additional installing from a third-party source.

Unfortunately the library is a waste of RAM when using more than one button, because each button uses:

protected:
    unsigned long previous_millis;
    uint16_t interval_millis;
    uint8_t state;
    uint8_t pin;

That way each button uses 8 bytes of RAM internally, plus the RAM each object derived from a class needs.

I'd prefer a button reading which uses less RAM, especially if more than one button is used in a sketch.

But of course, you can use the bounce2 or different third-party library.
As far as I've heard, it's working.

PaulMurrayCbr · December 17, 2015, 1:23pm

// if the current state is HIGH then the button
// wend from off to on:
speedCounter++;
lcd.setCursor(13, 0);
lcd.print(speedCounter);
EEPROM.update(speedAddress, speedCounter);
}

Yep, that would do it.

Load this code into an arduino, look at the output, and try to grokk what it is doing wrong.

boolean goingup = true;
int speedCounter = 1;

void setup() {
  // put your setup code here, to run once:
  Serial.begin(19200);
}

void loop() {
  if (speedCounter <= 1) speedCounter = 1;
  if (speedCounter >= 5) speedCounter = 5;

  if (goingup) speedCounter++;
  else speedCounter --;
  
  Serial.print("speedCounter is ");
  Serial.print(speedCounter);
  Serial.println();

  if(speedCounter < 1) goingup = true;
  if(speedCounter > 5) goingup = false;  

}

BulldogLowell · December 17, 2015, 3:12pm

jurs:
Unfortunately the library is a waste of RAM when using more than one button, because each button uses:

Well, you do need to keep variables for tracking pin changes, so I don't think there is all that much overhead in the little library. It scales nicely too, an array of buttons is easy as PI.

also, OP has SEVERAL delays in every call to loop() where (for primitive debounce) exactly one would suffice.

Topic		Replies	Views
Button counter not incrementing General Guidance	8	988	September 3, 2022
Using a for loop to increment counter to 8 and cancel General Guidance	22	2665	September 3, 2022
Skipping a number in a counting code Programming	7	1227	May 6, 2021
Button that increments a counter Programming	2	313	May 5, 2021
What is my increment code missing? Programming	9	865	May 5, 2021

Counter skipping numbers

Related topics