help with input in a Simon arduino uno game

Question

I' making a Simon game with the arduino uno.

I know something is wrong with the inputs but i don't know why or how to fix it.

The problem is the program takes an input and says it is correct no matter what it is or the length

#define NOTES

int tones[] = {NOTE_E5, NOTE_E6, NOTE_CS6, NOTE_A6};
String sequence;

void setup() {
    pinMode(13, OUTPUT); //green
    pinMode(12, OUTPUT); //yellow
    pinMode(11, OUTPUT); //blue
    pinMode(10, OUTPUT); //red
    pinMode(7, INPUT); //green
    pinMode(6, INPUT); //yellow
    pinMode(5, INPUT); //blue
    pinMode(4, INPUT); //red
    Serial.begin(9600);
}

void loop() {
    while(true) {
        delay(1000);
        randomSeed(random(random(1000000000)));
        sequence = String(random(4));
        boolean run = true;
        while(run) {
            for(int i = 0; i < sequence.length(); i++) {
                simonOut(sequence.charAt(i) - '0');
                delay(500);
                simonClear();
                delay(100);
            }
            for(int i = 0; i < sequence.length(); i++) {
                int input = simonIn();
                while(input < 0 || input > 3) {
                    delay(50);
                    input = simonIn();
                }
                if(sequence.charAt(i) - '0' == input) {
                    flash(0);
                    run = false;
                    goto reset;
                }
                else if(i = sequence.length() - 1)
                    flash(3);
            }
            sequence += random(4);
        }
        reset:;
    }
}

void boolOut(long pin, boolean val) {
    digitalWrite(pin,val ? HIGH : LOW);
}

void simonOut(long num) {
    boolOut(num + 10, true);
    tone(2, tones[num]);
}

 void flash(int led) {
     delay(300);
     for(int i = 0; i < 5; i++) {
         simonOut(led);
         delay(100);
         simonClear();
         delay(100);
     } 
 }

 void simonClear() {
     for(int i = 0; i < 4; i++){
         boolOut(i + 10, false);
     }
     noTone(2);
 }

 int simonIn() {
     int ret = -1;
     for(int i = 0; i < 4; i++) {
         if(simonIn(i)){
             ret = i;
         }
     }
     if(ret != -1) {
         simonOut(ret);
         delay(500);
         simonClear();
         delay(100);
     }
 }

 boolean simonIn(long num) {
     return digitalRead(num + 4) == HIGH;
 }

Answer 1

The problem in your code is there:

int simonIn() {
    int ret = -1;
    for(int i = 0; i < 4; i++) {
        if(simonIn(i)){
            ret = i;
        }
    }
    if(ret != -1) {
        simonOut(ret);
        delay(500);
        simonClear();
        delay(100);
    }
}

This method never returns any value, so that means the caller will just tke whatever is in the stack at that time:

            int input = simonIn();
            while(input < 0 || input > 3) {
                delay(50);
                input = simonIn();
            }

input could be anything there.

So the fix should be easy:

int simonIn() {
    int ret = -1;
    for(int i = 0; i < 4; i++) {
        if(simonIn(i)){
            ret = i;
        }
    }
    if(ret != -1) {
        simonOut(ret);
        delay(500);
        simonClear();
        delay(100);
    }
    return ret;
}

It's surprising you did not get a warning from the compiler on that one, which IDE do you use?

Finally, regarding your code, I would also advise you the following improvements:

• break down the loop() code further into functions that do ONE thing, e.g. showSequence(sequence), checkInput(seuquence), readSimonInput() (this last function would only return a value between 0 and 3 included. That would make your code much clearer.
• use the right types for your variables: I see you abuse usage of long where int or byte, or you mix different types for the same data, thus forcing the compiler to cast data on some function calls.
• avoid hardcoding constants in your code, e.g. you could define the number of buttons, the first pin of buttons, the first pin of LEDs, the various delays.

Answer 2

Is it possible that your problem might be just a typo?

#define NOTES

int tones[] = {NOTE_E5, NOTE_E6, NOTE_CS6, NOTE_A6};

In your array you talk about NOTE and you defined NOTES (See the S?)