search

adafruit / Adafruit_CircuitPython_HTTPServer

Simple HTTP Server for CircuitPython
MIT License
46 stars 30 forks source link

Allow user to specify request buffer size. #4

Closed cthulahoops closed 2 years ago

cthulahoops commented 2 years ago

The next limit we ran into was and 1kb buffer for reading the request. Not knowing how much memory different devices have, it seemed best to expose the buffer size rather than just increase it.

Co-authored-by: Shae Erisson shae@scannedinavian.com

dhalbert commented 2 years ago

I wrote this library in a hurry for a specific use case, hence the 0.1.x version. It could do partial `recv's and build up the request instead of having to specify the biggest possible buffer, so that the buffer size becomes irrelevant.

There are some other libraries that you might look at as well. I am not sure about what your requirements are. Are you serving files or mainly doing routing? https://github.com/Neradoc/circuitpython-native-wsgiserver/ https://github.com/adafruit/Adafruit_CircuitPython_WSGI

cthulahoops commented 2 years ago

We thought about doing the partial receives approach, but ended up going with the quick fix here. We could probably look a implementing that if you like.

We have a bunch of neopixels, and we're sending animation frames as http requests - the encoding is pretty naive so one of the requests ended up at about 1.5 kb. There's another pending change to add support for query parameters that's not quite ready, and then I think this library does everything we need. (So far, at least!)

We haven't looked at the wsgi libraries. Thanks for linking them, we'll take look!

dhalbert commented 2 years ago

We haven't looked at the wsgi libraries. Thanks for linking them, we'll take look!

Given that you are not trying to fetch large files (which was the main purpose of this quickly-made library), yes, the wsgi libraries are probably in better shape for what you want to do. We have talked about replacing this library with an adapted version of @Neradoc's https://github.com/Neradoc/circuitpython-native-wsgiserver/ and adding built-in file fetching to that library.

FoamyGuy commented 2 years ago

@dhalbert understanding that WSGI library may be a better fit for this use case. Do you have thoughts on the specific change here? Any potential problems with allowing the argument to be passed here to change the buffer size? It seems okay to me, but I'm not sure I'd understand all of the ramifications.

dhalbert commented 2 years ago

[Deleted dupe comment.]

@FoamyGuy Adding the size argument creates a new API, which I'm not sure we want to support in the long run. The best fix is to make the size specification unnecessary by doing partial receives and concatenating the pieces as necessary

I only provided this library so we could do off-line-Wordle-like stuff quickly. We could include this PR, but I would like the whole thing to be re-evaluated and possibly replaced by a better base library.

cthulahoops commented 2 years ago

I kind of agree that the expanded API here isn't worth supporting. We've worked around this with a local change, and can evaluate alternative libraries. Happy for this to be closed.

dhalbert commented 2 years ago

Thanks, I will close it then, and open an issue for the general problem.