loop() timeout parameter should be absolute

[vladak]

This changes the way how loop() deals with the timeout parameter. I think the original idea was to wait for the timeout and then return what was received in the mean time, regardless of whether anything was actually received. This means that the loop() will always return after the timeout, i.e. not earlier (assuming no fatal error happened).

Inspired by #68 (comment) and the discussion in PR #168.

[brentru]

This PR correctly addresses the comment:

loop() is blocking because _wait_for_msg() is blocking, which is because _sock_exact_recv() is blocking as currently implemented

@vladak I have some questions:

1. Do we need to change any example files to reflect the modifications to loop()
2. Have you tested this on hardware yet?

[FoamyGuy]

I'm doing some testing on adafruit/Adafruit_CircuitPython_ESP32SPI#167 which is aiming to make the esp32spi socket match the CPython socket behavior as close as possible, that led me to this problem and solution proposed here.

@brentru From my testing I think I can answer these:

1. Do we need to change any example files to reflect the modifications to loop()
2. Have you tested this on hardware yet?

1: The examples do not need to change as a result of the change in this PR because there is a default provided for timeout=0 and all of the examples call loop() without passing anything so they'll use this default value, which results in the expected behavior now as far as I understand it.

2: I have tested the change here in #169 successfully in 3 contexts:

• ESP32SPI*
• Builtin wifi with ESP32-S2 TFT
• CPython on a Linux PC.

The asterisks for ESP32SPI is because my testing was performed on the version of ESP32SPI from adafruit/Adafruit_CircuitPython_ESP32SPI#167 rather than the currently stock released version.

When using that modified ESP32SPI + The version from this PR the example scripts are working as intended and the blocking (or lack thereof) behavior of loop() appears to work correct to me.

On a somewhat related note, I do think it would be good to tweak the time value here:

Adafruit_CircuitPython_MiniMQTT/examples/esp32spi/minimqtt_pub_sub_nonblocking_esp32spi.py

Line 112 in 40b9096

time.sleep(0.5)

to be a little bigger. During testing I found that after a few dozen or so successful publishes it stops working and seems to fail silently. The publish code is called but the feed on aio doesn't get the new data, and the message callback isn't called.

Im not certain of the root cause, but I started suspecting we were publishing data "too fast" and that was causing it to start failing. I backed that off to a 3 second sleep and it's been running steadily without having the same problem. With regards to question 1 above: this change is not necessarily required, but it does seem good for stability.

[brentru]

1: The examples do not need to change as a result of the change in this PR

EXCELLENT! Thank you for testing.

On a somewhat related note, I do think it would be good to tweak the time value here:

@vladak Are you able to increase the delay value on the line specified by @FoamyGuy? After that, I'd like to go ahead and merge this PR.

[vladak]

This is super awesome if someone comes to the rescue and tests the code in the PR for you ! Thanks @FoamyGuy ! I have to admit I fired off the PR primarily to get the discussion started and skimped on proper testing. That said, I will think about a unit test for this.

[vladak]

Im not certain of the root cause, but I started suspecting we were publishing data "too fast" and that was causing it to start failing. I backed that off to a 3 second sleep and it's been running steadily without having the same problem. With regards to question 1 above: this change is not necessarily required, but it does seem good for stability.

Okay, changed to 3 seconds.

[FoamyGuy]

I'm doing some more testing on this and all the other changes associated with the ESP32SPI socket compatibility and I noticed that the mqtt_client.loop() function was never returning when used with Ethernet Featherwing if there was no data to recieve. It would never timeout successfully which made it effectively blocking until data does get recieved.

I opened: adafruit/Adafruit_CircuitPython_Wiznet5k#125 over there to address this and successfully tested the adafruitio ethernet example in this repo with those changes (and the changes from this PR).

[FoamyGuy]

Looks good to me. Thank you @vladak!

Merging this one now along with the changes to ESP32SPI library and others that were all tied together in various ways.