Delay device discovery during WeMo component initialization. #4435

jaharkes · 2016-11-17T15:07:23Z

Description:

When network discovery finds multiple WeMo platforms (in my case switch and light) then both platforms depend on the WeMo component. Because we are running a one-shot discovery during component initialization which takes some time the second initialization failed.

This patch delays running the pywemo specific discovery to occur after the component initialization has completed.

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

Example entry for configuration.yaml (if applicable):

Checklist:

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

Documentation added/updated in home-assistant.github.io

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.

Schedule device discovery to run after the WeMo component has finished
initializing. This avoids a situation where initialization fails because
discovery triggered initialization of two WeMo platforms that both
try to setup the WeMo component.

Schedule device discovery to run after the WeMo component has finished initializing. This avoids a situation where initialization fails because discovery triggered initialization of two WeMo platforms that both try to setup the WeMo component.

mention-bot · 2016-11-17T15:07:24Z

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

balloob · 2016-11-17T15:29:51Z

I don't think that I fully understand your description. We only setup each component once.

Discovery will wait doing setup until setup component is done.

jaharkes · 2016-11-17T15:36:08Z

I was seeing errors that the WeMo component was already being initialized.

The component is loaded either from the config because of a wemo: entry, or as a dependency of something like switch.wemo getting discovered by the discovery component.

The wemo component initialization takes quite a bit of time because it runs its own pywemo.discovery ans during this time another platform, for instance lights.wemo, may get initialized which also depends on the WeMo component.

On November 17, 2016 10:29:56 AM EST, Paulus Schoutsen notifications@github.com wrote:

I don't think that I fully understand your description. We only setup
each component once.

Discovery will wait doing setup until setup component is done.

balloob · 2016-11-17T15:51:50Z

But as far as I know, the discovery component doesn't start discovery till
home Assistant starts, which is always after setup of the wemo component

On Thu, Nov 17, 2016, 07:36 Jan Harkes notifications@github.com wrote:

I was seeing errors that the WeMo component was already being initialized.

The component is loaded either from the config because of a wemo: entry,
or as a dependency of something like switch.wemo getting discovered by
the discovery component.

The wemo component initialization takes quite a bit of time because it
runs its own pywemo.discovery ans during this time another platform, for
instance lights.wemo, may get initialized which also depends on the WeMo
component.

On November 17, 2016 10:29:56 AM EST, Paulus Schoutsen <
notifications@github.com> wrote:

I don't think that I fully understand your description. We only setup
each component once.

Discovery will wait doing setup until setup component is done.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4435 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABYJ2kjI1QZBb62PUFY1S5xS5BJfzSdZks5q_HRtgaJpZM4K1anE
.

jaharkes · 2016-11-17T15:54:37Z

I don't have wemo in my config at all, as well as chromecast and various others. Discovery is what gets those platforms and components loaded.

On November 17, 2016 10:52:00 AM EST, Paulus Schoutsen notifications@github.com wrote:

But as far as I know, the discovery component doesn't start discovery
till
home Assistant starts, which is always after setup of the wemo
component

On Thu, Nov 17, 2016, 07:36 Jan Harkes notifications@github.com
wrote:

I was seeing errors that the WeMo component was already being
initialized.

The component is loaded either from the config because of a wemo:
entry,
or as a dependency of something like switch.wemo getting discovered
by
the discovery component.

The wemo component initialization takes quite a bit of time because
it
runs its own pywemo.discovery ans during this time another platform,
for
instance lights.wemo, may get initialized which also depends on the
WeMo
component.

On November 17, 2016 10:29:56 AM EST, Paulus Schoutsen <
notifications@github.com> wrote:

I don't think that I fully understand your description. We only
setup
each component once.

Discovery will wait doing setup until setup component is done.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub

#4435 (comment),
or mute the thread

https://github.com/notifications/unsubscribe-auth/ABYJ2kjI1QZBb62PUFY1S5xS5BJfzSdZks5q_HRtgaJpZM4K1anE
.

jaharkes · 2016-11-17T16:01:49Z

I think we may be talking about different device discoveries here because of my poor choice of wording for the PR description. What this patch delays is a call to `pywemo.discover_devices`, not the device discovery using netdisco in the discovery component. The WeMo specific discovery duplicates what netdisco does, possibly in a slightly different way and is supposedly for people who want only their WeMo devices autodiscovered and to make the component more robust. My argument against it is that it duplicates existing functionality and only does a one-time shot discovery during startup, while netdisco periodically rescans in the background and will be able to see devices that come online after hass is already running.

balloob · 2016-11-17T16:55:58Z

What is happening currently:

discovery component starts discovery at homeassistant_start
discovery discovers Wemo. Wemo is a component based discovery so it calls discovery.discover
helpers.discovery.discover will setup Wemo (without checking/acquiring the setup lock in the discovery method)
Wemo will call helpers.discovery.load_platform (which will wait till an ongoing setup is finished.)
Since helpers.discovery.load_platform waits till Wemo is setup before it starts to setup the light component and fire the platform_discovered event for wemo.

I think that the problem is not inside the wemo component but it is a problem inside the discovery helpers. The discover method should be made async and also acquire a lock just like async_load_platform. That way the discovery will wait if a setup is currently going on.

jaharkes · 2016-11-17T17:32:01Z

So by making the component setup faster, I just made the window for the race condition smaller, but the problem is still there.

balloob · 2016-11-19T08:07:23Z

I have resolved the discovery issue in #4463 👍

jaharkes · 2016-11-21T14:05:15Z

#4463 did not seem to have resolved the issue. Yesterday I ran 0.34.dev0 with only a change to revert the netdisco dependency back to 0.7.5. I've extracted relevant parts of the log,

16-11-20 15:54:01 homeassistant.util.package: Attempting install of netdisco==0.7.5
16-11-20 15:54:19 homeassistant.bootstrap: Setting up discovery
16-11-20 15:54:19 homeassistant.core: Bus:Handling <Event component_loaded[L]: component=discovery>
16-11-20 15:55:23 homeassistant.core: Starting Home Assistant core loop
16-11-20 15:55:23 homeassistant.core: Starting Home Assistant
16-11-20 15:55:23 homeassistant.core: Bus:Handling <Event service_registered[L]: service=stop, domain=homeassistant>
16-11-20 15:55:23 homeassistant.core: Bus:Handling <Event service_registered[L]: service=restart, domain=homeassistant>
16-11-20 15:55:23 homeassistant.core: Bus:Handling <Event homeassistant_start[L]>
16-11-20 15:55:23 homeassistant.core: Timer:starting
16-11-20 15:55:23 netdisco.service: Scanning
16-11-20 15:55:52 homeassistant.components.discovery: Found new service: belkin_wemo ('Insight', 'Insight', 'http://192.168.0.100:49153/setup.xml', ...
16-11-20 15:55:52 homeassistant.components.discovery: Found new service: belkin_wemo ('WeMo Link', 'Bridge', 'http://192.168.0.40:49153/setup.xml', ...
16-11-20 15:55:53 homeassistant.loader: Loaded wemo from homeassistant.components.wemo
16-11-20 15:55:53 homeassistant.util.package: Attempting install of pywemo==0.4.7
16-11-20 15:56:12 homeassistant.bootstrap: Setting up wemo
16-11-20 15:56:13 homeassistant.components.wemo: Scanning for WeMo devices.
16-11-20 15:56:13 pywemo.subscribe: Listening on port 8989
16-11-20 15:56:21 homeassistant.components.wemo: Adding wemo at 192.168.0.40:49153
16-11-20 15:56:21 homeassistant.core: Bus:Handling <Event platform_discovered[L]: discovered=('WeMo Link', 'Bridge', 'http://192.168.0.40:49153/setup.xml', ...), service=belkin_wemo>
16-11-20 15:56:21 homeassistant.core: Bus:Handling <Event platform_discovered[L]: discovered=('WeMo Link', 'Bridge', 'http://192.168.0.40:49153/setup.xml', ...), platform=wemo, service=load_platform.light>
16-11-20 15:56:21 homeassistant.components.wemo: Adding wemo at 192.168.0.100:49153
16-11-20 15:56:21 homeassistant.loader: Loaded light.wemo from homeassistant.components.light.wemo
16-11-20 15:56:21 homeassistant.bootstrap: Attempt made to setup wemo during setup of wemo
16-11-20 15:56:21 homeassistant.bootstrap: Component wemo failed to setup
16-11-20 15:56:21 homeassistant.bootstrap: Unable to prepare setup for platform light.wemo because dependency wemo could not be initialized

balloob · 2016-11-22T07:50:01Z

Oh I think that I see the problem. Can you add or True to the end of this if-statement so it will always run the logic inside?

jaharkes · 2016-11-22T11:57:40Z

👍 that does the trick.

balloob self-assigned this Nov 18, 2016

balloob mentioned this pull request Nov 19, 2016

Better locking while setting up components + discovery #4463

Merged

2 tasks

balloob mentioned this pull request Nov 22, 2016

Fix platform discovery when platform discovered during discovery of a #4529

Merged

balloob closed this in #4529 Nov 22, 2016

home-assistant locked and limited conversation to collaborators Mar 17, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Delay device discovery during WeMo component initialization. #4435

Delay device discovery during WeMo component initialization. #4435

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Delay device discovery during WeMo component initialization. #4435

Delay device discovery during WeMo component initialization. #4435

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!