Mazon:
Ok then.. enlighten me: How are we supposed to read all these inputs in a single loop, without missing a ton of button clicks/user inputs? For example, at a certain point in my loop I read the state of a certain digital input pin. What if I push the button at a moment, where my loop is busy reading other input?
Then the button will be read on the next cycle through the loop, as it should be. In the process of de-bouncing, there is by definition, no hurry to detect the initial press.
Mazon:
That's why we thought we'd need interrupts.
A common mistake is to fail to conceptualise just how fast the loop() runs. If properly written, it should cycle at least 5000 times per second - at least five times per millisecond - and generally much more than that. Unless your encoder is driven by a power drill, that should be more than fast enough.
It is only the operations that take significant time that you need to manage - such as printing. But actually printing some information generally signifies that you do not need to collect further information at the time of printing. More about that in a moment.
Mazon:
But I'll put it in context for you:
Ooh, that is nasty! As best I understand that code - in either version - the interrupt executes a function which calls another function which uses Serial.print. So you are attempting to execute Serial.print within an interrupt.
Mazon:
To your second point: How I imagined this would debounce the input? I'd save the time of the last function call in lastEnconding, now every time the interrupt function is called, I check if the last call is at least 50ms ago.
You appear to be saving the time only after Serial.print is called within the interrupt routine. Given how long Serial.print takes to execute, this could be an awful lot longer than 50 ms and all sorts of things might have happened in that interval.
The general concept is not too bad, you make a note of when the encoder state changes (on only one pin - you are sensing half steps) and keep checking on every bounce (of that pin) in either direction, waiting for the interval to elapse in which case you act on whether the other encoder pin is high or not.
No, that is not going to work. If you insist in responding to every bounce, you must completely characterise the change of state to determine how to interpret it. 50 ms is a long time (a usual bounce period would be more like 5 ms for an encoder and 10 ms for a pushbutton) and all sorts of things could happen in that interval of which you would be entirely ignorant.
My method of de-bouncing is to poll the input(s) every time the loop cycles. If a change of state is detected, a decision is made as to whether the change is sustained since the last poll, otherwise it reverts to the previous state. If the change is sustained for every poll over the debounce interval (the timer is only reviewed if a change has been so maintained since the last cycle through the loop), then the corresponding action is taken. In so doing, changes that revert within the debounce interval are completely ignored and the interval is only started again when the next change is sensed. An example of the code is below.
By the way, if by
if(counter > 4){
counter = 0;
}
if(counter < 0){
counter = 4;
}
you really meant to restrict the range to 0 to 3, it could be abbreviated to
counter &= 3;
and lastEncoding and currMillis should be unsigned long and 50 should be 50UL
Debouncing:
// Binary cycling multiple LEDs
const int led1Pin = 13; // LED pin number
const int led2Pin = 11;
const int button1 = 4;
char led1State = LOW; // initialise the LED
char led2State = LOW;
char allState = 0;
char bstate1 = 0;
unsigned long bcount1 = 0; // button debounce timer. Replicate as necessary.
// Have we completed the specified interval since last confirmed event?
// "marker" chooses which counter to check
boolean timeout(unsigned long *marker, unsigned long interval) {
if (millis() - *marker >= interval) {
*marker += interval; // move on ready for next interval
return true;
}
else return false;
}
// Deal with a button read; true if button pressed and debounced is a new event
// Uses reading of button input, debounce store, state store and debounce interval.
boolean butndown(char button, unsigned long *marker, char *butnstate, unsigned long interval) {
switch (*butnstate) { // Odd states if was pressed, >= 2 if debounce in progress
case 0: // Button up so far,
if (button == HIGH) return false; // Nothing happening!
else {
*butnstate = 2; // record that is now pressed
*marker = millis(); // note when was pressed
return false; // and move on
}
case 1: // Button down so far,
if (button == LOW) return false; // Nothing happening!
else {
*butnstate = 3; // record that is now released
*marker = millis(); // note when was released
return false; // and move on
}
case 2: // Button was up, now down.
if (button == HIGH) {
*butnstate = 0; // no, not debounced; revert the state
return false; // False alarm!
}
else {
if (millis() - *marker >= interval) {
*butnstate = 1; // jackpot! update the state
return true; // because we have the desired event!
}
else
return false; // not done yet; just move on
}
case 3: // Button was down, now up.
if (button == LOW) {
*butnstate = 1; // no, not debounced; revert the state
return false; // False alarm!
}
else {
if (millis() - *marker >= interval) {
*butnstate = 0; // Debounced; update the state
return false; // but it is not the event we want
}
else
return false; // not done yet; just move on
}
default: // Error; recover anyway
{
*butnstate = 0;
return false; // Definitely false!
}
}
}
void setup() {
pinMode(led1Pin, OUTPUT);
pinMode(led2Pin, OUTPUT);
pinMode(button1, INPUT);
digitalWrite(button1, HIGH); // internal pullup all versions
}
void loop() {
// Cycle LEDs if button debounced
if (butndown(digitalRead(button1), &bcount1, &bstate1, 10UL )) {
allState++; allState &= 3; // Increment state, constrain to 0 to 3
if (allState & 1) led1State = HIGH; // LED 1 on if odd state
else led1State = LOW;
if (allState & 2) led2State = HIGH; // LED 2 on if 2 or 3
else led2State = LOW;
}
digitalWrite(led1Pin, led1State);
digitalWrite(led2Pin, led2State);
}