Skip to content

Make sure zwave nodes/entities enter the registry is proper state.#14251

Merged
balloob merged 5 commits intohome-assistant:devfrom
andrey-git:zwavereadd
May 8, 2018
Merged

Make sure zwave nodes/entities enter the registry is proper state.#14251
balloob merged 5 commits intohome-assistant:devfrom
andrey-git:zwavereadd

Conversation

@andrey-git
Copy link
Copy Markdown
Contributor

@andrey-git andrey-git commented May 2, 2018

Description:

Until zwave node's info is parsed it is read as blank causing inconvenient entity_id to be written into the entity registry. zwave entities (lights, sensors, etc.) suffer from the same problem.

This PR adds the following logic:

  • If node/entity is added but the node is not parsed yet - don't advertise unique_id, don't add the node/entity to HA. For entity additionally require that the label is not "unknown".
  • For 30 seconds check each second if the node/entity has unique_id. If it does - add it to HA (it will get also get added to the entity registry with a proper name and entity_id.
  • If 30 seconds passed - add the node/entity to HA without unique_id. It will get inconvenient name and entity_id, but won't "pollute" the registry with it.
  • If node got parsed or became "ready" advertise node/entity unique_id and remove-and-re-add it to HA so it will get a new, better entity_id and get added to entity registry. If node became ready, then even its "unknown" entities will get unique_id. This can happen do to OZW failing to properly scan an entity type.

Checklist:

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

@andrey-git andrey-git requested a review from a team as a code owner May 2, 2018 21:57
super().__init__()
from openzwave.network import ZWaveNetwork
from pydispatch import dispatcher
self.component = None
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not necessary. Entities already have access to the platform that contains them at self.platform.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe the "remove, clear_id, add back" functionality should be added to the Entity class then? WDYT?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think that it's a very common use case to do this. Better keep it in Z-Wave, we can always make it a generic function once we see more places need it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ack

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This can be removed since it's no longer being used?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@andrey-git andrey-git changed the title When zwave node's info is parsed remove it and re-add back. WIP: When zwave node's info is parsed remove it and re-add back. May 6, 2018
@andrey-git andrey-git changed the title WIP: When zwave node's info is parsed remove it and re-add back. Make sure zwave nodes/entities enter the registry is proper state. May 7, 2018
@andrey-git
Copy link
Copy Markdown
Contributor Author

Added more logic. Ready for review.

entity.node_id, sec)
hass.async_add_job(_add_node_to_component)

def _on_timeout(sec):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you tag this one and the other async callbacks with @callback so we know it's async

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@rofrantz
Copy link
Copy Markdown
Contributor

rofrantz commented May 8, 2018

@andrey-git after this gets merged do we need to clean db/remove entity registry or anything else...or renaming zwave will work right away reflecting the new entity id ?

@balloob balloob merged commit 10505d5 into home-assistant:dev May 8, 2018
@andrey-git andrey-git deleted the zwavereadd branch May 8, 2018 20:09
@andrey-git
Copy link
Copy Markdown
Contributor Author

@rofrantz this change is backwards compatible. However if you delete your entity registry you might get other entity IDs when it is regenerated.

@rofrantz
Copy link
Copy Markdown
Contributor

rofrantz commented May 8, 2018

Thanks for the clarifications @andrey-git

@balloob balloob mentioned this pull request May 28, 2018
girlpunk pushed a commit to girlpunk/home-assistant that referenced this pull request Sep 4, 2018
…ome-assistant#14251)

* When zwave node's info is parsed remove it and re-add back.

* Delay value entity if not ready

* If node is ready consider it parsed even if manufacturer/product are missing.

* Add annotations
@home-assistant home-assistant locked and limited conversation to collaborators Sep 5, 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.

4 participants