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

Make time trigger data trigger.now local #21544

Merged
merged 5 commits into from
Mar 2, 2019

Conversation

emontnemery
Copy link
Contributor

@emontnemery emontnemery commented Feb 28, 2019

Description:

Fix regression introduced by #17199 where time trigger is in local timezone but the time trigger data is in UTC. This fix changes back to previous behavior where both time trigger and the time trigger data is in local timezone

Related issue (if applicable): fixes #20110

Checklist:

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

@balloob
Copy link
Member

balloob commented Feb 28, 2019

If we're able to track down the cause, we should tag this PR for the 89 milestone.

@emontnemery
Copy link
Contributor Author

This is not the right fix. async_track_time_change operates on local time and so now should already be local. If it's not, we should fix that inside helpers/event.py.

Thanks for reviewing! Now fixed by making sure we call the event handler with local time instead of converting to local time in the event handler: https://github.com/home-assistant/home-assistant/blob/e8f122c5518275dc93c1aaf4879113dd258c17c7/homeassistant/helpers/event.py#L373-L375

@balloob balloob merged commit cd89809 into home-assistant:dev Mar 2, 2019
@ghost ghost removed the in progress label Mar 2, 2019
@@ -370,7 +370,9 @@ def pattern_time_change_listener(event):
last_now = now

if next_time <= now:
hass.async_run_job(action, event.data[ATTR_NOW])
if local:
now = dt_util.as_local(now)
Copy link
Member

Choose a reason for hiding this comment

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

Does it matter that we now pass local time to calculate_next?

Copy link
Member

Choose a reason for hiding this comment

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

@balloob balloob mentioned this pull request Mar 3, 2019
balloob pushed a commit that referenced this pull request Mar 5, 2019
* Make time trigger data trigger.now local

* Make time pattern trigger data trigger.now local

* Lint

* Rework according to review comment

* Lint
@balloob balloob mentioned this pull request Mar 6, 2019
@emontnemery emontnemery deleted the local_time_trigger_data branch April 3, 2019 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PR #17199 changed trigger.now from local to UTC breaking automations
5 participants