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

Decouple Konnected entity setup from discovery #16146

Merged
merged 2 commits into from
Aug 24, 2018

Conversation

heythisisnate
Copy link
Contributor

@heythisisnate heythisisnate commented Aug 23, 2018

Description:

This PR does not contain any new features or functional changes. It decouples Konnected entity setup from the network discovery process to improve stability and faster recovery in case of a Hass reboot or power failure.

Previously, the Konnected component would completely depend on discovery to find the Konnected device(s) on the network before the corresponding Hass entities (binary sensors & switches) were created. This caused a problem if Hass was rebooted while Konnected was still running -- Hass would "forget" all the discovered entities and the Konnected device could get into a reboot/failsafe cycle trying to update the states of devices that Hass didn't know about any longer. The reboot cycle prevented discovery from working again properly.

Included in this PR, we decouple the entity setup from discovery so that entity setup happens earlier when the konnected component first loads. This allows the Konnected device(s) to recover quickly after a reboot and update states even if the device hasn't been re-discovered yet. Once discovery occurs, the entity will be augmented with its initial state and network information.

Pull request in home-assistant.github.io with documentation: home-assistant/home-assistant.io#6066

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

@heythisisnate
Copy link
Contributor Author

This looks like a lot of changes, but it's really mostly moving code around and some simplification. I've split up the KonnectedDevice object into ConfiguredDevice and DiscoveredDevice. The former gets initialized when the konnected component first loads. The latter object is initialized in the discovery callback.

@jcooper-korg
Copy link

Great news! I've had to downgrade to an earlier version of the konnected mcu firmware because of this issue. My system was repeatedly getting into an unrecoverable unresponsive state, when restarting Hass after editing config.

@balloob balloob added this to the 0.77 milestone Aug 24, 2018
@balloob balloob merged commit 647b3ff into home-assistant:dev Aug 24, 2018
@ghost ghost removed the in progress label Aug 24, 2018
balloob pushed a commit that referenced this pull request Aug 25, 2018
* decouple entity setup from discovery

* validate that device_id is a full MAC address
@johntdyer
Copy link
Contributor

@heythisisnate - So my python is weak but does this mean I no longer need to use discovery w/ konnected ?

@heythisisnate
Copy link
Contributor Author

@johntdyer No, not quite. Discovery is still required for the devices to be fully set up and connected to Hass. This change is one step in that direction, allowing the entities to be created ahead of time before the device's network information is discovered.

I plan on adding a manual IP configuration option in a future release.

@balloob
Copy link
Member

balloob commented Aug 27, 2018

@heythisisnate you should implement a config flow that will do discovery once and store the IP in a config entry

@heythisisnate
Copy link
Contributor Author

@balloob Thank you for the suggestion. I didn't think it was possible for Hass to write to a config entry. Could you point me to an example of this or some documentation?
I would still have to solve for the case when the IP address changes via DHCP, but this is a good idea. Thank you.

@balloob
Copy link
Member

balloob commented Aug 27, 2018

https://developers.home-assistant.io/docs/en/config_entries_config_flow_handler.html#initializing-a-config-flow-from-an-external-source

And now let's no longer abuse this PR as a discussion place. You can find us on Discord.

@balloob balloob mentioned this pull request Aug 29, 2018
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
* decouple entity setup from discovery

* validate that device_id is a full MAC address
@home-assistant home-assistant locked and limited conversation to collaborators Dec 10, 2018
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.

5 participants