Reaction timer using millis

Question

I've written this code for a reaction timer game. The reaction time is to be displayed in milliseconds to the serial monitor as accurately as possible. The reaction time is not being displayed correctly. I'm including my timer setup routine and where I increment milliseconds.

TCCR1A=0; 
TCCR1B=0; 
timer1_counter=65536; 
TCNT1=timer1_counter; 
TCCR1B |=(1<<CS12); 
TIMSK1 |=(1<<TOIE1); 
interrupts(); 
}
ISR(TIMER1_OVF_vect) 
{
 TCNT1=timer1_counter; 
 int_flag=1;
 }
 digitalWrite(LED,LOW); 
 randomSeed(analogRead(A1)); 
 NumSecs=random(1000,7000); 
 delay(NumSecs); 
 digitalWrite(LED,HIGH);
 while(digitalRead(PUSHBUTTON) == HIGH) 
 {
 }
 if (int_flag==1){
 millisecs++;
 int_flag=0;
 }

Answer 1

You're over-thinking this:

 digitalWrite(LED,LOW); 
 randomSeed(analogRead(A1)); 
 NumSecs=random(1000,7000); 
 delay(NumSecs); 
 digitalWrite(LED,HIGH);
 unsigned int startTime = millis();
 while(digitalRead(PUSHBUTTON) == HIGH) 
 {
 }
 unsigned int elapsed = millis() - startTime;

After that code, elapsed will contain the number of milliseconds it took the user to press the button. Note, however, that the user could cheat and simply press the button the whole time while waiting for the LED to change states. You could add logic to make sure the button isn't pressed just before you change the LED state, but I'll leave that up to you.

Answer 2

Your ISR is not counting the elapsed milliseconds. Instead, it just sets a flag and lets the main program do the counting when it sees the flag. Here:

while(digitalRead(PUSHBUTTON) == HIGH)
{
}

you are waiting for the user to press the button. The program does nothing else during the wait. It doesn't event count the time. Here:

if (int_flag==1){
 millisecs++;
 int_flag=0;
}

you increment once the millisecond count after the wait.

The obvious solution is to count the milliseconds while waiting for the button press:

millisecs = 0; // don't forget to reset the stopwatch!
while (digitalRead(PUSHBUTTON) == HIGH) { // button not pressed
 if (int_flag) {
 millisecs++;
 int_flag = false;
 }
}

(side note: int_flag should be a bool).

There are still many issues with this program. First of all, it should check that the user did not press the button before the LED came up.

Then, the interrupt period is not one millisecond, because the interrupt takes some time to be processed by the CPU and to go through its prologue. By the time you do TCNT1=timer1_counter; you are at least several tens of cycles late. You should instead configure the timer to keep the correct period by itself, typically using CTC mode.

Also, there is no point in letting the main program count the milliseconds when the ISR could do it. E.g.

ISR(TIMER1_OVF_vect)
{
 millisecs++;
}
// Avoid a race condition when reading millisecs.
static inline unsigned int getMillisecs()
{
 noInterrupts();
 int millisecsCopy = millisecs;
 interrupts();
 return millisecsCopy;
}

Note that the time keeping variables should be unsigned in order to avoid undefined behaviour on overflows.

Finally, it may be a good learning experience for working with interrupts, but in the end just using millis() or micros() would be the easiest way to achieve what you want.