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

Add DB-Movas Provider #614

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

traines-source
Copy link

@traines-source traines-source commented Dec 20, 2024

As discussed in #610, I have now implemented the DB Vendo/Movas API.
The tests are green and I have manually tested with Oeffi, everything looks fine, but more testing surely can't harm.
Some notes:

  • The nearby endpoint doesn't allow for POI search, but that doesn't seem to be surfaced in Oeffi anyways
  • The departure boards are limited to one hour, but I think this is OK
  • More messages/remarks/notices seem to be shown than before, although I'm only surfacing the important ones (usually disruptions, but also static ones like phone numbers for on demand buses etc. With the legacy DB Provider, some "Rufbus" were not distinguishable from normal buses, e.g. from Rothenburg o.d.T. to Dombühl at 20:31).
  • The cheapest available adult price is shown.
  • Error handling is not (yet) very granular

I noticed that Oeffi always gulps the last departure on the departure boards, because in StationDetailsActivity.java:438 it should be selectedDepartures.size()+1 due to position 0 being a dummy position. This becomes much more obvious due to the one hour limitation of the new API (and at this time of the day :).

Let me know if you also want a PR for Oeffi, however I've only quickly added the new provider and one would need to decide whether one wants to completely replace the old DB Provider (ironically, right now, the HAFAS API seems to work beautifully again, let's see for how long...)

@schildbach
Copy link
Owner

Huge thanks for this PR!

The nearby endpoint doesn't allow for POI search, but that doesn't seem to be surfaced in Oeffi anyways

That's right. "Öffi Stations" is only about stations/stops, not POIs or street addresses.

The departure boards are limited to one hour, but I think this is OK

Is that a limit from the API? I think 1 hour is very short, given that delays can easily be more than one hour. But of course it's better than nothing. Maybe in future we can ask DB to extend it.

More messages/remarks/notices seem to be shown than before, although I'm only surfacing the important ones

The old messages/categories were badly curated (by me, ages ago), so feel free to apply whatever filter feels right. We don't need to copy old behaviour here.

The cheapest available adult price is shown.

Cool! We never had prices with DB, afair.

Error handling is not (yet) very granular

Yes, that's expected. There is never any API doc about errors, so we need to fine-tune them over the coming weeks and months.

Let me know if you also want a PR for Oeffi

Sure, feel free! But I can also look after this bug if you prefer.

one would need to decide whether one wants to completely replace the old DB Provider

This is a very important decision indeed. From my gut feeling, I'd like to stay with the old provider as long as it works reasonably reliable. The HCI implementation has proven to be pretty solid over the years.

We could maybe provide the Movas API as an alpha alongside HCI, to help testing? If the HCI is switched off permanently, we can migrate everything to just one "DB" network ID. Related: Do you know if stations IDs have changed between the APIs? If so, have you witnessed any pattern, like adding a fixed constant?

I should have brought this up earlier, it's maybe too late now: Currently the PTE uses the old (and probably deprecated?) Android JSON API (org.json.JSONObject). At some point – and we could have started with this impl – I think it would make sense to migrate to something more modern. I always had Moshi in mind, because it is built on OkIo. But I'm not sure if it's still the right choice – we also have access to almost all of JDK 8 API nowadays, and in maybe 2 years JDK 11+.

@traines-source
Copy link
Author

traines-source commented Dec 21, 2024

Is that a limit from the API? I think 1 hour is very short

At least I haven't yet found out a parameter with which to influence it. One could of course do multiple requests under certain conditions (such as not enough departures returned), but there is not even a laterRef or something, DB Navigator just requests for one hour later if you click on "later" 🙈

Maybe in future we can ask DB to extend it.

Maybe you have better contacts into DB, from my experience they won't do something like that. On a side note, I haven't asked for permission so far to use the API, but at least it's without any API Key this time. Or one could switch to another DB API for the boards that support longer durations, but which have other drawbacks:

  • RIS::Stations API, needs an API Key
  • bahnhof.de API, which only allows querying for the current datetime (which would be fine for Oeffi I guess)
  • regio-guide.de API, which doesn't surface cancelled trips (which is currently also the behavior of Oeffi/DB Provider) and no messages/notices either

Which is why already for the sake of consistency I had opted for the API that DB Navigator uses for both routing and boards. And I personally would propose to show cancelled trips in the departure board, even if there is no cancelled flag in PTE for them, at least they now reliably show "Halt entfällt"/"Stop cancelled".

We could maybe provide the Movas API as an alpha alongside HCI, to help testing?

Sounds good, I'll make a PR for Oeffi then since that's basically what I've done. – EDIT: don't want to go through the hassle of creating a Gitlab account for these five lines, a git patch instead: dbmovas.patch.txt

Do you know if stations IDs have changed between the APIs?

It still uses the EVA numbers and in the queries the lid created out of that. But for POI and ADDRESS, PTE before used the complete lid as an id as far as I can tell. I can change it to also use that, then the serialized locations should be compatible I guess.

Android JSON API (org.json.JSONObject)

Indeed I've used it throughout and since it does not seem to be deprecated and it works nicely, I am a bit reluctant to change everything now :D But if you want I can have a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants