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

Agency: additional events #275

Conversation

lionel-panhaleux
Copy link
Contributor

At the end of a trip or a maintenance, a vehicle may be unavailable.
The service_start transition signals the vehicle is available again, it can be used by programs operating 24/7.

Note this is missing a matching state machine diagram.

See #274

At the end of a trip or a maintenance,  a vehicle may be unavailable.
The `service_start` transition signals the vehicle is available again,
it can be used by programs operating 24/7.
@marie-x
Copy link
Collaborator

marie-x commented Apr 30, 2019

Definitely addresses a need. Trying to be sure that this is the best way to address that need. Don't yet have a counter-proposal. Will consider with @whereissean and @toddapetersen.

@marie-x marie-x self-requested a review April 30, 2019 01:20
@marie-x
Copy link
Collaborator

marie-x commented May 2, 2019

I have a counter-proposal. I think we should instead just add a transition from trip to unavailable via service_end.

@marie-x
Copy link
Collaborator

marie-x commented Jun 19, 2019

I wrote:

I have a counter-proposal. I think we should instead just add a transition from trip to unavailable via service_end.

Actually I have what I think is a full enumeration of the possibilities for resolving this, none of which I'm in love with:

  1. leave as-is (decompose trip -> trip_end -> unavailable into trip -> trip_end -> available -> service_end -> unavailable with, say 1ms delay)
  2. allow trip -> service_end -> unavailable
  3. create new event trip -> trip_end_unavailable -> unavailable
  4. use not just event_type but event_type_reason to determine the state change caused by trip_end
  5. change state machine such that trip_end puts you into unavailable such that you need an additional service_start
  6. variant on 1), decompose to multiple events with zero MS delay. pro: minimal change, zero time spent in available; con: requires some additional ordering method beyond timestamp (non-trivial)

I'm interested in how folks would rank these solutions, or are there others I've missed? @whereissean @hunterowens @lionel-panhaleux

@lionel-panhaleux
Copy link
Contributor Author

If I had to grade these alternatives, I'd say:

  • 2 is best because it is simpler. From an analytics point of view, a specific event reason would be nice (ie. technical_check, availability_check, or something similar).
  • 3 is good enough, but slightly more complicated because it adds a new event
  • 4 looks too tricky: a single events with two possible status seems bug-prone to me.
  • Definitely not 1. The 1ms delay is problematic in some edge cases. Plus the vehicles will still be seen as available for 1ms (in analytics and compliance), although they were not.

@marie-x
Copy link
Collaborator

marie-x commented Jun 27, 2019

@lionel-panhaleux I just remembered there's a fifth option, added for your grading. Though in some ways is suffers the same inherent pathology as option one.

My problem with option two (trip -> service_end -> unavailable) is that it makes identifying the end of the trip harder, when generating Trips. OTOH we're already in a place where not all trips have trip_end; some effectively end with a trip_leave.

@rf-
Copy link
Contributor

rf- commented Jun 27, 2019

There's also another option, which is to preserve the semantics of option 1 (modeling a single real-world event as a sequence of trip_end followed by service_end) but with identical timestamps, so that the intermediate state is zero seconds long. This is more semantically accurate but means there would need to be some way to correctly identify the ordering of the events, which could happen a few different ways:

  1. Allow some kind of compound event record that includes multiple sub-events
  2. Add a sequence identifier that's separate from the timestamp and disambiguates the order of simultaneous events
  3. Define heuristics in the spec that everyone can use consistently to identify the correct sequence based on event type, etc., when there are multiple simultaneous events

I'm not sure how appealing any of these options are in agency, or if there are other alternatives, but it would be nice to avoid the need for pseudo-events like trip_end_unavailable while still preserving each individual event's metadata.

@marie-x
Copy link
Collaborator

marie-x commented Jun 27, 2019

@rf- added to list. The three sub-options have new implications that we'd have to wade through. Each seems to add more complexity than it removes, seems to me.

@rf-
Copy link
Contributor

rf- commented Jun 27, 2019

Yeah, I'm not sure I find any of them super compelling, but that's also kind of how I feel about all the other options too. The core idea—having a relatively small set of events, allowing them to stack up on the same timestamp (consistent with the normal rules about state transitions), and then leaving the vehicle in the state indicated by the last event—seems like it has some nice properties, but I'm not sure anyone has come up with the right way to deal with the sequencing problem yet.

@hunterowens hunterowens added this to the 0.4.0 milestone Jun 28, 2019
@hunterowens hunterowens modified the milestones: 0.4.0 , 0.4.1 Oct 16, 2019
@CLAassistant
Copy link

CLAassistant commented Nov 6, 2019

CLA assistant check
All committers have signed the CLA.

@sarob sarob added Agency Specific to the Agency API enhancement New feature or request labels Dec 19, 2019
@HenriJ HenriJ assigned HenriJ and unassigned HenriJ Jan 3, 2020
@Retzoh
Copy link
Contributor

Retzoh commented Jan 3, 2020

I can follow up on this

@HenriJ
Copy link

HenriJ commented Jan 9, 2020

2020-01-09 WG meeting:

  • Add a new event trip_end_unavailable <- E&A, BlueSystems, Dirk
  • Maybe a new "reason-like" field ?

Morgan: should be a minor version because it's adding something ?

@Retzoh
Copy link
Contributor

Retzoh commented Jan 9, 2020

Until #422 is solved, there is no versioning on the Agency API.
So for now, I feel any of the following changes would be breaking:

  • add a trip_end_unavailable event_type -> agencies with old implementations would miss trip ends.
  • add a reason to trip_end deciding the status -> agencies would still get trip ends but the device would be in the wrong status.
  • add a state -> one of the two changes above is still needed since states are not explicitely sent in agency events.

@Retzoh
Copy link
Contributor

Retzoh commented Feb 26, 2020

@sarob as explained in #274 , this PR should be moved to the 0.5.0 milestone.

@thekaveman thekaveman modified the milestones: 0.4.1, Future Feb 26, 2020
@marie-x
Copy link
Collaborator

marie-x commented Mar 2, 2020

Propose we close this in favor of the Provider/Agency reconciliation project

@schnuerle
Copy link
Member

Ok closing this per the recommendation above in favor of the now completed reconciliation work.

@schnuerle schnuerle closed this Sep 29, 2020
@tonial tonial deleted the lionel-panhaleux/missing_events branch May 27, 2021 11:57
@schnuerle schnuerle removed this from the Future milestone Sep 2, 2021
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 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants