-
Notifications
You must be signed in to change notification settings - Fork 137
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
Conversation
Huge thanks for this PR!
That's right. "Öffi Stations" is only about stations/stops, not POIs or street addresses.
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.
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.
Cool! We never had prices with DB, afair.
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.
Sure, feel free! But I can also look after this bug if you prefer.
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+. |
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
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:
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".
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
It still uses the EVA numbers and in the queries the
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. |
I think it has happened, DB HAFAS mgate.exe has been shut off for good, at least it now returns 503. If that is true, it probably doesn't make a lot of sense to keep the old DB Provider. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks really good overall.
f439e98
to
8a2c1e5
Compare
I just tested this against Öffi, and it looks very good. Sadly I didn't have addresses or POIs in my history, but stations are perfectly compatible. |
Well, I guess we'll have to hope for the best with addresses then (but I think it should work, and it's a nice to have, not critical anyways IMHO). |
Feel free to rebase and squash, and add any last amendments.On Jan 10, 2025 16:05, Traines ***@***.***> wrote:
Well, I guess we'll have to hope for the best with addresses then (but I think it should work, and it's a nice to have, not critical anyways IMHO).
I suppose you have then done the rebasing and will do the amendments to ResultHeader and comments regarding request headers yourself, otherwise lmk.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: ***@***.***>
|
… transfer type, other minor fixes
8a2c1e5
to
4fe6602
Compare
Ok done. Haven't tested with Oeffi again, but (new) tests are green, and I have only changed the |
Merged, and submitted a new version of Öffi to Google Play. I'll monitor exceptions. Thanks again for your work! |
Nice 👍 However, as far as I can see, you haven't fixed the |
It's live on the test channel already. I'll look into the departures issue soon, I expect at least one quick follow-up release anyway. One particular thing I need to monitor is which devices will maybe choke on the Java streaming API (which wasn't used in PTE up to now). |
Ah I didn't realize streams are such an issue, even though being on Java 8. Haven't done any Android development in a long time... Then we should just have replaced them with plain old Java I guess. |
Well let's see. It would be heartbreaking to see your streams code being converted to old loops. |
I just verified Android 8.1 works fine with streams. So I assume 8.0 does, too, because it's just a minor version bump between the two. So only Android 7/7.1 remains unknown for now. |
Well according to the docs it should work starting API level 24, so Android 7.0 should be fine, right? I was rather worried about other projects that are using PTE and that maybe haven't yet upgraded to minSdk 24. |
Does the API limit include station board requests? If yes, a single user could hit that limit if scrolling a lot. Regarding |
Answering my own Q: That looks like two version 4 UUIDs concatenated, separated by an underscore. |
Jup thats exactly how marudor (and I in db-vendo-client now) does it, too 👍 |
I found an incompatibility: Would you like to have a look at this PR, which converts it to old-style DateFormat? |
@traines-source Another Java 8-ism: #623 Would you like to have a look at the proposed replacement? |
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:
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...)