-
Notifications
You must be signed in to change notification settings - Fork 51
Fix socket receive issues related to message buffer size #55
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
Conversation
1a23d7b
to
5947477
Compare
Hi @tannewt . I could use some much needed help on the CI failure, as well as code review from you on this PR. |
5947477
to
c9fdc40
Compare
https://learn.adafruit.com/contribute-to-circuitpython-with-git-and-github has some info about running black/pylint |
Thank you for the link. It seems that Travis is no longer used and https://learn.adafruit.com/contribute-to-circuitpython-with-git-and-github/open-pull-request needs an update, agree? It would be nice to add more info on running black tool @dherrada : The ci issues are not related to my changes so I will definitely need some help. I managed to get past the
Best, -- flaviof |
Hi @brentru ! Based on your involvement with https://learn.adafruit.com/pyportal-weather-station , I would like very |
@flavio-fernandes I'll get the REUSE thing fixed right away. |
@flavio-fernandes master is now passing, but there are a few conflicts. I would recommend running pre-commit locally to make the conflicts smaller and then resolving the remaining ones manually |
Sounds good. How do you run the pre-commit locally? All I got so far is "pip install black ; black file.py" ;) |
e353adb
to
b90bf88
Compare
@flavio-fernandes Install pre-commit with pip, and then just run it with |
b90bf88
to
a1a04d6
Compare
This change addresses a few issues in the handling of the MQTT messages that caused the library to become unstable: - Add wrapper for socket.recv() so that an exact number of bytes are read into the buffer before attempting to parse the MQTT message; - Fix handling of ping response packets via _wait_for_msg(); - Fix disconnect so it can gracefully handle cases when socket writes are not possible. Also re-init _subscribed_topics as an empty list instead of None. Related-to adafruit/Adafruit_CircuitPython_ESP32SPI#102 Fixes adafruit/Adafruit_CircuitPython_PyPortal#98 Fixes adafruit#54 Signed-off-by: Flavio Fernandes <[email protected]>
a1a04d6
to
3e5a14d
Compare
@flavio-fernandes This is passing, do you want me to re-review on hardware? |
Sure thing @brentru ! I have 2 pyportals running this code for over a week w/out any problems. If you or anyone would be interested on using that code.py, here is a link: Thanks, -- flaviof |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@flavio-fernandes Thanks for making the requested changes! Tested simpletest and adafruit io on Adafruit CircuitPython 6.1.0-rc.1 on 2021-01-15; Adafruit PyPortal with samd51j20
Updating https://github.com/adafruit/Adafruit_CircuitPython_CircuitPlayground to 4.2.1 from 4.2.0: > Merge pull request adafruit/Adafruit_CircuitPython_CircuitPlayground#100 from adafruit/REUSE > Added pre-commit-config file > Added pre-commit and SPDX copyright > Merge pull request adafruit/Adafruit_CircuitPython_CircuitPlayground#96 from adafruit/tap-matching Updating https://github.com/adafruit/Adafruit_CircuitPython_EPD to 2.7.1 from 2.7.0: > Merge pull request adafruit/Adafruit_CircuitPython_EPD#44 from adafruit/REUSE > Added pre-commit-config file > Added pre-commit and SPDX copyright Updating https://github.com/adafruit/Adafruit_CircuitPython_IL0373 to 1.3.3 from 1.3.2: > Merge pull request adafruit/Adafruit_CircuitPython_IL0373#19 from adafruit/REUSE > Merge pull request adafruit/Adafruit_CircuitPython_IL0373#18 from peterhinch/patch-1 > Added pre-commit-config file > Added pre-commit and SPDX copyright Updating https://github.com/adafruit/Adafruit_CircuitPython_LSM9DS1 to 2.1.5 from 2.1.4: > Merge pull request adafruit/Adafruit_CircuitPython_LSM9DS1#28 from adafruit/REUSE > Added pre-commit-config file > Added pre-commit and SPDX copyright Updating https://github.com/adafruit/Adafruit_CircuitPython_MLX90393 to 2.0.3 from 2.0.2: > Merge pull request adafruit/Adafruit_CircuitPython_MLX90393#26 from chrisbailey4/master Updating https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT to 4.1.0 from 4.0.2: > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#55 from flavio-fernandes/socket_read_fixes Updating https://github.com/adafruit/Adafruit_CircuitPython_RSA to 1.2.1 from 1.2.0: > Merge pull request adafruit/Adafruit_CircuitPython_RSA#17 from adafruit/REUSE > Added pre-commit-config file > Added pre-commit and SPDX copyright Updating https://github.com/adafruit/Adafruit_CircuitPython_TinyLoRa to 2.2.0 from 2.1.3: > Merge pull request adafruit/Adafruit_CircuitPython_TinyLoRa#36 from imliubo/master
Fix socket receive issues related to message buffer size
This change addresses a few issues in the handling of the MQTT
messages that caused the library to become unstable:
Add wapper for socket.recv() so that an exact number of bytes
are read into the buffer before attempting to parse the MQTT
message;
Fix handling of ping response packets via _wait_for_msg();
Fix disconnect so it can gracefully handle cases when socket writes
are not possible. Also re-init _subscribed_topics as an empty list
instead of None.
Related-to adafruit/Adafruit_CircuitPython_ESP32SPI#102
Fixes adafruit/Adafruit_CircuitPython_PyPortal#98
Fixes #54
Signed-off-by: Flavio Fernandes [email protected]