fast-increment code, need to include fast-decrement and "start"...stuck...

Hi- I'm trying to learn code by following the examples in Michael Margolis' book, "Arduino Cookbook". I've had a lot of luck with the earlier examples, but am stuck with expanding the functionality of this one (5.4: Determining how long a switch is pressed).
I am trying to get a second switch to decrement down, in the same manner that the upPin goes up...but I can't quite sort out the way to incorporate this into the switchTime() function (or if this is perhaps the wrong way to go about it?)

Here's the working sketch, with the addition of my buttonPin assignments for the other buttons. My goal is to count up as it does, wait. Then, if the downPin is pressed and held, it decrements the same way. Finally, nothing happens until the nextPin button, then, the countdown begins.

const int ledPin = 13;
 const int backPin = 5;
 const int upPin = 6;
  const int downPin = 7;
 const int nextPin = 8;

 const int debounceTime = 20;
 const int fastIncrement = 1000;
 const int veryFastIncrement = 4000;

 int count = 0;
 
 void setup() {
 pinMode (upPin, INPUT);
 digitalWrite (upPin, HIGH);
 pinMode (ledPin, OUTPUT);
 Serial.begin(9600);
 }

void loop() {
  int duration = switchTime();
  if(duration > veryFastIncrement)
  count = count + 10;
  else if (duration > fastIncrement)
  count = count + 4;
  else if (duration > debounceTime)
  count = count + 1;

  else
  {
    if (count == 0)
    digitalWrite (ledPin, HIGH);
    else
    {
      digitalWrite (ledPin, LOW);
      count = count - 1;
    }
    }
  
  Serial.println(count);
  delay(100);
  
  }

  long switchTime()
  {
    static unsigned long startTime = 0;
    static boolean state;
    if (digitalRead(upPin) != state)
    {
      state = ! state;
      startTime = millis();
    }
    if (state == LOW)
    return millis() - startTime;
    else
    return 0;
  }

Thank you in advance for any advice you can provide!

Since the switchTime() function reads the upPin pin, you either need to make that a parameter or duplicate the switchTime() function and rename it and then read the downPin. Since it also uses static variables and you seem to be learning, the easiest would be to create a second function switchDownTime()

Inside the loop() function, you would then test the switchDownTime() value. Something like this: (untested)

const int ledPin = 13;
const int backPin = 5;
const int upPin = 6;
const int downPin = 7;
const int nextPin = 8;

const int debounceTime = 20;
const int fastIncrement = 1000;
const int veryFastIncrement = 4000;

int count = 0;

void setup() {
  pinMode (upPin, INPUT);
  digitalWrite (upPin, HIGH);
  pinMode (ledPin, OUTPUT);
  Serial.begin(9600);
}

void loop() {
  int duration = switchUpTime();
  if (duration > veryFastIncrement)
    count = count + 10;
  else if (duration > fastIncrement)
    count = count + 4;
  else if (duration > debounceTime)
    count = count + 1;

  duration = switchDownTime();
  if (duration > veryFastIncrement)
    count = count - 10;
  else if (duration > fastIncrement)
    count = count - 4;
  else if (duration > debounceTime)
    count = count - 1;

  if (count == 0) {
    digitalWrite (ledPin, HIGH);
  }
  else
  {
    digitalWrite (ledPin, LOW);
    count = count - 1;
  }
  Serial.println(count);
  delay(100);

}

long switchUpTime()
{
  static unsigned long startTime = 0;
  static boolean state;
  if (digitalRead(upPin) != state)
  {
    state = ! state;
    startTime = millis();
  }
  if (state == LOW)
    return millis() - startTime;
  else
    return 0;
}

long switchDownTime()
{
  static unsigned long startTime = 0;
  static boolean state;
  if (digitalRead(downPin) != state)
  {
    state = ! state;
    startTime = millis();
  }
  if (state == LOW)
    return millis() - startTime;
  else
    return 0;
}

I would also recommend that you ALWAYS use braces for your if/else statements. It is a good habit to get into and will save you lots of headaches when you later add a line of code and break things.

Note that since Michael wrote that book the Arduino IDE has changed so that

pinMode (upPin, INPUT);
 digitalWrite (upPin, HIGH);

Can be replaced with

pinMode (upPin, INPUT_PULLUP);

Can be replaced with "[color=brown]pinMode (upPin, INPUT_PULLUP);[/color]"

Not just "Can be", but "should be"!

westfw:
Not just "Can be", but "should be"!

Aside from shorter, cleaner code, is there a technical consideration here I'm missing?

Thanks!

blh64:
Since the switchTime() function reads the upPin pin, you either need to make that a parameter or duplicate the switchTime() function and rename it and then read the downPin. Since it also uses static variables and you seem to be learning, the easiest would be to create a second function switchDownTime()

Inside the loop() function, you would then test the switchDownTime() value. Something like this: (untested)

const int ledPin = 13;

const int backPin = 5;
const int upPin = 6;
const int downPin = 7;
const int nextPin = 8;

const int debounceTime = 20;
const int fastIncrement = 1000;
const int veryFastIncrement = 4000;

int count = 0;

void setup() {
  pinMode (upPin, INPUT);
  digitalWrite (upPin, HIGH);
  pinMode (ledPin, OUTPUT);
  Serial.begin(9600);
}

void loop() {
  int duration = switchUpTime();
  if (duration > veryFastIncrement)
    count = count + 10;
  else if (duration > fastIncrement)
    count = count + 4;
  else if (duration > debounceTime)
    count = count + 1;

duration = switchDownTime();
  if (duration > veryFastIncrement)
    count = count - 10;
  else if (duration > fastIncrement)
    count = count - 4;
  else if (duration > debounceTime)
    count = count - 1;

if (count == 0) {
    digitalWrite (ledPin, HIGH);
  }
  else
  {
    digitalWrite (ledPin, LOW);
    count = count - 1;
  }
  Serial.println(count);
  delay(100);

}

long switchUpTime()
{
  static unsigned long startTime = 0;
  static boolean state;
  if (digitalRead(upPin) != state)
  {
    state = ! state;
    startTime = millis();
  }
  if (state == LOW)
    return millis() - startTime;
  else
    return 0;
}

long switchDownTime()
{
  static unsigned long startTime = 0;
  static boolean state;
  if (digitalRead(downPin) != state)
  {
    state = ! state;
    startTime = millis();
  }
  if (state == LOW)
    return millis() - startTime;
  else
    return 0;
}




I would also recommend that you ALWAYS use braces for your if/else statements. It is a good habit to get into and will save you lots of headaches when you later add a line of code and break things.

Thanks for the advice, I had to create another int (duration2), but your code worked. I added a for()loop so that when I hit the next button, it runs the countdown, and had to do a little finagling to get that behavior right, but it does what I was looking for, so thank you!!

blh64:
Since the switchTime() function reads the upPin pin, you either need to make that a parameter or duplicate the switchTime() function and rename it and then read the downPin. Since it also uses static variables and you seem to be learning, the easiest would be to create a second function switchDownTime()

OK- now that I'm playing around a bit, I'd like to know more about your initial suggestion of making the upPin a parameter...would you mind elaborating? It sounds like you're suggesting the function read both pins, and takes actions within one function, instead of two functions acting on the same static variable...but that's about the limit of my understanding at this point.

Out of curiosity, do you have a favorite reference book for Arduino? I get that the forum/internet is the most up to date, but I'm not a fan of taking a baby step and then having to ask for more help (despite me doing it repeatedly here! It's embarrassing for me and more work for you! :confused: ) And thank you again!

Not just "Can be", but "should be"!

Aside from shorter, cleaner code, is there a technical consideration here I'm missing?

Yeah, sort-of. Since it was documented that "digitalWrite(pin, HIGH)" turned on pullups for the AVR (before pinMode(pin, INPUT_PULLUP) existed), subsequent platforms have been including EXTRA code in digitalWrite() to cause that to still happen. Slowing them down significantly. :frowning:
If the digitalWrite() stuff fall out of use enough, perhaps that extra code can be removed.