Skip to content
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

smaller buffer size for file read to send as HTTPResponse. #15

Merged
merged 1 commit into from
Oct 18, 2022

Conversation

FoamyGuy
Copy link
Contributor

This lowers the buffer size that files are read into for sending as HTTPResponse down to 2048 bytes which puts it under a limit that is affecting raspberry pi pico w #7077 for details.

This change allows responses larger than 2920 bytes to be returned successfully. Tested successfully up to 5408 bytes.

Interestingly this change also seems to make the response come back quicker as well. With currently released version I am/was getting 300-500ms fairly consistently for the response times. With the version from this PR it's been more 100-150 with a few dipping below 100ms even.

@FoamyGuy FoamyGuy requested a review from a team October 17, 2022 21:53
@FoamyGuy
Copy link
Contributor Author

I tested this with both Pico W and ESP32-S2 Feather TFT using the reproducer script included in the core issue. Both work successfully with these changes even for files in excess of 2920 bytes.

@jepler
Copy link
Member

jepler commented Oct 18, 2022

Pico W has a bug where it won't send more than 2920 bytes (coincidentally?? 2*1460, a common TCP MSS value seen on wireless networks). However, the buffer size of 8192 bytes here is probably way too big.

The code should also deal with the fact that send() can return a value smaller than requested. A "sendall" wrapper function may need to be added so that values larger than one send() can handle are fully sent across multiple calls. I plan to PR the addition of sendall to the core, but an implementation here (or a max send so small it always works!) would be needed for airlift & for older versions of CircuitPython.

Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

This is fine by me.

@dhalbert dhalbert merged commit ba2da43 into adafruit:main Oct 18, 2022
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Oct 19, 2022
Updating https://github.com/adafruit/Adafruit_CircuitPython_ADXL34x to 1.12.7 from 1.12.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_ADXL34x#37 from tekktrik/dev/fix-value-error

Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306 to 1.5.10 from 1.5.9:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306#29 from tekktrik/dev/fix-type-annotation

Updating https://github.com/adafruit/Adafruit_CircuitPython_GPS to 3.10.6 from 3.10.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_GPS#90 from tekktrik/dev/fix-example-import

Updating https://github.com/adafruit/Adafruit_CircuitPython_PM25 to 2.1.13 from 2.1.12:
  > Merge pull request adafruit/Adafruit_CircuitPython_PM25#25 from tekktrik/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_SD to 3.3.15 from 3.3.14:
  > Merge pull request adafruit/Adafruit_CircuitPython_SD#51 from calcut/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_SHTC3 to 1.1.10 from 1.1.9:
  > Merge pull request adafruit/Adafruit_CircuitPython_SHTC3#17 from tcfranks/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_asyncio to 0.5.17 from 0.5.16:
  > Merge pull request adafruit/Adafruit_CircuitPython_asyncio#30 from jepler/make-tests-pass

Updating https://github.com/adafruit/Adafruit_CircuitPython_HTTPServer to 0.5.2 from 0.5.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_HTTPServer#15 from FoamyGuy/smaller_file_read_buffer

Updating https://github.com/adafruit/Adafruit_CircuitPython_ImageLoad to 1.16.7 from 1.16.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_ImageLoad#61 from deshipu/png

Updating https://github.com/adafruit/Adafruit_CircuitPython_Ticks to 1.0.8 from 1.0.7:
  > Merge pull request adafruit/Adafruit_CircuitPython_Ticks#8 from adafruit/try-time-ticksms
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.

3 participants