Implement MOTIS provider and Transitous - #713
dancojocaru2000 wants to merge 7 commits into
Conversation
6d757f9 to
f19c9b4
Compare
|
Very cool! I hope that this way we can finally get Transitous in Öffi etc.! |
schildbach
left a comment
There was a problem hiding this comment.
Thanks for your contribution! I did a first review, and this generally looks very good!
It's probably too late now, but do you like the org.json API? Would you have preferred using a different JSON API?
|
Unit tests pass as well 😃 |
|
|
a3f1bd9 to
d0e9973
Compare
|
One issue that has been brought to my attention which needs to be addressed is that the Transitous API Usage has the following requirements:
I'm unsure how to handle this in the library, given that the application which uses the library (Öffi, Transportr, etc) will have to set its own user agent. |
Co-authored-by: santawho <196087559+santawho@users.noreply.github.com>
MOTIS returns only one station ID corresponding to one dataset, but fetching the departures of that ID only returns the departures from that dataset. That means departures from the other datasets would get left out.
Yes, the apps need to call I think it would be nice to mention this requirement (maybe link to it) in the class-level JavaDoc, otherwise people maybe simply miss it. |
|
Would it be reasonable to set a default bad user agent (something like |
Rather than setting a weird user agent, we could also check against the default But it feels to me this is a bit over the top anyway. |
|
Then I'll add the JavaDoc and undraft the PR. |
|
I think over at santawho#3 people added some more tweaks that are relevant, e.g. making |
|
I just tried this branch in Öffi. See https://gitlab.com/oeffi/oeffi/-/tree/motis?ref_type=heads if interested. Öffi Stations:
Öffi Directions:
I'll investigate and will report back. |
| protected String nextPageCursor; | ||
| @Nullable | ||
| protected String previousPageCursor; | ||
| protected HttpUrl endpoint; |
There was a problem hiding this comment.
Sadly the OkHttp HttpUrl class is not serializable, while QueryContext is (and Serialization is used by Öffi).
I see these options:
- just store the URL as String – probably easiest
- keep HttpUrl but implement custom serialization for that field – somewhat ugly
- somehow get rid of Serialization completely (in Öffi, but probably other consumers too) – in this case, no changes necessary
Does Transitious actually have departure times (live or planned)? On https://transitous.org/ I only see trip planning, not any station boards. Edit: I just searched for "Hauptbahnhof" in Öffi Stations and got somewhat random 10 Hauptbahnhöfe, with 9 of them showing no departures. "Gera Hauptbahnhof" however shows a bunch and they are nicely formatted. However most departures are hours ago – not that useful. |
|
Transitous does have departure times, see e.g. https://api.transitous.org/?stopArriveBy=false&stopId=de-DELFI_de%3A05915%3A7756_G&time=2026-05-30T17%3A29%3A00.000Z (this is a link to the web UI, which uses the stoptimes API, as does this PR). I'm not sure what issues you are facing, in Germany the coverage should be mostly complete and over in ÖffiNG everything seems to work as well. (Also it can be beneficial to use the new For the nearby view, instead of the reverse-geocode endpoint it is probably better to also use the stoptimes endpoint with a larger radius or the /map/stops endpoint (which only includes the stops and no departures). Reverse-geocode is hardcoded to 500 meters radius indeed. |
|
This PR uses this endpoint for departure times: This example station (Stuttgart Hauptfriedhof) yields "no departures" in Öffi, despite including a list of 180 "stop times". However I don't actually see any date/time field in these stop times (but I'm new to the API so I just might oversee things). Edit: Ah, it's (somewhat unintuitively) in the |
|
The times are contained in |
| final JSONObject place = stopTime.getJSONObject("place"); | ||
| final String departureStopId = place.getString("stopId"); | ||
|
|
||
| if (!equivs && !stationId.equals(departureStopId)) { |
There was a problem hiding this comment.
I think this conditional fails way more often than it should, leading to no departures being returned if equivs is false (always false in Öffi Departures).
Would it be an option to also test againt parentId, or would that lead to too many results?
Yes, I just came to the same conclusions.
I'd be happy to remove the client-side filter, if that were possible. It would also save resources. So are you saying that if we remove the |
| endpointBuilder.setQueryParameter("window", String.valueOf(3600)); | ||
| } | ||
| if (time != null) { | ||
| endpointBuilder.addQueryParameter("time", new SimpleDateFormat("yyyy-MM-dd'T'h:m:ss.SZ").format(time)); |
There was a problem hiding this comment.
The time format is incorrect. Please use HH for hours to use 24 hour format. And mm for minutes so that it's always two digits.
About the S: Does motis really support milliseconds? Their examples don't imply so. I'd consider removing it.
About the Z: Have you considered passing in time as UTC always? In that case, you'd need to pass a literal Z as timezone.
| throw new IllegalArgumentException("to needs to be stop or have coordinates: " + to); | ||
| } | ||
|
|
||
| endpointBuilder.addQueryParameter("time", new SimpleDateFormat("yyyy-MM-dd'T'h:m:ss.SZ").format(date)); |
|
Yes it's unfortunately a bit complicated. And just today with the new MOTIS release deployed to Transitous, the equivalence logic has been considerably improved. I've thought about it again. For both
|
|
I just worked on some patches to However, I now seriously consider removing Any thoughts about this change? It would not affect |
|
Idk I think it is a useful distinction, depending on the usecase, and something that all timetable information systems do in one way or another. Just that the definition of "equivalent" and "same" is of course very vague and depending on use case as well. Also in some sense it would be a breaking change to PTE. |
I know the pain of changing stop IDs. It has always been a problem with the other APIs as well. However, geo-coordinates change too. As long as At the moment I suspect it might be a problem to use complex structures like a map with a geo-coordinates as a station ID in Öffi. |
|
The detour via |
I fear this will fail in too many ways, and be a pain to maintain – especially world-wide. I just tested removing the
So I wonder what's the specific reason for including a 50 meters radius in the query? |
|
50 meter radius would just be needed when using |
|
I propose PR #723 which removes the If/when it is merged, I can add the necessary changes to this PR. |
|
Hi @dancojocaru2000, hi @schildbach, I'd like to help move this PR forward. I'm particularly interested in using Transitous/MOTIS for Seattle, so I wanted to test the provider against a region outside Europe and report back with something reproducible. Based on this thread, I backported a few small fixes on top of this branch, verified with unit tests and a live run against
I also added two live test cases to I deliberately did not touch See this branch for details: https://github.com/cyroxx/public-transport-enabler/tree/motis Please let me know how you'd like to proceed. |
I got hyperfixated so I spent the night implementing a MOTIS provider.