Keeping last flag number

Hi, I'm a new chap on this so please be patient with me.
I am trying to make a model train speed check program that can check the train speed regardless of which direction it comes from.
The first sensor detects the train entering the monitored area and can still be sensing or not depending upon the length of the train being detected.
i.e. As train enters the area sensor 1 low and sensor 2 high
As the train front is leaving the area sensor 1 low or high and sensor 2 low.
Obviously the sequence is reversed for the opposite direction of travel.
I am trying to use 'flag' to indicate a train is in the monitoring area, but of course it may not latch depending upon the length of the train.
flag = 0 is no train, flag = 1 is train is being monitored, flag = 2 indicate time and speed.
Have attached zip of complete program as hoping someone out there will take pity of 70+ man trying hard to get a working unit that can be taken to club nights as well as used at home.
Arduino is genuino/uno, have pull up resistors on the sensors.

Ken

bi_direction_train_speed_check.zip (5.08 KB)

You need to detect the changes in the signals, only reading them will not work.

You will get two timestamps, one for each transition from free to blocked on both measurement points.

This is your code (after pressing ctrl-T to auto.format it), small codes are better embedded in code tags,
so it is easy to copy and can be viewed from any platform.

//Bi-direction train speed indicator
//Attempt 1 date 17/06/2018
//S is Speed to be in mph
//T is Time measured in seconds
//L is Distance between sensors in feet
//G is scale of model (ie gauge)
//This attempt is for n gauge hence G = 148

#include<Wire.h>
#include<LiquidCrystal_PCF8574.h>

LiquidCrystal_PCF8574 lcd(0x27); //lcd address
int error;
long int start = 0;
long int finish = 0;
float timetaken = 0.000;
int flag = 0;
float mph = 000.0;
float length = 1; //distance of probes in ft
float G = 148; //ngauge
int direction = 0; //direction of train
float realdistance = 0.000;


void setup() {
  Wire.begin();
  Wire.beginTransmission(0x27);
  error = Wire.endTransmission();
  Serial.begin(9600);
  Serial.print("Error: ");
  Serial.print(error);

  if (error == 0) {
    Serial.println("LCD found");
    lcd.home(); lcd.clear();
    lcd.print("Ready");
    delay(1000);

  } else {
    Serial.println("LCD not found");
  }
  lcd.begin(16, 2); //initialise the lcd
  lcd.setBacklight(200);
  pinMode(3, INPUT);
  pinMode(4, INPUT);
  digitalWrite(3, HIGH); //sensors go low on detect
  digitalWrite(4, HIGH);

} // End of setup

void loop() {
  if (flag == 0); {
    lcd.home(); lcd.clear();
    lcd.print("Ready for train");
    Serial.println("flag=");
    Serial.print(flag);
    delay(1000);
  }
  if (digitalRead(3) == LOW || (digitalRead(4) == LOW)) {
    flag = 1;
    Serial.println("flag=");
    Serial.print(flag);
  }

  if (digitalRead(3) == LOW && (digitalRead(4) == HIGH) && (flag == 1)) {
    direction = 0;
    Serial.println("direction=");
    Serial.print(direction);
  }
  if (digitalRead(3) == HIGH && (digitalRead(4) == LOW) && (flag == 1)) {
    direction = 1;
    Serial.println("direction");
    Serial.print(direction);
  }

  if (flag == 1) {
    lcd.home(); lcd.clear();
    lcd.print("Checking speed");
    start = millis();
    if ((direction) == 0 && digitalRead(4) == LOW) {
      finish = millis();
      flag = 2;
    }
    if ((direction) == 1 && digitalRead(3) == LOW) {
      finish = millis();
      flag = 2;
    }
    delay (1000);
  }
  if (flag == 2) {
    lcd.home(); lcd.clear();
    realdistance = (length * G / 5280); // ft in a mile
    mph = (3600 * realdistance); // now into hours
    timetaken = (finish - start) * 1.000 / 1000.000;
    lcd.setCursor(0, 0);
    lcd.print("Time taken=");
    lcd.print(timetaken);
    delay (1000);
    lcd.setCursor(0, 1);
    lcd.print("Speed MPH=");
    lcd.print(mph);
    delay (1000);
    flag = 0;
  }
}

I bet you already noticed that the first if in loop does not seem to work?

if (flag == 0); {
    lcd.home(); lcd.clear();
    lcd.print("Ready for train");
    Serial.println("flag=");
    Serial.print(flag);
    delay(1000);
  }

Notice the '; behind the condition?

That renders the whole if meaningless as it represents the empty action.

BTW displaying "Ready for train" and going to sleep for a full second is ridiculous.

  pinMode(3, INPUT);
  pinMode(4, INPUT);
  digitalWrite(3, HIGH); //sensors go low on detect
  digitalWrite(4, HIGH);

I would write

  pinMode(3, INPUT_PULLUP);
  pinMode(4, INPUT_PULLUP);

No comment necessary, more obvious, less letters to type.

I have a different lcd library, so you will have to adjust the inititialization (like I had to).

I would solve your problem by using a library for the state change detection, micros for the train timing,
millis for the autoclear of last result after 5 seconds and three states, idle, sensorAFirst and sensorBfirst.

//Bi-direction train speed indicator
//Attempt 1 date 17/06/2018
//S is Speed to be in mph
//T is Time measured in seconds
//L is Distance between sensors in feet
//G is scale of model (ie gauge)
//This attempt is for n gauge hence G = 148

#include<Wire.h>
//#include<LiquidCrystal_PCF8574.h>
#include<LiquidCrystal_I2C.h>
#include <Bounce2.h>  // https://github.com/thomasfredericks/Bounce2

LiquidCrystal_I2C lcd(0x27, 16, 2);

uint32_t senATime;
uint32_t senBTime;
uint32_t lastMeasurement;

const float length = 1; //distance of probes in ft
const float G = 148; //ngauge
const float realdistance = (length * G / 5280); // to miles

enum SState {
  idle,
  senAFirst,
  senBFirst,
} detState = idle;

Bounce senA, senB;

void setup() {
  Wire.begin();
  Wire.beginTransmission(0x27);
  int error = Wire.endTransmission();
  Serial.begin(250000);
  if (error == 0) {
    Serial.println(F("LCD found"));
    lcd.init();
    lcd.setBacklight(200);
    displayResults();
  } else {
    Serial.println(F("LCD not found"));
  }
  senA.attach(3, INPUT_PULLUP);
  senB.attach(4, INPUT_PULLUP);
}

void loop() {
  if (senA.update()) {
    if (senA.fell()) {
      if (detState == idle) {
        senATime = micros();
        oneLineMsg(F("--> expecting B"));
        detState = senAFirst;
      } else if (detState == senBFirst) {
        senATime = micros();
        displayResults();
        detState = idle;
      }
    }
  }
  if (senB.update()) {
    if (senB.fell()) {
      if (detState == idle) {
        senBTime = micros();
        oneLineMsg(F("<-- expecting A"));
        detState = senBFirst;
      } else if (detState == senAFirst) {
        senBTime = micros();
        displayResults();
        detState = idle;
      }
    }
  }
  if (detState == idle && lastMeasurement != 0 && (millis() - lastMeasurement) >= 5000) {
    lastMeasurement = 0;
    displayResults();
  }
}

void displayResults() {
  if (detState == idle) {
    oneLineMsg(F("Ready for train"));
  } else {
    uint32_t timetaken;
    if (detState == senAFirst) {
      timetaken = senBTime - senATime;
    } else if (detState == senBFirst) {
      timetaken = senATime - senBTime;
    }
    float seconds = timetaken / 1000000.0;
    float hours = seconds / 3600;
    float mph = realdistance / hours;
    oneLineMsg(F("Seconds "));
    lcd.print(seconds);
    lcd.setCursor(0, 1);
    lcd.print(F("Speed "));
    lcd.print(mph);
    lcd.print(F(" mph"));
    lastMeasurement = millis();
  }
}

void oneLineMsg(const __FlashStringHelper* msg) {
  lcd.clear();
  lcd.print(msg);
}

Thanks,
I have copied the code and will study it to try and understand what is happening.
Have only been attempting writing for about three weeks so still a lot to learn.

As far as too many ; oops, will change input pins to pullup. Thanks for being willing to help..