Skip to content

Improve Wemo setup speed#19563

Merged
MartinHjelmare merged 8 commits intohome-assistant:devfrom
sqldiablo:wemo_improve_setup_speed
Dec 30, 2018
Merged

Improve Wemo setup speed#19563
MartinHjelmare merged 8 commits intohome-assistant:devfrom
sqldiablo:wemo_improve_setup_speed

Conversation

@sqldiablo
Copy link
Copy Markdown
Contributor

@sqldiablo sqldiablo commented Dec 24, 2018

Description:

Breaking change:
Move WeMo device discovery to run after home assistant start so it won't block initial component setup from completing quickly.

Example entry for configuration.yaml (if applicable):

wemo:

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.

Move WeMo device discovery to an async context so it won't block initial component setup from completing quickly.
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

I don't see the benefit of this, and there are mistakes regarding I/O in the event loop and api use.

@fabaff fabaff changed the title Wemo - Improve setup speed Improve Wemo setup speed Dec 25, 2018
@sqldiablo
Copy link
Copy Markdown
Contributor Author

@MartinHjelmare I think I misunderstood some things here. I'm still getting used to how asyncio works. My hope was to allow the WeMo component to load quickly at startup by not having to wait for it to do its device discovery. That's why I moved the discovery process to a subroutine that I then called through hass.add_job(). If there's no good way to defer the discovery process until after setup of the component is complete, then we can close this PR. I was trying to eliminate the "taking a long time to load" messages at startup and make HA load faster on initial startup.

@sqldiablo
Copy link
Copy Markdown
Contributor Author

Is there an event that fires at the end of the setup phase that I could listen for and then run the wemo device discovery? That would accomplish my goal of starting up faster by deferring wemo discovery until after setup is complete.

@MartinHjelmare
Copy link
Copy Markdown
Member

MartinHjelmare commented Dec 27, 2018

We can listen for EVENT_HOMEASSISTANT_START that will fire when the setup phase is finished. Limitation of that could be that entitites that are loaded after home assistant start, via discovery, will not be ready to use in automations, immediately.

@sqldiablo
Copy link
Copy Markdown
Contributor Author

I've changed the way this works, per our conversation. This no longer adds an async task to run in the event loop, but rather listens for the start event and then schedules a wemo discovery scan immediately. This is copied from how the discovery component works.

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Good!

@MartinHjelmare MartinHjelmare merged commit 25e5864 into home-assistant:dev Dec 30, 2018
@ghost ghost removed the in progress label Dec 30, 2018
@MartinHjelmare
Copy link
Copy Markdown
Member

Marking this breaking change, since we change when discovery is run.

@sqldiablo sqldiablo deleted the wemo_improve_setup_speed branch December 30, 2018 04:21
@chelming
Copy link
Copy Markdown
Contributor

Will this speed up static discovery as well?

wemo:
  static:
    - 10.0.40.21
    - 10.0.40.22
    - 10.0.40.23
    - 10.0.40.26

My Wemo devices are on another subnet and even though I have mDNS enabled, they're not discovered by HomeAssistant (Google Cast devices are discovered just fine though).

Doing static discovery with the Wemo devices causes home assistant to hang or about 20 seconds before allowing me to log back in.

@sqldiablo
Copy link
Copy Markdown
Contributor Author

sqldiablo commented Dec 31, 2018

Yep, it'll let HA start up a little faster, because it won't add the wemo devices until after HA has started up and let's you log in.

@balloob balloob mentioned this pull request Jan 10, 2019
@juched78
Copy link
Copy Markdown

I think that this change means I now need to change my homekit component to not auto_start.

My wemo lights disappeared and I think it is because the wemo devices are discovered after, even though I have discovery disabled and my wemo devices statically defined.

Is there an event I could setup an automation on to turn on homekit once wemo devices are loaded? This would be helpful.

@MartinHjelmare
Copy link
Copy Markdown
Member

Please open an issue if you suspect a bug. If you need help please use our help channels:
https://home-assistant.io/help/#communication-channels

Merged PRs should not be used for support or bug reports. Thanks!

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Jan 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants