Hi all! I've been working on some code that works well, under one circumstance and have come to hopefully get some help understanding why. A tiny bit of background, the code I will attach has some functions which don't matter a ton for this discussion, but what matters is that there is some basic button polling going on. The first element of buttonPins is for a button that switches between 3 modes (live, preset, and sample/hold), the second element is for a button which switches between the presets when in preset mode. The states can be seen in the nested SwitchCase in the code.
All of this work beautifully, AS LONG as I have the delay(400) in the buttonPoll function as shown. If i take it out, the first button switches for a blink of an eye from state 0 to 1 (into preset mode than back to live). I'm sure it's not button bouncing because all my buttons are hardware debounced with RC filters and a schmitt trigger inverter. I've checked the scope traces and the transisions are clean as a whistle.
I'm just worried that the 400ms delay is a bandaid for something I'm doing wrong, as I've read on the interwebs that delay() should be avoided. Any help or guidance would be SUPER appreciated. Thanks so much, code gurus!
// Button poll function
void pollButtons() {
for (int z=0; z<4; z++) {
buttonPoll[z] = digitalRead(buttonPins[z]);
if (buttonPoll[z] == HIGH) {
buttonStates[z] = buttonStatesOld[z] + 1;
}
}
}
...checks for the state of all buttons and updates the array.
This code, which executes EVERY time round the loop...
delay(400);
if (buttonPoll[2] == HIGH) { // Actuate preset store function if the preset store button is pressed
presetStore();
}
if (buttonPoll[3] == HIGH) {
if (buttonStates[0] = 2) { // If in the S&H state & the sample button is pressed, actuate the sample grab function
//sandhSampleGrab();
}}
...has no business being inside the loop. It checks for individual buttons (2 and 3) and so there is no reason to be doing that for every other button.
I expect you need the delay for that reason, and mainly to slow things down. Because there doesn't seem to be any mechanism to wait for a key to be released. You are only checking if it is currently pressed.
Although TBH I'm having a hard time making sense of your code. What's this meant to be doing?
buttonStates[z] = buttonStatesOld[z] + 1;
For me "buttonStates" isn't a very good choice of name, since it doesn't actually represent the button state. In your code it seems the button state is actually given by varaible buttonPoll, but that doesn't seem a very good name either. To me, "buttonPoll" says if I should be testing the button or not....
For the buttonStates[z] = buttonStatesOld[z] + 1; This is I believe supposed to update the counter for that button and store it, incremented by one if that button is pressed. All the Shiftcase button tutorials I found do it this way, so I copied that idea. Is there a better way? I agree the variable naming needs work. If you have suggestions, I'm totally open to it.
So, would you suggest I move the
if (buttonPoll[2] == HIGH) { // Actuate preset store function if the preset store button is pressed
presetStore();
}
if (buttonPoll[3] == HIGH) {
if (buttonStates[0] = 2) { // If in the S&H state & the sample button is pressed, actuate the sample grab function
//sandhSampleGrab();
}}
and place it outside the main button poll, but still inside the main loop?
I think you are onto something about waiting for a button to be released. I had wondered about handling this, but none of the tutorials I found address this at all. They just look at presses and of course always have delays in them for basic software debounce... maybe that makes them work? Ha!
Do you have any examples of how to handle looking at button releases as well? I'm still sorta new to this, and really learn well by example.
Thanks so much again for the help and looking through my code.!
I think part of the problem is I'm convolving button state with poll and naming things in a confusing way. I think I need now:
-button state (for each button): looks at if the button is low or high <- use this to compare to previous state and check change as in the tutorial groundFungus posted.
-mode state counter (for each button) (perhaps there is a better name for this): increments by 1 if the button is determined pressed by the above. This is what the switchcase looks at.
Success! I took the advice of cleaning up my loop and also i realized there was nothing in the switch case that couldn't be solved with a few if statements! Now it works flawless, nice and snappy, and NO delay() needed:
// Start loop function
void loop() {
monitorButtons();
manageModes();
}
// Button monitor function
void monitorButtons() {
if (modeState[0] > 2) {
modeState[0] = 0;
}
if (modeState[1] > 4) {
modeState[1] = 0;
}
for (int z=0; z<4; z++) {
buttonStates[z] = digitalRead(buttonPins[z]);
if (buttonStates[z] != lastButtonStates[z]) {
if (buttonStates[z] == HIGH) {
modeState[z]++;
}
lastButtonStates[z] = buttonStates[z];
}
}
}
// Mode manager function
void manageModes() {
if (modeState[0] == 0) {
liveMode();
}
if (modeState[0] == 1) {
presetMode(modeState[1]);
}
if (modeState[0] == 2) {
sandhMode();
}
}
