Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework recv() #158

Merged
merged 10 commits into from
Jul 8, 2022
Merged

Rework recv() #158

merged 10 commits into from
Jul 8, 2022

Conversation

uzlonewolf
Copy link
Collaborator

@uzlonewolf uzlonewolf commented Jul 7, 2022

This PR reworks socket receiving (again). It separates read retries from send retries and tries a bit harder to receive a valid message.

If a null payload is received, it treats it as an ACK and will not resend a command even if no actual payload is ever received.
If a null payload or corrupt message is received, it retries the read (but not the send).
If a generic exception or socket timeout is encountered without first receiving a null payload, it retries the send. (This matches the existing behavior, it is not a change.)
If a generic exception is encountered after receiving a null payload, it retries the read (but not the send).
If a socket timeout is encountered after receiving a null payload, it immediately returns None.

Decryption failures now return a json error instead of None as that seems to be the more correct thing to do. If it is a null payload then decryption is skipped and None is returned.

In addition, it now searches the received data for the message start prefix. This could help with temperamental bulbs/devices if they send an extra NUL or other garbage before the start prefix or if a message somehow "tears" into 2 packets with the 2nd half badly delayed.

@uzlonewolf
Copy link
Collaborator Author

uzlonewolf commented Jul 7, 2022

Well, I started to implement a _recv_all() due to the fact that socket.recv() can return less data than requested, but then stopped as I figured "what are the chances that a <300 byte packet will get broken up?" But then I remembered a previous experience with a less-than-great 'thing' that was waaaay too trigger-happy with firing off packets before the data it was sending was fully buffered and thus would break up a few dozen bytes into 3+ packets. With the talk about problematic smartbulbs and WiFi being WiFi I thought it would be prudent to add it after all.

@jasonacox jasonacox merged commit 36467fc into jasonacox:master Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants