Changing increment speed based on button press length

Firstly; Hi all, new to the forum and pretty new to the Arduino and fairly ok-ish @ C, hoping I posted this in the right place!!

I already have 98.5% of my code working perfectly though (cutting a long story, and code, short) I need to change a value based on the input from two switches. One for up, the other for down… but I want the speed at which the rate goes up/down to be based on the button length press in real time…

I tried making my own timer which failed - it would do one action depending on how long the button had been pressed but nothing until I released the button… so some forum searching and Google led me to the millis() function… Either I don’t understand it properly or it’s not suitable for what I want to do. Below is my routine which houses the issue and any accompanying code/declarations etc.

Hopefully someone can see what I am trying to do and point me in the right direction!

I have my own code for checking whether it’s a ‘unique press’ rather than using a library with uniquePress()

#define S8 A0
#define S2 A1  // ran out of digital pins!!

int setV = 750;           // default set voltage value (0-1023)

//int debounce = 50;        // switch/button debounce time in ms (not used if hardware debounce used)
int button = 5;          // set increase/decrease value on single button press
int speed1 = 4;           // speed increase multiplier after button held for >time1
int speed2 = 8;           // speed increase multiplier after button held for >time2
int time1 = 3;            // time in seconds before first multiplier applied
int time2 = 8;            // time in seconds before second multiplier applied

long millisStore;
long millisHeld;          // how long the button was held (milliseconds)
long secondsHeld;         // how long the button was held (seconds)





void setVolts(){
  long millisCurrent;
  long millisHeld;         // how long the button was held (milliseconds)
  long secondsHeld;           // how long the button was held (seconds) */
  int button8 = digitalRead(S8);
  int button2 = digitalRead(S2);

  if (button8 == 0 || button2 == 0){         // this is a recent addition to try and make millis timer work
    millisStore = millis();
    secondsHeld = 0;
  }
  
  if (button8 == 0 && button2 == 1){              // if S8 keypress is unique
    //delay(debounce);                            // debounce timer (not req as using HW debounce)
    millisHeld = millis() - millisStore; 
    secondsHeld = millisHeld / 1000;
    
    if (secondsHeld >= time1 && secondsHeld <= time2){
      setV = setV-button*speed1;                  // increase speed if button held for >time1 seconds  
    }
    if (secondsHeld > time2){
      setV = setV-button*speed2;                  // increase speed if button held for >time2 seconds  
    }
    else {
      setV = setV-button; 
    }
  }
  
  else if (button2 == 0 && button8 == 1){
    //delay(debounce);                            // debounce timer
    millisHeld = millis() - millisStore;
    secondsHeld = millisHeld / 1000;
    
    if (secondsHeld <= time1){
      setV = setV+button;                         // pressing S2 increases the set value  
    }
    if (secondsHeld > time1 && secondsHeld <= time2){
      setV = setV+button*speed1;                  // increase speed if button held for >time1 seconds  
    }
    if (secondsHeld > time2){
      setV = setV+button*speed2;                  // increase speed if button held for >time2 seconds  
    }
  }
  
  if (setV >= 1023){                              
    setV = 1023;
  }
  else if (setV <= 0){
    setV = 0;
  }                                               // ensures that value of setV never goes beyond possible limits

I think you need to more thoroughly explain exactly what you're trying to accomplish and what you mean by "unique press".

I'm not sure whether this was intentional but you have millisHeld and secondsHeld declared as global and also local to setVolts().

I recommend using LOW and HIGH instead of 0 and 1 for pin states. Either way is valid but LOW/HIGH makes the code more readable. In this usage 0 and 1 are what is called magic numbers, which should be avoided in programming. Even better would be something like this:

const byte buttonReleasedState = HIGH;
const byte buttonPressedState = LOW;

This will make it very simple to understand the code even without any comments:

  if (button8 == buttonPressedState || button2 == buttonPressedState){

Or better yet:

  int button8State = digitalRead(button8Pin);
  int button2State = digitalRead(button8Pin);

  if (button8State == buttonPressedState || button2State == buttonPressedState){

pert:
I think you need to more thoroughly explain exactly what you’re trying to accomplish and what you mean by “unique press”.

I’m not sure whether this was intentional but you have millisHeld and secondsHeld declared as global and also local to setVolts().

I recommend using LOW and HIGH instead of 0 and 1 for pin states. Either way is valid but LOW/HIGH makes the code more readable. In this usage 0 and 1 are what is called magic numbers, which should be avoided in programming. Even better would be something like this:

const byte buttonReleasedState = HIGH;

const byte buttonPressedState = LOW;



This will make it very simple to understand the code even without any comments:


if (button8 == buttonPressedState || button2 == buttonPressedState){



Or better yet:


int button8State = digitalRead(button8Pin);
  int button2State = digitalRead(button8Pin);

if (button8State == buttonPressedState || button2State == buttonPressedState){

Thanks for the reply.

The unique keypress basically means so that if S8 and S2 are pushed it does nothing. [button 8 and button 2 respectively)

If S8 is pushed (switch goes low as using pullups, forgot to mention that bit - sorry!) then setV is to increase by a defined amount. If the switch is held in for time1 then the incremental amount is multiplied (speed1), if the switch is held in even longer then it is to be incremented by a higher multiple (speed2).

This is then the same for S2/Button2 though the value is to decrease.

I made a right mess of the code trying to do this bit as I still can’t figure it out.

in a nutshell

if S8=0 and S2=1

if time button held <3 seconds
setv = setv +a (constantly until button released - I have a 500ms delay elsewhere)

if time button held >3 & <8 seconds
setv = setv +a*b

if time button held > 8 seconds
setv = setv +a*c

I really hope that the above makes more sense! - As for the declarations the setVolts ones are commented out as I tried declaring them in different places

i think you should check each button in a separate function . first make it work (really work) for 1 function then complete for 2.
then you also need on the loop and update_output() function which can take your SetV value and update something ... like the speed of a motor i dont know... or just eliminate this one.

loop()
checkButton1()
checkButton2()
update_output()

...
when checking a button, define a minimum time held the button needs to be pressed to be considered as "pressed"
then update with status and on the next loop if you have the variable as pressed and the timer running check values.

function definition could include something like this

checkButton1()

if (button1 pressed and status1=0) 'was not pressed before
status1 = ON
timePressed = millis() 'update when i pressed the button
delay() 'optimize

if (button1 and status1=1) 'pressed before

if (time-timePressed > range1 and time-timePressed <range3) then Setv=Setv+a

i will write later this week if i find time a working procedure and send you.
sorry i dont have time to make a full delivery now

Right, I’ve tidied up the code and remove unnecessary bits too (which I discovered after naming things sensibly and testing them).

Below is the full code. It’s, in essence, a user programmable voltage comparator which lights an LED when the ‘read’ input voltage is equal to the ‘set’ voltage - within a defined tolerance.

Everything works perfectly apart from the issue I’m trying to fix - increasing/decreasing setv based on how long a button has been pressed. I have highlighted the affected area of code with
/***************************************************************************/ above and below the area that I know is the issue.

My comments/wording used may not be perfect as I am far from a ‘decent’ programmer so my knowledge is a little thin.

#include <LiquidCrystal.h>
#define onboardLED 13         // On-board LED
#define lcdEnablePin 9        // Pin to control backlight on/off
#define lcdContrastPin 10     // Pin for PWM lcdContrastPin control
#define inputPin A7           // Analogue input pin
#define outputPin 8           // outputPin pin [LED at the moment]
#define button8Pin A0
#define button2Pin A1

const byte buttonReleased = HIGH;
const byte buttonPressed = LOW;

int ledState = LOW;
int heartbeatDelay = 1000;
long millisPreviousHB = 0;    // variables for Heartbeat routine

float vset;                  
float vread;                  // global variables for comparison, loaded during set and read routines

unsigned long millisStore;
unsigned long millisHeld;
unsigned long secondsHeld;    // global variables for millis() timing handling

const int button = 5;         // set increase/decrease value on single button press
const int speed1 = 4;         // speed increase multiplier after button held for >time1
const int speed2 = 8;         // speed increase multiplier after button held for >time2
const int time1 = 3;          // time in seconds before first multiplier applied
const int time2 = 8;          // time in seconds before second multiplier applied
//const int debounce = 50;    // switch/button debounce time in ms (not used if hardware debounce used)

int setV = 750;               // default set voltage value (use adc value 0-1023)
float tolerance = 0.2;        // tolerance for comparison in 'real' value. 0.2 = 0.2V
float inputPinVoltage = 4.7;  // change to suit inputPin voltage [5.0 or 3.3 typical, for accuracy; measure Vsupply and adjust to suit]


LiquidCrystal lcd(12, 11, 5, 4, 3, 2);      // initialize the library with the numbers of the LCD interface pins

void setup() {
  digitalWrite(button8Pin, INPUT_PULLUP);   // Set pin modes and setup/lcdEnablePin LCD
  digitalWrite(button2Pin, INPUT_PULLUP);   // also lcdEnablePins internal pull ups for switches
  pinMode(onboardLED, OUTPUT);
  pinMode(lcdContrastPin, OUTPUT);
  pinMode(lcdEnablePin, OUTPUT);
  pinMode(outputPin, OUTPUT);
  lcd.begin(16,2);                          // set up the LCD's (columns, rows)
  analogWrite(lcdContrastPin, 60);          // Set LCD Contrast, PWM value to control lcdContrastPin, lower = brighter [0-255], controlling the LCD this way can lead to some 'flickering' at certain viewing angles
  lcd.clear();
  digitalWrite(lcdEnablePin, HIGH);;        // lcdEnablePin backlight after ensuring it's empty
  delay(1000);                              // Delay before running program
}

void loop() {
  setVolts();
  readVolts();
  compare();
  heartbeat();
  delay(500);       // 'global' delay between loop 
}

void readVolts(){
  int sample = analogRead(inputPin);              // sample analogue pin
  vread = inputPinVoltage/1023*sample;            // convert ADC reading to voltage
  char readVoltage[4];
  dtostrf(vread, 1, 2, readVoltage);              // convert float to string - very useful function this!
  char dispRead[15];
  sprintf(dispRead, "Read = %sV", readVoltage);   // sets prefix/suffix for 'new' string
  lcd.setCursor(0,0);                             // sets LCD cursor (Column, Row) - 0,0 is left most column, top row
  lcd.write(dispRead);                            // displays the 'new' string 
}




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

void setVolts(){
    
  int button8State = digitalRead(button8Pin);
  int button2State = digitalRead(button2Pin);
  millisStore = millis();
  secondsHeld = 0;
  
  if (button8State == buttonPressed && button2State == buttonReleased){             // if only button 8 is pressed
    //delay(debounce);                            // debounce timer (not req if using HW debounce)
    millisHeld = millis() - millisStore; 
    secondsHeld = millisHeld / 1000;
    
    if (secondsHeld >= time1 && secondsHeld <= time2){
      setV = setV-button*speed1;                  // increase speed if button held for >time1 seconds  
    }
    if (secondsHeld > time2){
      setV = setV-button*speed2;                  // increase speed if button held for >time2 seconds  
    }
    else {
      setV = setV-button; 
    }
  }
  
  else if (button8State == buttonReleased && button2State == buttonPressed){        // if only button 2 is pressed
    //delay(debounce);                              // debounce timer
    millisHeld = millis() - millisStore;
    secondsHeld = millisHeld / 1000;
    
    if (secondsHeld <= time1){
      setV = setV+button;                         // pressing button2Pin increases the set value  
    }
    if (secondsHeld > time1 && secondsHeld <= time2){
      setV = setV+button*speed1;                  // increase speed if button held for >time1 seconds  
    }
    if (secondsHeld > time2){
      setV = setV+button*speed2;                  // increase speed if button held for >time2 seconds  
    }
  }

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



  
  if (setV >= 1023){                              
    setV = 1023;
  }
  else if (setV <= 0){
    setV = 0;
  }                                               // ensures that value of setV never goes beyond possible limits
  
  vset = inputPinVoltage/1023*setV;               // convert setV reading to voltage
  char setVoltage[4];
  dtostrf(vset, 1, 2, setVoltage);                // convert float to string
  char dispSet[15];
  sprintf(dispSet, "Set  = %sV", setVoltage);     // sets pre/post fix for 'new' string, note additional space before = for alignment purposes
  lcd.setCursor(0,1);                             // sets LCD cursor (Column, Row) - 0,1 is left most column, bottom row
  lcd.write(dispSet);                             // displays the 'new' string  
}


void compare(){                                   // compares set and read voltages with defined tolerance 
    if (vset <= vread + tolerance && vset >= vread - tolerance){   
    digitalWrite(outputPin, LOW);  
    }
    else {
      digitalWrite(outputPin, HIGH);
    }
}


void heartbeat(){                                 // 'heartbeat LED' to show code running/working
  long millisCurrentHB = millis();
  if (millisCurrentHB - millisPreviousHB >= heartbeatDelay){
      millisPreviousHB = millisCurrentHB;
      if (ledState == LOW) {
      ledState = HIGH;
      } 
        else {
        ledState = LOW;
        }
    digitalWrite(onboardLED, ledState);
  }
}

You call setVolts() every pass through loop().

At the start of that function you assign millisStore = millis(). This will increment at the same rate as millis() and millisHeld will never grow.

You need a boolean control variable to only set it once.

if(timingStarted == false)
{
millisStore = millis();
timingStarted = true;
}

set timingStarted back to false when you release the button.

cattledog:
You call setVolts() every pass through loop().

At the start of that function you assign millisStore = millis(). This will increment at the same rate as millis() and millisHeld will never grow.

You need a boolean control variable to only set it once.

if(timingStarted == false)
{
millisStore = millis();
timingStarted = true;
}

set timingStarted back to false when you release the button.

This makes sense to me, thought it would be something like that. I'll give it a go and see how I get on, thanks :slight_smile:

cattledog:
You need a boolean control variable to only set it once.

Well I did some code editing and removed some extra maths and conversions that were not required.

I also did some rewiring which would allow me to use the Nano’s two interrupts thinking that this would solve my issue looking at the rising/falling edge of the press… though wither my understanding of interrupts is wrong or I’ve done something wrong… (very much an AVR/Arduino noob as only doing this for about a week, I did some RTES work many, many, many years ago but forgotten most of it. Used this https://www.arduino.cc/en/Reference/attachInterrupt as a resource)

Here is the new code as it stands:
I’ve added in an extra bit so that I can view the state of ‘secondsHeld’ and it’s purely a counter - which means that millisHeld is being constantly updated with millis() still…

I now know what the problem is - I’m just unsure on how to fix it…

#include <LiquidCrystal.h>
#define onboardLED 13         // On-board LED
#define lcdEnablePin 9        // Pin to control backlight on/off
#define lcdContrastPin 10     // Pin for PWM lcdContrastPin control
#define inputPin A7           // Analogue input pin
#define outputPin 8           // outputPin pin [LED at the moment]
#define button8Pin 3          // button 8 decrease (left button) button 2 increase (right button)
#define button2Pin 2          // pins 3 and 2 can be interrupts (edge detection)

LiquidCrystal lcd(12, 11, 7, 6, 5, 4);      // initialize the library with the numbers of the LCD interface pins

volatile int button8State = HIGH;
volatile int button2State = HIGH;
const int buttonReleased = HIGH;
const int buttonPressed = LOW;

int ledState = LOW;
int heartbeatDelay = 1000;
long millisPreviousHB = 0;    // variables for Heartbeat routine

float vset = 3.0;             // default 'set' voltage value on system start     
float vread;                  // 'read voltage' variable

unsigned long millisStore;
unsigned long millisHeld;
unsigned int secondsHeld;    // global variables for millis() timing handling
int timingStarted = false;
 
const int speed1 = 4;         // speed increase multiplier after button held for >time1
const int speed2 = 8;         // speed increase multiplier after button held for >time2
const int time1 = 2;          // time in seconds before first multiplier applied
const int time2 = 5;          // time in seconds before second multiplier applied
//const int debounce = 50;    // switch/button debounce time in ms (not used if hardware debounce used)

float buttonChange = 0.1;     // set increase/decrease value (in Volts) on single button press
float tolerance = 0.2;        // tolerance for comparison in 'real' value. 0.2 = 0.2V
float inputPinVoltage = 4.7;  // change to suit inputPin voltage [5.0 or 3.3 typical, for accuracy; measure Vsupply and adjust to suit]


void setup() {
  pinMode(button8Pin, INPUT_PULLUP);   // Set pin modes and setup/lcdEnablePin LCD
  pinMode(button2Pin, INPUT_PULLUP);   // also lcdEnablePins internal pull ups for switches
  pinMode(onboardLED, OUTPUT);
  pinMode(lcdContrastPin, OUTPUT);
  pinMode(lcdEnablePin, OUTPUT);
  pinMode(outputPin, OUTPUT);
  attachInterrupt(button8Pin, buttonISR, FALLING);
  attachInterrupt(button2Pin, buttonISR, FALLING);
  attachInterrupt(button2Pin, buttonCleanup, RISING);
  attachInterrupt(button2Pin, buttonCleanup, RISING);
  lcd.begin(16,2);                          // set up the LCD's (columns, rows)
  analogWrite(lcdContrastPin, 60);          // Set LCD Contrast, PWM value to control lcdContrastPin, lower = brighter [0-255], controlling the LCD this way can lead to some 'flickering' at certain viewing angles
  lcd.clear();
  digitalWrite(lcdEnablePin, HIGH);;        // lcdEnablePin backlight after ensuring it's empty
  delay(1000);                              // Delay before running program
}

void loop() {
  setVolts();
  readVolts();
  compare();
  //heartbeat();
  delay(500);       // 'global' delay between loop 
}

void readVolts(){
  int sample = analogRead(inputPin);              // sample analogue pin
  vread = inputPinVoltage/1024*sample;            // convert ADC reading to voltage
  char readVoltage[4];
  dtostrf(vread, 1, 2, readVoltage);              // convert float to string - very useful function this!
  char dispRead[15];
  sprintf(dispRead, "Read = %sV", readVoltage);   // sets prefix/suffix for 'new' string
  lcd.setCursor(0,0);                             // sets LCD cursor (Column, Row) - 0,0 is left most column, top row
  lcd.write(dispRead);                            // displays the 'new' string 
}


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

void setVolts(){
 
  button8State = digitalRead(button8Pin);
  button2State = digitalRead(button2Pin);

  if (button8State == buttonPressed && button2State == buttonReleased){             // if only button 8 is pressed
    millisHeld = millis() - millisStore;
    secondsHeld = millisHeld/1000;
    vset = vset-buttonChange;
  }
  
  else if (button8State == buttonReleased && button2State == buttonPressed){        // if only button 2 is pressed
    vset = vset+buttonChange;
  }

/***************************************************************************/
                                            
  if (vset >= inputPinVoltage){                   // ensures that value of vset never goes beyond possible limits                           
    vset = inputPinVoltage;
  }
  else if (vset <= 0){
    vset = 0;
  }        
  
  char setVoltage[4];
  dtostrf(vset, 1, 2, setVoltage);                // convert float to string
  char dispSet[15];
  sprintf(dispSet, "Set  = %sV, %d", setVoltage, secondsHeld);     // sets pre/post fix for 'new' string, note additional space before = for alignment purposes
  lcd.setCursor(0,1);                             // sets LCD cursor (Column, Row) - 0,1 is left most column, bottom row
  lcd.write(dispSet);                             // displays the 'new' string  
}


void compare(){                                   // compares set and read voltages with defined tolerance 
    if (vset <= vread + tolerance && vset >= vread - tolerance){   
    digitalWrite(outputPin, LOW);  
    }
    else {
      digitalWrite(outputPin, HIGH);
    }
}


void buttonISR(){
  if(timingStarted == false) {
  millisStore = millis();
  timingStarted = true;
}
  millisHeld = 0;
  secondsHeld = 0;
}

void buttonCleanup(){
  millisStore = 0;
  timingStarted = false; 
}


void heartbeat(){                                 // 'heartbeat LED' to show code running/working
  long millisCurrentHB = millis();
  if (millisCurrentHB - millisPreviousHB >= heartbeatDelay){
      millisPreviousHB = millisCurrentHB;
      if (ledState == LOW) {
      ledState = HIGH;
      } 
        else {
        ledState = LOW;
        }
    digitalWrite(onboardLED, ledState);
  }
}

just found a way of doing it, probably not the best way but it works! No idea why I hadn't thought of it before...

The main loop has a delay of 0.5 seconds which helps with all manner of things (reading adc, reading buttons etc - lazy fix I know but it works). I thought I could just use this to help count.

void setVolts(){
 
  button8State = digitalRead(button8Pin);
  button2State = digitalRead(button2Pin);

  if (button8State == buttonPressed && button2State == buttonReleased){             // if only button 8 is pressed
    counter++;
    secondsHeld = counter/2;
***'INCREASE' CODE HERE***
  }
  
  else if (button8State == buttonReleased && button2State == buttonPressed){        // if only button 2 is pressed
    counter++;
    secondsHeld = counter/2;
***'DECREASE' CODE HERE***
  }
  else {
    counter = 0;
  }

It's not accurate (it has the potential to be up to 0.5 seconds out) but it's close enough and works!!!