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

add dublin bus RTPI sensor #5257

Merged
merged 3 commits into from Jan 13, 2017

Conversation

ttroy50
Copy link
Contributor

@ttroy50 ttroy50 commented Jan 10, 2017

Description:
Adds a sensor for the Dublin Bus real time information so that you can trigger an action based on a bus being due in a certain timeframe

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#1745

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: dublin_bus_transport
    stopid: [Stop ID]
    route: [Route]
    name: [Friendly Name]

Checklist:

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • [* ] Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • [* ] New dependencies have been added to the REQUIREMENTS variable (example).
  • [* ] New dependencies are only imported inside functions that use them (example).
  • [* ] New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • [* ] New files were added to .coveragerc.

If the code does not interact with devices:

  • [* ] Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • Tests have been added to verify that the new code works.

@mention-bot
Copy link

@ttroy50, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @fabaff and @robbiet480 to be potential reviewers.

For more details about this platform, please refer to the documentation at
https://home-assistant.io/components/sensor.dublin_public_transport/

Example Configuration:
Copy link
Member

Choose a reason for hiding this comment

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

Lets keep the example config on our webpage only

datetime.strptime(item['departuredatetime'],
"%d/%m/%Y %H:%M:%S")))) -
dt_util.as_local(dt_util.utcnow()))}
for item in result['results']
Copy link
Member

Choose a reason for hiding this comment

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

Wow, this is super confusing...

To simplify, can you maybe try some of the following: (1) remove it from being inline in the dict. (2) Is the result of the diff between two UTC or two local times not the same? (3) maybe you can use age in some way to make this simpler/ display better. (4) why as_timestamp/from timestamp?

Copy link
Contributor Author

@ttroy50 ttroy50 Jan 11, 2017

Choose a reason for hiding this comment

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

I'll take a look at making it clearer. I originally took the skeleton for my implementation from the swiss_public_transport sensor and they have something similar. Basically I'm trying to take a timestamp that is a string and get it as a datetime to find the difference (in minutes) between it and now.

Looking at get_age, it won't work as it says "Make sure date is not in the future, or else it won't work." and the due date should always be in the future.

for item in result['results']
]

except KeyError:
Copy link
Member

Choose a reason for hiding this comment

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

Instead of catching KeyErrors, is it possible to use item.get(key, somedefault)?

self._stop = stop
self._route = route
self.update()
self._unit_of_measurement = "min"
Copy link
Member

Choose a reason for hiding this comment

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

If it is always a constant of minutes, maybe we can simply return this constant in the function below?

Copy link
Member

@kellerza kellerza left a comment

Choose a reason for hiding this comment

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

Thank you, looking good! 🐬

(One minor comment)

def due_in_minutes(timestamp):
"""Get the time in minutes from a timestamp.

The timestamp should be in the format day/mont/year hour/minute/second
Copy link
Member

Choose a reason for hiding this comment

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

day/month/year hour:minute:second

The timestamp should be in the format day/mont/year hour/minute/second
"""
diff = datetime.strptime(
timestamp, "%d/%m/%Y %H:%M:%S") - dt_util.now().replace(tzinfo=None)
Copy link
Member

Choose a reason for hiding this comment

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

👍

@kellerza kellerza merged commit 1219ca3 into home-assistant:dev Jan 13, 2017
@kellerza
Copy link
Member

🐬

nordlead2005 added a commit to nordlead2005/home-assistant that referenced this pull request Jan 14, 2017
This reverts commit 4b43537.

Revert "Version bump for kodi dependency (home-assistant#5307)"

This reverts commit 6abad6b.

Revert "Add HMWIOSwitch to sensor, binary (home-assistant#5304)"

This reverts commit 2c3f55a.

Revert "Remove GTFS default name & string change"

This reverts commit 6000c59.

Revert "Update pyhomematic 1.19 & small cleanups (home-assistant#5299)"

This reverts commit a30711f.

Revert "[sensor] Add Dublin bus RTPI sensor (home-assistant#5257)"

This reverts commit 1219ca3.

Revert "Bugfix group reload (home-assistant#5292)"

This reverts commit baa8e53.

Revert "Support for TrackR device trackers (home-assistant#5010)"

This reverts commit f7a1d63.

Revert "Bump pywemo version."

This reverts commit dc937cc.

Revert "Upgrade to voluptuous to 0.9.3 (home-assistant#5288)"

This reverts commit d12decc.

Revert "Upgrade distro to 1.0.2 (home-assistant#5291)"

This reverts commit 64800fd.

Revert "Don't build Adafruit_BBIO - doesn't work on all platforms. (home-assistant#5281)"

This reverts commit 9a3c0c8.

Revert "Convert flic to synchronous platform. (home-assistant#5276)"

This reverts commit eb9b95c.

Revert "Upgrade to aiohttp 1.2 (home-assistant#4964)"

This reverts commit e68e29e.

Revert "Fix TCP sensor to correctly use value_template (home-assistant#5211)"

This reverts commit 1cf9ae5.

Revert "Cleanup language support on TTS (home-assistant#5255)"

This reverts commit 3f3a3bc.

Revert "Add last triggered to script (home-assistant#5261)"

This reverts commit 467cb18.

Revert "Bump flux_led version and make use of PyPi package (home-assistant#5267)"

This reverts commit 34a9fb0.

Revert "Add support for NAD receivers (home-assistant#5191)"

This reverts commit 3b59e16.

Revert "Bugfix async device_tracker see callback (home-assistant#5259)"

This reverts commit 71fddd2.

Revert "Use SHA hash to make token harder to guess (home-assistant#5258)"

This reverts commit 922308b.
balloob pushed a commit that referenced this pull request Jan 27, 2017
* Added forecast support to DarkSky

	modified:   homeassistant/components/sensor/darksky.py
	modified:   tests/components/sensor/test_darksky.py

* Fix async_volume_up / async_volume_down (#5249)

async_volume_up / async_volume_down should be async versions of
volume_up / volume_down, not a async version of the default variants of
volume_up / volume_down.

The previous code always called into the mediaplayers set_volume_level,
and never into volume_up / volume_down.

Signed-off-by: Anton Lundin <glance@acc.umu.se>

* adding a default icon "blind" to a PowerView blinds scene. (#5210)

* adding a default icon "blind" to a PowerView blinds scene.

* Adding icon property to define blind icon. Removed it from the state attributes dict.

* fixing lint error

* Added forecast support to DarkSky

	modified:   homeassistant/components/sensor/darksky.py
	modified:   tests/components/sensor/test_darksky.py

* Use SHA hash to make token harder to guess (#5258)

* Use SHA hash to make token harder to guess

Use hashlib SHA256 to encode object id instead of using it directly.

* Cache access token

Instead of generating a token on the fly cache it in the constructor.

* Fix lint

* Bugfix async device_tracker see callback (#5259)

* Add support for NAD receivers (#5191)

* Add support for NAD receivers

* remove self.update() in various methods

* remove setting attributes in various methods

* Change import to hass style

* Updated Config Validation, extended daily forecast to all supported types

* Fix style errors from previous commit, fix test since adding daily for all supported types

* Removed temperature from daily as it isn't supported

* Added forecast support to DarkSky

	modified:   homeassistant/components/sensor/darksky.py
	modified:   tests/components/sensor/test_darksky.py

* Updated Config Validation, extended daily forecast to all supported types

* Fix style errors from previous commit, fix test since adding daily for all supported types

* Removed temperature from daily as it isn't supported

* Revert "Bugfix camera streams (#5306)"

This reverts commit 4b43537.

Revert "Version bump for kodi dependency (#5307)"

This reverts commit 6abad6b.

Revert "Add HMWIOSwitch to sensor, binary (#5304)"

This reverts commit 2c3f55a.

Revert "Remove GTFS default name & string change"

This reverts commit 6000c59.

Revert "Update pyhomematic 1.19 & small cleanups (#5299)"

This reverts commit a30711f.

Revert "[sensor] Add Dublin bus RTPI sensor (#5257)"

This reverts commit 1219ca3.

Revert "Bugfix group reload (#5292)"

This reverts commit baa8e53.

Revert "Support for TrackR device trackers (#5010)"

This reverts commit f7a1d63.

Revert "Bump pywemo version."

This reverts commit dc937cc.

Revert "Upgrade to voluptuous to 0.9.3 (#5288)"

This reverts commit d12decc.

Revert "Upgrade distro to 1.0.2 (#5291)"

This reverts commit 64800fd.

Revert "Don't build Adafruit_BBIO - doesn't work on all platforms. (#5281)"

This reverts commit 9a3c0c8.

Revert "Convert flic to synchronous platform. (#5276)"

This reverts commit eb9b95c.

Revert "Upgrade to aiohttp 1.2 (#4964)"

This reverts commit e68e29e.

Revert "Fix TCP sensor to correctly use value_template (#5211)"

This reverts commit 1cf9ae5.

Revert "Cleanup language support on TTS (#5255)"

This reverts commit 3f3a3bc.

Revert "Add last triggered to script (#5261)"

This reverts commit 467cb18.

Revert "Bump flux_led version and make use of PyPi package (#5267)"

This reverts commit 34a9fb0.

Revert "Add support for NAD receivers (#5191)"

This reverts commit 3b59e16.

Revert "Bugfix async device_tracker see callback (#5259)"

This reverts commit 71fddd2.

Revert "Use SHA hash to make token harder to guess (#5258)"

This reverts commit 922308b.

* Revert "Revert "Bugfix camera streams (#5306)""

This reverts commit 2ee8c44.

* Update darksky.py
@Ordep
Copy link

Ordep commented Feb 5, 2017

Had to edit my local dublin_bus_transport.py so the get request to the Dublin Bus website would look like this.
response = requests.get(
_RESOURCE,
params=params,
verify=False,
timeout=10)

@ttroy50
Copy link
Contributor Author

ttroy50 commented Feb 6, 2017

@Ordep your change seems to be required because of certificate errors. I have also noticed an issue with the SSL certificate over the weekend on my hassbian install. It looks like the certificate on the server was updated on 03/02/17

verify=False will cause it to not check the certificate validity and work around the issue but wouldn't be recommended for a full fix.

Also, we should probably also take the discussion of this from the closed pull request into an issue

@home-assistant home-assistant locked and limited conversation to collaborators May 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants