Protocol 3.4 support #606
Protocol 3.4 support #606
Conversation
* upgrade deps
|
@codetheweb Ok, lint and tests work now ... (and I adjusted a lot lint stuff) |
|
According to #234 the 3.2 support should be working |
|
Thanks for doing this! Just wanted to let you know I've been really busy the last few days but hope to take a look soon. |
|
@codetheweb In the meantime I should also have a 3.4 device ... So plan is to try the changes out myself today |
|
@codetheweb Works as expected ... @harryzz did great work! With my cleanups ... Ready for Review |
| /** | ||
| * Create a deferred promise that resolves as soon as the connection is established. | ||
| */ | ||
| createDeferredConnectPromise() { |
There was a problem hiding this comment.
I don't think we need this, can we just return new Promise() at the end?
There was a problem hiding this comment.
The problem is that the 3.4 "connect" is in the end 3 messages exchanged, sothe final "resolve" will come from out an other method but we also still want to have "connect timeout" logic. So in the end for a "clean" solution we in any case need to store a promise.
The alternative is that we resolve connect promise already after we sent out the "start session exchange message", but in fact we are not yet connected.
There was a problem hiding this comment.
Right, I agree we need to store a Promise, just not sure why we need to have this wrapper that doesn't seem to have any custom logic to create one.
There was a problem hiding this comment.
This wrapper is needed because you can not just store the two methods reject and resolve... I experimented around a long time on how to build a deferred promise and this solution is used in one of my other projects and originally came from an other developer that uses it in his projects.
In fact if you return a promise you have no real way to reference to the reject or resolve method of this object "outside" of the Promise constructor. Thats why this is the trick to allow this.
It cost me hours to come to this in my other project :-)
You find it also here e.g. https://www.getrevue.co/profile/masteringjs/issues/promises-and-the-deferred-antipattern-235313
There was a problem hiding this comment.
(and I decided against adding a dependency lib to get a Deffered option which is kind of just doing exactly also that - e.g. https://github.com/ljharb/promise-deferred/blob/main/index.js
There was a problem hiding this comment.
PS: If you mean that I should kill the extra function and directly integrate the code into the connect function then yes I can do this. ;-)
There was a problem hiding this comment.
Read the code again and I understand the issue now, seems ok for now.
|
For testing: I have one device and tested with this,but in the end no" long time testing" so far. I have also one more user using it - he had one time an issue that the devices went into a "restart loop", but honestly ... This could mean something is not 100%, but considering the quelity sometimes it must not mean anything ;-) And also after restarting problem did not re-occured till now. So I would consider as ok aka "best we can say for now" :-) |
|
@codetheweb Lets wait a day longer ... On user that tried this has oissues, so I now started a bit a "Long time test" ... lets see if it is stable for me |
|
@codetheweb Ok I would go and release this. In fact it works. I only have one device to verify and yes this device closes the connection every 2-3h, but a reconnect works after 30 or latest second try after 60s ... but from all the quality issues we know from Tuya devices this is not enough to proof it a "protocol implementation issue" (and also python lib do not have anything else). So best it can be right now |
|
Published in v7.5.0. |
|
Thank you very much! |
I catched the code from @harryzz repoditory where he posted that protocol 3.4 was implemented (see #481 (comment)). I adjusted code style to the project one and reworked the code a bit to be less clustered when it comes to the 3.4 changes.
Additionally I added a deferred promise for the "longer connect flow" to resolve the connect promise when the connection is really established.
This is completely untested! I hope that I get such a device tomorrow/tuesday, but if someone wants to : install from my Fork (https://github.com/Apollon77/tuyapi) and give it a shot. Ideally start with DEBUG=TuyAPI* and post your log when you tested it (successfuly or not) :-)
@codetheweb I will do a Draft PR for now, but if you have time please already have a look and tell me if you would like somethings to be adjusted.
I do not get any lint or tests running locally ... will try again, but if you have ideas....
fixes #234, fixes #481