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

GTFS-rt : wheelchair access update #87

Closed
wants to merge 4 commits into from

Conversation

gcamp
Copy link
Contributor

@gcamp gcamp commented Feb 27, 2018

This pull request add the ability to specify a wheelchair_accessible value in a TripUpdate that updates the value found in the static GTFS.

This pull is intentionally small, it doesn't support all cases of a accessibility values and instead relies on what is already accepted in the GTFS.

It partially goes against the carriage pull request. However, both can be accepted in their current form. In the case where a producer doesn't have complete carriage information but knows about updated wheelchair accessibility value, they could provide only this value.

We have two agencies who are interested in implementing this with us : STM (Montréal) and MBTA (Boston).

Note :

  • didn't include an spanish doc and protobuf. I can either add it with empty description or will need help to add the full description.
  • Value of WheelchairAccessible NO_VALUE is negative to be able to have the same value in the enum as the static GTFS. However, negative values in a protobuf enum are discouraged for compression reasons. Since NO_VALUE is the default and will not be provided most of the time, I think this is a reasonable tradeoff.

| _**Value**_ | _**Comment**_ |
|-------------|---------------|
| **NO_VALUE** | The trip doesn't have information about wheelchair accessibility. This is the **default** behavior. If the static GTFS contains a _wheelchair_accessible_ value, it won't be overwritten. |
| **UNKNOWN** | The trip has no accessibility value present. This value will overwrite the value from the GTFS. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really required? Why not keep it simple with NO_VALUE as UNKNOWN?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For example, if a static GTFS says trip A is wheelchair accessible, how can a trip_update changes the value to Unknown? If we drop NO_VALUE we won't support this case. Maybe this is an edge case and in the eventuality this happens the producer can put WHEELCHAIR_INACCESSIBLE instead. I don't feel too strongly on either choices.

Copy link
Collaborator

@barbeau barbeau Mar 13, 2018

Choose a reason for hiding this comment

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

This is definitely confusing at first glance, but the underlying issue is that protobufs don't allow enums to have a null value. I believe this is the first enumeration in GTFS that we're overriding in GTFS-rt, and as a result we haven't had this issue before.

From https://developers.google.com/protocol-buffers/docs/javatutorial?csw=1 (emphasis mine):

optional: the field may or may not be set. If an optional field value isn't set, a default value is used. For simple types, you can specify your own default value, as we've done for the phone number type in the example. Otherwise, a system default is used: zero for numeric types, the empty string for strings, false for bools. For embedded messages, the default value is always the "default instance" or "prototype" of the message, which has none of its fields set. Calling the accessor to get the value of an optional (or required) field which has not been explicitly set always returns that field's default value.

I think it's important to support the case where the GTFS says that the trip is wheelchair accessible, but then real-time information indicates that this is unknown for some reason (UNKNOWN) - for example, let's say an accessible vehicle was scheduled to run a trip, but then it broke down and was replaced with another vehicle, and dispatch doesn't know if that vehicle is wheelchair accessible. However, it's also important to say that there is no real-time information, and therefore the GTFS value should be used (NO_VALUE), especially for backwards compatibility and GTFS-rt feeds that aren't providing this field.

@gcamp The proto3 spec says:

  • There must be a zero value, so that we can use 0 as a numeric default value.
  • The zero value needs to be the first element, for compatibility with the proto2 semantics where the first enum value is always the default.

We're currently using proto2 for GTFS-realtime, but I'm assuming we should plan for eventually moving to proto3. This means that NO_VALUE would need to be 0, which makes this even more confusing, as the enum values in GTFS-rt wouldn't match GTFS.

I'm definitely supportive of trying simpler iterative implementations first especially if we have willing producers for them, but in this case because of the above I'm wondering if we should look at a simplified version of the carriage proposal instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@barbeau Good point for proto3. We could use values from 0 to 4. Not the same numerical value compared the static GTFS, but since protobuf libraries would use the name of the value anyway I'm not sure this is a problem.

Would the carriage proposal fix those issues? The WheelchairAccessible value there doesn't have a NO_DATA and I don't think there's a way to not specify WheelchairAccessible value for a carriage if you want to specify other information.

Copy link
Collaborator

@barbeau barbeau Mar 14, 2018

Choose a reason for hiding this comment

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

Not the same numerical value compared the static GTFS, but since protobuf libraries would use the name of the value anyway I'm not sure this is a problem.

Good point - in code I assume most/all protobuf libraries producing and consuming libraries would reference the enumeration name, not the ID, so maybe that's not too big of a stumbling block in software development, just confusing to read in the spec itself.

Would the carriage proposal fix those issues?

Yeah, thinking more, it doesn't. I was thinking that because CarriageDescriptor is new, we wouldn't have the overriding issue that we would with GTFS trip.wheelchair_accessible. But, we'd still have to specify some behavior for interaction between the two. Unless we make CarriageDescriptor.WheelchairAccessible required, it could be empty, in which case we have the same overriding behavior problem as the default would be chosen.

So in summary using values from 0 to 4 for TripUpdate.WheelchairAccessible enumeration seems like a reasonable approach.

Given some of the potential interaction issues with GTFS here, I'd like to see a working implementation before we vote on it. Having a producer and consumer is a requirement for calling a GTFS vote for new fields, but in the past those requirements have been relaxed for GTFS-rt given the potential cost of an agency reverting a deployed protobuf extension to the canonical protobuf after the field is approved. Instead, a field has been voted on an accepted as "experimental" (see occupancy proposal). Due to the interactions with GTFS, though, this proposal seems potentially more complicated to me.

@gcamp
Copy link
Contributor Author

gcamp commented Mar 8, 2018

@RachM @barbeau Would love your comments before I submit for a vote.

@gcamp
Copy link
Contributor Author

gcamp commented Mar 15, 2018

Updated WheelchairAccessible enum values to be compatible with proto3.

@RachM
Copy link
Contributor

RachM commented Apr 13, 2018

I'll most likely abandon the carriage pull request, so don't worry about this request conflicting :-)

Can this be added to VehiclePositions too? It seems like you'd also want it there?

@jakluk
Copy link

jakluk commented May 21, 2018

Anything new regarding this pull request? We would like to provide realtime wheelchair accessibility information in Prague region. In static timetables we can guarantee only a portion of accessible trips, but in general more low-floor vehicles are available for operation.

@gcamp
Copy link
Contributor Author

gcamp commented May 21, 2018

Unfortunately the agencies that we could have made the sample implementation with don't have the time to do it right now. We are still willing to make a sample implementation if a producer wants to do a feed. @jakluk if you have somebody in mind it would be awesome.

@barbeau barbeau added the GTFS Realtime Issues and Pull Requests that focus on GTFS Realtime label Aug 27, 2018
@gcamp
Copy link
Contributor Author

gcamp commented Oct 3, 2018

Closing this because no producer seems to have the willingness to go forward with it (even those who ask for it!). Anybody feel free to use that branch as a starting base for this implementation.

@gcamp gcamp closed this Oct 3, 2018
@skinkie
Copy link
Contributor

skinkie commented Oct 3, 2018

Why not? We can actually provide this information...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GTFS Realtime Issues and Pull Requests that focus on GTFS Realtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants