From 666eeb5682a83eba903f500dd885685afdd6b63b Mon Sep 17 00:00:00 2001 From: Brett Heath-Wlaz Date: Tue, 8 Oct 2024 08:41:54 -0400 Subject: [PATCH] streamline prediction destination code (#827) --- lib/content/message/predictions.ex | 86 +++++-------- lib/content/message/stopped_train.ex | 33 ++--- lib/content/utilities.ex | 129 +++++++++++-------- lib/signs/utilities/predictions.ex | 40 ++---- test/content/messages/predictions_test.exs | 55 -------- test/content/messages/stopped_train_test.exs | 6 - test/content/utilities_test.exs | 91 ++++++++++--- test/signs/realtime_test.exs | 21 --- 8 files changed, 207 insertions(+), 254 deletions(-) diff --git a/lib/content/message/predictions.ex b/lib/content/message/predictions.ex index a34e1e349..e4f176e22 100644 --- a/lib/content/message/predictions.ex +++ b/lib/content/message/predictions.ex @@ -88,34 +88,23 @@ defmodule Content.Message.Predictions do do: {nil, nil}, else: do_crowding(prediction, sign) - case Content.Utilities.destination_for_prediction( - prediction.route_id, - prediction.direction_id, - prediction.destination_stop_id - ) do - {:ok, destination} -> - %__MODULE__{ - destination: destination, - minutes: minutes, - approximate?: approximate?, - route_id: prediction.route_id, - stop_id: prediction.stop_id, - trip_id: prediction.trip_id, - direction_id: prediction.direction_id, - width: width, - new_cars?: sign.location_engine.for_vehicle(prediction.vehicle_id) |> new_cars?(), - station_code: station_code, - zone: zone, - platform: platform, - certainty: certainty, - crowding_data_confidence: crowding_data_confidence, - crowding_description: crowding_description - } - - {:error, _} -> - Logger.warn("no_destination_for_prediction #{inspect(prediction)}") - nil - end + %__MODULE__{ + destination: Content.Utilities.destination_for_prediction(prediction), + minutes: minutes, + approximate?: approximate?, + route_id: prediction.route_id, + stop_id: prediction.stop_id, + trip_id: prediction.trip_id, + direction_id: prediction.direction_id, + width: width, + new_cars?: sign.location_engine.for_vehicle(prediction.vehicle_id) |> new_cars?(), + station_code: station_code, + zone: zone, + platform: platform, + certainty: certainty, + crowding_data_confidence: crowding_data_confidence, + crowding_description: crowding_description + } end @spec terminal( @@ -137,32 +126,21 @@ defmodule Content.Message.Predictions do x -> compute_minutes(x, prediction.departure_certainty) end - case Content.Utilities.destination_for_prediction( - prediction.route_id, - prediction.direction_id, - prediction.destination_stop_id - ) do - {:ok, destination} -> - %__MODULE__{ - destination: destination, - minutes: minutes, - approximate?: approximate?, - route_id: prediction.route_id, - stop_id: prediction.stop_id, - trip_id: prediction.trip_id, - direction_id: prediction.direction_id, - width: width, - new_cars?: sign.location_engine.for_vehicle(prediction.vehicle_id) |> new_cars?(), - station_code: station_code, - zone: zone, - terminal?: true, - certainty: prediction.departure_certainty - } - - {:error, _} -> - Logger.warn("no_destination_for_prediction #{inspect(prediction)}") - nil - end + %__MODULE__{ + destination: Content.Utilities.destination_for_prediction(prediction), + minutes: minutes, + approximate?: approximate?, + route_id: prediction.route_id, + stop_id: prediction.stop_id, + trip_id: prediction.trip_id, + direction_id: prediction.direction_id, + width: width, + new_cars?: sign.location_engine.for_vehicle(prediction.vehicle_id) |> new_cars?(), + station_code: station_code, + zone: zone, + terminal?: true, + certainty: prediction.departure_certainty + } end defp compute_minutes(sec, certainty) do diff --git a/lib/content/message/stopped_train.ex b/lib/content/message/stopped_train.ex index 88b0e3cf6..05a1ece4c 100644 --- a/lib/content/message/stopped_train.ex +++ b/lib/content/message/stopped_train.ex @@ -27,28 +27,17 @@ defmodule Content.Message.StoppedTrain do @spec from_prediction(Predictions.Prediction.t()) :: t() | nil def from_prediction(%{boarding_status: status} = prediction) when not is_nil(status) do - case Content.Utilities.destination_for_prediction( - prediction.route_id, - prediction.direction_id, - prediction.destination_stop_id - ) do - {:ok, destination} -> - stops_away = parse_stops_away(prediction.boarding_status) - - %__MODULE__{ - destination: destination, - stops_away: stops_away, - certainty: prediction.arrival_certainty || prediction.departure_certainty, - stop_id: prediction.stop_id, - trip_id: prediction.trip_id, - route_id: prediction.route_id, - direction_id: prediction.direction_id - } - - {:error, _} -> - Logger.warn("no_destination_for_prediction #{inspect(prediction)}") - nil - end + stops_away = parse_stops_away(prediction.boarding_status) + + %__MODULE__{ + destination: Content.Utilities.destination_for_prediction(prediction), + stops_away: stops_away, + certainty: prediction.arrival_certainty || prediction.departure_certainty, + stop_id: prediction.stop_id, + trip_id: prediction.trip_id, + route_id: prediction.route_id, + direction_id: prediction.direction_id + } end defp parse_stops_away(str) do diff --git a/lib/content/utilities.ex b/lib/content/utilities.ex index d3bc10eba..2a063b6e9 100644 --- a/lib/content/utilities.ex +++ b/lib/content/utilities.ex @@ -19,22 +19,21 @@ defmodule Content.Utilities do |> Enum.max() end - @spec destination_for_prediction(String.t(), 0 | 1, String.t()) :: - {:ok, PaEss.destination()} | {:error, :not_found} - def destination_for_prediction("Mattapan", 0, _), do: {:ok, :mattapan} - def destination_for_prediction("Mattapan", 1, _), do: {:ok, :ashmont} - def destination_for_prediction("Orange", 0, _), do: {:ok, :forest_hills} - def destination_for_prediction("Orange", 1, _), do: {:ok, :oak_grove} - def destination_for_prediction("Blue", 0, _), do: {:ok, :bowdoin} - def destination_for_prediction("Blue", 1, _), do: {:ok, :wonderland} - def destination_for_prediction("Red", 1, _), do: {:ok, :alewife} - - def destination_for_prediction("Red", 0, last_stop_id) - when last_stop_id in ["70085", "70086", "70087", "70089", "70091", "70093"], - do: {:ok, :ashmont} - - def destination_for_prediction("Red", 0, last_stop_id) - when last_stop_id in [ + @spec destination_for_prediction(Predictions.Prediction.t()) :: PaEss.destination() + def destination_for_prediction(%{route_id: "Mattapan", direction_id: 0}), do: :mattapan + def destination_for_prediction(%{route_id: "Mattapan", direction_id: 1}), do: :ashmont + def destination_for_prediction(%{route_id: "Orange", direction_id: 0}), do: :forest_hills + def destination_for_prediction(%{route_id: "Orange", direction_id: 1}), do: :oak_grove + def destination_for_prediction(%{route_id: "Blue", direction_id: 0}), do: :bowdoin + def destination_for_prediction(%{route_id: "Blue", direction_id: 1}), do: :wonderland + def destination_for_prediction(%{route_id: "Red", direction_id: 1}), do: :alewife + + def destination_for_prediction(%{route_id: "Red", direction_id: 0, destination_stop_id: stop_id}) + when stop_id in ["70085", "70086", "70087", "70089", "70091", "70093"], + do: :ashmont + + def destination_for_prediction(%{route_id: "Red", direction_id: 0, destination_stop_id: stop_id}) + when stop_id in [ "70095", "70096", "70097", @@ -45,43 +44,67 @@ defmodule Content.Utilities do "Braintree-01", "Braintree-02" ], - do: {:ok, :braintree} - - def destination_for_prediction("Red", 0, _), do: {:ok, :southbound} - - def destination_for_prediction(_, 0, "70151"), do: {:ok, :kenmore} - def destination_for_prediction(_, 0, "71151"), do: {:ok, :kenmore} - def destination_for_prediction(_, 0, "70202"), do: {:ok, :government_center} - def destination_for_prediction(_, 0, "70201"), do: {:ok, :government_center} - def destination_for_prediction(_, 0, "70175"), do: {:ok, :reservoir} - def destination_for_prediction(_, 0, "70107"), do: {:ok, :boston_college} - def destination_for_prediction(_, 0, "70237"), do: {:ok, :cleveland_circle} - def destination_for_prediction(_, 0, "70161"), do: {:ok, :riverside} - def destination_for_prediction(_, 0, "70260"), do: {:ok, :heath_street} - - def destination_for_prediction(_, 1, "70205"), do: {:ok, :north_station} - def destination_for_prediction(_, 1, "70511"), do: {:ok, :medford_tufts} - def destination_for_prediction(_, 1, "70503"), do: {:ok, :union_square} - def destination_for_prediction(_, 1, "70501"), do: {:ok, :lechmere} - def destination_for_prediction(_, 1, "70201"), do: {:ok, :government_center} - def destination_for_prediction(_, 1, "70200"), do: {:ok, :park_street} - def destination_for_prediction(_, 1, "71199"), do: {:ok, :park_street} - def destination_for_prediction(_, 1, "70150"), do: {:ok, :kenmore} - def destination_for_prediction(_, 1, "71150"), do: {:ok, :kenmore} - def destination_for_prediction(_, 1, "70174"), do: {:ok, :reservoir} - - def destination_for_prediction(_, _, "Government Center-Brattle"), do: {:ok, :government_center} - - def destination_for_prediction("Green-B", 0, _), do: {:ok, :boston_college} - def destination_for_prediction("Green-C", 0, _), do: {:ok, :cleveland_circle} - def destination_for_prediction("Green-D", 0, _), do: {:ok, :riverside} - def destination_for_prediction("Green-E", 0, _), do: {:ok, :heath_street} - def destination_for_prediction("Green-B", 1, _), do: {:ok, :government_center} - def destination_for_prediction("Green-C", 1, _), do: {:ok, :government_center} - def destination_for_prediction("Green-D", 1, _), do: {:ok, :union_square} - def destination_for_prediction("Green-E", 1, _), do: {:ok, :medford_tufts} - - def destination_for_prediction(_, _, _), do: {:error, :not_found} + do: :braintree + + def destination_for_prediction(%{route_id: "Red", direction_id: 0}), do: :southbound + + def destination_for_prediction(%{direction_id: 0, destination_stop_id: "70151"}), do: :kenmore + def destination_for_prediction(%{direction_id: 0, destination_stop_id: "71151"}), do: :kenmore + + def destination_for_prediction(%{direction_id: 0, destination_stop_id: "70202"}), + do: :government_center + + def destination_for_prediction(%{direction_id: 0, destination_stop_id: "70201"}), + do: :government_center + + def destination_for_prediction(%{direction_id: 0, destination_stop_id: "70175"}), do: :reservoir + + def destination_for_prediction(%{direction_id: 0, destination_stop_id: "70107"}), + do: :boston_college + + def destination_for_prediction(%{direction_id: 0, destination_stop_id: "70237"}), + do: :cleveland_circle + + def destination_for_prediction(%{direction_id: 0, destination_stop_id: "70161"}), do: :riverside + + def destination_for_prediction(%{direction_id: 0, destination_stop_id: "70260"}), + do: :heath_street + + def destination_for_prediction(%{direction_id: 1, destination_stop_id: "70205"}), + do: :north_station + + def destination_for_prediction(%{direction_id: 1, destination_stop_id: "70511"}), + do: :medford_tufts + + def destination_for_prediction(%{direction_id: 1, destination_stop_id: "70503"}), + do: :union_square + + def destination_for_prediction(%{direction_id: 1, destination_stop_id: "70501"}), do: :lechmere + + def destination_for_prediction(%{direction_id: 1, destination_stop_id: "70201"}), + do: :government_center + + def destination_for_prediction(%{direction_id: 1, destination_stop_id: "70200"}), + do: :park_street + + def destination_for_prediction(%{direction_id: 1, destination_stop_id: "71199"}), + do: :park_street + + def destination_for_prediction(%{direction_id: 1, destination_stop_id: "70150"}), do: :kenmore + def destination_for_prediction(%{direction_id: 1, destination_stop_id: "71150"}), do: :kenmore + def destination_for_prediction(%{direction_id: 1, destination_stop_id: "70174"}), do: :reservoir + + def destination_for_prediction(%{destination_stop_id: "Government Center-Brattle"}), + do: :government_center + + def destination_for_prediction(%{route_id: "Green-B", direction_id: 0}), do: :boston_college + def destination_for_prediction(%{route_id: "Green-C", direction_id: 0}), do: :cleveland_circle + def destination_for_prediction(%{route_id: "Green-D", direction_id: 0}), do: :riverside + def destination_for_prediction(%{route_id: "Green-E", direction_id: 0}), do: :heath_street + def destination_for_prediction(%{route_id: "Green-B", direction_id: 1}), do: :government_center + def destination_for_prediction(%{route_id: "Green-C", direction_id: 1}), do: :government_center + def destination_for_prediction(%{route_id: "Green-D", direction_id: 1}), do: :union_square + def destination_for_prediction(%{route_id: "Green-E", direction_id: 1}), do: :medford_tufts @spec stop_track_number(String.t()) :: track_number() | nil def stop_track_number("Alewife-01"), do: 1 diff --git a/lib/signs/utilities/predictions.ex b/lib/signs/utilities/predictions.ex index 2bbfb4dd0..1b880edf6 100644 --- a/lib/signs/utilities/predictions.ex +++ b/lib/signs/utilities/predictions.ex @@ -134,35 +134,19 @@ defmodule Signs.Utilities.Predictions do end) |> Enum.sort_by(fn prediction -> prediction.seconds_until_passthrough end) |> Enum.flat_map(fn prediction -> - route_id = prediction.route_id - - case Content.Utilities.destination_for_prediction( - route_id, - prediction.direction_id, - prediction.destination_stop_id - ) do - {:ok, :southbound} when route_id == "Red" -> - [ - %Content.Audio.Passthrough{ - destination: :ashmont, - trip_id: prediction.trip_id, - route_id: prediction.route_id - } - ] - - {:ok, destination} -> - [ - %Content.Audio.Passthrough{ - destination: destination, - trip_id: prediction.trip_id, - route_id: prediction.route_id - } - ] + destination = + case Content.Utilities.destination_for_prediction(prediction) do + :southbound -> :ashmont + destination -> destination + end - _ -> - Logger.info("no_passthrough_audio_for_prediction prediction=#{inspect(prediction)}") - [] - end + [ + %Content.Audio.Passthrough{ + destination: destination, + trip_id: prediction.trip_id, + route_id: prediction.route_id + } + ] end) |> Enum.take(1) end diff --git a/test/content/messages/predictions_test.exs b/test/content/messages/predictions_test.exs index 8876a707c..55dbad826 100644 --- a/test/content/messages/predictions_test.exs +++ b/test/content/messages/predictions_test.exs @@ -12,24 +12,6 @@ defmodule Content.Message.PredictionsTest do :ok end - test "logs a warning when we cant find a headsign" do - prediction = %Predictions.Prediction{ - seconds_until_arrival: 0, - direction_id: 1, - route_id: "NON-ROUTE", - stopped?: false, - stops_away: 1, - destination_stop_id: "70261" - } - - log = - capture_log([level: :warn], fn -> - assert is_nil(Content.Message.Predictions.non_terminal(prediction, "test", "m", @sign)) - end) - - assert log =~ "no_destination_for_prediction" - end - test "puts ARR on the sign when train is 0 seconds away, but not boarding" do prediction = %Predictions.Prediction{ seconds_until_arrival: 0, @@ -288,43 +270,6 @@ defmodule Content.Message.PredictionsTest do :ok end - test "logs a warning when we cant find a headsign, even if it should be boarding" do - prediction = %Predictions.Prediction{ - seconds_until_departure: 0, - direction_id: 1, - route_id: "NON-ROUTE", - destination_stop_id: "70261", - stopped?: false, - stops_away: 0, - boarding_status: "Boarding" - } - - log = - capture_log([level: :warn], fn -> - assert is_nil(Content.Message.Predictions.terminal(prediction, "test", "m", @sign)) - end) - - assert log =~ "no_destination_for_prediction" - end - - test "logs a warning when we cant find a headsign" do - prediction = %Predictions.Prediction{ - seconds_until_departure: 0, - direction_id: 1, - route_id: "NON-ROUTE", - stopped?: false, - stops_away: 1, - destination_stop_id: "70261" - } - - log = - capture_log([level: :warn], fn -> - assert is_nil(Content.Message.Predictions.terminal(prediction, "test", "m", @sign)) - end) - - assert log =~ "no_destination_for_prediction" - end - test "puts boarding on the sign when train is supposed to be boarding according to rtr" do prediction = %Predictions.Prediction{ seconds_until_departure: 75, diff --git a/test/content/messages/stopped_train_test.exs b/test/content/messages/stopped_train_test.exs index 65fa251e4..81761ab1b 100644 --- a/test/content/messages/stopped_train_test.exs +++ b/test/content/messages/stopped_train_test.exs @@ -52,11 +52,5 @@ defmodule Content.Message.StoppedTrainTest do assert %Content.Message.StoppedTrain{destination: :alewife, stops_away: 10} = Content.Message.StoppedTrain.from_prediction(prediction) end - - test "handles unknown final stop_id" do - prediction = %{@prediction | route_id: "Fake Route", destination_stop_id: "123"} - - assert is_nil(Content.Message.StoppedTrain.from_prediction(prediction)) - end end end diff --git a/test/content/utilities_test.exs b/test/content/utilities_test.exs index 8f0ad5e07..845280eda 100644 --- a/test/content/utilities_test.exs +++ b/test/content/utilities_test.exs @@ -5,35 +5,96 @@ defmodule Content.UtilitiesTest do describe "destination_for_prediction/3" do test "handles child stops properly" do - assert destination_for_prediction("Red", 1, "Alewife-01") == {:ok, :alewife} - assert destination_for_prediction("Red", 1, "Alewife-02") == {:ok, :alewife} + assert destination_for_prediction(%{ + route_id: "Red", + direction_id: 1, + destination_stop_id: "Alewife-01" + }) == :alewife - assert destination_for_prediction("Red", 0, "Braintree-01") == {:ok, :braintree} - assert destination_for_prediction("Red", 0, "Braintree-02") == {:ok, :braintree} + assert destination_for_prediction(%{ + route_id: "Red", + direction_id: 1, + destination_stop_id: "Alewife-02" + }) == :alewife - assert destination_for_prediction("Orange", 0, "Forest Hills-01") == {:ok, :forest_hills} - assert destination_for_prediction("Orange", 0, "Forest Hills-02") == {:ok, :forest_hills} + assert destination_for_prediction(%{ + route_id: "Red", + direction_id: 0, + destination_stop_id: "Braintree-01" + }) == :braintree - assert destination_for_prediction("Orange", 1, "Oak Grove-01") == {:ok, :oak_grove} - assert destination_for_prediction("Orange", 1, "Oak Grove-02") == {:ok, :oak_grove} + assert destination_for_prediction(%{ + route_id: "Red", + direction_id: 0, + destination_stop_id: "Braintree-02" + }) == :braintree - assert destination_for_prediction("Green-D", 0, "Government Center-Brattle") == - {:ok, :government_center} + assert destination_for_prediction(%{ + route_id: "Orange", + direction_id: 0, + destination_stop_id: "Forest Hills-01" + }) == :forest_hills - assert destination_for_prediction("Green-E", 1, "71199") == {:ok, :park_street} + assert destination_for_prediction(%{ + route_id: "Orange", + direction_id: 0, + destination_stop_id: "Forest Hills-02" + }) == :forest_hills + + assert destination_for_prediction(%{ + route_id: "Orange", + direction_id: 1, + destination_stop_id: "Oak Grove-01" + }) == :oak_grove + + assert destination_for_prediction(%{ + route_id: "Orange", + direction_id: 1, + destination_stop_id: "Oak Grove-02" + }) == :oak_grove + + assert destination_for_prediction(%{ + route_id: "Green-D", + direction_id: 0, + destination_stop_id: "Government Center-Brattle" + }) == + :government_center + + assert destination_for_prediction(%{ + route_id: "Green-E", + direction_id: 1, + destination_stop_id: "71199" + }) == :park_street end test "Southbound headsign on Red Line trunk" do - assert destination_for_prediction("Red", 0, "70063") == {:ok, :southbound} + assert destination_for_prediction(%{ + route_id: "Red", + direction_id: 0, + destination_stop_id: "70063" + }) == :southbound end test "Regular headsigns for regular Red Line trips to Ashmont / Braintree" do - assert destination_for_prediction("Red", 0, "70093") == {:ok, :ashmont} - assert destination_for_prediction("Red", 0, "70105") == {:ok, :braintree} + assert destination_for_prediction(%{ + route_id: "Red", + direction_id: 0, + destination_stop_id: "70093" + }) == :ashmont + + assert destination_for_prediction(%{ + route_id: "Red", + direction_id: 0, + destination_stop_id: "70105" + }) == :braintree end test "Dont show kenmore on signs when the destination is blandford st" do - assert destination_for_prediction("Green-B", 0, "70149") == {:ok, :boston_college} + assert destination_for_prediction(%{ + route_id: "Green-B", + direction_id: 0, + destination_stop_id: "70149" + }) == :boston_college end end diff --git a/test/signs/realtime_test.exs b/test/signs/realtime_test.exs index ebfc09e39..4fc5840ba 100644 --- a/test/signs/realtime_test.exs +++ b/test/signs/realtime_test.exs @@ -595,14 +595,6 @@ defmodule Signs.RealtimeTest do Signs.Realtime.handle_info(:run_loop, @terminal_sign) end - test "properly handles case where destination can't be determined" do - expect(Engine.Predictions.Mock, :for_stop, fn _, _ -> - [prediction(route_id: "invalid", destination_stop_id: "invalid")] - end) - - Signs.Realtime.handle_info(:run_loop, @sign) - end - test "Correctly orders BRD predictions between trains mid-trip and those starting their trip" do expect(Engine.Predictions.Mock, :for_stop, fn _, _ -> [ @@ -663,19 +655,6 @@ defmodule Signs.RealtimeTest do Signs.Realtime.handle_info(:run_loop, @sign) end - test "handles passthrough audio where headsign can't be determined" do - expect(Engine.Predictions.Mock, :for_stop, fn _, _ -> - [prediction(seconds_until_passthrough: 30, route_id: "Foo", destination_stop_id: "Bar")] - end) - - log = - capture_log([level: :info], fn -> - Signs.Realtime.handle_info(:run_loop, @sign) - end) - - assert log =~ "no_passthrough_audio_for_prediction" - end - test "reads special boarding button announcement at Bowdoin" do expect(Engine.Predictions.Mock, :for_stop, fn _, _ -> [prediction(arrival: 0, destination: :wonderland, stops_away: 0)]