Solved: Pressing a button in a rhythm

So I'm making a game where someone has to press a button five times to the rhythm of Staying Alive of the Bee Gees. You know, like how they say that if you are in the situation where you have to revive someone, keep the rhythm of that song in your head. It's 103 bpm so that's 580ms/beat. I put a large window around it of 500-700ms.

Now what my code is doing right:

  • if the player presses in the right rhythm 5 times in a row, the LED flashes 10times to indicate that they've won.

What I can't get to work is when they "break the chain", i.e. press the button too fast or too slow. More detail is in the code below. I've worked around the too slow part by adding a bit where they have to finish within 2.7 seconds or the game restarts. But this is of course not as neat as: "you were too fast or too slow so the game restarted".

Any ideas?

const int buttonPin = 2;     // the number of the pushbutton pin
const int ledPin =  13;      // the number of the LED pin

int count = 0;               // Count the button presses
int buttonState = 0;         // variable for reading the pushbutton status
int lastButtonState = 0;
long detection_start = 0;
long detection_range = 0;
int buttonPushCounter = 0;

void setup() {
  pinMode(ledPin, OUTPUT);     
  pinMode(buttonPin, INPUT);
  Serial.begin(9600);    
}

void loop() {

  buttonState = digitalRead(buttonPin);
  
  if (buttonState != lastButtonState && buttonState == HIGH) {

    Serial.println(millis() - detection_range);
    Serial.println(millis() - detection_start);
    
    if (buttonPushCounter == 0){
     detection_start = detection_range = millis();
     buttonPushCounter++;
     Serial.print("number of button pushes: ");
     Serial.println(buttonPushCounter);
     digitalWrite(ledPin, HIGH);    
    }
    
    if (buttonPushCounter > 0 && millis() - detection_range >= 500 && millis() - detection_range <= 700) {
      buttonPushCounter++;
      Serial.print("number of button pushes: ");
      Serial.println(buttonPushCounter);
      detection_range = millis();
    }

[b]/* These lines below don't work because they fall into a loop where buttonPushCounter is reset to 0,
 *  because of this on the if buttonPushCounter == 0, millis and detection_range are set equal.
 *  This automatically means that the difference is < 500, triggering the reset again.
 */[/b]
 
//    if (buttonPushCounter > 0 && millis() - detection_range < 500) {
//      buttonPushCounter = 0;
//      Serial.print("reset");
//      }
//      
    delay(50);
  }
  
  if (buttonPushCounter > 0 && millis() - detection_start > 2700){
      buttonPushCounter = 0;
    }
    
  lastButtonState = buttonState;

  if (buttonPushCounter >= 1 && buttonPushCounter <= 5) {
    digitalWrite(ledPin, HIGH);
  } else {
    digitalWrite(ledPin, LOW);
  }

  if (buttonPushCounter == 5){
    for (int i = 0; i <= 10; i++){
    digitalWrite(ledPin, LOW);
    delay(100);
    digitalWrite(ledPin, HIGH);
    delay(100);
    }
//    delay(5000);
//    buttonPushCounter = 0;
   }
}
detection_start = detection_range = millis();

Don't do this. Make that two statements. Yes, I know it is legal but it hurts readability and you need help. Try to keep things simple for yourself if you're new to this.

Instead of relying on buttonPushCounter for everything, make yourself a couple of new variables so you don't have to reset buttonPushCounter to 0 every time they make a mistake. Maybe a boolean variable called oneWasOutOfRange, or a boolean everythingIsStillAllGoodForThisPlayer that can be set false when they miss one. You only want to get a new value from millis when the button is actually pressed.

Which leads me to a question about the rules for the game. If I am late on say the third press, do I need to press the fourth a little early to make up for it and get back on time with the song or do I want to still wait the entire 500 - 700ms?

Delta_G:
Which leads me to a question about the rules for the game. If I am late on say the third press, do I need to press the fourth a little early to make up for it and get back on time with the song or do I want to still wait the entire 500 - 700ms?

The song isn't playing during the game, that's maybe important to point out.

both the early and late presses won't be registered as a count, so that won't work. You have to wait to get back in the rhythm, but you will fail to get five presses because of the 2,7s cap. In theory you could get 6 or 7 (very quick) presses in and still get a correct five count

However, this situation won't happen when I've fixed the fallout issue because you fall out as soon as you press early or late. The you can try again to get into the rhythm.

Deleted for brevity

What happened to your other thread on this? Why did you start over? Don't do that! It wastes people's time if they have to give answers that already been given on the other thread.

You should be using ">1" when checking the interval because "detecton_range" (bad name) isn't set until the count goes from 0 to 1.

     if (buttonPushCounter > 1)
      if (millis() - detection_range >= 500 && millis() - detection_range <= 700) {
          // Second or later push was within the allowed time range
          buttonPushCounter++;
          Serial.print("number of button pushes: ");
          Serial.println(buttonPushCounter);
          detection_range = millis();
        } else {
           // Second or later push missed the time window
           buttonPushCounter = 0;  // Restart
           digitalWrite(ledPin, LOW);  // Turn out the light
           Serial.println("Missed the push window.");
        }

Delta_G:
What happened to your other thread on this?

Combination of cross-post and moderator snafu. All should be good now.

johnwasser:
You should be using ">1" when checking the interval because "detecton_range" (bad name) isn't set until the count goes from 0 to 1.

     if (buttonPushCounter > 1)

if (millis() - detection_range >= 500 && millis() - detection_range <= 700) {
          // Second or later push was within the allowed time range
          buttonPushCounter++;
          Serial.print("number of button pushes: ");
          Serial.println(buttonPushCounter);
          detection_range = millis();
        } else {
          // Second or later push missed the time window
          buttonPushCounter = 0;  // Restart
          digitalWrite(ledPin, LOW);  // Turn out the light
          Serial.println("Missed the push window.");
        }

Then it breaks on the second time round because on each succesful press detection_range is set equal to millis() to measure the time between presses.

 if (buttonState != lastButtonState && buttonState == HIGH) {

    Serial.print("millis() - detection range = ");
    Serial.println(millis() - detection_range);
    Serial.print("millis() - detection start = ");
    Serial.println(millis() - detection_start);
    
    if (buttonPushCounter == 0){
      detection_start = millis();
      detection_range = millis();
      buttonPushCounter++;
      Serial.print("number of button pushes: ");
      Serial.println(buttonPushCounter);
      digitalWrite(ledPin, HIGH);    
    }

    if (buttonPushCounter > 0){
      if (millis() - detection_range >= 500 && millis() - detection_range <= 700){
        buttonPushCounter++;
        Serial.print("number of button pushes: ");
        Serial.println(buttonPushCounter);
        digitalWrite(ledPin, HIGH);
      }
      else {
        Serial.print("millis() - detection range = ");
        Serial.println(millis() - detection_range);
        buttonPushCounter = 0;
        Serial.println("reset");
      }
      delay(50);    
      detection_range = millis();
    }
  }

Gives this output

millis() - detection range = 379
millis() - detection start = 499
number of button pushes: 1
millis() - detection range = 59
reset

Practically the same happens when I write >1 because then it just loops like that on the second time.
I'll try the first option with multiple variables later.

JorritK:
Gives this output

millis() - detection range = 379

millis() - detection start = 499
number of button pushes: 1
millis() - detection range = 59
reset

It's doing what you told it to do. You incremented the count from 0 to 1 and reset the timer. Now that the value is >0 it immediately checks the timer again it discovers that the value is <500 so it resets. That's why you should not check the time when the count is 1.

Again. I don't think you should be using the button press count to serve two purposes. You need another variable to track when the player has failed.

Thanks for the suggestions guys :slight_smile:

johnwasser:
It's doing what you told it to do. You incremented the count from 0 to 1 and reset the timer. Now that the value is >0 it immediately checks the timer again it discovers that the value is <500 so it resets. That's why you should not check the time when the count is 1.

I know, but the code is different from the first time around, I appended it with your code. However, if I check it on count >= 1 it does exactly the same thing, except that it never gets past 2. So I've moved the problem 1 count forward. So it disregards the if/else statement for some reason.

So A) why would it go to the else statement here

 if (buttonPushCounter > 0){
      if (millis() - detection_range >= 500 && millis() - detection_range <= 700){
        buttonPushCounter++;
        Serial.print("number of button pushes: ");
        Serial.println(buttonPushCounter);
        digitalWrite(ledPin, HIGH);
      }
      else {
        Serial.print("millis() - detection range = ");
        Serial.println(millis() - detection_range);
        buttonPushCounter = 0;
        Serial.println("reset");
      }

if the if statement was true, i.e. you got the rhythm right?

And B) assuming that it does, why would it print 50ms as time between range and millis? Range isn't set to millis until after the if > 0 loop. It almost looks like a denouncing issue.

Delta_G:
Again. I don't think you should be using the button press count to serve two purposes. You need another variable to track when the player has failed.

As I said above I'll try that option later, I have to wrap my head around resetting the button counter without using button counter = 0

So you want to solve it the impossible way first and when you figure that out you'll try the easy option?

The impossible solution turned out to be pretty simple. Instead of using two if statements I put every condition in if/else statements.

if (count =0) {
  count ++
  detection range = 0
  }

if (count > 0 && detection range >500 && detection range < 700 {
  count ++
  }
  else {
    reset
    }

I used

if (count = 0) {
  count ++
  detection range = 0
  }
  else {
    if detection range > 500 && detection range < 700 {
    count ++
    }
    else {
      reset
      }
if (count = 0) {

And that worked??? If you say so. We will see you again soon then I suppose.

Delta_G:

if (count = 0) {

And that worked??? If you say so. We will see you again soon then I suppose.

It works exactly as I intended. Or do you want me to use count == 0?

If you're just gonna keep bashing and being a sour guy I'd rather you just move on, because apart from your first post you've been anything but helpful.

Say what???? What did I do to you other than point out an error. How is that bashing? If you really had that in your code then I just caught a bug before it bit you. I really wish you'd point it out cause I just don't see what I said that upset you.

if (count = 0) {

It works exactly as I intended. Or do you want me to use count == 0?

Please can you tell me what you intended that line of code to do ?

JorritK:
I used

if (count = 0) {

count ++
 detection range = 0
 }
 else {
   if detection range > 500 && detection range < 700 {
   count ++
   }
   else {
     reset
     }

The compiler let you compile this line?!?

    if detection range > 500 && detection range < 700 {

or let you write a statement without the closing semicolon?!?

    else {
      reset
      }