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

Inconsistency documentation https://developers.google.com/transit/gtfs-realtime/guides/vehicle-positions#vehiclestopstatus #291

Closed
sven4all opened this issue Nov 9, 2021 · 10 comments
Assignees
Labels
Status: Stale Issues and Pull Requests that have remained inactive for 30 calendar days or more.

Comments

@sven4all
Copy link

sven4all commented Nov 9, 2021

The documentation in https://developers.google.com/transit/gtfs-realtime/guides/vehicle-positions#vehiclestopstatus is not consistent with the documentation in the .proto files https://developers.google.com/transit/gtfs-realtime/gtfs-realtime-proto and gtfs.org

On https://developers.google.com/transit/gtfs-realtime/guides/vehicle-positions#vehiclestopstatus IN_TRANSIT_TO is described as "the referenced stop is the next stop for the vehicle - default" while in the .proto file it's described the following:

enum VehicleStopStatus {
    // The vehicle is just about to arrive at the stop (on a stop
    // display, the vehicle symbol typically flashes).
    INCOMING_AT = 0;

    // The vehicle is standing at the stop.
    STOPPED_AT = 1;

    // The vehicle has departed and is in transit to the next stop.
    IN_TRANSIT_TO = 2;
  }
  // The exact status of the vehicle with respect to the current stop.
  // Ignored if current_stop_sequence is missing.
  optional VehicleStopStatus current_status = 4 [default = IN_TRANSIT_TO];

current_stop_sequence is described:

// The stop sequence index of the current stop. The meaning of
  // current_stop_sequence (i.e., the stop that it refers to) is determined by
  // current_status.
  // If current_status is missing IN_TRANSIT_TO is assumed.
  optional uint32 current_stop_sequence = 3;

What is the correct way to interpreted it? Should the current_stop_sequence be updated directly after the bus is departed from the previous stop (what https://developers.google.com/transit/gtfs-realtime/guides/vehicle-positions#vehiclestopstatus IN_TRANSIT_TO suggests) or should it be updated when the vehicle arrives at the next stop what the .proto file and gtfs.org are suggesting.

@barbeau
Copy link
Collaborator

barbeau commented Jan 5, 2022

IMHO this is another example of the confusion that results when we have multiple GTFS specs online, specifically the developers.google.com page, which for some reason doesn't always mirror the canonical spec reference on GitHub.

Here's the canonical spec and .proto reference on GitHub:

For IN_TRANSIT_TO, the canonical spec says:

The vehicle has departed the previous stop and is in transit.

...and the canonical .proto file says:

// The vehicle has departed and is in transit to the next stop.

So these align and are the canonical definitions (although for clarity they really should be word-for-word identical).

For current_stop_sequence, here's the canonical definition:

// The stop sequence index of the current stop. The meaning of
// current_stop_sequence (i.e., the stop that it refers to) is determined by
// current_status.
// If current_status is missing IN_TRANSIT_TO is assumed.
optional uint32 current_stop_sequence = 3;

For this question:

Should the current_stop_sequence be updated directly after the bus is departed from the previous stop

...my interpretation of the above canonical spec is yes, current_stop_sequence should be updated to the next stop (3) after the bus departs from the previous stop (2).

Specifically, this should be the sequence of the vehicle going from stops 2 to 3 if you're providing both fields:

  • current_stop_sequence = 2, current_status = INCOMING_AT
  • current_stop_sequence = 2, current_status = STOPPED_AT
  • current_stop_sequence = 3, current_status = IN_TRANSIT_TO // When vehicle departs stop 2

If you're not providing current_status, then you should update current_stop_sequence based on the definition of IN_TRANSIT_TO, which means that the value only changes after the vehicle departs a stop.

@sven4all Does that make sense to you?

Anyone else agree or disagree?

@scmcca
Copy link
Contributor

scmcca commented Jan 5, 2022

@barbeau

IMHO this is another example of the confusion that results when we have multiple GTFS specs online, specifically the developers.google.com page, which for some reason doesn't always mirror the canonical spec reference on GitHub.

In fairness I think the content at https://developers.google.com/transit/gtfs-realtime/guides/vehicle-positions#vehiclestopstatus is being pulled from this repo at: https://github.com/google/transit/blob/master/gtfs-realtime/spec/en/vehicle-positions.md

IMO the google/transit repo should always be the source of truth for canonical information. In this case there is actually inconsistent documentation within the repo itself.

@sven4all
Copy link
Author

sven4all commented Jan 5, 2022

@barbeau for me it's the most important that we agree on one approach which one doesn't really matter to me.

Mixing the words previous and next in canonical spec and .proto spec is causing the confusion. Also the order of the VehicleStopStatus ENUM is causing confusion because IN_TRANSIT_TO -> INCOMING_AT -> STOPPED_AT is the logical order in your interpetation.

@barbeau
Copy link
Collaborator

barbeau commented Jan 6, 2022

In fairness I think the content at https://developers.google.com/transit/gtfs-realtime/guides/vehicle-positions#vehiclestopstatus is being pulled from this repo at: https://github.com/google/transit/blob/master/gtfs-realtime/spec/en/vehicle-positions.md

Ah, good point, I stand corrected.

IMO the google/transit repo should always be the source of truth for canonical information. In this case there is actually inconsistent documentation within the repo itself.

Yes, agreed, and it looks like this particular issue is that we have the 3 vehicle-positions, trip-updates, and service-alerts markdown files, which are more of a narrative definition, in addition to the main reference.md file that actually enumerates all the fields. I know we've talked about combining these in the past because of these potential inconsistencies, and this is definitely another piece of evidence in favor of that approach.

@scmcca
Copy link
Contributor

scmcca commented Feb 11, 2022

For now I propose we make the narrative guide definitions for VehicleStopStatus consistent with the definitions in the spec and .proto file.

Are these descriptions suitable?

  • Incoming at - The vehicle is just about to arrive at the referenced stop (on a stop display, the vehicle symbol typically flashes).
  • Stopped at - The vehicle is standing at the referenced stop.
  • In transit to - The vehicle has departed the referenced stop and is in transit. - default

@barbeau
Copy link
Collaborator

barbeau commented Feb 11, 2022

Close, but to be consistent with my interpretation in #291 (comment) the last one should be something like:

In transit to - The vehicle has departed the stop prior to the referenced stop and is in transit to the referenced stop. - default

@scmcca
Copy link
Contributor

scmcca commented Feb 11, 2022

Ah right. How about:

  • In transit to - The vehicle has departed and is in transit to the referenced stop. - default

Should the descriptions in the spec and proto be clarified along these lines as well?

@barbeau
Copy link
Collaborator

barbeau commented Feb 11, 2022

Either could work - as long as others think it's clear that "has departed" refers to the stop prior to the referenced stop without explicitly saying that.

Should the descriptions in the spec and proto be clarified along these lines as well?

Yes, IMHO all 3 docs should match identically.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Status: Stale Issues and Pull Requests that have remained inactive for 30 calendar days or more. label Feb 12, 2023
@github-actions
Copy link

This issue has been closed due to inactivity. Issues can always be reopened after they have been closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Stale Issues and Pull Requests that have remained inactive for 30 calendar days or more.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants