Problem detecting short, mid and long presses. [Solved]

Hi guys, I have been trying for days now to solve a little problem I have with the code of my DIY guitar pedals's switching system. It uses an ATtiny85 running at 16Mhz internal, and the one wire bootloader that actually just lets me program the chip using two wires. TX/RX on pin 2 of the chip, and ground. easy to program these little ones like this :slight_smile:

Anyway, this is how the switching works:
Short press changes the state of a relay and mute pin combination, so, it turns the effect ON or OFF. Latching switch.

Long press, turns the effect ON on press or keeps the ON if it was already ON, until I release the footswitch, and if it was held for over 1 second. Momentary switching.

Middle press(between half a second and 1 second), puts the pedal in remote mode. It will then be able to receive commands from am external footswitch connected to arduino pin 3 of the ATtiny.

There is also code for this pin to switch the effect while the pedal is not in "Remote Mode", and this is intended. It doesn't do the latching switching. Just ON if the footswitch is ON and OFF if it is OFF. No latching.

All this is working as intended, even if the code is a mess :slight_smile:

The problem is as you can see on the video attached below, when I press the switch sometimes the relay (Yellow LED) behaves like it wasn't debounced, and it is. I suppose 20ms should be enough?
And also the pedal enters "Remote Mode" (Red LED) when switching the switch several times and slowly. I think it might be detecting 2 consecutive presses as middle press? I can't understand why :frowning:
Anyway, thanks a lot for your help. I am still a beginner in arduino coding, and at the moment I put together pieces of code that I find online. We gotta start somewhere, right? :slight_smile:

Here is the video and code. Thanks again

João

#define PRESSED LOW
#define NOT_PRESSED HIGH
 
const byte relay = 0;
const byte mute = 1;
const byte remote = 2;

 
unsigned long shortPress = 15;
unsigned long  longPress = 1000;
unsigned long midpress = 500;
bool relay_State = false;
bool remote_State = false;
//bool ledState = false;

typedef struct Buttons {
    const byte pin = 4;
    const int debounce = 20;
 
    unsigned long counter=0;
    bool prevState = NOT_PRESSED;
    bool currentState;
   
} Button;

typedef struct Buttons1 {
    const byte pin = 3;
    const int debounce = 20;
 
    unsigned long counter=0;
    bool prevState = NOT_PRESSED;
    bool currentState;
   
} Button1;

 
// create a Button variable type
Button button;
Button1 button1;
 
void setup() {
  pinMode(relay, OUTPUT);
  pinMode(mute, OUTPUT);
  pinMode(remote, OUTPUT);
  pinMode(button.pin, INPUT_PULLUP);
  pinMode(button1.pin, INPUT_PULLUP);

digitalWrite(relay, LOW);  
digitalWrite(mute, LOW);
digitalWrite(remote, LOW);

}


          

void loop() {

if (remote_State == true) {
  RemoteControl();

}             

    // check the button
    button.currentState = digitalRead(button.pin);
 
    // has it changed?
    if (button.currentState != button.prevState) {
        delay(button.debounce);
        // update status in case of bounce
        button.currentState = digitalRead(button.pin);
        if (button.currentState == PRESSED) {

           // a new press event occured
            // record when button went down
            button.counter = millis(); 

 
            digitalWrite(mute, HIGH);
        delay(35);

                  
           digitalWrite(relay, HIGH);
           
           delay(35);

   
           digitalWrite(mute, LOW);
  
         }

       if (button.currentState == NOT_PRESSED) {
            // but no longer pressed, how long was it down?
            unsigned long currentMillis = millis();


          if ((currentMillis - button.counter >= shortPress) && (currentMillis - button.counter < midpress)) {
                // Short press detected. 
               
                 digitalWrite(mute, HIGH); //Turn Opto ON
                delay(30);               
               
                relay_State = !relay_State;
                digitalWrite(relay, relay_State);

                delay(30);

                digitalWrite(mute,LOW);

             currentMillis == 0;
            }


  
            if ((currentMillis - button.counter >= longPress)) {
                // the long press was detected
               

                digitalWrite(mute, HIGH); //Turn Opto ON
                delay(30);

                 digitalWrite(relay, LOW); 

                delay(30);

              digitalWrite(mute, LOW); 

               relay_State = false;

               currentMillis == 0;

  
            }
           
               if ((currentMillis - button.counter >= midpress) && (currentMillis - button.counter < longPress)) {   // Mid press detected. 
             
                remote_State = !remote_State;
                digitalWrite(remote, remote_State);

                digitalWrite(mute, HIGH); //Turn Opto ON
                delay(30);
               
  
                 digitalWrite(relay, LOW); 

                delay(30);

              digitalWrite(mute, LOW); 

              relay_State = false;

              currentMillis == 0;

        }

      }
        // used to detect when state changes
        button.prevState = button.currentState;
    } 
     // check the button
    button1.currentState = digitalRead(button1.pin);
 
    // has it changed?
   // if (button1.currentState != button1.prevState) {
      if ((button1.currentState != button1.prevState) && (remote_State == false)) {
        delay(button1.debounce);
        // update status in case of bounce
        button1.currentState = digitalRead(button1.pin);
        if (button1.currentState == PRESSED) {


          if (relay_State == false) {

            digitalWrite(mute, HIGH);
        delay(35);

            }
                  
           digitalWrite(relay, HIGH);
           
           delay(35);

   
           digitalWrite(mute, LOW);

            relay_State = true;
            
        }

       if (button1.currentState == NOT_PRESSED) {
            // but no longer pressed, how long was it down?


             if (relay_State == true) {
                digitalWrite(mute, HIGH); //Turn Opto ON
                delay(35);
 
                  relay_State = false;
                digitalWrite(relay, relay_State);
            
                delay(35);

                digitalWrite(mute,LOW);

            }
         
        }
        // used to detect when state changes
        button1.prevState = button1.currentState;
    } 

}



 void RemoteControl()  { 


   // check the button
    button1.currentState = digitalRead(button1.pin);
 
    // has it changed?
    if (button1.currentState != button1.prevState) {
        delay(button1.debounce);
        // update status in case of bounce
        button1.currentState = digitalRead(button1.pin);
        if (button1.currentState == PRESSED) {
          
            // a new press event occured
            // record when button went down
            button1.counter = millis(); 


           digitalWrite(mute, HIGH);
           delay(35);
                  
           digitalWrite(relay, HIGH);
           
           delay(35);

           digitalWrite(mute, LOW);

        }


       if (button1.currentState == NOT_PRESSED) {
            // but no longer pressed, how long was it down?
            unsigned long currentMillis = millis();


           if (currentMillis - button1.counter <= longPress) {
                // short press detected. 
               
                digitalWrite(mute, HIGH); //Turn Opto ON
                delay(30);             
               
                relay_State = !relay_State;
                digitalWrite(relay, relay_State);
            
                delay(30);

                digitalWrite(mute,LOW);

            }

            if ((currentMillis - button1.counter > longPress)) {
                // the long press was detected
               

                digitalWrite(mute, HIGH); //Turn Opto ON
                delay(30);

                 digitalWrite(relay, LOW); 
                relay_State = false;
                delay(30);

              digitalWrite(mute, LOW); 
              
  
            }
        }
        // used to detect when state changes
        button1.prevState = button1.currentState;
    } 

 }

This

        button.prevState = button.currentState;

happens after several possible delays, and is probably not correct.

Sorry, I am a bit of a beginner, so what you are saying is that I should use something like (void relayOn() ) and put all the relay switching inside it? The same for relayOFF? In other words, remove the delays from the main loop??

Thanks

João

To make it act fast, you should remove all the delays.
My problem is the overall flow of the code. I see multiple problems. In my opinion it is not a little problem. Don't worry, my first program that I wrote was just like that. You have to take a step back and think about the overall structure of the code.

You read the second button four times, in two different parts of the code. That is not straightforward.

The current state of a button is read. But if it has changed, then it is read again ! That ruins the logic of the code. Since it is read the second time after a delay, that is a serious problem.

The 'currentMillis' is sometimes made zero. Is that to disable something ? That is not okay.

Can you make a good description of what you want ?
(1) What should happen at the moment the button is pressed (and is still being pressed) ?
(2) What should happen if the button is kept pressed for a long time (and is still being pressed) ?
(3) What should happen when the button is released ?

For number (1), I'm not sure what you want.
For number (2), I assume that nothing should happen.
For number (3), you check how long it was pressed. To check for medium press and long press. Nothing should happen if the button is released after a short press ?

You should start by seperating the different parts of the code.
I have a few examples, they do not have debouncing for buttons. You could try the Bounce2 library for that.
Maybe you can try a few examples to see what they do.
This is a simple sketch to detect a long press: millis_short_press_long_press.ino
This detects how long a button is pressed and performs actions while being pressed: millis_and_bool.ino

This link has a neat method for detecting different button presses.

If you want a responsive program do not use delay(). The functions delay() and delayMicroseconds() block the Arduino until they complete. Have a look at how millis() is used to manage timing without blocking in Several Things at a Time.

And see Using millis() for timing. A beginners guide if you need more explanation.

...R

Koepel:
To make it act fast, you should remove all the delays.

The delays are needed. It is like this, relays are noisy when switching audio, just like normal switches.. They will produce some loud pop sounds most of the times. To avoid this, when I turn the effect ON there is a sequence of events. The audio is muted for 30ms, then the relay switches, then wait another 30ms wjhile it is switching to avoid the pop sound, and then unmute the audio. This happens every time the relay is switched. Therefore so many delays, well unless there are other way of doing the same thing that i don't know, of course :slight_smile:

You have to take a step back and think about the overall structure of the code.

You read the second button four times, in two different parts of the code. That is not straightforward.

The current state of a button is read. But if it has changed, then it is read again ! That ruins the logic of the code. Since it is read the second time after a delay, that is a serious problem.

Are you talking about the button debouncing?

The 'currentMillis' is sometimes made zero. Is that to disable something ? That is not okay.

Reset the counter? so it starts from the beginning again to detect a new press?

Can you make a good description of what you want ?
(1) What should happen at the moment the button is pressed (and is still being pressed) ?

Every time the button is pressed, the effect/relay should turn ON.

(2) What should happen if the button is kept pressed for a long time (and is still being pressed) ?

As I said above, when the button is pressed, the effect turns ON. If it is released shortly, it changes state. If it is released between 0.5 and 1 second, it enters Remote mode. If it is released after more than 1 second, it will turn off. This means a momentary effect. The pedal turns on on press, and it stays ON until I release the switch.

You should start by seperating the different parts of the code.
I have a few examples, they do not have debouncing for buttons. You could try the Bounce2 library for that.
Maybe you can try a few examples to see what they do.
This is a simple sketch to detect a long press: millis_short_press_long_press.ino
This detects how long a button is pressed and performs actions while being pressed: millis_and_bool.ino

Thank you for your help.. will check this..

João

EDIT: Did you watch the video I posted? Just follow the youtube link. I explain what the different modes are..

Robin2:
This link has a neat method for detecting different button presses.

If you want a responsive program do not use delay(). The functions delay() and delayMicroseconds() block the Arduino until they complete. Have a look at how millis() is used to manage timing without blocking in Several Things at a Time.

And see Using millis() for timing. A beginners guide if you need more explanation.

...R

Thank you.. will check it all..

João

Yes, I watched the video, and thank you for the video :slight_smile:
I'm not sure about the short press, is that activated when the button is pressed or when the button is released ?
I don't understand that when the button is pressed the effect relay should turn on and when the button is released after a short time the effect relay should change state. Change state from which state ? The state before the button was pressed ?

Using the State Change Detection is good, but combining the State Change Detection with a debounce that way ruins the State Change Detection. You have to seperate the debouncing from the State Change Detection.

Koepel:
I'm not sure about the short press, is that activated when the button is pressed or when the button is released ?
I don't understand that when the button is pressed the effect relay should turn on and when the button is released after a short time the effect relay should change state. Change state from which state ? The state before the button was pressed ?

Yeah, I know it is not easy to understand and I do apologize for that. The code i found detected the short press also on release, but that is not acceptable for the pedal because it would introduce more delay when activating the switch. So at the beginning of the main loop, I've added some code to turn the effect ON as soon as the switch is pressed. this didn't affect the effect latching part of the code. because if the relay was OFF, it would turn ON. If it was ON already, it wouldn't do anything until released after the short, mid or long presses were detected (on release). This was a very bad solution, I know, but it worked.. The only problem is this going into remote mode when operating the switch a bit slower and repeatedly. :frowning:

João

The delays are needed.

Delays are almost never needed and the exceptions do not apply to your case. Delays almost always have unwanted side effects that create additional difficulties.

Button press detection is complicated by switch bouncing, which varies from switch to switch. Do not think for a moment that your task is simple, or that delays are required.

Especially as a beginner, you will have to work hard to come up with a completely satisfactory solution, if that is even possible.

Button A, B, C.

Button B clears button A press if A press was not processed.

Button C clears Button A and B pressed if not processed.

Button A, B, C toggle a Boolean when pressed.

loop() just does loop, checks for a button press, calls the relevant function per button press, and clears the Boolean value.

jremington:
Delays are almost never needed and the exceptions do not apply to your case. Delays almost always have unwanted side effects that create additional difficulties.

The delays are not on Button detection, apart from the debouncing of the buttons, as far as I can understand. They are there to turn a "Mute" pin ON a while before the relay switches, and then turn it OFF a while after the relay switched, to avoid pop sounds on the audio.
How can I achieve that without the delays? If there is a better solution I would like to learn about it :slight_smile: Or did we finally find something this type of code can't do? :wink: I don't really want to believe this is the case :slight_smile:

João

How can I achieve that without the delays? If there is a better solution I would like to learn about it

Use the millis() function and state change detection.

Study this excellent tutorial on Blink with Delay, and this collection of examples, which incidentally shows how to debounce a switch, without using delay.

So, I removed all the delays from the main loop.. Problem is still there :frowning: If i repeatedly hit the pedal footswitch, it will enter remote mode after a few clicks :frowning:
Apart from this, it works as it should..

#define PRESSED LOW
#define NOT_PRESSED HIGH
 
const byte relay = 0;
const byte mute = 1;
const byte remote = 2;

 
unsigned long shortPress = 15;
unsigned long  longPress = 1000;
unsigned long midpress = 500;
bool relay_State = false;
bool remote_State = false;
//bool ledState = false;

typedef struct Buttons {
    const byte pin = 4;
    const int debounce = 20;
 
    unsigned long counter=0;
    bool prevState = NOT_PRESSED;
    bool currentState;
   
} Button;

typedef struct Buttons1 {
    const byte pin = 3;
    const int debounce = 20;
 
    unsigned long counter=0;
    bool prevState = NOT_PRESSED;
    bool currentState;
   
} Button1;

 
// create a Button variable type
Button button;
Button1 button1;
 
void setup() {
  pinMode(relay, OUTPUT);
  pinMode(mute, OUTPUT);
  pinMode(remote, OUTPUT);
  pinMode(button.pin, INPUT_PULLUP);
  pinMode(button1.pin, INPUT_PULLUP);

digitalWrite(relay, LOW);  
digitalWrite(mute, LOW);
digitalWrite(remote, LOW);

}


          

void loop() {

if (remote_State == true) {
  RemoteControl();

}             

    // check the button
    button.currentState = digitalRead(button.pin);
 
    // has it changed?
    if (button.currentState != button.prevState) {
        delay(button.debounce);
        // update status in case of bounce
        button.currentState = digitalRead(button.pin);
        if (button.currentState == PRESSED) {

           // a new press event occured
            // record when button went down
            button.counter = millis(); 

          effectON();
       
  
         }

       if (button.currentState == NOT_PRESSED) {
            // but no longer pressed, how long was it down?
            unsigned long currentMillis = millis();


          if ((currentMillis - button.counter >= shortPress) && (currentMillis - button.counter < midpress)) {
                // Short press detected. 
               
               effectChangeState();

             currentMillis == 0;
            }


  
            if ((currentMillis - button.counter >= longPress)) {
                // the long press was detected
               

               effectOFF();

               currentMillis == 0;

  
            }
           
               if ((currentMillis - button.counter >= midpress) && (currentMillis - button.counter < longPress)) {   // Mid press detected. 
             
                remote_State = !remote_State;
                digitalWrite(remote, remote_State);

               effectOFF();

              currentMillis == 0;

        }

      }
        // used to detect when state changes
        button.prevState = button.currentState;
    } 
     // check the button
    button1.currentState = digitalRead(button1.pin);
 
    // has it changed?
   // if (button1.currentState != button1.prevState) {
      if ((button1.currentState != button1.prevState) && (remote_State == false)) {
        delay(button1.debounce);
        // update status in case of bounce
        button1.currentState = digitalRead(button1.pin);
        if (button1.currentState == PRESSED) {


          if (relay_State == false) {

            effectON();
            relay_State = true;
          }
            
        }

       if (button1.currentState == NOT_PRESSED) {
            // but no longer pressed, how long was it down?


             if (relay_State == true) {
             effectOFF();

            }
         
        }
        // used to detect when state changes
        button1.prevState = button1.currentState;
    } 

}



 void RemoteControl()  { 


   // check the button
    button1.currentState = digitalRead(button1.pin);
 
    // has it changed?
    if (button1.currentState != button1.prevState) {
        delay(button1.debounce);
        // update status in case of bounce
        button1.currentState = digitalRead(button1.pin);
        if (button1.currentState == PRESSED) {
          
            // a new press event occured
            // record when button went down
            button1.counter = millis(); 


        effectON();

        }


       if (button1.currentState == NOT_PRESSED) {
            // but no longer pressed, how long was it down?
            unsigned long currentMillis = millis();


           if (currentMillis - button1.counter <= longPress) {
                // short press detected. 
               
                effectChangeState();

            }

            if ((currentMillis - button1.counter > longPress)) {
                // the long press was detected
               

                effectOFF(); 
              
  
            }
        }
        // used to detect when state changes
        button1.prevState = button1.currentState;
    } 

 }
 
void effectON() {

       digitalWrite(mute, HIGH);
        delay(35);
               
           digitalWrite(relay, HIGH);
           
           delay(35);
 
           digitalWrite(mute, LOW);

}

void effectChangeState() {

    digitalWrite(mute, HIGH); //Turn Opto ON
                delay(30);               
               
                relay_State = !relay_State;
                digitalWrite(relay, relay_State);

                delay(30);

                digitalWrite(mute,LOW);
}
void effectOFF()  {

   digitalWrite(mute, HIGH); //Turn Opto ON
                delay(30);

                 digitalWrite(relay, LOW); 

                delay(30);

              digitalWrite(mute, LOW); 

               relay_State = false;
}

jhsa:
So, I removed all the delays from the main loop..

void loop() {

delay(button.debounce);

You actually left tons of delays in there.

Saying you “need” to use 20 or 30 milliseconds delays just emphasizes your not fully embracing using non-blocking code.

Start loop, capture current time, check time-based variables against current time, read inputs, set stuff, loop.

Your already setting variable for how long to delay, just add that and current millis to a variable.

jhsa:
If i repeatedly hit the pedal footswitch, it will enter remote mode after a few clicks :frowning:

That is a very common problem with user-input systems that try to do several things with one button / switch without providing suitable feedback to the user so s/he know what state the system is in and has the necessary time to respond.

Often the solution depends more on careful planning rather than on program code. Or maybe the solution requires a different hardware system - more than one button, for example.

...R

Can't have more than one button. If I can't find a solution, I will have to leave it as it is.. It works if I never hit that switch several times in 5 seconds. And I never will because that is not the way Guitar effects work :slight_smile: Unless they have tap tempo, but that is done with an extra switch. This one is a distortion pedal, so it doesn't need tap tempo :slight_smile:

I am working in replacing the delays with millis() Let'see what I can do :slight_smile:

João

Slumpert:
You actually left tons of delays in there.

Saying you “need” to use 20 or 30 milliseconds delays just emphasizes your not fully embracing using non-blocking code.

Start loop, capture current time, check time-based variables against current time, read inputs, set stuff, loop.

Your already setting variable for how long to delay, just add that and current millis to a variable.

This is what I am trying to do now.. Please keep in mind that I am a complete beginner. Until today "millis()" sounded a bit frightening :slight_smile: I'll get there, I think :slight_smile:

João

Sorry to bother you guys, I am trying this, but it is not working :frowning: Effect turns ON but it won't turn OFF on the second switch press..
Thanks a lot for your help..

  // check the button
    button.currentState = digitalRead(button.pin);
    button.counter = millis();
 
    // has it changed?
    if (button.currentState != button.prevState) {
       // delay(button.debounce);
        unsigned long currentMillis = millis();
       
        if ((currentMillis - button.counter >= 30) && (digitalRead(button.pin)== LOW)) { 

          button.currentState = PRESSED;

         // previousMillis = currentMillis;
          
        }

       
        // update status in case of bounce
       // button.currentState = digitalRead(button.pin);
        if (button.currentState == PRESSED) {

           // a new press event occured
            // record when button went down
            button.counter = millis(); 

          effectON();
       
  
         }

       if (button.currentState == NOT_PRESSED) {
            // but no longer pressed, how long was it down?
            unsigned long currentMillis = millis();


          if ((currentMillis - button.counter >= shortPress) && (currentMillis - button.counter < midpress)) {
        
                // Short press detected. 
               
               effectChangeState();

             //currentMillis == 0;
             previousMillis = currentMillis;
            }

Hi think I won the first battle :slight_smile: This seems to work Yaay :smiley: Now to try to get rid of the other delays :slight_smile:
Some learning curve here :slight_smile:

    // check the button
    button.currentState = digitalRead(button.pin);
 
    // has it changed?
    if (button.currentState != button.prevState) {
        lastDebounceTime = millis();
       if ((millis() - lastDebounceTime) > debounceDelay) { 
        // update status in case of bounce
        button.currentState = digitalRead(button.pin);
       }
        if (button.currentState == PRESSED) {

           // a new press event occured
            // record when button went down
            button.counter = millis(); 

          effectON();
       
  
         }