openmobilityfoundation / mobility-data-specification

A data standard to enable right-of-way regulation and two-way communication between mobility companies and local governments.
https://www.openmobilityfoundation.org/about-mds/
Other
676 stars 232 forks source link

Provider/Agency API alignment #796

Closed marie-x closed 1 year ago

marie-x commented 1 year ago

Explain pull request

Agency and Provider have been converging with each iteration. Time to fully align data structures and harmonize the APIs.

Is this a breaking change

A breaking change would require consumers or implementors of the API to modify their code for it to continue to function (ex: renaming of a required field or the change in data type of an existing field). A non-breaking change would allow existing code to continue to function (ex: addition of an optional field or the creation of a new optional endpoint).

Impacted Spec

Which spec(s) will this pull request impact?

Additional context

Github issue: https://github.com/openmobilityfoundation/mobility-data-specification/issues/759

Task force link: https://github.com/openmobilityfoundation/mobility-data-specification/wiki/Unification-WG-Task-Force

Design doc: https://docs.google.com/document/d/134DrPFjSpapW1auTT2M_BuL2E9tdO1At0yu6VII1cPA/edit#

marie-x commented 1 year ago

Still a fair amount to do - most changes are in the Provider doc, I'll come back to clean up Agency after we get a good look at the proposed Provider changes. Also there are a few open questions, denoted with (?) in the PR.

pierre-bouffort commented 1 year ago

Hello all, thanks a lot for the work that was put in this PR. I think it's is very good overall.

2 quick thoughts and comments:

marie-x commented 1 year ago

Good catches. I will explain the proposed recent/historical split at today's review. Omitting telemetry trip_id was an oversight which I have remedied.

alexdemisch commented 1 year ago

The current SFMTA taxi spec has a telemetry API very similar to the Agency telemetry endpoint. The SFMTA spec includes a driver_status and vehicle_status field, which makes it much easier to analyze telemetry data.

The driver_status values are:

And the vehicle_status values are:

Some of the use cases are cross-checking whether we have received trip data in all cases where the vehicle was 'hired', distinguish between dead-heading and just off-duty driving, and analyze driver shifts, including (but not limited to) detecting cases of excessive driving without breaks

The PR has a trip_id, which would allow us to identify telemetry records that are related to a revenue trip, there’s not an easy way to identify other statuses. One way to achieve this would be to have last_vehicle_state and last_event_types fields a la the vehicles endpoint. I’m not sure how much of a lift this would be on the provider side, or if there are other ways to get at this that don’t involve trying to cross reference the telemetry and events streams.

pierre-bouffort commented 1 year ago

Read through this PR again, and it seems fine by me. I already have expressed my doubts about the split between historical and recent events in the Provider API, but this has already been discussed, and if I'm the only one with this concern, I will obviously align with the general opinion. Thanks for your work here @marie-x :-)

schnuerle commented 1 year ago

It would be great to get this out of draft and ready to merge to dev by Monday Dec 19.

@marie-x could you complete the open work and questions to the best of your ability, and leave inline comments or general comments for areas you weren't sure of?

There are a number of conflicts to be resolved now too.

Also please double check links across the spec that now should go to the data types or general information page, and check reference links which could be broken now.

marie-x commented 1 year ago

I'm aiming for mergeable by end of year.

schnuerle commented 1 year ago

Could you review #740 and leave a comment there if you think it's ok to merge with the work there?

Also there are some conflicts to resolve if you can because of merging some other work.

pierre-bouffort commented 1 year ago

Are still looking at adding the surface location type now that the unification PR is mature ? https://github.com/openmobilityfoundation/mobility-data-specification/issues/767

schnuerle commented 1 year ago

Are still looking at adding the surface location type now that the unification PR is mature ? #767

Yes we can still work this in (and related ones) once #767 is merged to dev (still in progress). cc @marie-x

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.