home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
71.4k stars 29.9k forks source link

MVG Live integration is outdated and partially broken #81760

Closed mondbaron closed 3 months ago

mondbaron commented 1 year ago

The problem

The mvglive integration is build on top of the PyMVGLive module which has not been updated since May 2017. This leads to some problems documented in an issue from 2018 which remains open. This results in problems with stations, which have been changed since then. For Example "Hauptbahnhof" still returns valid results while "Pasing" does not.

When those broken stations are configured, Home Assistant displays Returned data not understood in the logs. I provided a minimal working YAML config with both, a working and a broken station.

What version of Home Assistant Core has the issue?

2022.11.1

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

mvglive

Link to integration documentation on our website

https://www.home-assistant.io/integrations/mvglive/

Diagnostics information

No response

Example YAML snippet

sensor:
  - platform: mvglive
    nextdeparture:
      - station: Hauptbahnhof
        name: mvg_hauptbahnhof
        number: 3
      - station: Pasing
        name: mvg_pasing
        number: 3

Anything in the logs that might be useful for us?

Logger: homeassistant.components.mvglive.sensor
Source: components/mvglive/sensor.py:148
Integration: mvglive (documentation, issues)
First occurred: 07:02:49 (7 occurrences)
Last logged: 07:05:49

Returned data not understood

Additional information

No response

home-assistant[bot] commented 1 year ago

mvglive documentation mvglive source

mondbaron commented 1 year ago

If I may add some suggestions:

I would also kindly offer my help and could prepare a pull request, as there is no code owner at the moment.

leftshift commented 1 year ago

Hey, cool to see people still stumble over mvg-api!

Feel free to switch to it as a dependency, but yeah, it's also totally fine by me if you want to use it as a base to figure out how to call the API and just do the calls yourself. Thanks for offering attribution in that case :)

One last note, my pronouns are 'she' or 'they', so it'd be 'give her some credit' for example.

mondbaron commented 1 year ago

Cool, thanks for the work and support! Pronoun edited, sorry for that :) The LICENSE in your project says Copyright (c) 2016 Adrian Schneider, could you check, if this is correct or leave a note, how you'd like to be attributed?

leftshift commented 1 year ago

Ouh, thanks for the heads-up, completely missed that. Yeah, I don't use that name anymore.

Just use my github handle leftshift for attribution :)

mondbaron commented 1 year ago

I meanwhile made some progress by developing the new PyPI package mvgapi (https://github.com/mondbaron/mvgapi) using the more recent API of the MVG Website. Asynchronous methods and calls are included to keep Home Assistant responsive when using the integration on top of it. Let me know if there are already comments on that, while I'll proceed to develop the "new" integration :)

Leopoldpaul commented 1 year ago

Hey i would love to see your new integration for Home Assistant. What is the state of development for the Home Assistant integration?

mondbaron commented 1 year ago

Glad to hear, there is some interest in this - this increases the motivation :) The PyPI package feels ready for the moment and I started working on the integration, which will be a complete rewrite to match the recent styleguide for integrations and include a nice config dialog to select the station and filter transport types.

But I could need some feedback: I still feel unsure about the way of presenting the results. At the moment my draft creates a sensor for each departure slot with its state value beeing the actual departure time. Additional information (e.g. line name, final destination, delay, ...) is added to that sensor as additional attributes. The integration will be a "hub", so it should be possible to add multiple configurations (e.g. different stations or different transport types) as separate devices. Do you think this would be convenient?

hoeni commented 1 year ago

Glad to hear, there is some interest in this - this increases the motivation :)

+1 :-)

The integration will be a "hub", so it should be possible to add multiple configurations (e.g. different stations or different transport types) as separate devices. Do you think this would be convenient?

In my case, it's only one station to watch, but I could imagine living between two or three station would make this a helpful feature :-)

Leopoldpaul commented 1 year ago

I currently need two stations. One only the bus and the other i use for the Subway.

Leopoldpaul commented 1 year ago

I think the Api is down or gone.

vpanichkin commented 1 year ago

Yeap, @Leopoldpaul. You are totally right. The maintainer of the old API package suggests to migrate to another package, thought I didn't go into deep research

manuelh19 commented 1 year ago

Glad to hear, there is some interest in this - this increases the motivation :) The PyPI package feels ready for the moment and I started working on the integration, which will be a complete rewrite to match the recent styleguide for integrations and include a nice config dialog to select the station and filter transport types.

But I could need some feedback: I still feel unsure about the way of presenting the results. At the moment my draft creates a sensor for each departure slot with its state value beeing the actual departure time. Additional information (e.g. line name, final destination, delay, ...) is added to that sensor as additional attributes. The integration will be a "hub", so it should be possible to add multiple configurations (e.g. different stations or different transport types) as separate devices. Do you think this would be convenient?

Thank you so much for your efforts already, really looking forward to this!

And yes, would definitely be useful to me as I am living close to 2 departures I take regularly

mondbaron commented 1 year ago

I can confirm the old mvglive integration has more and more problems. Luckily the mentioned new pypi package doesn't seem to be affected by recent changes and is still working well, so I'm pushing my efforts towards the release of a new package called mvgapi to allow a smooth migration for everyone.

mondbaron commented 1 year ago

Progress update: Today I released the new underlying python package mvg on PyPI and the HA config dialog seems to work well with it. You'll be able to either choose a station from list or select the nearest from coordinates. Configured HA coordinates are used as default. Results can be filtered by transport type and limited to a number of departures. Optionally an offset (e.g. walking distance) can be set to filter departures you won't be able to catch any more:

grafik

Leopoldpaul commented 1 year ago

@mondbaron Is there already a proper Home Assistant way to install this (HACS, etc.)?

richardwonka commented 1 year ago

Not sure what the status of this is, I just found the old integration and found it not working.

One feature I like from the old ‘deutschebahn’ integration is that it also offers the current delay, both as an attribute and as a ‘+n’ in the state string.

Seeing that I have to use S1 a lot, this is highly relevant to me. 😆😂😭

hoeni commented 1 year ago

Seeing that I have to use S1 a lot, this is highly relevant to me. 😆😂😭

If you can survive S1, you can survive everything ✌️

vpanichkin commented 1 year ago

Hey, @mondbaron. Great achievement so far. Could you provide info how can we setup full component as on the screenshot above?

Mika255 commented 1 year ago

Looking forward to this!

vpanichkin commented 1 year ago

Well, as I didn't see any further progress on this, I decided to fix at least old integration on my own. For that I took the new mvgapi library and patched old mvg live extension

Disclaimer: I don't have any Python knowledge, hence it's not ideal solution, but it works.

How to use it?

  1. Create a backup (I know, 99% of people will skip this step, but you acknowledge 😅)
  2. Using File Editor, adjust 2 files in config/custom_components/mvglive 2.1. Substitute old manifest with the new version 2.2. Substitute old sensor.yaml with the new version
  3. Restart home assistant
  4. You are brilliant, your sensors should be back again :)

Small favour

I spent on this solution ~6 hours. I fully understand that helped the community and don't demand anything from you. However, I would really appreciate if you donate a small amount of money to support Ukrainian Army🇺🇦 to make my home not only "smart", but "peaceful" again :)

🙏Charity link🙏: Visa, Mastercard, Google and Apple Pay are available

Side notes:

  1. There shouldn't be any changes to configuration.yaml needed (all previous configuration are reusable)
  2. In the extra attributes there is a new additionall parameter: "cancelled". You can use it to see if your transport was cancelled

Troubleshooting:

You still might have errors with not getting data. Check:

  1. In case you use products, lines or directions in configuration.yaml - they all need to be a array, even if single value, e.g ["U2"]

Screenshots:

  1. Example of mvglive _customcomponents folder to be edited image
  2. Extra property cancelled is exposed image
richardwonka commented 1 year ago

[…]

  1. Using File Editor, adjust 2 files in config/custom_components/mvglive 2.1. Substitute old manifest with the new version 2.2. Substitute old sensor.yaml with the new version […]

Could you submit this as a pull request to the project?

That would make it easy for the devs to incorporate your fix for everyone.

Uninator commented 1 year ago

How to use it?

  1. Create a backup (I know, 99% of people will skip this step, but you acknowledge 😅)
  2. Using File Editor, adjust 2 files in config/custom_components/mvglive

I was a little confused since I neither didn't have the folder nor the files. I simply created them manually and now everything works fine! Thank you for your work!!!

hubdihubdi commented 1 year ago

Hi, thanks for the description. But in my case it does not work. i also didn't have the folder and created it with the new input. but still the error in the logs. Logger: homeassistant.components.mvglive.sensor Source: components/mvglive/sensor.py:148 Integration: mvglive (documentation, issues) First occurred: 17:51:38 (52 occurrences) Last logged: 18:04:24

Returned data not understood here its written about components I have only the folder custom_components

JDTm commented 1 year ago

Hi, "Directions:" is not working anymore: how do i get the correct direction? Are delays directly included? which is the best card at lovelace to get the information?

Thanks for the updated version!

gpgmailencrypt commented 1 year ago

Just a small hint: line 145 in sensor.py should be self._state = None and not self._state = "-"

fellnerse commented 1 year ago

I also added some hours and created a pr here: https://github.com/home-assistant/core/pull/97271

hille721 commented 1 year ago

Progress update: Today I released the new underlying python package mvg on PyPI and the HA config dialog seems to work well with it. You'll be able to either choose a station from list or select the nearest from coordinates. Configured HA coordinates are used as default. Results can be filtered by transport type and limited to a number of departures. Optionally an offset (e.g. walking distance) can be set to filter departures you won't be able to catch any more:

@mondbaron : any updates here? this looks already really good.

And in general, the current implementation is not only "partially" broken, but complete broken. Thus a first quick fix as proposed from @vpanichkin would be great, then there can be still improvements in the future. I can also see that @fellnerse already provided this quick fix as PR (#97271) but also there seems to be no progress :(

issue-triage-workflows[bot] commented 10 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.

hille721 commented 10 months ago

issue still exists. But there is a WIP PR to solve it #97271

kimberlyeet commented 7 months ago

any updates on this? @mondbaron

eloo commented 6 months ago

sadly this integration is still broken... also the PR is closed as stale already

really sad that official integrations are not being supported in any way

hille721 commented 6 months ago

sadly this integration is still broken... also the PR is closed as stale already

really sad that official integrations are not being supported in any way

yeah, the PR was not directly approved as they don't want to break something (https://github.com/home-assistant/core/pull/97271#pullrequestreview-1734508587).

If I have a bit of time I can create a new PR where I try to satisfy the expectations.

hille721 commented 6 months ago

in the meanwhile you can fix it easily by yourself. Solutions were provided in this thread and there is now also a HACS integration from @danielpotthast (https://github.com/danielpotthast/mvg)

issue-triage-workflows[bot] commented 3 months ago

There hasn't been any activity on this issue recently. Due to the high number of incoming GitHub notifications, we have to clean some of the old issues, as many of them have already been resolved with the latest updates. Please make sure to update to the latest Home Assistant version and check if that solves the issue. Let us know if that works for you by adding a comment 👍 This issue has now been marked as stale and will be closed if no further activity occurs. Thank you for your contributions.