Using External interrupts correctly

Hi,
I am trying to write a sketch that measures the speed of a metal ball rolling down a track. I have an led and photo-transistor which is placed across the track. When the light is not blocked, the input from the photo-transistor(pin 19) is low, when light is blocked it is high. First I need to measure the time the led is blocking the photo-transistor. I am trying to use an Interrupt service routine to do this.
I am trying to interrupt when the led pin is going from low to high, take a time measurement, interrupt again when going from high to low, take a time measurement and determine delta time. Here is my code:

//Pin assignments
#define led_bottom 19  //led pin
int long unsigned time_1; //initial time
int long unsigned time_2; //final time

void setup () {
  Serial.begin(9600); 
  pinMode(led_bottom, INPUT);
  digitalWrite(led_bottom, HIGH);
  //interrupts
  attachInterrupt(4, bottom_interrupt, RISING);  //ISR 4 =pin19
  attachInterrupt(4, bottom_interrupt1, FALLING);
}
void loop(){}

void bottom_interrupt () {       //led pin rising
time_1=micros();
 Serial.print("T1= ");
Serial.println(time_1);
}

void bottom_interrupt1 () {      //led pin falling
    time_2=micros();
    Serial.print("T2= ");
    Serial.println(time_2);
    Serial.print("T=  ");
    Serial.println(time_2-time_1);
  }

I am very new to Arduino and programming in general. I would really appreciate it if someone could have a look and tell me if I am making some major mistakes, Thanks.

You can't attach two interrupt service routines to one interrupt.

AWOL:
You can't attach two interrupt service routines to one interrupt.

Thanks, that was something I was wondering about, I have deleted and I'm trying to think of another approach.

Also serial communications does not function inside an interrupt function as all interrupts are disabled at that time.
You should most likely only require one interrupt, the finished led/sensor. You can just loop in the main loop function testing the start led/sensor and when tripped capture the starting time and then wait for the finished interrupt sensor to set a flag variable the it has triggered and then again get the current time and subtract the difference to get the delta time between those two sensors and send the serial data out.

retrolefty:
You can just loop in the main loop function testing the start led/sensor and when tripped capture the starting time and then wait for the finished interrupt sensor to set a flag variable the it has triggered and then again get the current time and subtract the difference to get the delta time between those two sensors and send the serial data out.

Thanks for you help,
I have tried to follow what you told me, just wondering am I going the right way?

//Pin assignments
#define led_bottom 19  //led pin
int long unsigned time_1; //initial time
int long unsigned time_2; //final time
int counter =0;
void setup () {
  Serial.begin(9600);
  pinMode(led_bottom, INPUT);
  digitalWrite(led_bottom, HIGH);

  attachInterrupt(4, bottom_interrupt, FALLING);
}
void loop(){
  if (digitalRead(led_bottom)>0.2) {
    time_1=micros();
    Serial.print("T1= ");
    Serial.println(time_1);
  }
  if (counter >0) {
    time_2=micros();
    Serial.print("T2= ");
    Serial.println(time_2);
    Serial.print("T=  ");
    Serial.println(time_2-time_1);
    counter=0;
  }
}

void bottom_interrupt () {       //led rising
  counter++;
}

You should really qualify “counter” as “volatile”

How long will it take for the ball to pass through the beam?

Have you considered having two beams so the elapsed time is longer? Also it would allow two interrupt pins to be used.

If the time is long enough maybe you don’t need interrupts at all.

If one interrupt is set to trigger on change, presumably it will trigger both when it goes Low-High and subsequently High_Low.

digitalRead() only returns 1 or 0.

…R

Getting there.

This makes no sense: digitalRead(led_bottom)>0.2)
A digitalRead() returns a high or low not a float value.

Do all your printing at the end after the finish, just save the start time in a variable to print along with the finish time and duration time calculation after the finish event.

There should probably be some timeout function in case the finish never happens.

retrolefty:
Getting there.

This makes no sense: digitalRead(led_bottom)>0.2)
A digitalRead() returns a high or low not a float value.

I have changed this to digitalRead(led_bottom)==1

retrolefty:
Do all your printing at the end after the finish, just save the start time in a variable to print along with the finish time and duration time calculation after the finish event.

Yes I will do this, where exactly is the best place to print?

//Pin assignments
#define led_bottom 19  //led pin
int long unsigned time_1; //initial time
int long unsigned time_2; //final time
volatile int counter =0;
void setup () {
  Serial.begin(9600); 
  pinMode(led_bottom, INPUT);
  digitalWrite(led_bottom, HIGH);
  
  attachInterrupt(4, bottom_interrupt, FALLING); 
}
void loop(){
  if (digitalRead(led_bottom)==1) {
    time_1=millis();
    Serial.print("T1= ");
    Serial.println(time_1);
    //delay(10);
  }
 if (counter >0) { 
    time_2=millis();
    Serial.print("T2= ");
    Serial.println(time_2);
    Serial.print("T=  ");
    Serial.println(time_2-time_1);
    counter=0;
  }
}

void bottom_interrupt () {       //led rising
counter++;
}

In the first if loop, time_1 is being read 1-3 times and displayed on the serial monitor. The delta time calculation is carried out with the last time_1 value and not the first, how can I have this if loop executed only once?

retrolefty:
There should probably be some timeout function in case the finish never happens.
Where can I find a timeout function?

Thanks so much for you help, really appreciate it.