Explicitly allow associated_trip for any event type
#297
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rf-
changed the title
associated_trip_id for any event typeassociated_trip for any event type
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.
rf-
force-pushed
the
ryanf/clarify-associated-trip-field
branch
from
1309054
to
29760f8
Compare
thekaveman
added
Schema
|
@rf- @hunterowens so just to circle back, are we OK with this being a language-only change - and leaving the JSON schema alone? Or do we want to add additional |
|
I think |
thekaveman
approved these changes
Apr 18, 2019
Merged
rf-
added a commit
to remix/mobility-data-specification
that referenced
this pull request
Aug 16, 2019
PR openmobilityfoundation#297 already updated the spec language to make it clear that any status change is allowed to have an `associated_trip`, but I didn't realize this was incompatible with the existing schema. The schema should now match the spec, which states that `user_pick_up` and `user_drop_off` events must have an `associated_trip` but other events may have one when applicable.
hunterowens
pushed a commit
that referenced
this pull request
Oct 1, 2019
PR #297 already updated the spec language to make it clear that any status change is allowed to have an `associated_trip`, but I didn't realize this was incompatible with the existing schema. The schema should now match the spec, which states that `user_pick_up` and `user_drop_off` events must have an `associated_trip` but other events may have one when applicable.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements the change suggested in #289 by clarifying that any
trip-ending event can and should have an
associated_trip, not justuser_drop_offevents.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_upevent with anunavailable/low_batteryevent, and this is the mostsemantically-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_tripfield in both eventsso that API consumers can correctly identify these events as the
endpoints of the relevant trip.
Is this a breaking change
Provideroragencyprovideragency