I am writing a small code that uses a momentary switch (on-off-on) to print a sentence to serial. If this code were working properly, it would print to serial ONCE per button press. Instead, if I press the button in one direction, it prints "Up Shift" two or three times. I attached a picture of the serial monitor, after pressing and holding the button once.
This is the start of a code that will control a linear actuator. If I press the switch forward, it moves the linear actuator x-distance. Likewise if I press the switch in reverse. My concern is that it will move the actuator two or three x-distances, rather than just one.
I am interested to know if anyone sees problems with my code, or if it can be written in a more clever method. I am wondering if the problem is with my code, or rather, with the switch. Could the momentary switch have some play in it (making contact intermittently a couple times) when it is switching terminals? Please let me know if you need more information to consider the issue.
The switch is momentary in the sense that the contacts open when you release the switch. But even a quick "press and release" will close the contacts long enough for the loop() function in your program to repeat several times.
The code is checking if the switch is currently closed, rather than "has it just changed from open to close". That would explain why the code prints "Up Shift" several times.
int val;
unsigned long waitTime;
val = digitalRead(switchPin);
if (val == HIGH)
{
waitTime = millis() + 500; // half second for the switch to settle, and reopen
while( waitTime < mills()) {}; //do nothing for the defined interval, by now switch is back up
..do the code to be invoked by the button press
}
While it will mostly work, there are two issues with this proposal:
(1)
waitTime and millis() are of type unsigned long. To avoid rollover problems, the recommendation is to only use subtraction. The code shown uses addition. When millis() gets almost large enough to rollover, millis() + 500 WILL rollover. At this point, waitTime will IMMEDIATELY be less than millis() and the debounce will end far too early.
I have not done the math, but I believe the user who made the recommendation and I follow the recommendation.
(2)
This is as bad as using delay(...). For 500 milliseconds (also known as 0.5 seconds, a l-o-n-g time in the Arduino's world) nothing else can happen. As the comment says, {} is a do-nothing where no other switches can be scanned, serial input cannot be processed, etc.
Ahh, I've heard the term debounce, but never actually knew what it meant. Learn something new every day.
So in my code, The delay is useful. I will probably make it longer (probably 250 to 500 ms) so that a user can not hurriedly tap the switch over and over. This is an acceptable way to do this, correct? I understand the issue that delays stop the Arduino from doing anything else, but it this case, I think that is a good thing. It is essentially ignoring any inputs while still processing the first one.
Reading switches is more about watching for signal changes. The pins on the chip always emit a signal. That signal can only be HIGH or LOW. You want to store the old signal state and keep comparing it to a new reading. They won't be equal if there has been a change. Here's a sketch. Read it carefully. It's really fairly simple and shows how to read a switch with debouncing. I haven't tested it but it should work.
const int shiftUpPin = 2;
const int shiftDownPin = 3;
const unsigned long DEBOUNCE_MILLIS = 25;
int shiftUpState, shiftDownState;
int oldUpShiftState, oldDownShiftState;
boolean debounceUpShiftFlag, debounceDownShiftFlag;
unsigned long upShiftTimer, downShiftTimer;
void setup()
{
Serial.begin(9600);
pinMode(shiftUpPin, INPUT_PULLUP);
pinMode(shiftDownPin, INPUT_PULLUP);
Serial.println(F("Setup Complete."));
}
void loop()
{
unsigned long currentMillis = millis();
shiftUpState = digitalRead(shiftUpPin);
if (oldUpShiftState != shiftUpState)
{
oldUpShiftState = shiftUpState;
debounceUpShiftFlag = true;
upShiftTimer = currentMillis;
}
if (debounceUpShiftFlag && currentMillis - upShiftTimer >= DEBOUNCE_MILLIS)
{
if (oldUpShiftState == LOW)
{
Serial.println(F("Up Shift"));
}
debounceUpShiftFlag = false;
}
shiftDownState = digitalRead(shiftDownPin);
if (oldDownShiftState != shiftDownState)
{
oldDownShiftState = shiftDownState;
debounceDownShiftFlag = true;
downShiftTimer = currentMillis;
}
if (debounceDownShiftFlag && currentMillis - downShiftTimer >= DEBOUNCE_MILLIS)
{
if (oldDownShiftState == LOW)
{
Serial.println(F("Down Shift"));
}
debounceDownShiftFlag = false;
}
}
harmon3:
So in my code, The delay is useful. I will probably make it longer (probably 250 to 500 ms) so that a user can not hurriedly tap the switch over and over. This is an acceptable way to do this, correct?
Not if you intend to run any code while that is happening. As explained in (2) of reply #5. Also, sluggish switches can frustrate users, which sometimes causes heavy handed banging of the switch. That can be a reliability issue.
You want to store the old signal state and keep comparing it to a new reading. They won't be equal if there has been a change.
I definitely understand now. The code you posted is using flags to keep track of the previous button state. I will try writing a new sketch that incorporates this method. Thank you all for the help!
