Skip to content

Remove pre 7.0 compatibility; other cleanup #102

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 2 commits into from
Mar 23, 2022
Merged

Conversation

dhalbert
Copy link
Contributor

Simplify and shorten the current code. I will have a further PR later that changes a bunch of thrown exceptions to allow for easier error handling.

  • CircuitPython 7 supports split() on bytes and bytearrays. Remove hand-coded routines no longer needed.
  • CircuitPython 6 supports streaming json decoding. Remove workaround for 5.x non-streaming decoding.
  • Refactor the duplicated check for gzip content.

Please test with your own wifi code - thanks. I tested this with the simple wifi testing code, and with the Google calendar example, but not with other examples.

Copy link
Member

@anecdata anecdata left a comment

Choose a reason for hiding this comment

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

Running fine so far on PyPortal (7.2.0).

Getting some of these on Metro ESP32-S2 (7.2.0):

  File "adafruit_requests.py", line 727, in post
  File "adafruit_requests.py", line 667, in request
  File "adafruit_requests.py", line 578, in _send_request
  File "adafruit_requests.py", line 554, in _send
NameError: name 'EAGAIN' is not defined

@dhalbert
Copy link
Contributor Author

Re-pushed; due to git problems, the code that was pushed was wrong.

@dhalbert dhalbert requested review from a team and anecdata March 21, 2022 17:19
@anecdata anecdata dismissed their stale review March 21, 2022 17:40

old code

@anecdata
Copy link
Member

Passes initial testing with my "production" code on PyPortal and Metro ESP32-S2 (CircuitPython 7.2.0). I'll load it on more devices and let it run.

@askpatrickw
Copy link
Contributor

I ran pytest locally and hit the same failure as the CI.

Changing https://github.com/adafruit/Adafruit_CircuitPython_Requests/blob/main/tests/post_test.py#L83
to sock.send.assert_called_with(b'Date=July 25, 2019')

Fixes the test... well it passed. But I don't understand why. ;-)

Copy link
Contributor

@Neradoc Neradoc left a comment

Choose a reason for hiding this comment

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

That's why ;) (well that's why there's the error)

Could we just import json as json_module at the start since it's a built-in module ? Or does that get us some mild memory optimization on M4 or other ?

@dhalbert
Copy link
Contributor Author

Also tested with a PyPortal (ESP32 coprocessor) with the OpenWeather demo. Ready for re-review.

Copy link
Member

@anecdata anecdata left a comment

Choose a reason for hiding this comment

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

Earlier version (w/o the json fix) has been running for a day on two types of 'production' code on Airlift and ESP32-S2/3 without issue. Expanded the test with current library to more applications, no issues found.

@dhalbert dhalbert merged commit a1b5050 into adafruit:main Mar 23, 2022
@dhalbert dhalbert deleted the cleanup branch March 23, 2022 03:50
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Mar 23, 2022
Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SH1106 to 1.2.3 from 1.2.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_SH1106#9 from tekktrik/dev/remove-int
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_SH1106#8 from tekktrik/dev/change-data-to-bytes
  > Fixed readthedocs build
  > Post-patch cleanup pt 2
  > Post-patch cleanup
  > Consolidate Documentation sections of README

Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SH1107 to 1.5.2 from 1.5.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_SH1107#13 from tekktrik/dev/change-data-to-bytes
  > Fixed readthedocs build
  > Consolidate Documentation sections of README

Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306 to 1.5.3 from 1.5.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306#26 from tekktrik/dev/change-data-to-bytes
  > Fixed readthedocs build
  > Post-patch cleanup
  > Consolidate Documentation sections of README

Updating https://github.com/adafruit/Adafruit_CircuitPython_FocalTouch to 1.4.0 from 1.3.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_FocalTouch#25 from winneymj/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_SSD1322 to 1.3.1 from 1.3.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_SSD1322#16 from tekktrik/dev/send-bytes

Updating https://github.com/adafruit/Adafruit_CircuitPython_VL53L4CD to 1.0.1 from 1.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_VL53L4CD#2 from caternuson/typo_fix

Updating https://github.com/adafruit/Adafruit_CircuitPython_asyncio to 0.5.8 from 0.5.7:
  > Merge pull request adafruit/Adafruit_CircuitPython_asyncio#21 from FoamyGuy/display_button_example

Updating https://github.com/adafruit/Adafruit_CircuitPython_PortalBase to 1.11.3 from 1.11.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_PortalBase#67 from FoamyGuy/docs_fixes
  > Merge pull request adafruit/Adafruit_CircuitPython_PortalBase#66 from FoamyGuy/docs_fixes

Updating https://github.com/adafruit/Adafruit_CircuitPython_Requests to 1.11.0 from 1.10.7:
  > Merge pull request adafruit/Adafruit_CircuitPython_Requests#102 from dhalbert/cleanup
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.

4 participants