Return to Revisions

Many of the lines shown in the program box are superfluous – totally redundant and unnecessary. I suggest that you revise it [see (a)-(d) below], then replace all the stuff in the program box with the revised version.

(a) Use more subroutine parameters. In cases where you have four or five otherwise-identical routines that differ only in some constants, pass those values into one routine as parameters. Don't write out long lists of nearly-identical routines.

(b) Don't create artificial dependencies. Your code in the five checkEntered- routines is obscure, so I can't say without taking more time than I care to spend, but it appears that you have made tests for button 2 depend on tests for button 1, and so on down the line. That makes no sense. Test each button separately and in an identical manner, using a parameterized subroutine. To enforce an order of button presses, create an array P of 5 bytes (or however many you like) and initialize a counter Q to zero. Whenever your debounce routine detects a valid button press, if Q is small enough store the button number in P[Q] and then increase Q. (If Q is (size P) or more, don't store.)

(c) Don't nest subroutine calls more deeply than necessary. Your checkEntered1() can call checkEntered2(), which can call checkEntered3(), and so forth. Besides not making sense, that series of calls uses five times as much stack memory as necessary. (Generally, RAM is a scarce resource on AVR microcontrollers.) Just use five calls, one after another, to a properly parameterized subroutine.

(d) For repetitious sequences like the following,

digitalWrite(greenLed1, HIGH); //turn the green LED on
digitalWrite(greenLed2, HIGH); //turn the green LED on
delay(1000); //wait for a bit
digitalWrite(greenLed1, LOW); //turn the green LED off
digitalWrite(greenLed2, LOW); //turn the green LED off
delay(250); //wait for a bit

which seem to recur with minor variations about a dozen times, write a properly parameterized subroutine and call it as necessary. For example:

void flashPair(byte L1, byte L2, int del1, int del2) {
 digitalWrite(L1, HIGH); // turn the green LED on
 digitalWrite(L2, HIGH); // turn the green LED on
 delay(del1); // wait a little
 digitalWrite(L1, LOW); // turn the green LED off
 digitalWrite(L2, LOW); // turn the green LED off
 delay(del2); // wait a little
}
...
flashPair(greenLed1, greenLed2, 1000, 250);
flashPair(redLed1, redLed2, 50, 2500);
...

There are, of course, other ways of handling those dozen tedious repetitions. For example, one could write a for loop that gets pin numbers and delays from an array and turns LEDs on or off as needed. For example, see an answer to question 17453 for a method of treating multiple buttons systematically using information in arrays along with a for loop.