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

Update launch library to use datetime #19521

Closed
wants to merge 2 commits into from

Conversation

DoloresHA
Copy link
Contributor

@DoloresHA DoloresHA commented Dec 22, 2018

Previous version used "start" from pylaunches which is a string that is not parsed by home assistant into a datetime. This change updates launch library to use the timestamp from pylaunches and parses it to make the launch_time attribute easier to template and automate with.

Description:

Modified launch library to pass datetime-parsable string instead of string.

**Related issue (if applicable): None

Example entry for configuration.yaml (if applicable):

Checklist:

  • The code change is tested and works locally.
  • [Not done] Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

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

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

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated 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:

  • Tests have been added to verify that the new code works.

Previous version used "start" from pylaunches which is a string that is not parsed by home assistant into a datetime. This change updates launch library to use the timestamp from pylaunches and parses it to make the launch_time attribute easier to template and automate with.
@@ -62,7 +63,7 @@ def __init__(self, launches, name):
try:
data = self.launches.launches[0]
self._state = data['name']
self._attributes['launch_time'] = data['start']
self._attributes['launch_time'] = template_help.timestamp_local(data['wsstamp'])

Choose a reason for hiding this comment

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

line too long (92 > 79 characters)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with commit 2.

Reduced line length to comply with 79 character limit.
@amelchio
Copy link
Contributor

IMO this should preferably be changed in pylaunches.

@ludeeus
Copy link
Member

ludeeus commented Dec 22, 2018

I agree @amelchio
Are ISO formated acceptable?
example output:

{'stream': 'https://www.youtube.com/watch?v=aVIrgayWlI0', 'agency': 'United States Air Force', 'agency_country_code': 'USA', 'wsstamp': 1545573060, 'name': 'Falcon 9 Block 5 | GPS III SV01', 'start': '2018-12-23T14:51:00'}
'start': '2018-12-23T14:51:00'

Since this is used in an attribute I'm not sure if this apply home-assistant/architecture#39, but consistancy is good i guess :)

@DoloresHA
Copy link
Contributor Author

@ludeeus I have no idea what the right answer is, presumably @amelchio does. I just knew that templates such as state_attr('sensor.next_launch', 'launch_time') didn't work as-is. Thank you for this awesome integration! I used it to automatically play the spacex launch today. :)

@ludeeus
Copy link
Member

ludeeus commented Dec 24, 2018

@DoloresHA I pushed version 0.2.0 of pylaunches with that change.
You can try that one, and see if that give you the control you are after.

@@ -62,7 +63,8 @@ def __init__(self, launches, name):
try:
data = self.launches.launches[0]
self._state = data['name']
self._attributes['launch_time'] = data['start']
self._attributes['launch_time'] = template_help.timestamp_local(
Copy link
Member

Choose a reason for hiding this comment

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

Why can't we use as_local from util/dt.py?

@DoloresHA
Copy link
Contributor Author

@ludeeus hmm I updated my pylaunches dependency to 0.2.0, but
{{ as_timestamp(state_attr('sensor.next_launch', 'launch_time')) }} still renders "None" instead of a timestamp. :-/

@ludeeus
Copy link
Member

ludeeus commented Dec 24, 2018

@DoloresHA https://i.ibb.co/FDpknW6/image.png 🤷‍♂️

@DoloresHA
Copy link
Contributor Author

@ludeeus I'm a doofus. I must have not actually rebooted or something? It's working for me as well. I've created PR #19570 to update the dependency and am closing this one. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants