1

Everyone, I am new to this. Currently I am trying to make a Whack-a-mole game for my school project. There will be a total of 6 moles (corresponding to 6 LEDs, which will light up randomly). At each moment, there will only be one LED lit up. The whole game duration is 30 seconds. At the end of the game, the score will be displayed.

I use a piezo electric sensor to detect the knock of the user. I program it in such a way that if the reading of the piezo sensor is greater than 50, it will take it as knock, and if the user knocks it correctly, one point will be awarded.

But unfortunately my code isn't working. The score displayed appears to be random numbers, doesn't work as it is supposed to be.

Here is my code, please hep me to have a look and provide me with some guidance. Thank you.

unsigned long time_since_reset;
int previousNum=0;
int currentNum =0;
int randomGenerator (void);
void light(int port);
int checker(int port);
int score =0;

void setup() {
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  Serial.begin(9600);
}

void loop() {
  time_since_reset = millis();
  while((millis()-time_since_reset)<30000) {
    randomSeed(analogRead(0));
    currentNum = randomGenerator();
    light(currentNum);
    checker(currentNum);
  }
  Serial.println("End Game");
  Serial.println(score);
}

int randomGenerator(void) {
  int cNum;
  cNum = random(3, 8);
  if(cNum!= previousNum) {
    return cNum;
  }
}

void light(int port) {
  digitalWrite(port,HIGH);
  delay(1000);
}

int checker(int port) {
  jump:
  int mole1 = analogRead(0);
  int mole2 = analogRead(1);
  int mole3 = analogRead(2);
  int mole4 = analogRead(3);
  int mole5 = analogRead(4);
  int mole6 = analogRead(5);
  while(mole1<10 && mole2<10 && mole3<10 && mole4<10 &&mole5<10 &&mole6<10) {
    goto jump;
  }
  if(mole1>50 && port == 3) {
    score++;
    digitalWrite(port, LOW);
  }
  if(mole2>50 && port == 4) {
    score++;
    digitalWrite(port, LOW);
  }
  if(mole3>50 && port == 5) {
    score++;
    digitalWrite(port, LOW);
  }
  if(mole4>50 && port == 6) {
    score++;
    digitalWrite(port, LOW);
  }
  if(mole5>50 && port == 7) {
    score++;
    digitalWrite(port, LOW);
  }
  if(mole6>50 && port == 8) {
    score++;
    digitalWrite(port, LOW);
  }
}
  • 1
    When including code in a stackexchange question or answer, first paste the code into the question or answer editing box; then in that editing box, highlight the code and press ctrl-k. Or highlight the code and click the {} icon in the toolbar at the top of the editing box. To highlight text click-drag across it or use the arrow keys while holding the shift key. This will make code appear as code. – James Waldby - jwpat7 Feb 19 '17 at 4:03
  • Could you add some sample serial output? – Gerben Feb 19 '17 at 8:26
  • light() prevents checker() from reading anything for 1 second because of delay(1000); – dandavis Feb 19 '17 at 8:41
  • if I don't include the delay(1000), the led will lights up and off very quickly, the players are unable to respond that fast. any suggestion to improve it? – Jet Herng Chion Feb 20 '17 at 14:28
1

The code as shown has a bunch of problems; the probably-most-important are as follows.

score is not initialized. Add score = 0; after time_since_reset = millis();

• As noted in a comment, the delay(1000) statement in light() prevents checking for whacks during the first second of a light being on. This probably is undesirable – most people expect to whack a mole much sooner than a second after it appears. If you want a delay there, make it at most 20 milliseconds. [See edit 2]

The sequence randomSeed(analogRead(0)); currentNum = randomGenerator(); has a good chance of generating the same value of currentNum several times in a row, which may result in multiple counts for a given whack.

• Instead of saying randomSeed(analogRead(0)); within the inner loop, say it just after time_since_reset = millis(); or (probably better) in setup(). If analogRead(0) changes slowly, the same random seed may get used over and over, resulting in the same sequence of numbers from the random(3,8) expression.

• Your randomGenerator() routine has undefined behavior: It doesn't return a valid result if the current random number is the same as the previous one. You could add a loop that calls random() again if the value is the same as before; but not returning a value results in undefined behavior.

Edit 2: When you turn a light on, record the current millis() time reading, say in onTime[i] for light i, and set flag isOn[i] true. Later, when isOn[i] && (millis()-onTime[i] > targetDelay[i]), turn the light off and clear the flag (via isOn[i] = false). Within checker(), a condition like port == 6 translates to isOn[6], an expression which is true when light 6 is on, else false.

The approach of using arrays like onTime[] and isOn[] and targetDelay[], variable subscripts, and recorded on-times has several benefits: It lets you loop through lights, getting rid of that needlessly repetitious code in checker(); it enables other processing in the main game loop (such as whack-testing) to proceed independently of light control; it allows multiple moles to pop up at the same time; and it allows different moles to have different or varying up times.

  • in cryptography you want to re-seed only once per session. use a timer to get more than 255 seeds. – dandavis Feb 19 '17 at 9:32
  • if I don't include the delay(1000), the led will lights up and off very quickly, before the players are able to respond to it, any suggestion on how to improve this? thanks – Jet Herng Chion Feb 20 '17 at 14:29
  • If you don't have a delay the LED will stay in the last state you set it to. Why would it turn off? – Andrew Feb 20 '17 at 17:34
1

Others have already mentioned the issues with the timing, score initialization and random number generation.

On your checker function you have 2 errors and one ugly piece of code.

Ugly code first - goto is BAD. About the only time it's acceptable is under an error condition, the rest of the time if you find yourself needing to use a goto then look at how you can improve the code structure.

Errors - You exist after an analog input is > 10 but only count >50 as a hit. As they hit it the value will ramp up so they hit a target you could see a value of 10, exit your loop and not score anything. You need to use the same threshold for both. (Another style point, if you have the same magic number in two places (e.g. 10 or 50) consider making them constants or #defines rather than hard coded numbers. It makes it a lot easier to change them without forgetting one of the locations).

The second error is that you only turn the LED off if they hit the correct target, if they get the wrong one you light the next target but never turn the first LED off.

The code below should solve all of these issues.

int checker(int port){

  char exit = 0;
  int mole;
  while (!exit) {                // until we get a hit
    for (int i = 0; i<5; i++) {  // check each mole in turn
      mole = analogRead(i);      // read the sensor
      if (mole < 50) {           // if over the threshold
        exit = 1;                // exit the while loop after this check
        if (i + 3 == port)       // see if they got the correct target. i goes 0-5, port goes 3-8
          score++;
        digitalWrite(port,LOW);  // turn the LED off
        break;                   // break out of the for loop
      }                          // end if hit
    }                            // end for loop
  }                              // end while loop
}
  • after amending my code with your suggestions, it still don't work. after I removed the delay(1000) for the LED, it turns on and off very fast and the score showed at the end is a random number. please provide more guidance. Thanks – Jet Herng Chion Feb 24 '17 at 4:21

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.