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

feat: Pass along more enhanced JSON fields from RTR #252

Closed
wants to merge 1 commit into from

Conversation

losvedir
Copy link
Contributor

This adds two new fields to the TripUpdates_enhanced.json file, which RTR produces but currently Concentrate ignores. The fields are used by realtime signs. With the recent OL surge, there was a question about updating realtime-signs to use concentrate's output, in order to have access to CR data. They decided against it, for now, but it seemed like a good idea to me to update concentrate so that it would be possible in the future, if a situation like this came up again. The two fields are in StopTimeUpdate and are:

  • stops_away - this is a field that says how many stations away the predicted train is from the stop the prediction is for.
  • passthrough_time - this is arrival_time || departure_time for SKIPPED stops, prior to those values being nullified by RTR. realtime-signs uses this to play the "next train does not take passengers, please stand back from the yellow line" message.

I think both of them could arguably be implemented differently in realtime-signs such that these values aren't necessary but that's a larger lift, and in an app that our team doesn't maintain anymore. I didn't want to put that work on @PaulJKim , but I am open to potentially implementing it over there myself. It didn't seem so bad to me to have extra fields in our enhanced JSON here, so I went with the easier solution, but I could be convinced to take the other approach.

@github-actions
Copy link

Coverage of commit 7690be8

Summary coverage rate:
  lines......: 94.9% (1034 of 1090 lines)
  functions..: 78.0% (347 of 445 functions)
  branches...: no data found

Files changed coverage rate:
                                                            |Lines       |Functions  |Branches    
  Filename                                                  |Rate     Num|Rate    Num|Rate     Num
  ================================================================================================
  lib/concentrate/encoder/trip_updates_enhanced.ex          | 100%      4| 100%     3|    -      0
  lib/concentrate/parser/gtfs_realtime_enhanced.ex          |96.3%     54|87.5%    16|    -      0
  lib/concentrate/stop_time_update.ex                       |91.3%     23|64.7%    34|    -      0

Download coverage report

@paulswartz
Copy link
Member

  • stops_away: is it possible to calculate this inside the app, using the vehicle_id from the TripUpdate?
  • passthough_time: I wonder if this should be implemented differently now, with keeping the times in the feed along with SKIPPED?

In general, I'm wary of adding data to Concentrate or the API that only has one client. Do you see other use cases for this data?

Copy link
Member

@paulswartz paulswartz left a comment

Choose a reason for hiding this comment

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

Code itself looks fine, but I think we need some more discussion about the general concept of adding these fields.

@PaulJKim
Copy link

@losvedir let me know if you want to discuss how to implement those features without those fields. I can sort of see how we could go about that, but if you want to lay out what you have in mind I'm more than happy to take on the work.

@bklebe
Copy link
Member

bklebe commented Apr 5, 2024

We've determined this is no longer necessary.

@bklebe bklebe closed this Apr 5, 2024
@bklebe bklebe deleted the gjd-more-enhanced-json-fields branch April 5, 2024 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants