client.py report_mode may yield empty object #42

New Issue

topquark12 · 2021-01-06T11:38:02+08:00

topquark12 commented

2021-01-06 11:38:02 +08:00

In pytec/pytec/client.py, def report_mode(self) may yield empty object once every few seconds when polled continously.

pytec/autotune.py shows a temporary workaround and shows where the issue can arise.

In `pytec/pytec/client.py`, `def report_mode(self)` may yield empty object once every few seconds when polled continously. `pytec/autotune.py` shows a temporary workaround and shows where the issue can arise.

topquark12 was assigned by sb10q

2021-01-25 22:25:44 +08:00

topquark12 commented

2021-01-26 17:46:19 +08:00

Just logging what I've discovered so far.
The phrasing of this issue might not be very accurate. The issue is actually a second set of {} sent after the pwm x i_set y command.
As you can see in the Wireshark screen capture, the client side was a bit slow to ack the {} sent after pwm x i_set y, so the thermostat just decided to send it again, which ends up in the pytec client input buffer, which is interpreted as an "empty" object in the next cycle.
Not sure if the workaround in autotune.py is good enough, or should pytec/client.py be fixed, or should something be done in thermostat firmware.

Just logging what I've discovered so far. The phrasing of this issue might not be very accurate. The issue is actually a second set of ```{}``` sent after the ```pwm x i_set y``` command. As you can see in the Wireshark screen capture, the client side was a bit slow to ack the ```{}``` sent after ```pwm x i_set y```, so the thermostat just decided to send it again, which ends up in the pytec client input buffer, which is interpreted as an "empty" object in the next cycle. Not sure if the workaround in autotune.py is good enough, or should pytec/client.py be fixed, or should something be done in thermostat firmware.

Screenshot from 2021-01-26 17-44-45.png

251 KiB

sb10q commented

2021-01-26 17:54:45 +08:00

That's definitely a bug and the workaround is not acceptable. A TCP connection should never duplicate any data.
But I'm not sure if it's a TCP-level retransmission - normally Wireshark flags TCP retransmission packets as such, no? Isn't the TCP sequence number incremented between the two packets?

That's definitely a bug and the workaround is not acceptable. A TCP connection should never duplicate any data. But I'm not sure if it's a TCP-level retransmission - normally Wireshark flags TCP retransmission packets as such, no? Isn't the TCP sequence number incremented between the two packets?

sb10q commented

2021-01-26 17:55:44 +08:00

So likely it's a bug in the way the firmware passes TCP TX buffers to smoltcp.

topquark12 commented

2021-01-28 11:56:13 +08:00

Well I've done a bit of digging, so the send_line function is actually called twice in the main loop to send the {}, so TX buffer is working as expected.

However, I started to count the frequency of receiving double {} at the client, and it happens every 146 or 147 or 293 pwm 0 i_set 1 sent, which is 14 characters long including newline.

14 * 146 = 2044, 14* 147 = 2058, 14 * (293 / 2) = 2051 which are all suspiciously close to the socket RX buffer size of 2048.

I think something funny is happening at the Rx side of things.

Well I've done a bit of digging, so the send_line function is actually called twice in the main loop to send the {}, so TX buffer is working as expected. However, I started to count the frequency of receiving double {} at the client, and it happens every 146 or 147 or 293 ```pwm 0 i_set 1``` sent, which is 14 characters long including newline. 14 * 146 = 2044, 14* 147 = 2058, 14 * (293 / 2) = 2051 which are all suspiciously close to the socket RX buffer size of 2048. I think something funny is happening at the Rx side of things.

topquark12 referenced a pull request that will close this issue

2021-01-29 12:14:58 +08:00

main: fix double brackets sent when socket rx ring buffer wraps around #55

sb10q closed this issue

2021-01-29 16:18:08 +08:00

Sign in to join this conversation.