Working on a reaction test for the Arduino Uno using an led and an lcd shield

Question

The situation involves some Asm inline practice and a basic circuit. The program turns an LED on after a somewhat random period of time then waits until the button is pressed to report back how many milliseconds it took. This is then printed by the LCD shield. My confusion occurs because after each reset it prints a 1 before the button is pressed, which overlaps returned millisecond value on the screen giving me some false data. I tried to cut the code down so if there is anything else that might be helpful let me know. I think the issue may be in the test reaction time function but am not sure.

/* --------------------------------------------------------------------------------------
  Includes
--------------------------------------------------------------------------------------*/
#include <LiquidCrystal.h>   // include LCD library



/*--------------------------------------------------------------------------------------
  Init the LCD library with the LCD pins to be used
--------------------------------------------------------------------------------------*/
LiquidCrystal lcd( 8, 9, 4, 5, 6, 7 );   //Pins for the freetronics 16x2 LCD shield. LCD: ( RS, E, LCD-D4, LCD-D5, LCD-D6, LCD-D7 )
/*--------------------------------------------------------------------------------------
  setup()
  Called by the Arduino framework once, before the main loop begins
--------------------------------------------------------------------------------------*/


void setup()
{
   //button adc input
   pinMode( BUTTON_ADC_PIN, INPUT );         //ensure A0 is an input
   digitalWrite( BUTTON_ADC_PIN, LOW );      //ensure pullup is off on A0
   //lcd backlight control
   digitalWrite( LCD_BACKLIGHT_PIN, HIGH );  //backlight control pin D3 is high (on)
   pinMode( LCD_BACKLIGHT_PIN, OUTPUT );     //D3 is an output
   pinMode(3,OUTPUT);                         //d4 is are LED
   //set up the LCD number of columns and rows: 
   lcd.begin( 16, 2 );
   //Print some initial text to the LCD.
   lcd.setCursor( 0, 0 );   //top left
   //          1234567890123456
   lcd.print( "LinkSprite  16x2" );
   //
   lcd.setCursor( 0, 1 );   //bottom left
   //          1234567890123456
  // lcd.print( "Btn:" );

    Serial.begin(9600);

 pinMode(2, INPUT);

 }
/*--------------------------------------------------------------------------------------
  loop()
  Arduino main loop
--------------------------------------------------------------------------------------*/
void loop()
{

      //read the button 
     byte result;//  = digitalRead(2);
      asm volatile(
          "ldi %0, 0 \n\t"
          "sbic 0x09, 2 \n\t"
          "ldi %0,1 \n\t"
          "nop \n\t"
          :"=r" (result)
          :
          :
      );
      int t =(random(0, 10000)+1000);
      unsigned long currentM = millis();
      //int t =(random(0, 10000)+1000 +millis());
      while( t < currentM){
      int x = testReactionTime();
       lcd.setCursor( 0, 1 );
     //if (result){
         lcd.print( "on " );
         lcd.setCursor(7,1);
         lcd.print(" ");
         //int x = testReactionTime();
         lcd.print(x,DEC);
         //delay(1000);
         //lcd.print("     ");
      //}else{
        //testReactionTime();
        // delay(random(0, 1000)+1000);
        //lcd.print( x,DEC );
        //digitalWrite(3, LOW);
      }
//}
}


/*
 *  after a random delay call this function to measure their reaction
 *  time in milliseconds
 *  It will immediately turn on a LED (pin 3)
 *  And then start timing the milliseconds until they hit the button (pin 2)
 * 
 */
int testReactionTime()
{
  int answer;
  //delay(rand());
  asm volatile(
    "ldi %A0, 0 \n\t"  // initializw anwer to 0
    "ldi %B0, 0 \n\t"  // answer should be the number of ms until they hit the button
    "sbi 0x0b,3 \n\t"  //turn on the led pin 3
    "ldi r16, 1 \n\t"
    "ldi r17,0 \n\t"
    "movw r26, %0 \n\t"
    "rcall killms%= \n\t"
   "cbi 0x0b,3 \n\t"  //turn on the led pin 3   

      "rjmp end%= \n\t" //skip over the routine
      //start of kill millesecond subrouting, need to waste ~16000 clock cycles
      "killms%=:ldi r24, 0x90 \n\t"  //lo8(3984)
      "ldi 25, 0x0f  \n\t"      //hi8(3984)
      "kloop%=: sbiw r24, 1 \n\t"   //2 clocks
       "brne kloop%=   \n\t"       //2 clocks since most times true!! 
       "add r26, r16 \n\t"
       "adc r26, r17 \n\t"
       "movw %0, r26 \n\t"
       "sbis 0x09, 2 \n\t"
       "ret       \n\t"
       "rcall killms%= \n\t"
      //end of the kill millesecond loop
      "end%=: nop \n\t"
    :"=r" (answer)
    :
    :"r24", "r25"
    );

  return answer;
}

Answer 1

Expanding on my comment... I see a few issues in your asm code, not all as serious as the first one:

1. killms is calling itself recursively indefinitely, which should overflow your stack. The last rcall killms%= should probably be an rjmp.
2. There is no need to move the answer around between register pairs. Just use the compiler-allocated registers instead of r17:r16.
3. By adc r26, r17, you probably mean adc r27, r17.
4. Instead of add and adc, you could just adiw, which saves a register pair.

Below is my version of testReactionTime(). I tested it on an Uno (with Serial instead of an LCD) and it works fine. The assumptions on the hardware are:

• The LED is on pin 13 (PB5): it's the built-in LED.
• The button is on pin 2 (PD2), which goes HIGH when the button is pressed.

/*
 * After a random delay, call this function to measure the reaction time
 * in milliseconds. It will immediately turn the built-in LED and then
 * start timing the milliseconds until the user hits the button (pin 2
 * going HIGH).
 */
int testReactionTime()
{
    int result, counter;
    asm volatile(
        "    clr %A[result]                      \n" // clear result
        "    clr %B[result]                      \n" // ditto
        "    sbi %[led_port], %[led_bit]         \n" // LED on

        // delay to make the loop 0 last exactly one millisecond
        "0:  ldi %A[counter], lo8(%[iterations]) \n"
        "    ldi %B[counter], hi8(%[iterations]) \n"
        "1:  sbiw %[counter], 1                  \n"
        "    brne 1b                             \n"
        "    nop                                 \n"

        "    sbic %[button_pin], %[button_bit]   \n" // unless button up:
        "    rjmp 2f                             \n" //     we are done
        "    adiw %[result], 1                   \n" // increment result
        "    rjmp 0b                             \n" // loop back
        "2:  cbi %[led_port], %[led_bit]         \n" // done: LED off

        : [result]     "=w" (result)
        , [counter]    "=w" (counter)
        : [led_port]   "I"  (_SFR_IO_ADDR(PORTB))  // LED on PB5
        , [led_bit]    "I"  (5)
        , [button_pin] "I"  (_SFR_IO_ADDR(PIND))   // button on PD2
        , [button_bit] "I"  (2)
        , [iterations] "i"  ((16000 - 8) / 4)
    );
    return result;
}