search

TheAgentK / tuya-mqtt

Nodejs-Script to combine tuyaapi and openhab via mqtt
MIT License
175 stars 80 forks source link

Unhandled exception if Tuya device is removed #2

Closed platgeo closed 3 years ago

platgeo commented 5 years ago

The script is working wonderful for me, but I found one issue:

When removing a controlled tuya device physically (e.g. removing a bulb from the socket), the script ends with a uncaught exception.

So far I worked around this issue with a Shell-Script checking every 5 minutes if the script is running, and if not start it again, but it would be nice if the script catches the exception and handles it when device is removed.

TheAgentK commented 5 years ago

Unfortunately, I can't reproduce the error. If I interrupt one of my Tuya devices (buld and socket), my script stays running.

I have integrated more error handlers, but I believe that this error comes from the original TuyAPI.

It would be great if you could test if the script still crashes.

tsightler commented 5 years ago

I definitely see some very bad issues if you remove or change a device. In my case I changed the IP addresses of the devices (moved to static IP assignment so they won't change in the future), but I could not get the script to ignore the old devices until I completely removed them from the existing MQTT broker (mosquitto with persist option, I just stopped the service, deleted the persistence file, and restarted the service). I'm thinking this issue might not exist if the broker has no persistence file and was restarted, or if the messages were not sent with qos.

If I can somehow find the time I'll dig into this issue a little more. Should be pretty easy to reproduce by just forcing a static IP change on a device.

BTW, to be clear, I see the issue when I restart the script, otherwise the script seems to stay running.

TheAgentK commented 5 years ago

Yes, if a device is changed (IP, removed), the status remains available through the qos of 2. But I don't have a solution yet. I also moved the QoS to the configuration file (config.json). So that the QoS can be easily adapted.

Maybe we can think of an elegant solution.

tsightler commented 5 years ago

My current thought is that the best way to solve this is really to implement device auto-discovery. I've really moved away from OpenHAB to Home Assistant and have been playing with several scripts that were build specifically for HA MQTT and use their discovery protocol. It's really quite simple in that, for each device, it sends a specific notification to MQTT that describes the device as well as the state and command topics for the device. Since OpenHAP 2.4 MQTT supports the Home Assistant MQTT discovery protocol you can get all your devices that support this discovered in either tool automatically, no configuration at all (well, once MQTT is configured).

It might not be obvious how this helps with the above issue, but bear with me. My idea is that the script should have a simple JSON configuration file that list all of the devices, really, the same basic format that is used for the MITM discovery process is perfect.

When the script is first run it would read in the devices/IP's/IDs from the config file and the publish the state information with QOS 1 and the discovery information to the correct topics. As soon as you started the script the devices from the config would just appear automatically, no configuration required. To change the config, just edit the config file to add new devices/remove devices, etc. and restart the script.

This idea is based on the fact that I've been playing with a similar MQTT script for the Ring alarm system that automatically publishes your alarm and sensors via MQTT during startup.

Heck, we could probably even automate the discovery of the devices completely, since HA already has a plugin that controls lights via the Tuya web API vs locally anyway, but admittedly doesn't offer local control. That being said it makes me think we should be able to figure out how to get the keys automatically.

Of course, all of these ideas take time, which I seem to have very little of right now. My lack of javascript skill is also a problem, although I'm slowly wrapping my head around it I think. In the shorter term I wonder if a simple process exit handler that just publishes empty value to all topics would solve the issue. Maybe I'll get some time next week to at least try that.

tsightler commented 3 years ago

This should be addressed with the 3.0 release.