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

Fix automations listening to HOMEASSISTANT_START #6936

Merged
merged 14 commits into from
Apr 6, 2017
Merged
20 changes: 16 additions & 4 deletions homeassistant/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,17 @@
from homeassistant.util.async import run_callback_threadsafe


def attempt_use_uvloop():
"""Attempt to use uvloop."""
import asyncio

try:
import uvloop
asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())
except ImportError:
pass


def monkey_patch_asyncio():
"""Replace weakref.WeakSet to address Python 3 bug.

Expand Down Expand Up @@ -311,8 +322,7 @@ def open_browser(event):
EVENT_HOMEASSISTANT_START, open_browser
)

hass.start()
return hass.exit_code
return hass.start()


def try_to_restart() -> None:
Expand Down Expand Up @@ -359,11 +369,13 @@ def try_to_restart() -> None:

def main() -> int:
"""Start Home Assistant."""
validate_python()

attempt_use_uvloop()

if sys.version_info[:3] < (3, 5, 3):
monkey_patch_asyncio()

validate_python()

args = get_arguments()

if args.script is not None:
Expand Down
6 changes: 2 additions & 4 deletions homeassistant/bootstrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ def async_from_config_dict(config: Dict[str, Any],
This method is a coroutine.
"""
start = time()
hass.async_track_tasks()
Copy link
Member Author

Choose a reason for hiding this comment

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

Track tasks is now enabled by default. It's what we want during startup.


core_config = config.get(core.DOMAIN, {})

try:
Expand Down Expand Up @@ -140,10 +138,10 @@ def async_from_config_dict(config: Dict[str, Any],
continue
hass.async_add_job(async_setup_component(hass, component, config))

yield from hass.async_stop_track_tasks()
yield from hass.async_block_till_done()

stop = time()
_LOGGER.info('Home Assistant initialized in %ss', round(stop-start, 2))
_LOGGER.info('Home Assistant initialized in %.2fs', stop-start)

async_register_signal_handling(hass)
return hass
Expand Down
19 changes: 16 additions & 3 deletions homeassistant/components/automation/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
Offer event listening automation rules.

For more details about this automation rule, please refer to the documentation
at https://home-assistant.io/components/automation/#event-trigger
at https://home-assistant.io/docs/automation/trigger/#event-trigger
"""
import asyncio
import logging

import voluptuous as vol

from homeassistant.core import callback
from homeassistant.const import CONF_PLATFORM
from homeassistant.core import callback, CoreState
from homeassistant.const import CONF_PLATFORM, EVENT_HOMEASSISTANT_START
from homeassistant.helpers import config_validation as cv

CONF_EVENT_TYPE = "event_type"
Expand All @@ -31,6 +31,19 @@ def async_trigger(hass, config, action):
event_type = config.get(CONF_EVENT_TYPE)
event_data = config.get(CONF_EVENT_DATA)

if (event_type == EVENT_HOMEASSISTANT_START and
hass.state == CoreState.starting):
_LOGGER.warning('Deprecation: Automations should not listen to event '
"'homeassistant_start'. Use platform 'homeassistant' "
'instead. Feature will be removed in 0.45')
hass.async_run_job(action, {
'trigger': {
'platform': 'event',
'event': None,
},
})
return lambda: None

@callback
def handle_event(event):
"""Listen for events and calls the action when data matches."""
Expand Down
55 changes: 55 additions & 0 deletions homeassistant/components/automation/homeassistant.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
"""
Offer Home Assistant core automation rules.

For more details about this automation rule, please refer to the documentation
at https://home-assistant.io/components/automation/#homeassistant-trigger
"""
import asyncio
import logging

import voluptuous as vol

from homeassistant.core import callback, CoreState
from homeassistant.const import (
CONF_PLATFORM, CONF_EVENT, EVENT_HOMEASSISTANT_STOP)

EVENT_START = 'start'
EVENT_SHUTDOWN = 'shutdown'
_LOGGER = logging.getLogger(__name__)

TRIGGER_SCHEMA = vol.Schema({
vol.Required(CONF_PLATFORM): 'homeassistant',
vol.Required(CONF_EVENT): vol.Any(EVENT_START, EVENT_SHUTDOWN),
})


@asyncio.coroutine
def async_trigger(hass, config, action):
"""Listen for events based on configuration."""
event = config.get(CONF_EVENT)

if event == EVENT_SHUTDOWN:
@callback
def hass_shutdown(event):
"""Called when Home Assistant is shutting down."""
hass.async_run_job(action, {
'trigger': {
'platform': 'homeassistant',
'event': event,
},
})

return hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP,
hass_shutdown)

# Automation are enabled while hass is starting up, fire right away
# Check state because a config reload shouldn't trigger it.
elif hass.state == CoreState.starting:
hass.async_run_job(action, {
'trigger': {
'platform': 'homeassistant',
'event': event,
},
})

return lambda: None
4 changes: 2 additions & 2 deletions homeassistant/components/automation/litejet.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def pressed():
nonlocal held_less_than, held_more_than
pressed_time = dt_util.utcnow()
if held_more_than is None and held_less_than is None:
call_action()
hass.add_job(call_action)
if held_more_than is not None and held_less_than is None:
cancel_pressed_more_than = track_point_in_utc_time(
hass,
Expand All @@ -88,7 +88,7 @@ def released():
held_time = dt_util.utcnow() - pressed_time
if held_less_than is not None and held_time < held_less_than:
if held_more_than is None or held_time > held_more_than:
call_action()
hass.add_job(call_action)

hass.data['litejet_system'].on_switch_pressed(number, pressed)
hass.data['litejet_system'].on_switch_released(number, released)
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/automation/mqtt.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Offer MQTT listening automation rules.

For more details about this automation rule, please refer to the documentation
at https://home-assistant.io/components/automation/#mqtt-trigger
at https://home-assistant.io/docs/automation/trigger/#mqtt-trigger
"""
import asyncio
import json
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/automation/numeric_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Offer numeric state listening automation rules.

For more details about this automation rule, please refer to the documentation
at https://home-assistant.io/components/automation/#numeric-state-trigger
at https://home-assistant.io/docs/automation/trigger/#numeric-state-trigger
"""
import asyncio
import logging
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/automation/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Offer state listening automation rules.

For more details about this automation rule, please refer to the documentation
at https://home-assistant.io/components/automation/#state-trigger
at https://home-assistant.io/docs/automation/trigger/#state-trigger
"""
import asyncio
import voluptuous as vol
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/automation/sun.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Offer sun based automation rules.

For more details about this automation rule, please refer to the documentation
at https://home-assistant.io/components/automation/#sun-trigger
at https://home-assistant.io/docs/automation/trigger/#sun-trigger
"""
import asyncio
from datetime import timedelta
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/automation/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Offer template automation rules.

For more details about this automation rule, please refer to the documentation
at https://home-assistant.io/components/automation/#template-trigger
at https://home-assistant.io/docs/automation/trigger/#template-trigger
"""
import asyncio
import logging
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/automation/time.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Offer time listening automation rules.

For more details about this automation rule, please refer to the documentation
at https://home-assistant.io/components/automation/#time-trigger
at https://home-assistant.io/docs/automation/trigger/#time-trigger
"""
import asyncio
import logging
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/automation/zone.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Offer zone automation rules.

For more details about this automation rule, please refer to the documentation
at https://home-assistant.io/components/automation/#zone-trigger
at https://home-assistant.io/docs/automation/trigger/#zone-trigger
"""
import asyncio
import voluptuous as vol
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/light/litejet.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def __init__(self, hass, lj, i, name):
def _on_load_changed(self):
"""Called on a LiteJet thread when a load's state changes."""
_LOGGER.debug("Updating due to notification for %s", self._name)
self._hass.async_add_job(self.async_update_ha_state(True))
self.schedule_update_ha_state(True)

@property
def supported_features(self):
Expand Down
1 change: 1 addition & 0 deletions homeassistant/components/sensor/mqtt_room.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ def update_state(device_id, room, distance):

self.hass.async_add_job(self.async_update_ha_state())

@callback
def message_received(topic, payload, qos):
"""A new MQTT message has been received."""
try:
Expand Down
4 changes: 2 additions & 2 deletions homeassistant/components/switch/litejet.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ def __init__(self, hass, lj, i, name):
def _on_switch_pressed(self):
_LOGGER.debug("Updating pressed for %s", self._name)
self._state = True
self._hass.async_add_job(self.async_update_ha_state())
self.schedule_update_ha_state()

def _on_switch_released(self):
_LOGGER.debug("Updating released for %s", self._name)
self._state = False
self._hass.async_add_job(self.async_update_ha_state())
self.schedule_update_ha_state()

@property
def name(self):
Expand Down
27 changes: 9 additions & 18 deletions homeassistant/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,14 @@
EVENT_TIME_CHANGED, MATCH_ALL, EVENT_HOMEASSISTANT_CLOSE,
EVENT_SERVICE_REMOVED, __version__)
from homeassistant.exceptions import (
HomeAssistantError, InvalidEntityFormatError, ShuttingDown)
HomeAssistantError, InvalidEntityFormatError)
from homeassistant.util.async import (
run_coroutine_threadsafe, run_callback_threadsafe)
import homeassistant.util as util
import homeassistant.util.dt as dt_util
import homeassistant.util.location as location
from homeassistant.util.unit_system import UnitSystem, METRIC_SYSTEM # NOQA

try:
import uvloop
asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())
except ImportError:
pass

DOMAIN = 'homeassistant'

# How long we wait for the result of a service call
Expand Down Expand Up @@ -86,10 +80,6 @@ def async_loop_exception_handler(loop, context):
kwargs = {}
exception = context.get('exception')
if exception:
# Do not report on shutting down exceptions.
if isinstance(exception, ShuttingDown):
return

kwargs['exc_info'] = (type(exception), exception,
exception.__traceback__)

Expand Down Expand Up @@ -123,7 +113,7 @@ def __init__(self, loop=None):
self.loop.set_default_executor(self.executor)
self.loop.set_exception_handler(async_loop_exception_handler)
self._pending_tasks = []
self._track_task = False
self._track_task = True
self.bus = EventBus(self)
self.services = ServiceRegistry(self)
self.states = StateMachine(self.bus, self.loop)
Expand All @@ -148,6 +138,7 @@ def start(self) -> None:
# Block until stopped
_LOGGER.info("Starting Home Assistant core loop")
self.loop.run_forever()
return self.exit_code
except KeyboardInterrupt:
self.loop.create_task(self.async_stop())
self.loop.run_forever()
Expand All @@ -165,9 +156,10 @@ def async_start(self):

# pylint: disable=protected-access
self.loop._thread_ident = threading.get_ident()
_async_create_timer(self)
self.bus.async_fire(EVENT_HOMEASSISTANT_START)
yield from self.async_stop_track_tasks()
Copy link
Member Author

@balloob balloob Apr 5, 2017

Choose a reason for hiding this comment

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

So this fixes the fact that CoreState.starting was never detectable before, as we would immediately overwrite it.

However, this has the downside that it is impossible for components/platforms to start a permanent async job as a result from EVENT_HOMEASSISTANT_START.

(note that things like MQTT server rely on loop.create_server which will work. Z-Wave depends on a thread and that will work)

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe add a timeout to async_stop_track_tasks after which we will no longer stop tracking tasks?

Copy link
Member

Choose a reason for hiding this comment

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

I think that should not be needed to add a timeout

self.state = CoreState.running
_async_create_timer(self)

def add_job(self, target: Callable[..., None], *args: Any) -> None:
"""Add job to the executor pool.
Expand Down Expand Up @@ -238,6 +230,8 @@ def block_till_done(self) -> None:
@asyncio.coroutine
def async_block_till_done(self):
"""Block till all pending work is done."""
assert self._track_task, 'Not tracking tasks'

# To flush out any call_soon_threadsafe
yield from asyncio.sleep(0, loop=self.loop)

Expand All @@ -252,7 +246,8 @@ def async_block_till_done(self):

def stop(self) -> None:
"""Stop Home Assistant and shuts down all threads."""
run_coroutine_threadsafe(self.async_stop(), self.loop)
self.loop.call_soon_threadsafe(
self.loop.create_task, self.async_stop())
Copy link
Member

Choose a reason for hiding this comment

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

Why not only call loop.create_task?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because that's not threadsafe

Copy link
Member

Choose a reason for hiding this comment

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

You are sure? I found no comments in doc that will be not threadsafe.

Copy link
Member Author

Choose a reason for hiding this comment

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

https://docs.python.org/3/library/asyncio-dev.html#concurrency-and-multithreading

To schedule a coroutine object from a different thread, the run_coroutine_threadsafe() function should be used.

Also, the tests won't pass if I just call create_task.

Copy link
Member Author

@balloob balloob Apr 5, 2017

Choose a reason for hiding this comment

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

(note, we include the run_coroutine_threadsafe function from Python in HASS because it was only introduced in Python 3.5.1)

Copy link
Member

Choose a reason for hiding this comment

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

asyncio.ensure_future maby the same but don't support with python 3.4.2


@asyncio.coroutine
def async_stop(self, exit_code=0) -> None:
Expand Down Expand Up @@ -368,10 +363,6 @@ def async_fire(self, event_type: str, event_data=None,

This method must be run in the event loop.
"""
if event_type != EVENT_HOMEASSISTANT_STOP and \
Copy link
Member Author

Choose a reason for hiding this comment

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

This was silently (because exception suppressed) killing shutdown tasks (noticed it when adding shutdown event in automation). It should be fine to just wait till all is done. Timer is already stopped so nothing new should come in.

self._hass.state == CoreState.stopping:
raise ShuttingDown("Home Assistant is shutting down")

listeners = self._listeners.get(event_type, [])

# EVENT_HOMEASSISTANT_CLOSE should go only to his listeners
Expand Down
6 changes: 0 additions & 6 deletions homeassistant/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@ class HomeAssistantError(Exception):
pass


class ShuttingDown(HomeAssistantError):
"""When trying to change something during shutdown."""

pass


class InvalidEntityFormatError(HomeAssistantError):
"""When an invalid formatted entity is encountered."""

Expand Down
Loading