Skip to content

Commit

Permalink
fix: handle unscheduled commuter rail departures (#2168)
Browse files Browse the repository at this point in the history
The commuter rail departures widget made the assumption that there would
never be a departure that had a prediction but no matching schedule. We
have observed that this isn't always the case.

Adds handling of a `nil` schedule to the commuter rail departures
widget. The only thing relying on a present schedule in
`serialize_departure/5` was calculating if a departure was delayed. If
there is no schedule we do not flag the departure as delayed; otherwise,
the delayed calculation remains unchanged.
  • Loading branch information
sloanelybutsurely authored Aug 28, 2024
1 parent 0180877 commit 18d5874
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 14 deletions.
11 changes: 8 additions & 3 deletions lib/screens/v2/departure.ex
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,14 @@ defmodule Screens.V2.Departure do
end
end

def fetch_predictions_and_schedules(params, now) do
with {:ok, predictions} <- Prediction.fetch(params),
{:ok, schedules} <- Schedule.fetch(params) do
def fetch_predictions_and_schedules(
params,
now,
fetch_predictions_fn \\ &Prediction.fetch/1,
fetch_schedules_fn \\ &Schedule.fetch/1
) do
with {:ok, predictions} <- fetch_predictions_fn.(params),
{:ok, schedules} <- fetch_schedules_fn.(params) do
relevant_predictions = Builder.get_relevant_departures(predictions, now)
relevant_schedules = Builder.get_relevant_departures(schedules, now)

Expand Down
25 changes: 15 additions & 10 deletions lib/screens/v2/widget_instance/cr_departures.ex
Original file line number Diff line number Diff line change
Expand Up @@ -134,17 +134,16 @@ defmodule Screens.V2.WidgetInstance.CRDepartures do
now
) do
{:ok, scheduled_departure_time} =
try do
if is_nil(schedule) do
Logger.error(
"[cr_departures serialize_time] Could not get schedule time: #{inspect(departure)}"
)

{:ok, nil}
else
%Departure{schedule: schedule}
|> Departure.time()
|> DateTime.shift_zone("America/New_York")
rescue
ex ->
Logger.error(
"[cr_departures serialize_time] Could not get schedule time: #{inspect(departure)}"
)

reraise ex, __STACKTRACE__
end

cond do
Expand Down Expand Up @@ -175,7 +174,7 @@ defmodule Screens.V2.WidgetInstance.CRDepartures do
|> Departure.time()
|> DateTime.shift_zone("America/New_York")

is_delayed = DateTime.compare(scheduled_departure_time, predicted_departure_time) == :lt
is_delayed = delayed?(scheduled_departure_time, predicted_departure_time)

%{departure_time: scheduled_departure_time, departure_type: :schedule, is_delayed: is_delayed}
end
Expand All @@ -196,7 +195,7 @@ defmodule Screens.V2.WidgetInstance.CRDepartures do
stop_type = Departure.stop_type(departure)
second_diff = DateTime.diff(predicted_departure_time, now)
minute_diff = round(second_diff / 60)
is_delayed = DateTime.compare(scheduled_departure_time, predicted_departure_time) == :lt
is_delayed = delayed?(scheduled_departure_time, predicted_departure_time)

departure_time =
cond do
Expand Down Expand Up @@ -256,4 +255,10 @@ defmodule Screens.V2.WidgetInstance.CRDepartures do
# Forrest Hills should not show a track number, only wayfinding arrow.
defp get_track_number(_, "place-forhl"), do: nil
defp get_track_number(departure, _station), do: Departure.track_number(departure)

defp delayed?(%DateTime{} = scheduled_departure_time, %DateTime{} = predicted_departure_time) do
DateTime.compare(scheduled_departure_time, predicted_departure_time) == :lt
end

defp delayed?(_, _), do: false
end
32 changes: 32 additions & 0 deletions test/screens/v2/departure_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -450,4 +450,36 @@ defmodule Screens.V2.DepartureTest do
assert nil == Departure.vehicle_status(departure)
end
end

describe "fetch_predictions_and_schedules/2" do
test "maintains schedules even if they are in the past" do
now = ~U[2024-08-28 17:13:14.116713Z]
# The train is _very_ late!!
schedule =
%Schedule{
trip: %Trip{id: "trip-1"},
arrival_time: DateTime.add(now, -10, :minute),
departure_time: DateTime.add(now, -8, :minute)
}

# The train is almost here!
prediction =
%Prediction{
trip: %Trip{id: "trip-1"},
arrival_time: DateTime.add(now, 2, :minute),
departure_time: DateTime.add(now, 5, :minute)
}

fetch_predictions_fn = fn _ -> {:ok, [prediction]} end
fetch_schedules_fn = fn _ -> {:ok, [schedule]} end

assert {:ok, [%Departure{schedule: schedule, prediction: prediction}]} ==
Departure.fetch_predictions_and_schedules(
[],
now,
fetch_predictions_fn,
fetch_schedules_fn
)
end
end
end
82 changes: 81 additions & 1 deletion test/screens/v2/widget_instance/cr_departures_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ defmodule Screens.V2.WidgetInstance.CRDeparturesTest do
# alias Screens.Alerts.Alert
# alias Screens.Config.Dup.Override.FreeTextLine
# alias Screens.Config.Screen
alias Screens.Schedules.Schedule
alias Screens.Predictions.Prediction
# alias Screens.Routes.Route
# alias Screens.Schedules.Schedule
alias Screens.Trips.Trip
# alias Screens.Vehicles.Vehicle
alias Screens.V2.{Departure, WidgetInstance}
Expand Down Expand Up @@ -53,6 +53,86 @@ defmodule Screens.V2.WidgetInstance.CRDeparturesTest do
end
end

describe "serialize_departure/5" do
test "serializes a departure with a schedule and a prediction" do
now = ~U[2024-08-28 18:08:30.883227Z]

departure = %Departure{
schedule: %Schedule{
id: "schedule-1",
trip: %Trip{id: "trip-1", headsign: "Somewhere"},
arrival_time: DateTime.add(now, 5, :minute),
departure_time: DateTime.add(now, 7, :minute)
},
prediction: %Prediction{
id: "prediction-1",
trip: %Trip{id: "trip-1", headsign: "Somewhere"},
arrival_time: DateTime.add(now, 5, :minute),
departure_time: DateTime.add(now, 7, :minute)
}
}

assert %{
prediction_or_schedule_id: "prediction-1"
} =
CRDeparturesWidget.serialize_departure(
departure,
"Somewhere",
%{},
"place-smwhr",
now
)
end

test "serializes a departure with only a schedule" do
now = ~U[2024-08-28 18:08:30.883227Z]

departure = %Departure{
schedule: %Schedule{
id: "schedule-1",
trip: %Trip{id: "trip-1", headsign: "Somewhere"},
arrival_time: DateTime.add(now, 5, :minute),
departure_time: DateTime.add(now, 7, :minute)
}
}

assert %{
prediction_or_schedule_id: "schedule-1"
} =
CRDeparturesWidget.serialize_departure(
departure,
"Somewhere",
%{},
"place-smwhr",
now
)
end

test "serializes a departure with only a prediction" do
now = ~U[2024-08-28 18:08:30.883227Z]

departure = %Departure{
prediction: %Prediction{
id: "prediction-1",
trip: %Trip{id: "trip-1", headsign: "Somewhere"},
arrival_time: DateTime.add(now, 5, :minute),
departure_time: DateTime.add(now, 7, :minute)
}
}

assert %{
prediction_or_schedule_id: "prediction-1"
} =
CRDeparturesWidget.serialize_departure(
departure,
"Somewhere",
%{},
"place-smwhr",
now
)
end
end

# describe "serialize_times_with_crowding/2" do
# setup do
# bus_eink_screen = %Screen{
Expand Down

0 comments on commit 18d5874

Please sign in to comment.