8000 Update nederlandse_spoorwegen.py to include platform information by hmmbob · Pull Request #10494 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Update nederlan 8000 dse_spoorwegen.py to include platform information #10494

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

Merged
merged 3 commits into from
Nov 11, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions homeassistant/components/sensor/nederlandse_spoorwegen.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,21 @@ def device_state_attributes(self):
'departure_delay':
self._trips[0].departure_time_planned !=
self._trips[0].departure_time_actual,
'departure_platform':
self._trips[0].trip_parts[0].stops[0].platform,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test these changes? Your PR seems to have been made via GitHub…

Copy link
Contributor Author
@hmmbob hmmbob Nov 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I did. I’m fairly new to both Github and Python, so I actually figured this out and tested this on my local home assistant installation (it worked as expected). It wasn't too difficult as all info was already avail in the object - i just needed to make sure it gets returned as well, along with the other attributes. Then I wanted to publish my changes on the forums but decided the Github option would be the better option ;)

What’s the best way? Still learning...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your approach works (as this PR shows) but it's not great for tracking changes or migrating changes to a branch. Best way is to set up a development environment so that you can run all checks locally: https://home-assistant.io/developers/development_environment/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks much. If I ever get to submit another idea / improvement, I will :)

'departure_platform_changed':
self._trips[0].trip_parts[0].stops[0].platform_changed,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does changed mean here? Is it if the platform has changed from their original platform or is it a timestamp when it was last updated? (because the latter should never be stored in state attributes)

Copy link
Contributor Author
@hmmbob hmmbob Nov 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is the first: it will be either true or false based upon the api a 75DE nswer (via the NSapi.py). It is not a time stamp.

'arrival_time_planned':
self._trips[0].arrival_time_planned.strftime('%H:%M'),
'arrival_time_actual':
self._trips[0].arrival_time_actual.strftime('%H:%M'),
'arrival_delay':
self._trips[0].arrival_time_planned !=
self._trips[0].arrival_time_actual,
'arrival_platform':
self._trips[0].trip_parts[0].stops[-1].platform,
'arrival_platform_changed':
self._trips[0].trip_parts[0].stops[-1].platform_changed,
'next':
self._trips[1].departure_time_actual.strftime('%H:%M'),
'status': self._trips[0].status.lower(),
Expand Down
0