Protocol 3.3 and encrypted discovery #214

kueblc · 2019-06-14T00:50:56Z

Preliminary support for protocol 3.3, including support for the new UDP broadcast mechanism.

Written to prioritize minimal change (hacky) and no breaking API changes. To be refactored in v6.0.0.

Tests pass. Additional testing was done with simulated 3.1 and 3.3 devices.

It wouldn't hurt to get a few more in vivo tests before merge. @Apollon77 @tsightler @kezakez @EricPalmquist @sylwester- @ChristianHoefer @thirug010

Cipher.decrypt now always outputs a string, as MessageParser handles JSON parsing Updated cipher tests to account for this API change Tests now pass

Minor version bump to v5.1.0 as a result of the added protocol support

Fixed 3.3 crc being taken before the payload is written

coveralls · 2019-06-14T00:55:22Z

Pull Request Test Coverage Report for Build 415

79 of 84 (94.05%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+1.8%) to 91.415%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
index.js	35	40	87.5%

Totals
Change from base Build 409:	1.8%
Covered Lines:	284
Relevant Lines:	312

💛 - Coveralls

Apollon77 · 2019-06-14T08:14:48Z

Coool. I‘m currently on vacation in tunesia but can give it a try in 1-2 weeks. So maybe all the others are faster then me ... ;-(

Apollon77 · 2019-06-14T20:44:13Z

Code Review wise looks ok to me, but doc is missing for the new methods ;-)

I currently try to get such a device with new protocol so that I can test by myself too.

Apollon77 · 2019-06-14T20:49:49Z

One more: was not the port for the tcp connection for the new device firmware also different? Or am I wrong with this?

…path in encode()

kueblc · 2019-06-14T21:08:52Z

Code Review wise looks ok to me, but doc is missing for the new methods ;-)

New methods should be considered internal for the time being, they will be refactored out in v6.0.0. Perhaps I can clarify this with an underscore?

One more: was not the port for the tcp connection for the new device firmware also different? Or am I wrong with this?

It is the same, at least with the samples I have received. The UDP discovery process happens on a new port however.

Apollon77 · 2019-06-14T21:33:01Z

Underscore is a Good convention for „private“ ... but in fact to start only with them maybe is confusing. Other idea is an @Private as doc for the method. But in fact the same.
So all ok ;-) (my point of view)

Thanks for the tcp port info.

…uld be contained to payload treatment

kueblc · 2019-06-14T21:45:43Z

Actually turned out to be an easier refactor than I was anticipating, so I've taken out the protocol specific functions. Now the change set is isolated to payload treatment.

EricPalmquist · 2019-06-16T05:38:47Z

I tested with my device today - an outdoor 3 socket power strip and can read and write to all three. You might consider in your constructor to show that the version must be specified as a string - it shows as an integer, which does not work. Excellent work though - I appreciate your hard work.

Apollon77 · 2019-06-16T07:29:12Z

Oh that it right. So we should adjust the check (use == ?) or add type conversion.

kueblc · 2019-06-16T12:42:57Z

You might consider in your constructor to show that the version must be specified as a string - it shows as an integer, which does not work.

Good catch, though if you're using .find() you don't need to specify the version.

Should be fixed now. MessageParser now ensures version is a string on construction.

Oh that it right. So we should adjust the check (use == ?) or add type conversion.

Funny I had used the == operator, but the linter complained, so I took it out to get tests passing without realizing the consequence. This linter drives me crazy sometimes, it doesn't like my coding style and fights with me on legitimate language features 😛

Switched constructor to terse defaults pattern

thirug010 · 2019-06-16T16:18:42Z

Hi,
I have tested with 2 devices ( firmware 3.1.1 , 1.0.7) for both it is not finding the device with device id, with IP-Address it is able find it but it fails on 'data' with below error and it is not decrypted, Please let me know if i am missing something

kueblc · 2019-06-16T16:35:03Z

Hi,
I have tested with 2 devices ( firmware 3.1.1 , 1.0.7) for both it is not finding the device with device id, with IP-Address it is able find it but it fails on 'data' with below error and it is not decrypted, Please let me know if i am missing something

Hi @thirug010, could you please post the test code you are using? If you are able to capture your network traffic please post this as well.

lib/cipher.js

index.js

codetheweb · 2019-06-16T17:48:18Z

Looks pretty good @kueblc; ready to merge once those changes are made.

I can't test it myself as I don't have any v3.3 devices, but I trust that everything works.

And if you give me some examples of bad linter rules, I can change the config. 😉

Added note about using protocol 3.3 without find()

kueblc · 2019-06-16T18:08:36Z

Looks pretty good @kueblc; ready to merge once those changes are made.

Ready when you are 👍

And if you give me some examples of bad linter rules, I can change the config. wink

Cool, let's revisit for v6.0.0. Got a bunch of notes to go over.

Apollon77 · 2019-06-16T18:11:02Z

I also currently try to get sich devices. Seems I need to purchase some :-(

It is not really about „bad“ Linter rules. In fact they are right and eg === is best practice also nimmt eyes but sometimes you want to have the „feature“ that == brings ;-) but in those rare cases you need to tell the Linter that you know what you did and to ignore it.

* Update dependency coveralls to v3.0.4 (#206) * Update dependency ava to v2.1.0 (#211) * Update dependency documentation to v11.0.1 (#213) * Update dependency delay to v4.3.0 (#212) * Protocol 3.3 and encrypted discovery (#214) * Add check for set resolver * Don't emit event on set ACK * Don't emit event on set ACK * Hacked in support for protocol 3.3 and new UDP broadcast * MessageParser.decode now attempts to parse JSON after decryption Cipher.decrypt now always outputs a string, as MessageParser handles JSON parsing Updated cipher tests to account for this API change Tests now pass * Rollback cipher changes for now -- saving breaking changes for v6.0.0 Minor version bump to v5.1.0 as a result of the added protocol support * Fixed 3.1 producing a buffer on decryption failure Fixed 3.3 crc being taken before the payload is written * Removed debug used during testing * Removed redundant try catch block, as it throws the errors it catches * Moved some shared code from encode31() and encode33() to shared code path in encode() * Updated tests for improved coverage, including 3.3 protocol code paths * Refactored out encode31 and encode33 entirely as their differences could be contained to payload treatment * Ensure that version is a string Switched constructor to terse defaults pattern * Moved UDP key to its own file Added note about using protocol 3.3 without find()

thirug010 · 2019-06-16T22:14:15Z

@kueblc,

Please find the network logs (logs.zip) and I use the basic code tuya-dev.txt, Please let me know if you need any other information

Thanks
Thiru

kueblc · 2019-06-16T22:45:17Z

Thanks @thirug010, can you capture the network activity of the device? It looks like these are of your phone. You can use tcpdump or wireshark to create a pcap file.

Since find isn't working for you, I'd be interested to see what, if any, data is being broadcasted on UDP ports 6666-6668.

It also looks like you're passing 30 to find, which isn't valid

tsightler · 2019-06-17T02:12:46Z

I tested the same code as @thirug010 (tuya-dev.txt) with my switches and the 5.1.0 tuya-api and they worked fine with both find and IP. It was pretty exciting to seem the actually work with local control for the first time since the firmware update was pushed.

Unfortunately my primary use case for tuyapi is to use it with tuya-mqtt which currently doesn't work with tuya-api 5.x although it looks like it should be easy to modify for that part. However, looks like I'll also need to modify it for supporting 3.3 firmware since that code currently requires specifying the IP for each device (no support for find) but has no method to specify firmware version.

AS137430 · 2019-06-18T08:39:49Z

@tsightler : https://github.com/TheAgentK/tuya-mqtt/blob/master/tuya-mqtt.js#L181 and #175 (comment).
Probably https://github.com/TheAgentK/tuya-mqtt/blob/master/tuya-mqtt.js#L125 needs to change too.

I have an (updated firmware) device new out from the box to test as well and have been waiting for @kueblc's changes. Thanks @kueblc and @codetheweb for your great work.

tsightler · 2019-06-18T12:17:44Z

Yeah, I actually hacked up tuya-mqtt last night to not require the IP and just use the find() capability and it worked just fine with the newer firmware. Thanks to everyone for all the work here, I'm happy to be able to return to local control for my devices.

AS137430 · 2019-06-18T12:27:33Z

@tsightler : Can you share your code please?

kueblc · 2019-06-18T16:59:06Z

Thanks @tsightler that's great to hear it worked out well for you. Your data was instrumental in understanding the new protocol.

tsightler · 2019-06-18T22:46:52Z

@AS137430 I will share the patched tuya-mqtt code in my own tree after flushing out a few remaining issues. Mainly, while the code works to toggle the lights, it doesn't actually update the MQTT status as it seems like it's not correctly parsing the returned data. I haven't had time to look at why but I suspect it's something pretty simple. Hopefully in a day or two at most.

thirug010 · 2019-06-19T00:24:16Z

Thanks @thirug010, can you capture the network activity of the device? It looks like these are of your phone. You can use tcpdump or wireshark to create a pcap file.

Since find isn't working for you, I'd be interested to see what, if any, data is being broadcasted on UDP ports 6666-6668.

It also looks like you're passing 30 to find, which isn't valid

@kueblc,
I have checked with new devices + different Tuya based app now it is working without any issues.
I can confirm it works for firmware version 1.0.7, 3.1.1 and 3.3 without using the firmware versions in constructors ( works for both Tuya switch, dimmer and fan light dimmer)

Thanks
Thiru

tsightler · 2019-06-19T03:21:52Z

@AS137430 I've published an initial version of tuya-mqtt that support 5.1.1 and discovery for 3.3 devices in my fork: https://github.com/tsightler/tuya-mqtt
It seems to work for me but I only have simple, single switches from three different vendors and no lightbulbs so it's only tested in a small subset of possible use cases. That being said, the overall changes are quite minimal.

codetheweb and others added 9 commits June 5, 2019 17:21

Add check for set resolver

9454774

Merge branch 'master' into development

9e43402

Don't emit event on set ACK

ee145f9

Don't emit event on set ACK

2fb72e8

Hacked in support for protocol 3.3 and new UDP broadcast

1006519

MessageParser.decode now attempts to parse JSON after decryption

713f0f8

Cipher.decrypt now always outputs a string, as MessageParser handles JSON parsing Updated cipher tests to account for this API change Tests now pass

Rollback cipher changes for now -- saving breaking changes for v6.0.0

b59ec5c

Minor version bump to v5.1.0 as a result of the added protocol support

Fixed 3.1 producing a buffer on decryption failure

ca93520

Fixed 3.3 crc being taken before the payload is written

Removed debug used during testing

4c0f3ca

kueblc added 3 commits June 14, 2019 17:00

Removed redundant try catch block, as it throws the errors it catches

774b0ce

Moved some shared code from encode31() and encode33() to shared code …

c92034a

…path in encode()

Updated tests for improved coverage, including 3.3 protocol code paths

9aaa628

Refactored out encode31 and encode33 entirely as their differences co…

bf832d1

…uld be contained to payload treatment

Merge branch 'master' into feature-3.3

806c4eb

kueblc requested a review from codetheweb June 14, 2019 22:04

Ensure that version is a string

c4b2068

Switched constructor to terse defaults pattern

codetheweb requested changes Jun 16, 2019

View reviewed changes

lib/cipher.js Outdated Show resolved Hide resolved

index.js Show resolved Hide resolved

index.js Show resolved Hide resolved

Moved UDP key to its own file

9b79567

Added note about using protocol 3.3 without find()

codetheweb self-requested a review June 16, 2019 18:28

codetheweb approved these changes Jun 16, 2019

View reviewed changes

codetheweb merged commit ab5c85e into codetheweb:development Jun 16, 2019

aperkins1310 mentioned this pull request Sep 9, 2019

Update tuyapi version apexad/eufy-robovac#1

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Protocol 3.3 and encrypted discovery #214

Protocol 3.3 and encrypted discovery #214

kueblc commented Jun 14, 2019

coveralls commented Jun 14, 2019 •

edited

Loading

Apollon77 commented Jun 14, 2019

Apollon77 commented Jun 14, 2019

Apollon77 commented Jun 14, 2019

kueblc commented Jun 14, 2019

Apollon77 commented Jun 14, 2019

kueblc commented Jun 14, 2019

EricPalmquist commented Jun 16, 2019

Apollon77 commented Jun 16, 2019

kueblc commented Jun 16, 2019

thirug010 commented Jun 16, 2019

kueblc commented Jun 16, 2019

codetheweb commented Jun 16, 2019

kueblc commented Jun 16, 2019

Apollon77 commented Jun 16, 2019

thirug010 commented Jun 16, 2019

kueblc commented Jun 16, 2019

tsightler commented Jun 17, 2019 •

edited

Loading

AS137430 commented Jun 18, 2019

tsightler commented Jun 18, 2019

AS137430 commented Jun 18, 2019

kueblc commented Jun 18, 2019

tsightler commented Jun 18, 2019 •

edited

Loading

thirug010 commented Jun 19, 2019

tsightler commented Jun 19, 2019

Protocol 3.3 and encrypted discovery #214

Protocol 3.3 and encrypted discovery #214

Conversation

kueblc commented Jun 14, 2019

coveralls commented Jun 14, 2019 • edited Loading

Pull Request Test Coverage Report for Build 415

💛 - Coveralls

Apollon77 commented Jun 14, 2019

Apollon77 commented Jun 14, 2019

Apollon77 commented Jun 14, 2019

kueblc commented Jun 14, 2019

Apollon77 commented Jun 14, 2019

kueblc commented Jun 14, 2019

EricPalmquist commented Jun 16, 2019

Apollon77 commented Jun 16, 2019

kueblc commented Jun 16, 2019

thirug010 commented Jun 16, 2019

kueblc commented Jun 16, 2019

codetheweb commented Jun 16, 2019

kueblc commented Jun 16, 2019

Apollon77 commented Jun 16, 2019

thirug010 commented Jun 16, 2019

kueblc commented Jun 16, 2019

tsightler commented Jun 17, 2019 • edited Loading

AS137430 commented Jun 18, 2019

tsightler commented Jun 18, 2019

AS137430 commented Jun 18, 2019

kueblc commented Jun 18, 2019

tsightler commented Jun 18, 2019 • edited Loading

thirug010 commented Jun 19, 2019

tsightler commented Jun 19, 2019

coveralls commented Jun 14, 2019 •

edited

Loading

tsightler commented Jun 17, 2019 •

edited

Loading

tsightler commented Jun 18, 2019 •

edited

Loading