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

Entities Update #341

Merged
merged 1 commit into from
Aug 12, 2018
Merged

Entities Update #341

merged 1 commit into from
Aug 12, 2018

Conversation

Odianosen25
Copy link
Collaborator

Allowed for the 10 mins entities updates in utilities(), not to delete those entities made using the self.set_app_state() function

Allowed for the 10 mins entities updates in `utilities()`, not to delete those entities made using the `self.set_app_state()` function
@ccremer
Copy link

ccremer commented Jul 22, 2018

I had this bug too. I can confirm this PR works. Until it is merged, I did the following in my dockerfile:

    curl https://github.com/home-assistant/appdaemon/commit/179d36e16b630142dc22d2f35c89152096f53540.diff > file.patch && \
    patch /usr/local/lib/python3.6/site-packages/appdaemon/appdaemon.py file.patch && \

@Odianosen25
Copy link
Collaborator Author

Glad it works for you @ccremer. Hopefully it will be accepted as said.

@Swamp-Ig
Copy link

I ran into an issue with this one.

ZWave devices don't come up immediately on boot-up of hass, they take a little while to process. If you put this patch in and update the state of a switch or some other device in the init function for an app, it prevents the connection between the state in HA and the actual device.

The result is that changing the state of the switch in HA fails. If you toggle the actual switch, it does change the state in ha however. Maybe this is a bug in ha / feature that needs to be implemented in ha?

If this does get accepted, you'd want to have a parameter that determines if the state should be set if it doesn't exist. Maybe create=False ? If it doesn't exist, maybe throw an exception or return false?

Difficult to know the best way to handle it. In my app I'm just testing if the object exists and registering a listener on the zwave.network_ready event.

@Odianosen25
Copy link
Collaborator Author

Hello @Swamp-Ig,

Thanks for the feedback. To be honest I don't fully understand what you mean, as I don't use zwave. If I understand you, you need this to be quicker so it sees on time the zwave devices? The utility function in appdaemon.py only runs every 10 mins. This 10 mins is by design and not mine. You can have @acockburn make changes to that.

Even if it happens every 10 mins, it still takes place when the plugin starts. Though thinking about what you wrote, I don't know if it is possible that an incident of an entity no longer existing, could still be kept in AppD (if it is possible), since its its updating and not replacing.

@Swamp-Ig
Copy link

Swamp-Ig commented Aug 10, 2018

Nope the other way around. Zwave starts up slowly (low power network), so if you set the state on something that's going to exist but doesn't exist yet, and then ZWave starts up and the device is already there then it borks the connection between HA and the physical device. Same thing I expect would happen with anything that gets found through network discovery and isn't there from boot up.

The simple way around is just to wait for the state to actually exist before attempting to set it, but that's not entirely obvious in the docs. IDK if this is really the place to worry about it but it is an issue that was uncovered when I added this patch, whereas before the patch it wasn't an issue so I posted it here for consideration / contemplation.

I expect really HA should handle this situation a bit better since potentially anything setting the state between boot up and a device actually being ready and configured would be affected by the race condition, which probably includes stuff like IFTTT integration and probably google home integration etc. It's not really an appdaemon problem entirely.

Hope that makes sense.

@Odianosen25
Copy link
Collaborator Author

issue that was uncovered when I added this patch, whereas before the patch it wasn't an issue

Hmmm that's quite interesting @Swamp-Ig. As I really didn't touch anything. Normall this is how I understand AppD works

When a plugin starts, it informs Appdaemon and Appdaemon requests its complete state. At this point I believe that your zwave devices are not available. This still happens and this patch doesn't affect it.

After that every 10 mins, Appdaemon requests from the plugin for its complete state, so as to update its own internal state. Within this time, I am certain your zwave devices are now available.

Now if you say your app tries to set it before, and you didn't see an issue, it honestly shouldn't be an issue now. Also if your app tried to set the zwave device, appdaemon should have raised a warning stating that Entity doesn't exist, and will therefore not execute, as it checks for if an entity exist before attempting to do anything to it.

Not sure why its showing up now, but what you could do is that to avoid errors or the likes, you could listen for the event in AppD zwave.network_ready for when the zwave devices have been loaded and ready. When it does, then execute the zwave related things in a function, instead of putting it directly in the init function.

Maybe @ReneTode could give some more advice, as he is more experienced than I am in this stuff, and maybe he has some zwave devices.

Regards

@Swamp-Ig
Copy link

Swamp-Ig commented Aug 10, 2018

Yeh with some thought it's really a HA issue. Will discuss it there.

@ReneTode
Copy link
Contributor

@Odianosen25 the warning "entity doesn't exist doesnt prevent execution.
set_state gives also such a warning the first time when you set the state from a sensor.

but only set_state should create anything in HA. and set_state shouldnt be used for zwave devices, in that case should the services be used.

to prevent creation from AD its also possible to check before the state is set.

if self.entity_exists(some_entity):
    self.set_state(some_entity,state=some_state)
else:
    # wait for some time and try again

@acockburn
Copy link
Member

I am adding the ability to delay app restart when connecting to HASS which should hopefully sovle this issue.

@acockburn acockburn merged commit fcd6fa5 into AppDaemon:dev Aug 12, 2018
@Odianosen25 Odianosen25 deleted the Odianosen25-patch-1 branch July 5, 2019 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants