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

Confusion about Reserved state #417

Closed
concept47 opened this issue Nov 26, 2019 · 27 comments · Fixed by #506
Closed

Confusion about Reserved state #417

concept47 opened this issue Nov 26, 2019 · 27 comments · Fixed by #506
Assignees
Labels
Agency Specific to the Agency API Provider Specific to the Provider API question Further information is requested
Milestone

Comments

@concept47
Copy link

concept47 commented Nov 26, 2019

Is your feature request related to a problem? Please describe.

I was looking at the provider event types spec as I was getting ready to implement the reserved event update. I was curious about why in the spec for the reserved event_type it says in the description

"A customer reserves a device (even if trip has not started yet)"

but then an associated_trip (UUID) is required because the reserved event maps to the event_type_reason user_pick_up

Describe the solution you'd like

associated_trip (UUID) not required for reserved event_type

Is this a breaking change

  • No, not breaking

Impacted Spec

For which spec is this feature being requested?

  • provider

Describe alternatives you've considered

Additional context

@thekaveman
Copy link
Collaborator

I would be more inclined to update the spec language to remove the even if trip has not started yet piece. Associated trip UUID for the trip-specific events seems like something we would want to continue requiring.

@concept47
Copy link
Author

concept47 commented Dec 3, 2019

I'm fine with that too. Although I am curious as to what your rationale for saying

Associated trip UUID for the trip-specific events seems like something we would want to continue requiring.

... is, @thekaveman?

It seems odd to me to require a UUID for a trip that has not started yet (specifically in the case of reservations), but then again I'm an absolute newb to all this, so I'm just trying to understand the design of the spec on a deeper level.

Thanks for taking the time to respond!

@geobir
Copy link
Contributor

geobir commented Dec 3, 2019

I am not sure to understand perfectly but for the "A customer reserves a device (even if trip has not started yet)"
If the provider let the user reserved vehicle before they start the trip.

For the UUID I understand your concern giving the trip UUID before the trip start can be interesting. Even more if the reservation is canceled.

associated_trip

associated_trip | UUID | Required if Applicable | 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.

We may want to switch user_pick_up event type by another or remove it from the applicable one.

@concept47
Copy link
Author

For the UUID I understand your concern giving the trip UUID before the trip start can be interesting. Even more if the reservation is canceled.

exactly my concern @geobir. Thanks for stating that better than I did 😅

We may want to switch user_pick_up event type by another or remove it from the applicable one.

This is what I was hoping was possible ...

@thekaveman
Copy link
Collaborator

It seems odd to me to require a UUID for a trip that has not started yet (specifically in the case of reservations), but then again I'm an absolute newb to all this, so I'm just trying to understand the design of the spec on a deeper level.

I think the use of the word "reserved" in the spec was from a time where the "pre-reserved trips" kind of scenario you describe @concept47 were not being considered.

My understanding is the language was more about the possibly short time (seconds-minutes) between scanning a vehicle with the app and actually pulling the throttle / pumping the pedals to "start the trip" - like a group of friends all getting devices but not going anywhere until everyone has one.

True that the event types may need to be adjusted to account for this "future hold" type scenario.

@concept47
Copy link
Author

concept47 commented Dec 5, 2019

My understanding is the language was more about the possibly short time (seconds-minutes) between scanning a vehicle with the app and actually pulling the throttle / pumping the pedals to "start the trip" - like a group of friends all getting devices but not going anywhere until everyone has one.

Oh wow. That's very interesting!

My understanding was that "reserved" was more in line with the "future hold" situation that you are talking about.

Is there anyway to confirm this

@thekaveman thekaveman added Provider Specific to the Provider API question Further information is requested labels Dec 6, 2019
@thekaveman
Copy link
Collaborator

Is there anyway to confirm this, because it would seem we've been doing it wrong this whole time if thats the case.

reserved: user_pick_up has always been the only event in status_changes that indicates trip start. The corresponding available: user_drop_off - User ends reservation event that indicates trip end also uses the word reservation in the more possessive sense.

When companies first started offering rental services, the only way to get a device was the more immediate on-the-fly rental. More recently we are hearing about the operational evolution of future hold type rentals (is a company doing this in the wild yet? I'm not sure...) Because of the reality on the ground when MDS launched, this future hold rental interpretation seems novel.

@thekaveman
Copy link
Collaborator

thekaveman commented Dec 6, 2019

Related: #399, #345, #271

@concept47
Copy link
Author

concept47 commented Dec 6, 2019

reserved: user_pick_up has always been the only event in status_changes that indicates trip start. The corresponding available: user_drop_off - User ends reservation event that indicates trip end also uses the word reservation in the more possessive sense.

Thanks @thekaveman! ... My question is, is this the official position of the Open Mobility Foundation? I apologize for asking this as I'm a newb to this and just need to understand where I would look to get official word.

As a follow up, in the case of the "full hold" model of reservations you've mooted above, where a vehicle can be reserved for an extended period of time, how would you suggest a team handle this using the current spec?

@wellorder
Copy link
Contributor

Bird does "future hold" reservations, as does Spin (as seen in #399). I think the other major scooter operators do as well, but I wouldn't swear to it. So it would be good to clarify what to do in this situation.

@dirkdk
Copy link
Contributor

dirkdk commented Dec 10, 2019

Yes Spin is working on Reservations as future hold.

Agency is clear, it distinguishes between reserve and and trip_start and even has cancel_reservation.

Provider status_changes only has reserved and no trip_start or rented event types.

Option 1 for us is to use reserved for the reserved action and then skip the transition from reserved to trip start in status_changes, as the explanation of reserved is A customer reserves a device (even if trip has not started yet) . In Provider trips we would not show trips that are reserved but canceled. Drawback is that a reservation reserved with associated_trip would have an earlier timestamp than the trip's start_time with the same trip_id in trips. This could confuse API consumers and look like our systems are inconsistent.

Second option would be to not surface future hold reservation events in status_changes at all, and to return a reserved event only at the trip start. An event reserved with associated_trip would have the same timestamp as in trips on start_time. Also reservation cancelations would not show up in status_changes. Drawback is then that the definition of Agency reserve becomes completely different from Provider reserved.

Third option we would completely go the other way, and also include canceled reservations in trips. This would at least have matching trip_id values, just different timestamps

@thekaveman
Copy link
Collaborator

My question is, is this the official position of the Open Mobility Foundation?

Right, and we can ping @jfh01 here to weigh in - but the OMF was only formalized and officially launched earlier in Summer 2019. In practice, as part of using MDS to regulate shared mobility services in Santa Monica (and to my understanding in LA) since Fall 2018, we have thus-far framed it as reserved == take into possession.

The mismatches between Agency and Provider are certainly a point of discussion; see the issues I referenced above for more context. This is really a by-product of the Provider spec being "live" (i.e. required for permit issuance) 6 or so months before Agency was first implemented and rolled out by LADOT, and the two specs having slightly different use-cases.

As a next step, I might suggest putting forth a PR to work on some of this reserved ambiguity - perhaps rewording, perhaps adding additional event_type/event_type_reason, or something else... But enough concrete work that the larger community can discuss on the bi-weekly calls and/or comment and consider here in GitHub.

@concept47
Copy link
Author

concept47 commented Dec 10, 2019

I might suggest putting forth a PR to work on some of this reserved ambiguity - perhaps rewording, perhaps adding additional event_type/event_type_reason

I like this idea! So is this something you want the community to do, or do you want to start that to kick things off @thekaveman. Either is fine, just wanted to clarify.

@marie-x
Copy link
Collaborator

marie-x commented Dec 10, 2019

I am interested in reconciling most/all of the differences between Agency and Provider, except for the obvious push vs. pull. New and forthcoming APIs like Policy, Compliance, and Audit are intended to be compatible with either means of data ingest. I would like to see a small working group help sort out the Agency/Provider discrepancies. I believe @jfh01 is onboard with this thinking.

@bhandzo
Copy link
Contributor

bhandzo commented Dec 10, 2019

Just to echo comments above:

  • Bird has had future hold reserves for quite some time, and a real % of these reservations don't result in trips.
  • Aligning the Provider and Agency events/state machines would be very helpful.

@dirkdk
Copy link
Contributor

dirkdk commented Dec 10, 2019

Just to echo comments above:

  • Bird has had future hold reserves for quite some time, and a real % of these reservations don't result in trips.
  • Aligning the Provider and Agency events/state machines would be very helpful.

thanks @bhandzo

does Bird report the future hold reservation as a status_change event of type reserved or exclude this completely?

@sarob sarob added the Agency Specific to the Agency API label Dec 19, 2019
@sarob sarob added this to the Future milestone Jan 8, 2020
@schnuerle
Copy link
Member

Noted on call that a change to the definition of reserved is breaking since it changes how older data is interpreted. Agree on that point.

For 0.5.0, adding some language to the definition that clarifies if reserved means the vehicles is on the street or not would be good, which is a key clarification for deployed vehicle counts and caps. I think it's implied it's on the street and not in motion, but should be clarified. For example, unavailable now says "A device is on the street but becomes unavailable for customer use." So reserved could say "A customer takes virtual or physical possession of a device on the street, and a trip has started or is about to start".

Long term I think reserved should become an event_type_reason underneath unavailable to make it clear that this device is on the street, but it's not available for someone to reserve/ride since it's in ride or reserved. To that point, seems like adding an on_trip event_type_reason would be good to distinguish if the reserved device is reserved and sitting there waiting, or on a trip. I could see scenarios where riders reserve a device they are not near yet via app, and even start paying, but don't get to the device for 5-10 minutes and start the ride.

@jfh01 recommended for now making a note that the reserved event_type could be interpreted different ways depending on the provider until it is clarified 0.5.0. @dirkdk may put this language in a new PR for 0.4.1.

@thekaveman
Copy link
Collaborator

Long term I think reserved should become an event_type_reason underneath unavailable to make it clear that this device is on the street, but it's not available for someone to reserve/ride since it's in ride or reserved.

My guess is that part of the current confusion comes from reserved event type being interpreted/considered as the reason, where considering the pair reserved:user_pick_up is important. Why is the device reserved (event_type)? Because a user picked it up (event_type_reason). You don't get one without the other.

This (mis)interpretation seems evident in the new PR #439 where the ambiguity note was added to the description for the event_type_reason instead of the event_type (thank you for working on this @dirkdk!)

All this to say: @schnuerle's proposed reorganization makes a lot of sense, and seems like a nice way to clear up this point of ambiguity. Similarly moving the reason user_pick_up under the event type unavailable makes for a nice symmetry with available:user_drop_off and avoids creating a new on_trip or similar reason.

@thekaveman
Copy link
Collaborator

Moving this to 1.0.0 as we'll (hopefully) take care of this confusion once and for all with the unified Agency/Provider status model.

@thekaveman thekaveman modified the milestones: Future, 1.0.0 Apr 9, 2020
@marie-x marie-x self-assigned this Apr 23, 2020
@marie-x
Copy link
Collaborator

marie-x commented May 27, 2020

@concept47 would you mind taking a quick peek at #506 and see if this resolves the issue for you? reserved ("future hold") becomes distinct from trip in both Agency and Provider.

@schnuerle schnuerle linked a pull request May 27, 2020 that will close this issue
@schnuerle
Copy link
Member

@concept47 Can you check out the PR #506? We will assume this addresses your concern unless we hear otherwise from you. We are working on finalizing these changes for the 1.0.0 release in the next 2 weeks. Thanks!

@concept47
Copy link
Author

concept47 commented Jun 5, 2020

Sorry have been distracted with recent events 😬
I absolutely will @karcass and @schnuerle. Thanks for taking care of it so quickly

@schnuerle
Copy link
Member

@concept47 We are finalizing #506 this week and will likely merge to dev tomorrow. Let us know if it addresses your needs here. Thanks!

@concept47
Copy link
Author

concept47 commented Jun 23, 2020

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

Thank you @schnuerle @karcass this looks good to me! 🎉

PS: I imagine reservation_canceled could be ambiguous about fitting in the above description of an event that marks the end of a trip?

@thekaveman
Copy link
Collaborator

@concept47 thanks for the feedback. The requirement for including the Trip UUID that you reference is only for events that take the vehicle into or out of the on_trip state. The reservation_canceled event takes the vehicle from the reserved state to the available state - it never gets to the on_trip (since there was no trip_start event). Whereas when a trip ends, we should have a trip_end or trip_cancel depending on what happens.

In #534 we're going to clarify this a bit more by saying:

Trip UUID (foreign key to Trips API), required if event_types contains trip_start, trip_end, trip_cancel, trip_enter_jurisdiction, or trip_leave_jurisdiction

Hopefully this clears up any ambiguity. Feel free to re-open this Issue if there are further questions.

@concept47
Copy link
Author

Perfect Thank you!

@marie-x
Copy link
Collaborator

marie-x commented Jun 27, 2020

Cities might want the more expansive definition and include these zero-length trips in the /trips endpoint as well, in order to get fee info. But we can adjudicate that in 1.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agency Specific to the Agency API Provider Specific to the Provider API question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants