1

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;
 }
  • Schematic where? – Ignacio Vazquez-Abrams May 23 '14 at 2:43
  • i already thoroughly tested the schematic it is good. pins 10 - 13 are light outputs; pins 4-7 are push button inputs. – deme72 May 23 '14 at 2:58
  • So then your inputs and outputs are working perfectly? – Ignacio Vazquez-Abrams May 23 '14 at 3:01
  • yes, a test program was created so that when button pressed corresponding lights were turned on and outputted corresponding sound, and it worked as intended. – deme72 May 23 '14 at 3:06
  • Please provide us of a schematic. With a schematic we've got a reference how your hardware setup is and how the programming should be. We don't know anything. Have you included other libraries into your sketch? That's important to know when we want to try to debug your sketch. When I get a full documentation of your script and a schematic I can try to help you, but I can't now. – Roxas May 25 '14 at 1:20
2

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.
| improve this answer | |
1

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?)

| improve this answer | |
  • that was shorthand because i didn't want to copy and past all 100+ lines of note definitions from my code – deme72 May 23 '14 at 18:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.