Help with multifunction button and potentiometer

mamassingale · January 9, 2019, 10:46am

So I have been working on a project for a little while now that is controlling a stepper with the use of a pot and a multifunction button. Thanks to the help from everyone here, I got it mostly working. The control functions as such:

pot controls the speed
click of the button changes direction
holding the button for 2 seconds disables the motor

It is mostly working, but it seems to be having some glitches. Here is what happens:

problem 1:
power on and nothing happens until I press the button for 2 seconds (This is the behavior I want)
when I press the button, it powers on and moves for a few seconds then stops (not what I want)

Problem 2:
I click the button and the direction changes but after about 2 seconds it changes back on it's on (not what I want)

Here is the code:

// define pins
#define stepPin 3
#define dirPin 4
#define ms1 8
#define ms2 9
#define ms3 10
#define buttonPin 2
#define motorPin 13
#define debounce 20
#define holdTime 2000

// Potentiometer Variables
int cutomDelay,customDelayMapped;

// Button variables
int buttonVal = 0;
int buttonLast = 0;
long btnDnTime;
long btnUpTime;
boolean ignoreUp = false;

// Motor variables
boolean motorVal = false;
boolean dirVal = false;

void setup() {
  // initialize and set outputs
 pinMode(buttonPin, INPUT_PULLUP);
 digitalWrite(buttonPin, HIGH);
 pinMode(motorPin, INPUT_PULLUP);
 digitalWrite(motorPin, HIGH);
 pinMode(dirPin, OUTPUT);
 digitalWrite(dirPin, dirVal);
 pinMode(stepPin, OUTPUT);
 pinMode(ms1, OUTPUT);
 pinMode(ms2, OUTPUT);
 pinMode(ms3, OUTPUT);

}

void loop() {
  // put your main code here, to run repeatedly:
  // Read the state of the button
buttonVal = digitalRead(buttonPin);

// Test for button pressed and store the down time
if (buttonVal == LOW && buttonLast == HIGH && (millis() - btnUpTime) > long(debounce))
{
btnDnTime = millis();
}

// Test for button release and store the up time
if (buttonVal == HIGH && buttonLast == LOW && (millis() - btnDnTime) > long(debounce))
{
if (ignoreUp == false) dirChange();
else ignoreUp = false;
btnUpTime = millis();
}

// Test for button held down for longer than the hold time
if (buttonVal == LOW && (millis() - btnDnTime) > long(holdTime))
{
motorOnOff();
ignoreUp = true;
btnDnTime = millis();
}

buttonLast = buttonVal;

motorRun();
motorSpeed();
  
  

}
void motorOnOff() {
  motorVal = digitalRead(motorPin);
  motorVal = !motorVal;
  digitalWrite(motorPin, motorVal);
    
}
void dirChange() {
  dirVal = !dirVal;
  digitalWrite(dirPin, dirVal);
}
int motorSpeed() {
  int customDelay = analogRead(A0);
  int newCustom = map(customDelay, 0, 1023, 200, 4000);
  return newCustom;
}
void motorRun() {
  customDelayMapped = motorSpeed();
  digitalWrite(stepPin, LOW);
  delayMicroseconds(customDelayMapped);
  digitalWrite(stepPin, HIGH);
  delayMicroseconds(customDelayMapped);
  digitalWrite(ms1,HIGH);
  digitalWrite(ms2,HIGH);
  digitalWrite(ms3, HIGH);
}

Any ideas where I've messed up?

septillion · January 9, 2019, 11:17am

First, do yourself a favor and press Ctrl+T in the IDE, looks better doesn't it?

Next I'm confused about the motorPin. What does it do? You never talk about it in your text. Also ms1, ms2 and ms3 are?? Might have to do with the hardware which I don't know.

But you can start the debug by adding a couple of print statements in some places. Because I don't see the error right away. I like to use a library like Bounce2 to do the debounce and state change detection for me to make the code clearer.

One thing I do notice, holding down the button for more then 2 seconds will call motorOnOff() every 2 seconds. That's what you want?

Some random tips:

Drop the macro's (aka #define) and change them to const byte for pins and const unsigned int for timing
Not only a tip but a real error, when using millis(), use unsigned long!
Try
Not all variables need to be global. For example, buttonVal is only used in loop(), and motorVal only used in motorOnOff() etc. Just declare them there like you do with motorSpeed()
Why do you call motorSpeed() in loop()?

JaBa · January 9, 2019, 12:47pm

I find the coding confusing and hard to follow. I really think you Need to look at some examples on how to debounce.

On the side, why bother tracking upTime and dnTime separately? The purpose is to detect the first Change of a button state (it just got pressed, or released) so that rapid changes in state (bouncing) can be ignored. It should be sufficient to just track one time. In a program of this size, it may not matter. Later, as your Projects get larger, you may start having Problems of running out of Memory, in part due to excessive use of variables.

More importantly, you are Setting buttonLast = buttonVal; each time through the Loop.

Looking at the code, for example:

if (buttonVal == LOW && buttonLast == HIGH && (millis() - btnUpTime) > long(debounce))

it appears that you are using buttonLast to remember if the program is currently in button up or button down mode. If that were the case, buttonLast should only be changed when your program has determined a button press/release - including debouncing.

septillion · January 9, 2019, 2:06pm

Yeah, he can do with one check. Or check both with the same variable.

But you are right about the 'buttonVal' thing! 'buttonLast' should contain the last stable button state. For that matter, setting 'btnDnTime' and 'btnUpTime' (which can be the same variable) is independant of the check for a stable state (aka debounce).

So I was kind of right, use a library like Bounce2 and have working, readable and easy code

mamassingale · January 9, 2019, 2:21pm

Thanks for the insight guys. And Septillian, it does look better after pressing ctrl + T haha. I had a lot of trouble figuring out how to get the button set up to do two types of presses (short and long). This is the first real code I have ever tried to write and I found this part tricky and a bit confusing. I found some info online which is where most of the button code came from. The motorPin is actually connected to the enable pin on a A4988 driver. This is being used to enable and disable the motor. It's not necessary, I could just have a power switch for the whole thing, but I just wanted to figure it out.

*- Drop the macro's (aka #define) and change them to const byte for pins and const unsigned int for timing

Not only a tip but a real error, when using millis(), use unsigned long! *

I am still learning about these things. At the moment, this part of the code is actually over my head. I'll do some more reading and see if can figure out how to clean this all up. Any more advice will be warmly welcomed!

septillion · January 9, 2019, 2:33pm

Yeah, you're trying to figure out two things at one: debounce and long press. (Actually three if you count state change) Which can become confusing.

If you want the full learning experience, change the code and split the debounce bit from the long press bit

If you want the easy way (which is also the recommended way after doing the "the full learning experience" once), grab Bounce2 and start over It will only take a couple of lines that way.

JaBa · January 9, 2019, 10:05pm

Once you have detected that the button has been pressed, wait for it to be released. You are debouncing the press, so probably no reason to debounce the release. Simply do a while Loop until the button is released. At that Point, simply check the millis() again. If it has been less than the required two seconds, it was a simple press. If it is at least two seconds, it was a Long press. This also solves the Problem mentioned by Septillion in Post #1. Of course, if you want it to respond quicker, turn the Motor on/off and then wait for the button to be released.

Coding is actually pretty easy if you Keep in mind that all big Projects are just lots of Little short pieces. If your Problem seems too complex, break it down into two or more steps and code each separately.

mamassingale · January 10, 2019, 3:16am

So I had a read through the bounce2 library and it seems pretty straight forward for button clicks. I have tried to separate the code into sections so that it can be better understood. Below is void setup() and variables to control the initialization and the direction change (potentiometer and motor controls have been omitted)

the desired function of the below code is to power on and initialize the system but prevent the motor from turning so that the system sits idle.

#include <Bounce2.h>
const byte buttonPin = 2;
const byte stepPin = 3; // is translated automatically by a4988 to control all the coils on the motor 
const byte dirPin = 4; // controls the direction the motor spins
const byte ms1 = 8;
const byte ms2 = 9; // ms1, ms2, ms3 pins set to (high) tell the a4988 driver to microstep at 1/16th
const byte ms3 = 10;
int dirState = LOW;
const byte enable = 13; // connects to the enable pin on a4988 driver (high) disables outputs (low) enables outputs
Bounce debouncer = Bounce();

void setup() {
  // everything initializes, but motor doesn't start spinning
  debouncer.attach(buttonPin, INPUT_PULLUP);
  debouncer.interval(25);
  pinMode(dirPin, OUTPUT);
  digitalWrite(dirPin, dirState);
  pinMode(enable, OUTPUT);
  digitalWrite(enable, HIGH); // disables outputs from the a4988 driver so that the motor doesn't start spinning when system is powered on
  pinMode(stepPin, OUTPUT);
  pinMode(ms1, OUTPUT);
  digitalWrite(ms1, HIGH);
  pinMode(ms2, OUTPUT);
  digitalWrite(ms2, HIGH);
  pinMode(ms3, OUTPUT);
  digitalWrite(ms3, HIGH);

}

mamassingale · January 10, 2019, 3:32am

for the loop, the motor should run after the button is pressed and held for 2 seconds ( I haven't added this yet because I haven't figured it out so just assume it works). The speed is controlled by the potentiometer and the direction changes everytime I click the button. This is the desired function of the following code (omitting the press and hold)

void loop() {
  motorRun(); // this will only start when the the button is pressed for more than 2 sec, but I havent figured it out yet so just assume the motor is runnning for the rest of the loop
  debouncer.update();
  if (debouncer.fell()) {
    dirState = !dirState;  // direction of the motor spin will change with each click of the button
    digitalWrite(dirPin, dirState);
  }

}
void motorRun () {
  customDelayMapped = speedUp();
  digitalWrite(stepPin, LOW);
  delayMicroseconds(customDelayMapped); // uses potentiometer reading to control speed of the motor
  digitalWrite(stepPin, HIGH);
  delayMicroseconds(customDelayMapped);
}
int speedUp() {
  int customDelay = analogRead(A0); // Reads the potentiometer
  int newCustom = map(customDelay, 0, 1023, 300, 4000); // Convrests the read values of the potentiometer from 0 to 1023 into desireded delay values (300 to 4000)
  return newCustom;
}

septillion · January 10, 2019, 9:11am

Please don't post your code in two sections please. Makes me have to copy/past it into a single piece. More work, and I might add/remove a blank line which results in posted error messages not matching. Aka, just plain annoying

Do do the 2 second press thing you still need to time the press. Which means, you can't distinguish between a short (change direction) push and a long (start/stop) press the moment a buttons become pressed. You can only do that on the release or when the long press time has passed.

But you knew that already in your past code You can now just save the time on a fell() and do the desired action on the rose() (or when the time for a long press passed).

But you did a great job! Especially making variables local where they can be

But I did some minor changes.

Name things for there function in your application. Yes, 'debouncer' debounces but it's more useful to know what it means. So I named it just 'button'. Now button.fell() makes more sense
Added the power of arrays to the ms-variables. Also added 'Pins' to it to indicate they are also pins.
Did split stuff into some extra functions. Keeps the loop() clear to read and easy to do the same stuff elsewhere in your code without repeating yourself.
changed 'speedUp' to 'getSpeed' because you can also speed down, can't you? And simplified it.
Added the variable 'potPin', it deserves it
Added Pin to 'enable'
Did you really needed 65536 possible directions? Changed 'dirState' to a bool.
Changed the map(), this way it's uniform all the way. Although here it may not changes things very much.

Might seem like a long list but really they are minor compared to the changes you made! Great job!

#include <Bounce2.h>
const byte buttonPin = 2;
const byte potPin = A0;
const byte stepPin = 3; // is translated automatically by a4988 to control all the coils on the motor
const byte dirPin = 4; // controls the direction the motor spins
const byte msPins[] = {8, 9, 10}; // ms1, ms2, ms3 pins set to (high) tell the a4988 driver to microstep at 1/16th
bool dirState = LOW;
const byte enablePin = 13; // connects to the enable pin on a4988 driver (high) disables outputs (low) enables outputs
Bounce button = Bounce();

void setup() {
  // everything initializes, but motor doesn't start spinning
  button.attach(buttonPin, INPUT_PULLUP);
  button.interval(25);
  pinMode(dirPin, OUTPUT);
  digitalWrite(dirPin, dirState);
  pinMode(enablePin, OUTPUT);
  digitalWrite(enablePin, HIGH); // disables outputs from the a4988 driver so that the motor doesn't start spinning when system is powered on
  pinMode(stepPin, OUTPUT);
  
  for(byte i = 0; i < sizeof(msPins); i++){
    digitalWrite(msPins[i], HIGH);
    pinMode(msPins[i], OUTPUT);
  }
}

void loop() {
  motorRun(); // this will only start when the the button is pressed for more than 2 sec, but I havent figured it out yet so just assume the motor is runnning for the rest of the loop
  checkButton();
}

void motorRun () {
  int customDelayMapped = getSpeed();
  digitalWrite(stepPin, LOW);
  delayMicroseconds(customDelayMapped); // uses potentiometer reading to control speed of the motor
  digitalWrite(stepPin, HIGH);
  delayMicroseconds(customDelayMapped);
}

int getSpeed() {
  int potValue = analogRead(potPin); // Reads the potentiometer
  return map(potValue, 0, 1024, 300, 4001); // Converts the read values of the potentiometer from 0 to 1023 into desireded delay values (300 to 4000)
}

void changeDir(){
  dirState = !dirState;  // direction of the motor spin will change with each click of the button
  digitalWrite(dirPin, dirState);
}

void checkButton(){
  button.update();
  if (button.fell()) {
    changeDir();
  }
}

mamassingale · January 10, 2019, 1:14pm

Wow! Thanks a lot Septillian! That looks much cleaner! I'll go through everything and try to wrap my brain around it all. Thanks for your help and constructive feedback! Everything makes sense to me except this part:

for(byte i = 0; i < sizeof(msPins); i++){
digitalWrite(msPins*, HIGH);*
_ pinMode(msPins*, OUTPUT);*_
Now to add that long press and hopefully move to the final step adding Bluetooth!

septillion · January 10, 2019, 1:25pm

If you post it without code tags it indeed becomes pretty weird

But it simply loops over all members (aka, all 3) of 'msPins' and calls digitalWrite() and pinMode() for them. That's the power of an array, you can index them in code. Aka msPins[1] equals 9 in the code above. but you can do it with a variable, 'i' from index in this case.

mamassingale · January 10, 2019, 1:37pm

Haha sorry about the code tags!

septillion · January 10, 2019, 1:42pm

Do note the way of setting the speed isn't ideal and may fluctuate a bit, especially if you start doing more. Can be solved by using a hardware timer / PWM. But it will make it a lot more advanced and (in most cases) not portable (= not easy to run on another type of micro).

mamassingale · January 11, 2019, 3:30am

So after reading through lots of examples and documentation, adding the long press still seems really complicated and the example codes I found are hard to follow. Is it possible to do this with fell() like with the direction change. something like this

 button.update();
  if (button.fell() && (millis() - pressTime) >  unsigned long(lngPressTime)) {
    motorOnOff();
  }
  if (button.fell() && (millis() - pressTime) <  unsigned long(lngPressTime)) {
    changeDir();
  }

septillion · January 11, 2019, 3:20pm

Like I said, you can only determine a long press from a short press:
a) when you release the button (aka .rose())
b) when the button is still down and 2 seconds have passed since you pushed the button (aka fell())

So it becomes something like:

const unsigned int LongPressTime = 2000; //in ms

unsigned long buttonMillis;
bool longPressHappened = false;

void checkButton(){
  button.update();
  //save time when it became pressed
  if (button.fell()) {
    buttonMillis = millis();
  }
  //is it down long?
  //and long press did not happen yet
  else if(!button.read() &&
          (millis() - buttonMillis >= LongPressTime) &&
          !longPressHappened)
  {
    longPressHappened = true;
    motorOnOff();
  }
  //or was it a short press?
  //do nothing on release after a long press
  else if(button.rose() && (millis() - buttonMillis < LongPressTime)){
    changeDir();
  }
  
  //independent!!
  //forget a long press happened (or not)
  if(button.rose()){
    longPressHappened = false;
  }
}

mamassingale · January 12, 2019, 12:47am

Wow that is definitely over my head. :o haha! But, it works now (after going back and adjusting some of my other mistakes ). I'm gonna need to ponder over this a while to see if I can figure out why it works. Thanks so much for your wisdom. I'm halfway there now!

septillion · January 12, 2019, 5:27pm

It's not really that complicated. Try to go over that statement block and try to see why my question in the comment matches the code.

And yes, I could have used 'longPressHappened' instead of the millis part in the "//or was it a short press?" part

mamassingale · January 12, 2019, 11:02pm

The short press statement makes perfect sense to me. But in the long press statement, why did you use !button.read() and !longPressHappened Why do you need the independent statement at the end if you already set the short press in its own statement?

septillion · January 12, 2019, 11:17pm

If you add a ! in front it's the NOT operator. Aka, !button.read() is true is when button.read() returns low (aka 0 aka false).

Same for 'longPressHappened', we set that to true is we already handled with the long press. Aka, !longPressHappened means 'longPressHappened'is false aka "no long press happened".

And I do need the independent part because I want to reset everything to accept a new button press when the button is released. Aka, I want to forget I handled a long press so I can handle a new long press.

Couple of options with that:

I could make that

if(button.rose() && longPressHappened){

Aka, only make it forget a long press happened if a long press really happened. But setting 'longPressHappened' to false when it is already false isn't an issue so why check more stuff than needed?

Instead of doing the long press thing only once when you keep the button pressed you could also do it every 2 seconds by doing:

 else if(!button.read() &&
          (millis() - buttonMillis >= LongPressTime))
  {
    buttonMillis = millis();
    motorOnOff();
  }

But doing it once seems more logical but it's up to you.

Or instead of the "independent" check, reset 'longPressHappened' when the button becomes pressed again:

 if (button.fell()) {
    buttonMillis = millis();
    longPressHappened = false;
  }

I think this is actually the neatest solution but I think the logic behind this one is a bit harder because now current en previous button press are kind of merging. I thought that might be confusing. But it does save a complete check.

Topic		Replies	Views
Trying to merge two sketches Programming	5	1043	May 5, 2021
2 potentiometer 2 stepper motor and 2 switch Programming	10	1452	October 18, 2022
how can i include a potentiometer in this code ??? Programming	12	967	May 5, 2021
How to have potentiometer separate from button actuation for LED on/off LEDs and Multiplexing	19	1914	May 6, 2021
Confused: Need to use to Potentiometers Frequently-Asked Questions	6	2415	May 6, 2021

Help with multifunction button and potentiometer

Related topics