Add support for passive devices #171
Conversation
|
Hi @postlund , localtuya:
With the wall switch on and the bulb off, I restarted Home Assistant and this is the log for the bulb: 2020-11-19 00:28:49 DEBUG (MainThread) [custom_components.localtuya.pytuya] [654...f9d] Started heartbeat loop I turned off the wall switch and found that even after many minutes, the state of the light bulb is off instead of unavailable. I tried to turn on the light bulb from the button in Home Assistant and this is the reported error: Logger: homeassistant.components.websocket_api.http.connection.54...92 Traceback (most recent call last): During handling of the above exception, another exception occurred: Traceback (most recent call last): REGISTER DETAIL 2020-11-19 00:34:21 DEBUG (MainThread) [custom_components.localtuya.pytuya] [654...f9d] Sending command set (device type: type_0a) I turned on the wall switch and very quickly it changed the state from off to on 2020-11-19 00:47:16 DEBUG (MainThread) [custom_components.localtuya.pytuya] [654...f9d] Started heartbeat loop The problem is that when the light bulb is on and I turn it off from the wall switch, the status remains fixed on ON, while with the previous version, after 15 seconds it was unavailable. From unavailable to ON, the change of state is very fast, about 5 seconds, but tomorrow I try better, leaving the wall switch off for many hours For my "needs", the very fast change of state from "unavailable" to "ON" is what I was looking for, while for the very long state change from "ON" to "unavailable" is what I would like to avoid. After how long is the parameter set to verify that the wall switch changes state? Sorry for the very detailed message, but maybe you need to understand the logs Thanks again Big Pierre :-) |
|
I will try to fix the error soon. You will get instant state changes once it works, but when there's an exception something isn't working so we can't make any assumptions yet. |
|
So, I have pushed an update now. You don't have to wait any long period of time when testing. Just turn the light off, wait for it to go unavailable and then turn it off. If it doesn't work, I need the debug log to verify what is happening. |
|
Just tested and waaaaoooouuu. I had the same problem as @SmartM-ui as i have multiple lights with physical switches. |
| @@ -71,7 +71,7 @@ | |||
|
|
|||
| from .common import TuyaDevice | |||
| from .config_flow import config_schema | |||
| from .const import DATA_DISCOVERY, DOMAIN, TUYA_DEVICE | |||
| from .const import CONF_PASSIVE_DEVICE, DATA_DISCOVERY, DOMAIN, TUYA_DEVICE | |||
There was a problem hiding this comment.
Shouldn't we add the "passive_device" option in the YAML sample in init.py ?
I'd also document it in the README.md and info.md.
Great work!
There was a problem hiding this comment.
Right, I missed that part when I split up the previous PR. Will fix that. Documentation is certainly needed. Maybe we should start looking at GitHub pages for documentation, it's pretty ok (I use it for pyatv over at pyatv.dev).
You mean the discovery time is between 5-20 seconds with this change? Could be right I guess, I depends on how the device acts. Can you try power-toggling a device and attach the complete log, so I can see what is really going on? That error message shouldn't be there of course. |
|
@postlund i mean status change from on/off to unavailable, 5 to 20s. |
|
@postlund
|
@postlund I congratulate you and I am really satisfied with your quick intervention! LOG from UNAVAILABLE to ON: LOG from ON to UNAVAILABLE Logger: custom_components.localtuya.pytuya Traceback (most recent call last): |
Can't see any problems here, did it work? |
Sounds great then! Heartbeats are sent sent every 20s (to verify if connection is still alive) with a timeout of 5s. So it should take roughly at most 25s to detect if connection was lost. But generally faster. Great that it works 👍 Maybe we can merge this then. |
|
Hi @postlund, to go further with this post Context: Passive Devices:
For the already on devices, i get two Tell me if you need more data. P.S. I was hitting an issue before but was not able to find a way to ensure i can reproduce it. It's like there are two HA connections to the same device. I know when it happens because in that situation each time I change color or brightness of some of my devices, they are flickering. Will open an issue if i can reproduce it. I just mention it as it may be related to the devices that are physically off some times. |
It works well for me! @postlund In your opinion, do I leave passive recognition only for the bulbs that I turn off through the wall switch or is it preferable to provide it for all the bulbs? |
This part is a bit tricky to deal with as we still want to log when it happens, but it might not always be a "big deal" to scream loud about it. I gladly take input for improvements.
In theory, this could be default mode and to some extent I would prefer to make it so. But it will only work as long as discovery works, which might not always be the case for some people. So I'm not sure yet. But the answer is that you can use this as default for all your devices, it's better. |
Thanks for the log, I will have a log after lunch and see what I can find. |
|
Just for information, I report that when you start Home Assistant the following errors are reported in the log: Logger: homeassistant Traceback (most recent call last): 2020-11-22 00:23:13 ERROR (MainThread) [homeassistant] Error doing job: Exception in callback _SelectorDatagramTransport._read_ready() Thanks |
|
@postlund checking my log, I encountered the same exception on a one device sometimes, the id is in core registry, do not know why this error occurs. |
|
Hi @postlund, I just installed the latest release and tried to use the passive_device feature but I keep getting this error. `Logger: homeassistant.components.hassio
Invalid config for [localtuya]: [passive_device] is an invalid option for [localtuya]. Check: localtuya->localtuya->0->passive_device. (See /config/configuration.yaml, line 17).` This is my configuration.yaml `localtuya:
Is there something wrong with my configuration? |
|
@sqazi95 this PR is not yet merged, you need to get the passive branch of this repository. |
Okay thanks. |
|
Ok, so I pushed some changes. The "passive device" option has been removed and is now default: all connection attempts are initiated when discovery broadcasts are received. Should work fine. When doing this I reverted back to version 1 of the config entry (since it does not have to change anymore), so that means you have to remove and re-create devices if you tried the PR prior to that. You can also change the version manually for each config entry to 1 in |
|
@ultratoto14 I can revert the seqno change, no problem. Do you see any other problems, e.g. error messages that could be used to debug? Seems strange to me that the mentioned situation could happen. The implementation is a bit jiffy right now. I map the sequence number to a semaphore used to wait for the message. When the message has been received, the semaphore is replaced with the message before releasing it. After I revert the seqno change we should be able to merge, no? |
This fixes the following error: AttributeError: 'NoneType' object has no attribute 'write'
From now on there's no connect loop but all attempts are initiated by received discovery messages.
|
Removed |
|
@postlund, i let it run all the night, the reconnect is now ok, but without the closer heartbeats, some devices switch to available/unavailable every now and then I think we can merge that as it will help a lot of users but I will keep my closer heartbeats locally as my connections was rock stable with them. edit: if self._connect_task:
self._connect_task.cancel()
await self._connect_task
if self._interface:
await self._interface.close()should be if self._connect_task is not None:
self._connect_task.cancel()
await self._connect_task
if self._interface is not None:
await self._interface.close()It's not linked to that PR but it's involved. |
|
@ultratoto14 I'm not sure I follow, what do you mean by "without closer heartbeats? You mean the tighter interval? I guess we can decrease that as well, but it would be great if we could try decreasing in steps to find a better threshold. Can you try 15s for instance? Is the heartbeat retry counter ever triggered and actually work? I mean, have you seen actual proof that the second heartbeat actually receives a response? I suggest that you add a log point when a heartbeat succeeds and `count
|
|
@postlund Yep i mean the tighter interval, i'll check and report, i think that it's already so much better that what we had, it can be merged, i'll check during the next days and open a PR if it's more conclusive, what do you think ? |
|
@ultratoto14 That sounds very good 👍 |
|
We need to make a clarification that discovery broadcasts are needed now, I'll see what I can come up with. |
|
I added something about the service here. We should probably rename the page to something like "working with datapoints", just to avoid having a billion pages with a sentence or two on. |
|
@postlund you were right, I reduced to 10 seconds and added a log in case, second heartbeat succeeds but it never occurred in the last 4 hours. With 10 seconds interval, the devices are rock stable. I found another crazy thing. The device i debug with that seemed to not support two connections at a time is now able to have Home Assistant and Tuya App at the same time, changing the light functions in the tuya app almost immediately replicated in HA 👍, |
|
@ultratoto14 I still have problemes with my wifi bulbs. It goes unavaliable, after a few minutes or hours turn on |
|
PS .. I have the last "master" branch |
|
Seems to be a failure from my pylint fixes. I will try to fix that. |
|
@postlund if you want a beta tester, i'm avaliable lol |
|
@DavidFFerreira Then I have an assignment for you: #252 |
|
Hey guys, just a question. I see we need to foward port 6666 and 6667 from the devices to the HA instance (on my side it's from my covers to my RPI4). Is this really necessary when they are on the same network? If my understanding is correct, this needs to be done directly on the router right? |
|
Hi
If the tuya devices and the raspberry pi are all on the same subnet then it
is plug and play.
The tuya devices will broadcast their UDP packet and the homeassistant will
pick them up.
This would typically be the case if all your devices are connected to a
home WiFi router with the default configuration.
The need to broadcast is if the tuya devices and the machine hosting
homeassistant are on different subnet.
The tuya devices UDP packet would need to be forwarded to the machine.
This would typically be the case if you have set up a guest network to
isolate the IoT devices from the rest of the network.
If that is your case I have a script ready and I will share it in a week
or two once I have been able to test it (I am on holiday and I am
refraining to hack on my home assistant set up as it is pretty much running
the home right now)
…On Sat, 2 Jan 2021, 11:20 Matssa, ***@***.***> wrote:
Hey guys, just a question. I see we need to foward port 6666 and 6667 from
the devices to the HA instance (on my side it's from my covers to my RPI4).
Is this really necessary when they are on the same network? If my
understanding is correct, this needs to be done directly on the router
right?
Thanks a bunch!
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#171 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAL6ZZ2VY5OILIF7LSXYEDSX3QNVANCNFSM4T2RXF2A>
.
|
|
I'm also working on a similar re-connect loop as before since this backfired more than I thought, see #288. |
A passive device will by-pass the re-connect loop and not try to connect or re-connect at all by itself. Instead, connects and re-connects are fully driven by discovery (which we do all the time in the background). When a discovery message is received from a passive device, a connection attempt will be made.
This has several benefits, e.g. we can deal with devices where power is cut. We will not try to re-connect randomly in vain, also giving potentially long re-connect times (due to backoff). A passive device will handle this gracefully, giving almost instead feedback. It also helps us deal with battery powered devices that goes to sleep most of the time. Additional improvements to the latter case will be done separately, namely keeping previous state after a disconnect.