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

Use pyhafas #35

Closed
Kurisudes opened this issue Dec 20, 2022 · 10 comments
Closed

Use pyhafas #35

Kurisudes opened this issue Dec 20, 2022 · 10 comments

Comments

@Kurisudes
Copy link

Kurisudes commented Dec 20, 2022

Hello,

Use HAFAS instead off scraping. It should be more reliable and approved by Home Assistant as proper integration again.

https://github.com/FahrplanDatenGarten/pyhafas

I am happy to try and help to change it. Do you see any issue in using pyhafas?
I am now trying to add it into schiene. If you can give me any hints, I'd be very happy.

@derhuerst
Copy link

related: #1

@Kurisudes
Copy link
Author

I have a solution for this now running.
But I do not know what the HTML link in detail is for. I don't know what I should do with it.
Also the price is supported in HAFAS but not supported in pyhafas.
If you agree to take this change, then I will also do a pull request for pyhafas as well.

@kennell
Copy link
Owner

kennell commented Dec 20, 2022

@Kurisudes Hello and thank you for the work/research done.

Some things:

  1. I don't know how feature-complete HAFAS - more specifically the pyhafas lib - is. It seems it supports delays, possibly also prices(?). Also not sure what sort of dependencies it has, i will look into it over the next few days.

  2. I need to look at how similar the data is that HAFAS gives versus Bahn.de as i don't want to have major breaks in icompatibility with existing code.

  3. Overall i am a bit skeptical of merging this. If pyhafas exists, why not use that directly? Because the pyhafas API is too inconvenient/non-intuitive or what is your motivation? Happy to hear your thoughts on this to understand what the motivation is :)

Again thank you and i will be looking into it, very busy right now and limited in time so please apologize any delay.

@Kurisudes
Copy link
Author

@kennell
Thanks for you quick answer.

  1. I added delays to the program already. As far as I tested, it is working perfectly and better as before because before delay was only showing for the first connection. Prices are supported with HAFAS but pyhafas does not support it. I think, I can change it in pyhafas. If you agree to merge my pull request, then I will do a pull request to pyhafas for that topic.
  2. HAFAS is the base for all searches on bahn.de as far as I know. So the data should be the same. Even in the ticket office, they are using HAFAS.
  3. True, it could be done completely separately. For me, this was the easiest way to fix the delay issue and to fix that Home Assistant does not want any scraping anymore. Additionally, I think, it is more stable way over HAFAS and because it is the same data, I can see no drawback. But not sure, wether I got the complete thing.

@derhuerst
Copy link

Overall i am a bit skeptical of merging this. If pyhafas exists, why not use that directly? Because the pyhafas API is too inconvenient/non-intuitive or what is your motivation? Happy to hear your thoughts on this to understand what the motivation is :)

  1. True, it could be done completely separately. For me, this was the easiest way to fix the delay issue and to fix that Home Assistant does not want any scraping anymore. Additionally, I think, it is more stable way over HAFAS and because it is the same data, I can see no drawback. But not sure, wether I got the complete thing.

As the author of hafas-client, which pyhafas has been ported from, I can attest the same: HAFAS is indeed a very stable API because it is hard for both HaCon and especially their customers to change it without breaking all those mobile apps out there.

Two drawbacks come to my mind though: While the HAFAS mgate.exe APIs are publicly available, without (reasonable) protection, they are not intended for usage by "random people on the internet", i.e. people outside of specific contracts allowing the API access. This has two consequences:

  • There might be legal problems with accessing the HAFAS API, given that it has some (arguably insignificant) protection mechanisms in place.
  • They might be able to block schiene (and/or the Home Assistant instances using on it) more easily than with web scraping.

Still I think that using the HAFAS API will likely never become a problem; Rather, it should make those tools based on it more stable than scraping-based ones. Both the MagicMirror public transport module as well as the ioBroker adapter has used it for a while now. cc @KristjanESPERANTO @gaudes

@FaserF
Copy link
Contributor

FaserF commented Dec 20, 2022

@Kurisudes i don’t think this should get merged, even if your work really is great, good and I also understand the intention about it.

I think hafas should stay separated. And for Homeassistant a complete new integration should be done.
There is already a pull request which would implement exactly your feature:
home-assistant/core#78839

Unfortunately it did not get merged yet.

@FaserF
Copy link
Contributor

FaserF commented Dec 20, 2022

And if you just want to get the existing code working again with HA, you could just use my custom integration if you want:
https://github.com/FaserF/ha-deutschebahn

@kennell
Copy link
Owner

kennell commented Dec 20, 2022

@FaserF Yes im also leaning towards keeping the code of schiene based on web scraping, the way it is.

There are HAFAS-based projects out there, free to use for those who need it. I consider this project to be more off a backup/alternative if the official APIs ever get blocked and/or reduced.

@Kurisudes
Copy link
Author

I understand your argument.
It is a little disappointing for me but okay.

@Kurisudes
Copy link
Author

Kurisudes commented Dec 21, 2022

@FaserF

I think hafas should stay separated. And for Homeassistant a complete new integration should be done. There is already a pull request which would implement exactly your feature: home-assistant/core#78839

Unfortunately it did not get merged yet.

Thanks, I could not find this pull request but this looks promising. Unfortunately, it is already very old.

@Kurisudes Kurisudes closed this as not planned Won't fix, can't repro, duplicate, stale Dec 22, 2022
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

No branches or pull requests

4 participants