Change code so motion sensor prints time instead of a button

Question

I have the code below. At the moment there is a proximity sensor that makes a speaker play happy birthday when it is set off. I also have an LCD shield attached and when one of the buttons on LCD screens interface is pressed, the time that it was when the button was pressed prints on the screen. I need to get it so that when the proximity sensor is set off it plays the sound on the speaker and then prints the time which is was set off on the screen.

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

int timer = 1000;
int C = 262;
int D = 294;
int E = 330;    
int F = 349;
int G = 392;
int A = 440;
int B = 494;
int Bflat = 466;
int Chigh = 523;


const int analogPin = A1;    // pin that the sensor is attached to
const int threshold = 400;


/*--------------------------------------------------------------------------------------
 Defines
 --------------------------------------------------------------------------------------*/
// Pins in use
#define MILLIS_OVERFLOW 34359738
#define BUTTON_ADC_PIN           A0  // A0 is the button ADC input
#define LCD_BACKLIGHT_PIN         3  // D3 controls LCD backlight
// ADC readings expected for the 5 buttons on the ADC input
#define RIGHT_10BIT_ADC           0  // right
#define UP_10BIT_ADC            145  // up
#define DOWN_10BIT_ADC          329  // down
#define LEFT_10BIT_ADC          505  // left
#define SELECT_10BIT_ADC        741  // right
#define BUTTONHYSTERESIS         10  // hysteresis for valid button sensing     window
//return values for ReadButtons()
#define BUTTON_NONE               0  // 
#define BUTTON_RIGHT              1  // 
#define BUTTON_UP                 2  // 
#define BUTTON_DOWN               3  // 
#define BUTTON_LEFT               4  // 
#define BUTTON_SELECT             5  // 
//some example macros with friendly labels for LCD backlight/pin control,     tested and can be swapped into the example code as you like
#define LCD_BACKLIGHT_OFF()     digitalWrite( LCD_BACKLIGHT_PIN, LOW )
#define LCD_BACKLIGHT_ON()      digitalWrite( LCD_BACKLIGHT_PIN, HIGH )
#define LCD_BACKLIGHT(state)    { if( state ){digitalWrite(     LCD_BACKLIGHT_PIN, HIGH );}else{digitalWrite( LCD_BACKLIGHT_PIN, LOW );} }
/*--------------------------------------------------------------------------------------
 Variables
 --------------------------------------------------------------------------------------*/
unsigned long currentMillis, previousMillis, elapsedMillis;
int seconds = 0, minutes = 34, hours = 8;

byte buttonJustPressed  = false;         //this will be true after a        ReadButtons() call if triggered
byte buttonJustReleased = false;         //this will be true after a     ReadButtons() call if triggered
byte buttonWas          = BUTTON_NONE;   //used by ReadButtons() for detection of button events
/*--------------------------------------------------------------------------------------
 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
  //set up the LCD number of columns and rows: 
   pinMode(13, OUTPUT);
  lcd.begin( 16, 2 );

  //Print some initial text to the LCD.
  Serial.begin(9600);
  lcd.begin( 16, 2 ); 



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

  setClock();
  byte button;


  button = ReadButtons();


  /**
   * After set clock now you have 3 int variables with the current time
   */
  //seconds
  //minutes
  //hours
  lcd.setCursor ( 0, 1);
  lcd.print(hours);
  lcd.print(":");
  lcd.print(minutes);
  lcd.print(":");
  lcd.print(seconds);
  lcd.print(":");
  lcd.print(elapsedMillis);


  //get the latest button pressed, also the buttonJustPressed,     buttonJustReleased flags

  //blank the demo text line if a new button is pressed or released, ready for a new label to be written

  //show text label for the button pressed
   if(buttonJustPressed || buttonJustReleased)
  switch( button )
  {

  case BUTTON_LEFT:
    {
      lcd.setCursor( 0, 0 );
               lcd.print(hours);
               lcd.print(":");
               lcd.print(minutes);
               lcd.print(":");
               lcd.print(seconds);
               lcd.print(":");
               lcd.print(elapsedMillis);

      break;         

    }
  default:
    {
      break;
    }
  }

  if( buttonJustPressed )
    buttonJustPressed = false;
  if( buttonJustReleased )
    buttonJustReleased = false;
}
/*--------------------------------------------------------------------------------------
 ReadButtons()
 Detect the button pressed and return the value
 Uses global values buttonWas, buttonJustPressed, buttonJustReleased.
 --------------------------------------------------------------------------------------*/
byte ReadButtons()
{
  unsigned int buttonVoltage;
  byte button = BUTTON_NONE;   // return no button pressed if the below checks don't write to btn

  //read the button ADC pin voltage
  buttonVoltage = analogRead( BUTTON_ADC_PIN );
  //sense if the voltage falls within valid voltage windows
  if( buttonVoltage < ( RIGHT_10BIT_ADC + BUTTONHYSTERESIS ) )
  {
    button = BUTTON_RIGHT;
  }
  else if(   buttonVoltage >= ( UP_10BIT_ADC - BUTTONHYSTERESIS )
    && buttonVoltage <= ( UP_10BIT_ADC + BUTTONHYSTERESIS ) )
  {
    button = BUTTON_UP;
  }
  else if(   buttonVoltage >= ( DOWN_10BIT_ADC - BUTTONHYSTERESIS )
    && buttonVoltage <= ( DOWN_10BIT_ADC + BUTTONHYSTERESIS ) )
  {
    button = BUTTON_DOWN;
  }
  else if(   buttonVoltage >= ( LEFT_10BIT_ADC - BUTTONHYSTERESIS )
    && buttonVoltage <= ( LEFT_10BIT_ADC + BUTTONHYSTERESIS ) )
  {
    button = BUTTON_LEFT;
  }
  else if(   buttonVoltage >= ( SELECT_10BIT_ADC - BUTTONHYSTERESIS )
    && buttonVoltage <= ( SELECT_10BIT_ADC + BUTTONHYSTERESIS ) )
  {
    button = BUTTON_SELECT;
  }
  //handle button flags for just pressed and just released events
  if( ( buttonWas == BUTTON_NONE ) && ( button != BUTTON_NONE ) )
  {
    //the button was just pressed, set buttonJustPressed, this can optionally be used to trigger a once-off action for a button press event
    //it's the duty of the receiver to clear these flags if it wants to detect a new button change event
    buttonJustPressed  = true;
    buttonJustReleased = false;
  }
  if( ( buttonWas != BUTTON_NONE ) && ( button == BUTTON_NONE ) )
  {
    buttonJustPressed  = false;
    buttonJustReleased = true;
  }

  //save the latest button value, for change event detection next time round
  buttonWas = button;

  return( button );
}
void setClock()
{
  currentMillis = millis();
  /**
   * The only moment when currentMillis will be smaller than previousMillis
   * will be when millis() oveflows
   */
  if (currentMillis < previousMillis){
    elapsedMillis += MILLIS_OVERFLOW - previousMillis + currentMillis;
  } 
  else {
    elapsedMillis += currentMillis - previousMillis;
  }

  /**
   * If we use equals 1000 its possible that because of the mentioned loop limitation
   * you check the difference when its value is (999) and on the next loop     its value is (1001)
   */
  while (elapsedMillis > 999){
    seconds++;
    elapsedMillis = elapsedMillis - 1000;
  }

  if (seconds == 60){
    minutes++;
    seconds = 0;
  }
  if (minutes == 60){
    hours++;
    minutes = 0;
  }
  if (hours == 24){
    hours = 0;
  }

  previousMillis = currentMillis;


 int analogValue = analogRead(analogPin);
  if (analogValue > threshold) {

tone(13,C,timer/2);// Happy Bday to You
delay(200);
tone(13,C,timer/2);
delay(200);
tone(13,D,timer/2);
delay(400);
tone(13,C,timer/2);
delay(400);
tone(13,F,timer/2);
delay(400);
tone(13,E,timer);
delay(750);
 }
else {

}
}

Answer 1

The code seems a little hotchpotch; is the code for reading the sensor supposed to be within the setClock() function? It would be better to put the following within loop():

if(analogRead(analogPin) > threshold)
{
    playMelody();
    printTriggerTime();
}

And create two new functions that handle the playing of 'Happy Birthday' and the printing of the time:

void playMelody()
{
    tone(13, C, timer / 2);// Happy Bday to you
    ...
    delay(750);
}

...which is just a cut-paste of what you've got in the setClock() function, and:

void printTriggerTime()
{
    lcd.setCursor(0, 0);
    ...
    lcd.print(elapsedMillis);
}

...which is a copy-paste of the code from within case BUTTON_LEFT:.

This will now play 'Happy Birthday' and print the time whenever the proximity sensor is triggered, AND it will print the time when the left-button is pressed (unless you remove that part of the code).

On a separate point, it's bad coding practice to use #define for a function, as you have done with #define LCD_BACKLIGHT(state) { if ( state )... - it makes it quite difficult to read the code, and can't conceive any reason why this isn't its own function. However, a nice trick in C is the following 'shortcut' statement:

( CONDITION ? DO_IF_TRUE : DO_IF_FALSE )

Thus you should change that line to:

#define LCD_BACKLIGHT(state) ( state ? digitalWrite(LCD_BACKLIGHT_PIN,HIGH) : digitalWrite(LCD_BACKLIGHT_PIN,LOW))

That being said, "LCD_BACKLIGHT(state)" isn't actually used anywhere in the program!