-
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
Trigger PA Messages from RTS #764
Merged
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
47c2b62
Trigger PA Messages from RTS
PaulJKim 9fe3e57
Make logs errors and remove something used for testing
PaulJKim 839dac3
Address PR feedback
PaulJKim f066eca
Put active messages path only in application env
PaulJKim e86ecc4
fix config
PaulJKim 288bb2e
Remove extraneous config
PaulJKim 453d042
fix tests
PaulJKim fe1511f
Trigger PA Messages from RTS
PaulJKim 4a6719c
Make logs errors and remove something used for testing
PaulJKim 5b0e2da
Address PR feedback
PaulJKim 63f72ec
Put active messages path only in application env
PaulJKim 585de73
fix config
PaulJKim bdd9232
Remove extraneous config
PaulJKim 54fb458
fix tests
PaulJKim e161092
try adding stubs
PaulJKim 71f2846
Merge branch 'pk/pa-messages' of https://github.com/mbta/realtime_sig…
PaulJKim 7c505b6
add TTS expects
PaulJKim e5bc8be
Address PR feedback
PaulJKim File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
defmodule Engine.PaMessages do | ||
use GenServer | ||
require Logger | ||
|
||
alias PaMessages.PaMessage | ||
|
||
@type state :: %{ | ||
pa_messages_last_sent: %{non_neg_integer() => DateTime.t()} | ||
} | ||
|
||
@minute_in_ms 1000 * 60 | ||
|
||
def start_link([]) do | ||
GenServer.start_link(__MODULE__, [], name: __MODULE__) | ||
end | ||
|
||
@impl true | ||
def init(_) do | ||
# Add some delay to wait for sign processes to come up before sending PA messages | ||
Process.send_after(self(), :update, 10000) | ||
{:ok, %{pa_messages_last_sent: %{}}} | ||
end | ||
|
||
@impl true | ||
def handle_info(:update, state) do | ||
schedule_update(self()) | ||
|
||
state = | ||
case get_active_pa_messages() do | ||
{:ok, pa_messages} -> | ||
recent_sends = send_pa_messages(pa_messages, state.pa_messages_last_sent) | ||
|
||
%{state | pa_messages_last_sent: recent_sends} | ||
|
||
{:error, %HTTPoison.Response{status_code: status_code, body: body}} -> | ||
Logger.error("pa_messages_response_error: status_code=#{status_code} body=#{body}") | ||
state | ||
|
||
{:error, %HTTPoison.Error{reason: reason}} -> | ||
Logger.error("pa_messages_response_error: reason=#{reason}") | ||
state | ||
|
||
{:error, error} -> | ||
Logger.error("pa_messages_response_error: error=#{inspect(error)}") | ||
state | ||
end | ||
|
||
{:noreply, state} | ||
end | ||
|
||
defp send_pa_messages(pa_messages, pa_messages_last_sent) do | ||
for %{ | ||
"id" => pa_id, | ||
"visual_text" => visual_text, | ||
"audio_text" => audio_text, | ||
"interval_in_minutes" => interval_in_minutes, | ||
"priority" => priority, | ||
"sign_ids" => sign_ids | ||
} <- pa_messages, | ||
into: %{} do | ||
active_pa_message = %PaMessage{ | ||
id: pa_id, | ||
visual_text: visual_text, | ||
audio_text: audio_text, | ||
priority: priority, | ||
sign_ids: sign_ids, | ||
interval_in_ms: interval_in_minutes * @minute_in_ms | ||
} | ||
|
||
{_, last_sent_time} = Map.get(pa_messages_last_sent, pa_id, {nil, nil}) | ||
|
||
time_since_last_send = | ||
if last_sent_time, | ||
do: DateTime.diff(DateTime.utc_now(), last_sent_time, :millisecond), | ||
else: active_pa_message.interval_in_ms | ||
|
||
if time_since_last_send >= active_pa_message.interval_in_ms do | ||
send_pa_message(active_pa_message) | ||
{pa_id, {active_pa_message, DateTime.utc_now()}} | ||
else | ||
{pa_id, {active_pa_message, last_sent_time}} | ||
end | ||
end | ||
end | ||
|
||
defp send_pa_message(pa_message) do | ||
Enum.each(pa_message.sign_ids, fn sign_id -> | ||
send( | ||
String.to_existing_atom("Signs/#{sign_id}"), | ||
{:play_pa_message, pa_message} | ||
) | ||
end) | ||
end | ||
|
||
defp get_active_pa_messages() do | ||
active_pa_messages_url = | ||
Application.get_env(:realtime_signs, :screenplay_base_url) <> | ||
Application.get_env(:realtime_signs, :active_pa_messages_path) | ||
|
||
http_client = Application.get_env(:realtime_signs, :http_client) | ||
|
||
with {:ok, response} <- | ||
http_client.get( | ||
active_pa_messages_url, | ||
[ | ||
{"x-api-key", Application.get_env(:realtime_signs, :screenplay_api_key)} | ||
], | ||
timeout: 2000, | ||
recv_timeout: 2000 | ||
), | ||
%{status_code: 200, body: body} <- response, | ||
{:ok, pa_messages} <- Jason.decode(body) do | ||
{:ok, pa_messages} | ||
else | ||
error -> | ||
{:error, error} | ||
end | ||
end | ||
|
||
defp schedule_update(pid) do | ||
Process.send_after(pid, :update, 1_000) | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
defmodule PaMessages.PaMessage do | ||
defstruct id: nil, | ||
visual_text: nil, | ||
audio_text: nil, | ||
priority: nil, | ||
sign_ids: [], | ||
interval_in_ms: nil | ||
|
||
@type t :: %__MODULE__{ | ||
id: integer(), | ||
visual_text: String.t(), | ||
audio_text: String.t(), | ||
priority: integer(), | ||
sign_ids: [String.t()], | ||
interval_in_ms: non_neg_integer() | ||
} | ||
|
||
defimpl Content.Audio do | ||
def to_params(%PaMessages.PaMessage{visual_text: visual_text}) do | ||
{:ad_hoc, {visual_text, :audio_visual}} | ||
end | ||
|
||
def to_tts(%PaMessages.PaMessage{visual_text: visual_text, audio_text: audio_text}) do | ||
{audio_text, PaEss.Utilities.paginate_text(visual_text)} | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I'm not sure I understand the benefit of tracking this here. Is this so in case the engine crashes, we won't spam messages? If that's the concern, and we're willing to accept a slight behavior change, we could schedule new messages with their full duration instead of immediately, which would result in fewer messages in that failure mode, rather than excess ones.
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.
Yeah that was the intention here, just to prevent consecutive plays due to an engine crashes/restarts. Ah so you mean that a PA Message wouldn't play immediately upon scheduling and instead wait one interval before it's first play?
I had actually sort of brought that up in the product channel last week but it was only discussed briefly. Kevin said a PA message should play as soon as it can right after it's created, but I'm still not sure if that's also what the ARINC software does currently. Seems like its still certainly up for discussion though. I'll raise the question again to the product folks and see if we would be okay with that behavior change.
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.
Discussed a bit further in the thread and I think I lean towards avoiding scheduling a new message with the full duration. I don't think it's been fully confirmed or is explicitly a standard practice, but it seems that OIOs tend to schedule a message and look immediately for a "proof of play" and I'd rather not disrupt their workflow if we can avoid it. I'm open to other ideas for preventing spammed message plays though if you have any thoughts?