uzlonewolf
commented
2 years ago 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.
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.