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

Add an associated trip id to status changes when a trip ends #289

Closed
billdirks opened this issue Apr 4, 2019 · 2 comments
Closed

Add an associated trip id to status changes when a trip ends #289

billdirks opened this issue Apr 4, 2019 · 2 comments
Labels
State Machine Changes in the vehicle state events and state machine diagram
Milestone

Comments

@billdirks
Copy link
Contributor

Currently the provider specification only requires an associated_trip for status change events if the event_type_reason is user_pick_up or user_drop_off. For trip starts an event with event_type = reservation and event_type_reason = user_pick_up appears mandatory. For trip ends, however, it seems possible that the vehicle doesn't become available (a scooter with low battery when a trip ends for example). In this example a trip would end with the state transition (available, reserved) to (unavailable, low_battery) and no associated_trip would be present in the status change event. It seems the intent is to have an associated_trip id present at end of trips even when there is no allowable user_drop_off event_type_reason.

@hunterowens hunterowens added the State Machine Changes in the vehicle state events and state machine diagram label Apr 9, 2019
@rf-
Copy link
Contributor

rf- commented Apr 13, 2019

It seems like a good idea to me to require an associated_trip for any status change that marks the beginning or end of a trip, regardless of the specific event_type and event_type_reason it represents. The spec is currently somewhat ambiguous about whether associated_trip is allowed for events that don't have a reason of user_pick_up or user_drop_off. I'd suggest the following language for 0.4.0:

Trip UUID (foreign key to Trips API), required if event_type_reason is user_pick_up or user_drop_off, or for any other status change event that marks the end of a trip.

For 0.3.1, we could find a middle ground that explicitly allows providers to include the field for other event types but doesn't require it.

If and when the provider state machine is defined more clearly in a future version of MDS, I think it would make sense to allow edges going from the reserved state to available, unavailable, and potentially other statuses, with the stipulation that all edges going into or out of reserved should have an associated_trip.

rf- added a commit to remix/mobility-data-specification that referenced this issue Apr 17, 2019
This implements the change suggested in openmobilityfoundation#289 by clarifying that any
trip-ending event can and should have an `associated_trip_id`, not just
`user_drop_off` events.

Since the Provider event system currently doesn't define which states
are allowed to transition to which other states, it's already
spec-compliant to follow a `reserved`/`user_pick_up` event with an
`unavailable`/`low_battery` event, and this is the most
semantically-accurate way to describe the scenario where a device shuts
down during a trip because of low battery. In this situation, the
implementer should include an `associated_trip_id` field in both events
so that API consumers can correctly identify these events as the
endpoints of the relevant trip.
rf- added a commit to remix/mobility-data-specification that referenced this issue Apr 17, 2019
This implements the change suggested in openmobilityfoundation#289 by clarifying that any
trip-ending event can and should have an `associated_trip`, not just
`user_drop_off` events.

Since the Provider event system currently doesn't define which states
are allowed to transition to which other states, it's already
spec-compliant to follow a `reserved`/`user_pick_up` event with an
`unavailable`/`low_battery` event, and this is the most
semantically-accurate way to describe the scenario where a device shuts
down during a trip because of low battery. In this situation, the
implementer should include an `associated_trip` field in both events
so that API consumers can correctly identify these events as the
endpoints of the relevant trip.
@hunterowens hunterowens added this to the 0.3.2 milestone May 21, 2019
@thekaveman
Copy link
Collaborator

The language of the spec was updated in #297 to be more explicit that associated_trip is allowed with any status_change event type. I think that closes this issue for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
State Machine Changes in the vehicle state events and state machine diagram
Projects
None yet
Development

No branches or pull requests

4 participants