Skip to content
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

Modified approaching announcement window from 30-45 to 0-45 seconds #886

Merged
merged 1 commit into from
Feb 25, 2025

Conversation

deanshi
Copy link
Contributor

@deanshi deanshi commented Feb 24, 2025

Summary of changes

Asana Ticket: Prevent predictions from skipping approaching messages

Modified approaching announcement window from 30-45 to 0-45 seconds

  • Fixed tests associated with it, now calls approaching message with two ARR on a sign

Reviewer Checklist

  • Meets ticket's acceptance criteria
  • Any new or changed functions have typespecs
  • Tests were added for any new functionality (don't just rely on Codecov)
  • If signs.json was changed, there is a follow-up PR to signs_ui to update its dependency on this project
  • This branch was deployed to the staging environment and is currently running with no unexpected increase in warnings, and no errors or crashes (compare on Splunk: staging vs. prod)

@deanshi deanshi requested a review from a team as a code owner February 24, 2025 18:44
Copy link
Collaborator

@panentheos panentheos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment, then looks good.

@@ -552,7 +552,7 @@ defmodule PaEss.Utilities do
@spec prediction_approaching?(Predictions.Prediction.t(), boolean()) :: boolean()
def prediction_approaching?(prediction, terminal?) do
!terminal? and !prediction.stopped_at_predicted_stop? and
prediction_seconds(prediction, terminal?) in 31..45
prediction_seconds(prediction, terminal?) in 1..45
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Predictions can go all the way down to 0, so let's make this 0..45 just to avoid any surprise edge cases in the future.

- Fixed tests associated with it, now calls approaching message with two
  ARR on a sign
@deanshi deanshi force-pushed the deanshi/change_approaching_message_time branch from efb80a7 to 3c9af28 Compare February 25, 2025 15:03
@deanshi deanshi merged commit b50f6c0 into main Feb 25, 2025
2 checks passed
@deanshi deanshi deleted the deanshi/change_approaching_message_time branch February 25, 2025 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants