From 3690065b5d9769f7b72659af937b9bf4225da476 Mon Sep 17 00:00:00 2001 From: Brett Heath-Wlaz Date: Thu, 14 Sep 2023 11:35:21 -0400 Subject: [PATCH] replace 20+ minutes with actual predictions (#690) --- lib/content/audio/predictions.ex | 14 ------ .../message/platform_prediction_bottom.ex | 4 +- lib/content/message/predictions.ex | 45 ++++++++++++------- lib/content/utilities.ex | 4 -- lib/pa_ess/utilities.ex | 6 +-- lib/signs/utilities/predictions.ex | 35 ++++++++++----- .../audio/next_train_countdown_test.exs | 4 +- test/content/audio/predictions_test.exs | 5 ++- test/content/messages/predictions_test.exs | 5 ++- test/signs/realtime_test.exs | 19 +++++--- 10 files changed, 75 insertions(+), 66 deletions(-) diff --git a/lib/content/audio/predictions.ex b/lib/content/audio/predictions.ex index 4d6865ff7..345466e9a 100644 --- a/lib/content/audio/predictions.ex +++ b/lib/content/audio/predictions.ex @@ -89,20 +89,6 @@ defmodule Content.Audio.Predictions do } ] - predictions.minutes == :max_time -> - [ - %NextTrainCountdown{ - destination: predictions.destination, - route_id: predictions.route_id, - minutes: div(Content.Utilities.max_time_seconds(), 60), - verb: if(predictions.terminal?, do: :departs, else: :arrives), - track_number: Content.Utilities.stop_track_number(predictions.stop_id), - platform: predictions.platform, - station_code: predictions.station_code, - zone: predictions.zone - } - ] - is_integer(predictions.minutes) -> [ %NextTrainCountdown{ diff --git a/lib/content/message/platform_prediction_bottom.ex b/lib/content/message/platform_prediction_bottom.ex index a593f5dd6..9ce358d69 100644 --- a/lib/content/message/platform_prediction_bottom.ex +++ b/lib/content/message/platform_prediction_bottom.ex @@ -3,13 +3,13 @@ defmodule Content.Message.PlatformPredictionBottom do @type t :: %__MODULE__{ stop_id: String.t(), - minutes: integer() | :boarding | :arriving | :approaching | :max_time, + minutes: integer() | :boarding | :arriving | :approaching, destination: PaEss.destination() } defimpl Content.Message do def to_string(%Content.Message.PlatformPredictionBottom{stop_id: stop_id, minutes: minutes}) do - if minutes == :max_time or (is_integer(minutes) and minutes > 5), + if is_integer(minutes) and minutes > 5, do: "platform TBD", else: "on #{Content.Utilities.stop_platform_name(stop_id)} platform" end diff --git a/lib/content/message/predictions.ex b/lib/content/message/predictions.ex index a142b1ae6..c806a6015 100644 --- a/lib/content/message/predictions.ex +++ b/lib/content/message/predictions.ex @@ -13,14 +13,15 @@ defmodule Content.Message.Predictions do require Logger require Content.Utilities - @max_time Content.Utilities.max_time_seconds() @terminal_brd_seconds 30 @terminal_prediction_offset_seconds -60 + @reverse_prediction_certainty 360 @enforce_keys [:destination, :minutes] defstruct [ :destination, :minutes, + :approximate?, :route_id, :station_code, :stop_id, @@ -38,7 +39,8 @@ defmodule Content.Message.Predictions do @type t :: %__MODULE__{ destination: PaEss.destination(), - minutes: integer() | :boarding | :arriving | :approaching | :max_time, + minutes: integer() | :boarding | :arriving | :approaching, + approximate?: boolean(), route_id: String.t(), stop_id: String.t(), trip_id: Predictions.Prediction.trip_id() | nil, @@ -72,13 +74,12 @@ defmodule Content.Message.Predictions do do: prediction.arrival_certainty, else: prediction.departure_certainty - minutes = + {minutes, approximate?} = cond do - prediction.stops_away == 0 -> :boarding - predicted_time <= 30 -> :arriving - predicted_time <= 60 -> :approaching - predicted_time >= @max_time -> :max_time - true -> predicted_time |> Kernel./(60) |> round() + prediction.stops_away == 0 -> {:boarding, false} + predicted_time <= 30 -> {:arriving, false} + predicted_time <= 60 -> {:approaching, false} + true -> compute_minutes(predicted_time, certainty) end {crowding_data_confidence, crowding_description} = do_crowding(prediction) @@ -92,6 +93,7 @@ defmodule Content.Message.Predictions do %__MODULE__{ destination: destination, minutes: minutes, + approximate?: approximate?, route_id: prediction.route_id, stop_id: prediction.stop_id, trip_id: prediction.trip_id, @@ -118,12 +120,11 @@ defmodule Content.Message.Predictions do def terminal(prediction, width) do stopped_at? = prediction.stops_away == 0 - minutes = + {minutes, approximate?} = case prediction.seconds_until_departure + @terminal_prediction_offset_seconds do - x when x <= @terminal_brd_seconds and stopped_at? -> :boarding - x when x <= @terminal_brd_seconds -> 1 - x when x >= @max_time -> :max_time - x -> x |> Kernel./(60) |> round() + x when x <= @terminal_brd_seconds and stopped_at? -> {:boarding, false} + x when x <= @terminal_brd_seconds -> {1, false} + x -> compute_minutes(x, prediction.departure_certainty) end case Content.Utilities.destination_for_prediction( @@ -135,6 +136,7 @@ defmodule Content.Message.Predictions do %__MODULE__{ destination: destination, minutes: minutes, + approximate?: approximate?, route_id: prediction.route_id, stop_id: prediction.stop_id, trip_id: prediction.trip_id, @@ -151,6 +153,16 @@ defmodule Content.Message.Predictions do end end + defp compute_minutes(sec, certainty) do + min = round(sec / 60) + + cond do + min > 60 -> {60, true} + certainty == @reverse_prediction_certainty && min > 20 -> {div(min, 10) * 10, true} + true -> {min, false} + end + end + defp do_crowding(prediction) when prediction.route_id in ["Orange"] do case Engine.Locations.for_vehicle(prediction.vehicle_id) do %Locations.Location{} = location -> @@ -249,11 +261,11 @@ defmodule Content.Message.Predictions do @boarding "BRD" @arriving "ARR" - @max_time "20+ min" def to_string(%{ destination: destination, minutes: minutes, + approximate?: approximate?, width: width, stop_id: stop_id, station_code: station_code, @@ -266,8 +278,7 @@ defmodule Content.Message.Predictions do :boarding -> @boarding :arriving -> @arriving :approaching -> "1 min" - :max_time -> @max_time - n -> "#{n} min" + n -> "#{n}#{if approximate?, do: "+", else: ""} min" end track_number = Content.Utilities.stop_track_number(stop_id) @@ -277,7 +288,7 @@ defmodule Content.Message.Predictions do platform_name = Content.Utilities.stop_platform_name(stop_id) {headsign_message, platform_message} = - if minutes == :max_time or (is_integer(minutes) and minutes > 5) do + if is_integer(minutes) and minutes > 5 do {headsign, " (Platform TBD)"} else {headsign <> " (#{String.slice(platform_name, 0..0)})", " (#{platform_name} plat)"} diff --git a/lib/content/utilities.ex b/lib/content/utilities.ex index 8616399a5..1fd81eeea 100644 --- a/lib/content/utilities.ex +++ b/lib/content/utilities.ex @@ -2,10 +2,6 @@ defmodule Content.Utilities do @type track_number :: non_neg_integer() @type green_line_branch :: :b | :c | :d | :e - defmacro max_time_seconds do - quote do: 20 * 60 - end - def width_padded_string(left, right, width) do max_left_length = width - (String.length(right) + 1) new_left = left |> String.slice(0, max_left_length) |> String.pad_trailing(max_left_length) diff --git a/lib/pa_ess/utilities.ex b/lib/pa_ess/utilities.ex index 9d7ab4ee6..a72f687ff 100644 --- a/lib/pa_ess/utilities.ex +++ b/lib/pa_ess/utilities.ex @@ -158,14 +158,10 @@ defmodule PaEss.Utilities do Integer.to_string(9100 + n) end - def countdown_minutes_var(n) when n >= 0 and n < 30 do + def countdown_minutes_var(n) when n >= 0 and n <= 100 do Integer.to_string(5000 + n) end - def countdown_minutes_var(n) when n >= 30 do - Integer.to_string(5030) - end - @doc "Constructs message from TAKE variables" @spec take_message([String.t()], Content.Audio.av_type()) :: Content.Audio.canned_message() def take_message(vars, av_type) do diff --git a/lib/signs/utilities/predictions.ex b/lib/signs/utilities/predictions.ex index d88675947..e6f041baf 100644 --- a/lib/signs/utilities/predictions.ex +++ b/lib/signs/utilities/predictions.ex @@ -9,6 +9,10 @@ defmodule Signs.Utilities.Predictions do require Content.Utilities alias Signs.Utilities.SourceConfig + @reverse_prediction_certainty 360 + @max_prediction_sec 60 * 60 + @reverse_prediction_cutoff_sec 20 * 60 + @spec get_messages(Signs.Realtime.predictions(), Signs.Realtime.t()) :: Signs.Realtime.sign_messages() def get_messages( @@ -150,19 +154,28 @@ defmodule Signs.Utilities.Predictions do |> Enum.take(1) end - @spec stopped_train?(Predictions.Prediction.t()) :: boolean() - defp stopped_train?(%{ - seconds_until_arrival: arrival_seconds, - seconds_until_departure: departure_seconds - }) - when arrival_seconds >= Content.Utilities.max_time_seconds() or - departure_seconds >= Content.Utilities.max_time_seconds() do - false + defp approximate_time?(sec, certainty) do + sec && + (sec > @max_prediction_sec || + (sec > @reverse_prediction_cutoff_sec && certainty == @reverse_prediction_certainty)) end - defp stopped_train?(prediction) do - status = prediction.boarding_status - status && String.starts_with?(status, "Stopped") && status != "Stopped at station" + @spec stopped_train?(Predictions.Prediction.t()) :: boolean() + defp stopped_train?(%{ + boarding_status: boarding_status, + seconds_until_arrival: seconds_until_arrival, + seconds_until_departure: seconds_until_departure, + arrival_certainty: arrival_certainty, + departure_certainty: departure_certainty + }) do + # Note: This performs a similar (but not identical) calculation to the one in the Message + # code for determining whether a prediction will show an approximate time. Ideally they + # should both call the same logic. + approximate_arrival? = approximate_time?(seconds_until_arrival, arrival_certainty) + approximate_departure? = approximate_time?(seconds_until_departure, departure_certainty) + + boarding_status && String.starts_with?(boarding_status, "Stopped") && + boarding_status != "Stopped at station" && !approximate_arrival? && !approximate_departure? end defp allowed_multi_berth_platform?(source_list, p1, p2) do diff --git a/test/content/audio/next_train_countdown_test.exs b/test/content/audio/next_train_countdown_test.exs index a36c373b1..a152a25da 100644 --- a/test/content/audio/next_train_countdown_test.exs +++ b/test/content/audio/next_train_countdown_test.exs @@ -229,7 +229,7 @@ defmodule Content.Audio.NextTrainCountdownTest do assert Content.Audio.to_params(audio) == {:canned, {"90", ["4021", "503", "5005"], :audio}} end - test "Uses audio for 30 minutes when train is more than 30 minutes away" do + test "can read larger numbers" do audio = %Content.Audio.NextTrainCountdown{ destination: :wonderland, route_id: "Blue", @@ -239,7 +239,7 @@ defmodule Content.Audio.NextTrainCountdownTest do platform: nil } - assert Content.Audio.to_params(audio) == {:canned, {"90", ["4044", "503", "5030"], :audio}} + assert Content.Audio.to_params(audio) == {:canned, {"90", ["4044", "503", "5050"], :audio}} end test "Next D train in 5 minutes" do diff --git a/test/content/audio/predictions_test.exs b/test/content/audio/predictions_test.exs index 6b4d77509..1ed85566d 100644 --- a/test/content/audio/predictions_test.exs +++ b/test/content/audio/predictions_test.exs @@ -216,10 +216,11 @@ defmodule Content.Audio.PredictionsTest do ] = from_sign_content(predictions, :top, false) end - test "returns a NextTrainCountdown with 30 mins if predictions is :max_time" do + test "returns a NextTrainCountdown with approximate minutes" do predictions = %Message.Predictions{ destination: :ashmont, - minutes: :max_time, + minutes: 20, + approximate?: true, route_id: "Red", stop_id: "70065" } diff --git a/test/content/messages/predictions_test.exs b/test/content/messages/predictions_test.exs index 0d23e6888..c0745a810 100644 --- a/test/content/messages/predictions_test.exs +++ b/test/content/messages/predictions_test.exs @@ -96,9 +96,10 @@ defmodule Content.Message.PredictionsTest do assert Content.Message.to_string(msg) == "Mattapan 1 min" end - test "Says 20+ min when train is 20 or more minutes away" do + test "shows approximate minutes for longer turnaround predictions" do prediction = %Predictions.Prediction{ - seconds_until_arrival: 45 * 60, + seconds_until_arrival: 25 * 60, + arrival_certainty: 360, direction_id: 0, route_id: "Mattapan", stopped?: false, diff --git a/test/signs/realtime_test.exs b/test/signs/realtime_test.exs index 8eee82b0d..431997e40 100644 --- a/test/signs/realtime_test.exs +++ b/test/signs/realtime_test.exs @@ -334,11 +334,17 @@ defmodule Signs.RealtimeTest do test "When the train is stopped a long time away from a terminal, shows max time instead of stopped" do expect(Engine.Predictions.Mock, :for_stop, fn _, _ -> - [prediction(destination: :mattapan, seconds_until_departure: 2020, stopped: 8)] + [ + prediction( + destination: :mattapan, + seconds_until_departure: 2020, + stopped: 8, + departure_certainty: 360 + ) + ] end) - expect_messages({"Mattapan 20+ min", ""}) - expect_audios([{:canned, {"90", ["4100", "502", "5020"], :audio}}]) + expect_messages({"Mattapan 30+ min", ""}) Signs.Realtime.handle_info(:run_loop, %{ @sign @@ -348,11 +354,10 @@ defmodule Signs.RealtimeTest do test "When the train is stopped a long time away, shows max time instead of stopped" do expect(Engine.Predictions.Mock, :for_stop, fn _, _ -> - [prediction(destination: :mattapan, arrival: 1200, stopped: 8)] + [prediction(destination: :mattapan, arrival: 3700, stopped: 8)] end) - expect_messages({"Mattapan 20+ min", ""}) - expect_audios([{:canned, {"90", ["4100", "503", "5020"], :audio}}]) + expect_messages({"Mattapan 60+ min", ""}) Signs.Realtime.handle_info(:run_loop, @sign) end @@ -594,7 +599,7 @@ defmodule Signs.RealtimeTest do seconds_until_arrival: Keyword.get(opts, :seconds_until_arrival), arrival_certainty: nil, seconds_until_departure: Keyword.get(opts, :seconds_until_departure), - departure_certainty: nil, + departure_certainty: Keyword.get(opts, :departure_certainty), seconds_until_passthrough: Keyword.get(opts, :seconds_until_passthrough), direction_id: Keyword.get(opts, :direction_id, 0), schedule_relationship: nil,