Initial work on refactoring pytuya #58

postlund · 2020-09-28T21:02:15Z

I'm not entirely done yet, I want to work a bit on the "receive" code as I haven't touched that yet. But I wanted to share what I'm working on, to give an idea where I'm heading (@rospogrigio ).

rospogrigio · 2020-09-28T21:12:10Z

Wow, amazing job! Still a bit unclear what you're aiming to with that "segno" but excellent code cleaning job. 💪
Let me know when you think you're done so I can do some testing and review.

postlund · 2020-09-28T21:17:20Z

Each message has a sequence number that should be matched when receiving the answer. I'm not sure if it's used for anything practical, but it should be increased for every new message. At least in theory, two message could be sent in order and responses received in opposite order.

Yeah, I'll let you know 👍

postlund · 2020-09-29T10:58:34Z

@rospogrigio Now would be a great time to test. I've made most of the changes I want, a few more are remaining and I will deal with those in due time. Want to get cracking on asyncio though. Tried to make everything more general and remove magic constants. So we parse more things than we use, but that's ok.

Still lots of things to improve, but it's a start. Will continue with receive code.

ultratoto14

Just something to think about, in case of RGBW lights, we may need to send multiple dps at the same time.
We may need to have a method to send all theses dps in a single message.
Sending them in a sequence will produce weird effects as the light may flash or quickly change color.

postlund · 2020-09-29T14:08:22Z

Yeah, you can do that by calling exchange manually. But we can extend set_dps to do that too, it's a trivial change.

rospogrigio

See comments, almost working...

custom_components/localtuya/pytuya/__init__.py

rospogrigio · 2020-09-30T05:30:23Z

custom_components/localtuya/pytuya/__init__.py

+                dev_type,
+                self.dev_type,
+            )
+            return self.exchange(command, data)


This is giving problems, since you are using the same variable (data) that is used to receive the data from the device, so if I print out the json_data of the payload I get:
DEBUG:localtuya.pytuya:json_data={'devId': 'bf4ce0d8be2bc62e2fgdoy', 'uid': 'bf4ce0d8be2bc62e2fgdoy', 't': '1601443774', 'dps': b'\x00\x00U\xaa\x00\x00\x00\x00\x00\x00\x00\n\x00\x00\x00,\x00\x00\x00\x01\x82\x86>el\xe2\xd1\xe5\xe90(\x05\x9b\xc8\xea\x8aA\xd1\xfb\xc0\x98)Z\x1e\x8er\x05dd?5UTH^\xc4\x00\x00\xaaU'}

Ah, yes, that's not very good.

rospogrigio · 2020-09-30T05:32:49Z

custom_components/localtuya/pytuya/__init__.py

+            try:
+                data = self.status()
+            except Exception as e:
+                _LOGGER.warning("Failed to get status: [{}]", e)


All the warning and error messages for Exceptions trigger the following errors:
Traceback (most recent call last):
File "/usr/lib/python3.7/logging/init.py", line 1034, in emit
msg = self.format(record)
File "/usr/lib/python3.7/logging/init.py", line 880, in format
return fmt.format(record)
File "/usr/lib/python3.7/logging/init.py", line 619, in format
record.message = record.getMessage()
File "/usr/lib/python3.7/logging/init.py", line 380, in getMessage
msg = msg % self.args
TypeError: not all arguments converted during string formatting
Call stack:
File "./tuyadebug/test.py", line 42, in
localtuya.devicePrint(DEVID, DEVIP, DEVKEY, DEVVERS)
File "/root/tuyadebug/localtuya/init.py", line 162, in devicePrint
err,data,dev_type = deviceInfo(deviceid, ip, key, vers)
File "/root/tuyadebug/localtuya/init.py", line 108, in deviceInfo
data = d.detect_available_dps()
File "/root/tuyadebug/localtuya/pytuya/init.py", line 274, in detect_available_dps
_LOGGER.error("Failed to get status: [{}]", e)
Message: 'Failed to get status: [{}]'
Arguments: (TypeError('Object of type bytes is not JSON serializable'),)

Bad formatting of course, will fix.

rospogrigio · 2020-09-30T05:45:16Z

custom_components/localtuya/pytuya/__init__.py

@@ -301,40 +343,20 @@ def generate_payload(self, command, data=None):
            json_data["dps"] = data
        if command_hb == "0d":


Now this should be
if command_hb == 0x0d:
It took a lot to figure it out!!

Yeah, moved from strings to integer instead where possible.

postlund · 2020-09-30T06:57:21Z

Should be fixed now!

rospogrigio

Everything works on my side.
Merging to master, I also updated the tuyadebug.tgz package.

rospogrigio · 2020-09-30T07:45:48Z

Merged #58 into master.

postlund · 2020-09-30T07:47:16Z

Great! Moving steadily with the port to asyncio, let's see if I have something that can be tested today or tomorrow.

rospogrigio · 2020-09-30T07:48:26Z

OK I'll update #55 to fix the "bouncing" effect and ask for your review.

postlund · 2020-09-30T07:49:32Z

That'd be great!

postlund · 2020-09-30T10:25:15Z

I think I'm more or less done with the pytuya changes for asyncio, seems to work in my little test case. Just need to adjust the rest. I still haven't decided exactly how to deal with connection problems and re-connects, but I'm starting with a simple approach. If we have an open connection (when needed), use that otherwise connect again.

postlund · 2020-09-30T18:01:40Z

Another short status update.... Today I learned something new. I had some problems with the device suddenly started to respond with status updates using sequence number 0, mainly after changing a datapoint (the regular status polling worked as expected). I kinda ran into what we have a workaround for today, right here:

https://github.com/rospogrigio/localtuya-homeassistant/blob/2e46d65f9a89496bcfe512d462dc83ad5aaedba1/custom_components/localtuya/pytuya/__init__.py#L222

This message is actually the ACK message to set_dps, just saying that everything went ok (or not). A return code is included in the message indicating if an error occurred or not. After changing a state however, a spontaneous status update is sent by the device (always with sequence number zero), to indicate that status has changed. So the response to setting a datapoint doesn't really return the value of that datapoint, the update coming afterward does. The same update comes if a datapoint is changed for some other reason, like pressing the physical button on a wall plug. This is how live-updates are propagated back.

I looked at the tuyapi code and figured it out that way. Here is the ACK message:

https://github.com/codetheweb/tuyapi/blob/6b50f74842cd61b40935ef7646f7185172d8c8c8/index.js#L430

And here is the update:

https://github.com/codetheweb/tuyapi/blob/6b50f74842cd61b40935ef7646f7185172d8c8c8/index.js#L447

This means that we can scrap the polling part altogether. We just poll status one time at start, then we rely on push updates. We need to add heart beat and send at an interval, but that is trivial. Pretty good discovery I would say, but it requires a few changes.

rospogrigio · 2020-09-30T20:41:12Z

Wow, this looks very interesting. Couple of questions though:

are you sure this works both for 0a and 0d devices?
are we really relying 100% on push updates? what happens if we miss some message?
Looks very cool anyway...

postlund · 2020-09-30T21:03:14Z

I would expect it to be the same, message type 0x08 is used for status updates. Should have something you can try tomorrow (maybe) to be sure.

I will modify what I currently have to work with push updates only. This is a TCP connection, we can't miss updates (unless there is bug in the tuya firmware).

postlund · 2020-10-01T07:39:04Z

One thing that came to my mind now is fields like power consumption. I need to verify if we get updates when they change.

postlund · 2020-10-01T10:25:25Z

I hooked things up and I have verified that updates are sent when any DP changes. Even so for power consumption, which means we are good!

postlund force-pushed the pytuya_refactor branch from 5234f27 to da6628b Compare September 28, 2020 21:05

rospogrigio mentioned this pull request Sep 28, 2020

Updating the status of TuyaDevice using the response of the set_dps() call #55

Merged

postlund force-pushed the pytuya_refactor branch from da6628b to c963a37 Compare September 29, 2020 10:54

Initial work on refactoring pytuya

0c953be

Still lots of things to improve, but it's a start. Will continue with receive code.

postlund force-pushed the pytuya_refactor branch from c963a37 to 0c953be Compare September 29, 2020 11:11

ultratoto14 reviewed Sep 29, 2020

View reviewed changes

rospogrigio reviewed Sep 30, 2020

View reviewed changes

Fix review comments

ecf1cac

rospogrigio approved these changes Sep 30, 2020

View reviewed changes

Merge branch 'master' into pytuya_refactor

8e843a9

rospogrigio merged commit d9c6d66 into master Sep 30, 2020

rospogrigio deleted the pytuya_refactor branch September 30, 2020 07:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial work on refactoring pytuya #58

Initial work on refactoring pytuya #58

postlund commented Sep 28, 2020

rospogrigio commented Sep 28, 2020

postlund commented Sep 28, 2020

postlund commented Sep 29, 2020

ultratoto14 left a comment

postlund commented Sep 29, 2020

rospogrigio left a comment

rospogrigio Sep 30, 2020

postlund Sep 30, 2020

rospogrigio Sep 30, 2020

postlund Sep 30, 2020

rospogrigio Sep 30, 2020

postlund Sep 30, 2020

postlund commented Sep 30, 2020

rospogrigio left a comment

rospogrigio commented Sep 30, 2020

postlund commented Sep 30, 2020

rospogrigio commented Sep 30, 2020

postlund commented Sep 30, 2020

postlund commented Sep 30, 2020

postlund commented Sep 30, 2020

rospogrigio commented Sep 30, 2020

postlund commented Sep 30, 2020

postlund commented Oct 1, 2020

postlund commented Oct 1, 2020

		@@ -301,40 +343,20 @@ def generate_payload(self, command, data=None):
		json_data["dps"] = data
		if command_hb == "0d":

Initial work on refactoring pytuya #58

Initial work on refactoring pytuya #58

Conversation

postlund commented Sep 28, 2020

rospogrigio commented Sep 28, 2020

postlund commented Sep 28, 2020

postlund commented Sep 29, 2020

ultratoto14 left a comment

Choose a reason for hiding this comment

postlund commented Sep 29, 2020

rospogrigio left a comment

Choose a reason for hiding this comment

rospogrigio Sep 30, 2020

Choose a reason for hiding this comment

postlund Sep 30, 2020

Choose a reason for hiding this comment

rospogrigio Sep 30, 2020

Choose a reason for hiding this comment

postlund Sep 30, 2020

Choose a reason for hiding this comment

rospogrigio Sep 30, 2020

Choose a reason for hiding this comment

postlund Sep 30, 2020

Choose a reason for hiding this comment

postlund commented Sep 30, 2020

rospogrigio left a comment

Choose a reason for hiding this comment

rospogrigio commented Sep 30, 2020

postlund commented Sep 30, 2020

rospogrigio commented Sep 30, 2020

postlund commented Sep 30, 2020

postlund commented Sep 30, 2020

postlund commented Sep 30, 2020

rospogrigio commented Sep 30, 2020

postlund commented Sep 30, 2020

postlund commented Oct 1, 2020

postlund commented Oct 1, 2020