Skip to content

Make loop() in esp32spi implementation non-blocking #69

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

Merged
merged 3 commits into from
Feb 19, 2021

Conversation

dlizotte-uwo
Copy link
Contributor

Have been having a discussion with @brentru about an issue with the esp32spi implementation of loop() always blocking until the end of the keepalive period. We think raising OSError(errno.ETIMEDOUT) at the right point may fix that issue. Also replaces some %x format codes with %s in the logging statements because bytearrays can't be formatted with %s. (The %s is ugly but it works.)

… OSError like socketpool does if no bytes are present and none arrive before first socket timeout. This allows loop to be non-blocking.
@brentru
Copy link
Member

brentru commented Feb 19, 2021

@dlizotte-uwo Great work - thank you for the PR! I exercised the client against the issue in #68 by removing the calls to publish within the while True loop, and just pumped the message queue:

# Connect the client to the MQTT broker.
print("Connecting to Adafruit IO...")
mqtt_client.connect()

photocell_val = 0
while True:
    # Poll the message queue
    mqtt_client.loop()

On master, it stalls within _exact_recv which was the behavior described in #68.

On this pull request, it sends a PINGREQ and gets a PINGRESP to keep the connection with the broker open.

959.437: DEBUG - _sock_exact_recv timeout
959.938: DEBUG - KeepAlive period elapsed - requesting a PINGRESP from the server...
959.94: DEBUG - Sending PINGREQ
960.146: DEBUG - Got PINGRESP
961.851: DEBUG - _sock_exact_recv timeout

Copy link
Member

@brentru brentru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dlizotte-uwo The CI is currently failing on running the black formatter: https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT/pull/69/checks?check_run_id=1905751739#step:11:22

Could you run formatting tools on this pull request (https://learn.adafruit.com/improve-your-code-with-pylint) so I can merge it in?

Thank you for the contribution.

Copy link
Member

@brentru brentru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checks passing, lgtm!

@brentru brentru merged commit b0b4418 into adafruit:master Feb 19, 2021
@dlizotte-uwo
Copy link
Contributor Author

OK great! Thanks for walking me though the process.

adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Feb 24, 2021
Updating https://github.com/adafruit/Adafruit_CircuitPython_DHT to 3.5.6 from 3.5.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_DHT#62 from jposada202020/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306 to 1.3.0 from 1.2.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306#19 from SAK917/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_IL0373 to 1.3.6 from 1.3.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_IL0373#21 from makermelissa/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_INA260 to 1.3.0 from 1.2.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_INA260#15 from gpongelli/patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_MCP2515 to 1.0.4 from 1.0.3:
  > Changed py_modules to packages

Updating https://github.com/adafruit/Adafruit_CircuitPython_SCD30 to 2.0.3 from 2.0.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_SCD30#10 from caternuson/remove_asc

Updating https://github.com/adafruit/Adafruit_CircuitPython_SSD1608 to 1.2.5 from 1.2.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_SSD1608#10 from makermelissa/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_SSD1681 to 1.0.2 from 1.0.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_SSD1681#3 from makermelissa/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_ST7789 to 1.4.4 from 1.4.3:
  > Merge pull request adafruit/Adafruit_CircuitPython_ST7789#22 from wildestpixel/patch-2

Updating https://github.com/adafruit/Adafruit_CircuitPython_VL53L0X to 3.3.6 from 3.3.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_VL53L0X#26 from caternuson/iss25

Updating https://github.com/adafruit/Adafruit_CircuitPython_VL6180X to 1.2.6 from 1.2.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_VL6180X#17 from OleMchls/patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_Gizmo to 1.3.0 from 1.2.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_Gizmo#15 from makermelissa/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_MagTag to 1.7.0 from 1.6.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_MagTag#56 from KTibow/patch-2

Updating https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT to 5.0.2 from 5.0.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#69 from dlizotte-uwo/master

Updating https://github.com/adafruit/Adafruit_CircuitPython_Motor to 3.2.7 from 3.2.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_Motor#52 from jedgarpark/pico-dc-motor-example

Updating https://github.com/adafruit/Adafruit_CircuitPython_PIOASM to 0.1.6 from 0.1.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_PIOASM#8 from jepler/commas

Updating https://github.com/adafruit/Adafruit_CircuitPython_PortalBase to 1.2.3 from 1.2.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_PortalBase#13 from Neradoc/patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Added the following libraries: Adafruit_CircuitPython_SSD1681
rtwfroody pushed a commit to rtwfroody/Adafruit_CircuitPython_MiniMQTT that referenced this pull request Sep 18, 2022
Make loop() in esp32spi implementation non-blocking
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