-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Crowding data confidence determination #673
Changes from all commits
373e511
e075fba
67d4d85
abf5778
7b85ec7
8e7d492
7e98cfd
f6ce48b
db7d146
165b961
1dd530d
a5f1f17
edd033c
fe97d8d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,8 @@ defmodule Content.Message.Predictions do | |
platform: nil, | ||
new_cars?: false, | ||
terminal?: false, | ||
certainty: nil | ||
certainty: nil, | ||
crowding_data_confidence: nil | ||
] | ||
|
||
@type t :: %__MODULE__{ | ||
|
@@ -47,7 +48,8 @@ defmodule Content.Message.Predictions do | |
zone: String.t() | nil, | ||
platform: Content.platform() | nil, | ||
terminal?: boolean(), | ||
certainty: non_neg_integer() | nil | ||
certainty: non_neg_integer() | nil, | ||
crowding_data_confidence: :high | :low | nil | ||
} | ||
|
||
@spec non_terminal( | ||
|
@@ -77,6 +79,14 @@ defmodule Content.Message.Predictions do | |
true -> predicted_time |> Kernel./(60) |> round() | ||
end | ||
|
||
crowding_data_confidence = | ||
calculate_crowding_data_confidence( | ||
prediction, | ||
Engine.Locations.for_vehicle(prediction.vehicle_id) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doing this lookup here means we'll end up grabbing location info for every prediction message, every tick, even ones far away or on other lines. Granted, it's an indexed operation, so it may be fast enough, but it's not needed the vast majority of the time. What about storing the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah that's a good point. I did end up thinking about that later down the line in a later PR and changed the logic so that we only fetch location if it's an Orange Line prediction. Take a look at this logic for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That does improve things. And the lookup should be fast enough, so it's probably not worth worrying about. |
||
) | ||
|
||
# TODO: Calculate crowding data classification and pass that along as well | ||
|
||
case Content.Utilities.destination_for_prediction( | ||
prediction.route_id, | ||
prediction.direction_id, | ||
|
@@ -95,7 +105,8 @@ defmodule Content.Message.Predictions do | |
station_code: station_code, | ||
zone: zone, | ||
platform: platform, | ||
certainty: certainty | ||
certainty: certainty, | ||
crowding_data_confidence: crowding_data_confidence | ||
} | ||
|
||
{:error, _} -> | ||
|
@@ -143,6 +154,17 @@ defmodule Content.Message.Predictions do | |
end | ||
end | ||
|
||
defp calculate_crowding_data_confidence(_prediction, nil), do: nil | ||
|
||
defp calculate_crowding_data_confidence(prediction, location) | ||
when prediction.route_id in ["Orange"] and location.stop_id == prediction.stop_id do | ||
if location.status in [:incoming_at, :in_transit_to], | ||
do: :high, | ||
else: :low | ||
end | ||
|
||
defp calculate_crowding_data_confidence(_prediction, _location), do: nil | ||
|
||
defimpl Content.Message do | ||
require Logger | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
defmodule Locations.CarriageDetails do | ||
defstruct label: nil, | ||
occupancy_status: nil, | ||
occupancy_percentage: nil, | ||
carriage_sequence: nil | ||
|
||
@type t :: %__MODULE__{ | ||
label: String.t(), | ||
occupancy_status: | ||
:many_seats_available | ||
| :few_seats_available | ||
| :standing_room_only | ||
| :crushed_standing_room_only | ||
| :full, | ||
occupancy_percentage: non_neg_integer(), | ||
carriage_sequence: non_neg_integer() | ||
} | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking out loud: Not sure how this will end up looking in the following PRs but hopefully there's an opportunity to unify some of this logic a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep! This does get unified down the line