-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
mvglive bug fixes and improvements #6953
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
Conversation
This pull requests builds on the first work with the mvglive 8000 sensor: - Refactoring the code so that multiple sensors for departures can be added - Rewrites the transport mode restrictions ("products") to be more modular - Fixes bugs, such as missing implementation of line restriction - Other improvements, such as including data attribution
@mountainsandcode, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fabaff and @DavidMStraub to be potential reviewers. |
It seems you haven't yet signed a CLA. Please do so here. Once you do that we will be able to review and accept this pull request. Thanks! |
- The API returns the property 'direction', which can be used to filter U-Bahn trains by direction without having to enter all final destinations - The sensor icon now corresponds to the mode of transport of the next departure
else: | ||
self._state = self.data.nextdeparture.get('time', '-') | ||
self._state = self.data.departures.get('time', '-') | ||
self._icon = ICONS_PRODUCTS[self.data.departures.get('product', '-')] |
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.
line too long (81 > 79 characters)
U-Bahn SEV (bus replacement services) have unexpected direction values, fixed resulting bug and hound issues
Hi, thanks for the fixes and improvements. What is the rationale for the |
By the way, the previous version is part of the PR #6956 for 0.42 which I guess will be merged on the weekend, so unless this PR can be merged in time probably the whole platform should be postponed to 0.43... |
@DavidMStraub - Agree on your reasoning regarding release since there doesn't seem much of a point releasing something if configuration will be broken in the next release. From my side I think this should be good to go already for 0.42 (have been testing the different combination of options during the afternoon), so if you don't see any problems with the changes and the maintainers are happy with code quality, maybe this is good to go already for this release. With regards to the |
OK so the original version was released as part of 0.42 now. @mountainsandcode, I suggest you (I can do it as well) open an issue and a PR to resolve the bug of the missing implementation of line restrictions. Then as a second step you can add the improvements for 0.43. OK? |
@DavidMStraub I'm travelling for the week without access to my dev machine, so if you have time for the line restriction part then that would be super! Will get to it next week otherwise |
Hi, I added an issue and PR just adding the line name restriction for now. |
@DavidMStraub cheers for the work on the line restrictions! I've updated this PR to adress the resulting merge conflict from that bug fix with the changes proposed in PR. |
Description:
@DavidMStraub provided a first implementation of public transport departures in Munich through MVG-live.de. This pull requests carries forward this work by:
The change breaks the configuration, however, since it is currently still in dev, this should be acceptable.
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#2386
Example entry for
configuration.yaml
(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code does not interact with devices:
tox
run successfully. Your PR cannot be merged unless tests pass(Have had some trouble getting tox to run locally - Travis tests appear to pass)