Skip to content

Handle initial event after entity is added.#6760

Merged
balloob merged 1 commit into
home-assistant:devfrom
aequitas:rflink_update_after_add
Mar 30, 2017
Merged

Handle initial event after entity is added.#6760
balloob merged 1 commit into
home-assistant:devfrom
aequitas:rflink_update_after_add

Conversation

@aequitas
Copy link
Copy Markdown
Contributor

Description: Make sure entity is not used before it can be used.

Recently entity creation has been made asynchronous. The Rflink codebase assumes it to be synchronous. If an event for a unknown entity is received a new entity object is created and added. The event handler is also directly called on the new object to update the new entities state with the incoming event. The subsequent async_update_ha_state is called on an 'unadded' entity which causes errors as described in: #5965

homeassistant.exceptions.NoEntitySpecifiedError: No entity id specified for entity auriolv2_5e01_bat
17-03-23 08:40:42 ERROR (MainThread) [homeassistant.core] Error doing job: Task exception was never retrieved
Traceback (most recent call last):
  File "/usr/lib/python3.4/asyncio/tasks.py", line 237, in _step
    result = next(coro)
  File "/srv/hass/lib/python3.4/site-packages/homeassistant/helpers/entity.py", line 200, in async_update_ha_state
    "No entity id specified for entity {}".format(self.name))

@mention-bot
Copy link
Copy Markdown

@aequitas, thanks for your PR! By analyzing the history of the files in this pull request, we identified @fabaff, @pvizeli and @martinfrancois to be potential reviewers.

@pvizeli
Copy link
Copy Markdown
Member

pvizeli commented Mar 23, 2017

I don't think that is a real bugfix.
Maby it is not bad to make a small refactory. Instead to use hass.data to store every things, you can use helpers/dispatcher.py for process and communicate beetween component and platform. A Entity should first register a dispatcher signal on async_added_to_hass. For the main problem, store it temp in entity init and refire it on async_added_to_hass or init all device on startup and if a device new so it lost first event.

Maby we can add this for 0.41.0 but it need a future work for make it realy rubust and move some stuff from hass.data to dispatcher.

@aequitas
Copy link
Copy Markdown
Contributor Author

It would be nice to have a feed of updates for developers to keep up to date with the latest changes in Core without having to follow every commit and change. Like a summary of (breaking) changes and the directions that are chosen architecture wise, new best practices, etc. There is some nice documentation for developers already. But keeping on track with developments is hard if your not full-time invested in the project or part of the core team. I feel responsible for my contributions and like to support other users with problems. But it frustrates the process when I'm lagging behind on information because I don't have the time to catch up on changes.

It is not a complete bugfix indeed as race conditions can occur. But local testing reduced the reported errors to zero so at least this change prevents some of the error chatter for users. Worst case like it is without the change the initial event is not handled by the newly created entity but an error is still logged.

I think refactoring to the dispatcher would make sense, I will start a separate PR for that. I used the event bus in a similar fashion before switching to this method as the dispatcher was not implemented back then and the event bus was not designed for this type of events.

I don't think passing it to the init is the cleanest solution though. Ignoring the initial event is also not the nicest solution. I will try to figure something out.

Feel feel to merge or close this PR. As it currently is not breaking functionality, just causing error messages there should be no harm.

@balloob
Copy link
Copy Markdown
Member

balloob commented Mar 30, 2017

I agree that we can do better with communicating the core changes. If I had more time I would spend some more time on the docs, they are getting somewhat outdated. For what it's worth, I don't expect any more changes in the near future. The core + systems around it are now all as much async as it can be.

I'll merge this PR and looking forward to the port to dispatcher. 🐬

@balloob balloob merged commit ead00e9 into home-assistant:dev Mar 30, 2017
@fabaff fabaff mentioned this pull request Apr 6, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Jul 17, 2017
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