Add Server creation and management support #59

mscosti · 2019-07-11T03:10:56Z

This is still a work in progress, but opening it up for early feedback and suggestions.

Core Library additions

Adds Server creation methods to esp32spi.py
- Instantiates a server on the esp32 on the specified port and a specified socket that was previously requested
Adds WSGIServer class
- This is a simple WSGI compatible server implemtnation
- Accepts an application callable to be passed to it.
- Invokes the application callable when a request comes in
  - Passes an environment dictionary with request values per wsgi requirements.
  - Passes a start_response method that must be invoked before the application callable returns
  - Application callable must return with an iterable form of response data.

New example file

Shows how to write a simple WSGI compatible application callable, in the form of a callable class
- supports basic request handling
- supports serving files from disk
Shows how to use the example Application class to write a simple web app.
- serves an index.html and JS file from disk when hitting the server from your browser
- adds an led_on and led_off handler to turn on /off an LED
- adds in ajax/led_color handler to set the LED color

Todo outside of this PR

Start discussing what a Circuit Python WSGI web application framework might look like.

Would live in a separate repository, and would be compatible with any WSGI Server

siddacious · 2019-07-11T16:36:53Z

Looks like you ran into some pylint issues. Let me know if you need some assistance in resolving them

mscosti · 2019-07-12T22:54:40Z

Thanks. I installed an in editor extension to lint as a type so shouldn't be seeing anymore of those errors moving forward.

This PR is going to change a lot, as I originally had Server + AP in one, but we're going to be splitting them up. Since Server code is going to go in first, I'll need to do some git manipulation to get this PR to be independent of any AP code (#57)

ladyada · 2019-07-12T23:30:50Z

thank you for the effort, we'll get everything going, together!
let us know if you need a hand. you may want to start 2 new branches?

arturo182 · 2019-07-13T17:57:37Z

This is great, something I wanted to look into as well, thanks for doing this!

A request handling like you suggested would be great, I think it should also be possible to serve .html files directly from the filesystem.

Also, having a simple template system, but that's a separate library, I think.

mscosti · 2019-07-13T18:35:17Z

Yeah, definitely want to look into serving .html files from filesystem so you don't need to inline it in your python code, but I'm not sure yet if this would be a something built in to the lib or not. I also agree that a templating system would be separate.

Next up I think is to figure out what the request handling looks like. I'm thinking Server will maybe support two "modes".

In one mode you can register all your request routes and request handlers upfront before starting the server, and they will be automatically called when that particular request comes in. Unless there is something I'm not thinking of right now, I think this would only be possible if the server is in control of the main event loop. So it's simple to use for responding to requests, but you would only be act on that.
The other mode I'm thinking would be more manual and more user code, but with it more flexibility. You would have control over the main event loop in your code, but you'd be responsible for asking Server class to see if an incoming request matches a specific route, then handle it. This should make it so you can still do things outside of handling a specific event, like reading sensors or what have you.

I'm curious if its possible to support both modes at the same time if you have the server request listening loop and callback handling on a separate thread from code.py? I'm not familiar with threading in python, let alone know what is possible in circuitpython/micropython, though.

- can register callbacks on specific HTTP methods and paths - Can call update_poll in event loop to check for new incoming requests - update server example script

…tory

mscosti · 2019-07-14T22:19:50Z

@arturo182 I agree with you that static asset handling is an important enough feature for getting a useful webserver running quickly.

I've made it so you can configure a single directory as your 'static assets' folders by doing

server.set_static_dir("/path/to/folder")

and if a request comes through that doesn't match any of your own configured request handlers, but does match a GET request for one of the files in that directory, then that file will get automatically returned.

It tries to determine the content-type based on the file extension. Right now I've only tested .html, .css, and .js, but images should work in theory, but might need some special handling.

I've also made the method server.serve_file(self, file_path, dir=None) public so you can still have writing files to the socket handled for you in your own request handlers

mscosti · 2019-07-14T22:40:55Z

adafruit_esp32spi/adafruit_esp32spi_socket.py

+            return True
+        else:
+            status = _the_interface.socket_status(self.socknum)
+            # TODO: why is esp.<ConstantName> not defined? using magic numbers in mean time


I wanted to use the same constants as defined in adafruit_esp32spi, but for some reason I wasn't able to get that to work. The constants weren't accessible off of the that module when running in CP (but my editor was showing them as available)

Any ideas? We obviously don't want the magic numbers here.

Which line are you referring to here?

all of the numbers in status not in (magic numbers)

All of those are constants corresponding to ones defined in esp32spi.py https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/blob/master/adafruit_esp32spi/adafruit_esp32spi.py#L101-L111

My initial attempt to use them wasn't working, I'm sure there is just something silly i'm missing with being able to re-use those constants here.

mscosti · 2019-07-14T22:42:08Z

adafruit_esp32spi/adafruit_esp32spi_socket.py

+        else:
+            status = _the_interface.socket_status(self.socknum)
+            # TODO: why is esp.<ConstantName> not defined? using magic numbers in mean time
+            result = status not in (1,


for context, stole this logic from the arduino WiFiNINA lib: https://github.com/arduino-libraries/WiFiNINA/blob/master/src/WiFiClient.cpp#L221-L242

mscosti · 2019-07-14T22:46:45Z

examples/static/index.html

+        <script src="https://cdnjs.cloudflare.com/ajax/libs/jquery-minicolors/2.3.4/jquery.minicolors.min.js"></script>
+        <script async src="led_color_picker_example.js"></script>
+        <link rel="stylesheet" type="text/css" href="https://cdnjs.cloudflare.com/ajax/libs/jquery-minicolors/2.3.4/jquery.minicolors.css" />
+        <link rel="stylesheet" type="text/css" href="https://cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/4.3.1/css/bootstrap.min.css" />


I'm not familiarity with how licensing works with open source projects using other libraries. I double checked that jQuery, jQuery minicolors plugin, and bootstrap are all on MIT license, at least.

Let me know if this is not sufficient, we can re-write the the frontend to not need 3rd party libs.

adafruit_esp32spi/adafruit_esp32spi.py

sommersoft

Looks like an excellent start!! Thanks for this; I'm sure it will get used a lot.

Just a few things that jump out at me.

Note to other reviewers: I don't have any hardware to test this.

adafruit_esp32spi/adafruit_esp32spi.py

adafruit_esp32spi/adafruit_esp32spi_server.py

siddacious · 2019-07-15T00:44:10Z

Thanks @mscosti ! This is all great work, definitely will be very useful and make it much easier to setup REST-ey or webservices or pages to interact with the hardware.

My main concern right now is if the SPI32SPI library is the place to have a webserver live. It seems like this library should handle setting up the network connection and listening on a given port, but a separate library should be responsible for accepting and processing requests.

Great work, I'm really looking forward to using this and helping out round out the feature set, but I think we should make sure it's being put in the right place.

mscosti · 2019-07-15T02:51:41Z

I don't disagree @siddacious , but I think it's a larger discussion. You're right that this adds a lot of functionality that is definitely not a part of purely sending and receiving commands to/from the esp32 over SPI. We have the adafruit_esp32spi.py class for this.

But, this library has already gone past that line by introducing helper classes for making HTTP requests easier and parsing the response out into a response class (headers and body separated and all).

So, I do agree with you, but if that's the line we want to draw then I think we would then also have to move over everything else that has an understanding of the HTTP protocol (incoming HTTP request handling/routing and responding. Outgoing request creation and response parsing).

I can also see that server could be a little different because the feature set could grow pretty large, but it also doesn't have to. We just need to agree on where that line is drawn. Happy to continue to the discussion, I want this to end up in the right place too :)

siddacious · 2019-07-15T04:25:17Z

You're right @mscosti this certainly is a part of a larger discussion. This lib has definitely grown past purely talking to the ESP32 and has started including useful helpers for managing a wifi connection, making HTTP requests and packaging the responses into response objects as you say. While this is indeed beyond the obvious scope of "manage talking to the ESP32", what's been added is useful and for now at least makes sense to live here.

I think the dividing line that makes sense to me is to have this library do an incoming/server side equivalent to the outgoing/client side helpers that already exist. The way I see it, the existing helpers allow the user to create an HTTP request , then it will send the request and package up the response for the user.

In my mind, the correlate to that would be allowing the user to create an endpoint for clients to call, package up incoming requests into an object that something can be done with, and then handle returning the response to the user.

In terms of how I would see that changing what you've already done, I would imagine that the server object that this library creates would have an attribute like request_processor or similar that takes a single handler function with a defined interface. The handler function would be passed a request object and would return a response object. Calling update_poll in the main loop would call the handler in response to new requests. Basically as it is now but there would only be one on endpoint. Another library could handle farming that request out to different handlers depending on method, URI, etc.

arturo182 · 2019-07-15T11:02:07Z

I should have some time tomorrow to verify this on HW :)

mscosti · 2019-07-15T11:13:59Z

Thanks @arturo182 ! 😄

arturo182 · 2019-07-16T20:48:08Z

Oh damn, it slipped my mind! Will do it tomorrow for sure 😅

mscosti · 2019-07-17T04:18:29Z

No rush @arturo182, my weeknights this week are a bit busy so I won't be able to do much until tomorrow and Friday, and I have to address some PR comments and possibly some re working depending on what feature set we want in this library anyway.

Speaking of which:

@siddacious I think I'm understanding you correctly but want to make sure and to clarify. I think what you're suggesting is that Server would accept a generic request_handler function. This would be called from a stripped down version of update_poll whenever a request comes in, where it would simply pass on some parsed data, maybe something like a Request object that has

A parsed header dict
The raw body content
The request method (GET, POST, etc)
The request path
It wouldn't act on any of the above, just pass it along to the registered request_handler

That would then mean that the "endpoint registering and management" (on GET /my-endpoint call getFunc, on POST /my-endpoint call postFunc), could be moved somewhere else, along with auto static asset serving among other things and future features. It would be a pretty smooth transition to move the extra functionality that I already have to wherever we want to move it.

I guess, my only concern/question is, where do we think is the best place to put in all the extra server management goodies (both what we have right now + future features)? Would it be a new adafruit CP lib? The esp32SPI Server.request_handler would allow for pretty basic and unbiased flexible use, but I would imagine that most users will want this secondary library in order to get the more 'complete' server experience.

If we spin up a new lib, I feel like we would need a lot more design discussions to make sure we get it right. Another thing to think about is while it would be easy to just slot in what we have as middleware that goes on top of ESP32SPI (so it would likely have a dependency on ESP32SPI), if we put the effort in to make the new library I feel like it it shouldn't actually have the dependency at all and would need to be agnostic; the hardware api would need to conform to a strict interface. Otherwise there would be little gain in separating them out in the first place. Are there more circuit python wifi chips/coprocessors that can act as a server and would thus benefit from a generic server library?

Sorry for the long train of thought post and if i'm over analyzing our options 😅 I'm just really torn between what is maybe more correct from an engineering perspective and with what provides the best user experience for the ESP32 airlift boards. We both already acknowledged it's a big discussion, so just adding more questions and things to think about to it. I'm curious what a path forward might look like.

siddacious · 2019-07-17T06:38:21Z

@mscosti it sounds like you understood my suggestion perfectly, based on your summary. In all candor I'm leaning on my experience with the Rack framework in Ruby:
https://www.engineyard.com/blog/understanding-rack-apps-and-middleware

Rack defines a simple but robust interface for web apps or middleware to plug into a request handling stack on top of a range of different app servers. The whole middleware stack is way beyond what I have in mind which is more the clearly defined interface as you suggested.

I just did a bit of looking and of course there is an equivalent standard in Python, WSGI:
http://ivory.idyll.org/articles/wsgi-intro/what-is-wsgi.html
We can probably take a lot of inspiration from it, if not steal/use the idea outright.

With regards to where the extra server stuff would go, I was imagining a separate lib. I agree that if the intent is for it to be transport/hardware agnostic it would make sense to have a strictly defined interface. Perhaps I'm being naive but I don't think we need to rack our brains too hard over the API. WSGI, Rack or something similar would be a perfectly good first pass and we could refine it later.

The only other socket providing thing I know is somewhere near the horizon is the Ethernet FeatherWing though I imagine others are possible. It's certainly possible, even likely that Airlift/ESP32SPI would be the only hardware supporting the server library for a while. With that in mind I wouldn't be entirely opposed to keeping them together for the time being, though they would still want to be internally fairly distinct.

Thanks for all the thought (and work!) you're putting into this. If you're feeling torn between between doing things "correctly" and what provides the best UI, in my opinion UI wins every time. I would suggest trying to avoid getting too caught up trying to find an optimal solution; better IMHO to iterate more quickly, fail early, and roll with the punches as they come.

I'm happy to opine more but unless you have more questions for me, I'm content to leave it up to you to figure out the best approach. Other's opinions are always welcome and valued.

mscosti · 2019-07-21T18:54:19Z

@siddacious @brentru @sommersoft , I have completed the refactoring effort to have the main functionality that this lib provides to be a WSGI compatible server implementation, as discussed.

https://www.python.org/dev/peps/pep-0333/

The examples folder now contains an example that includes a simple 'web application framework' to illustrate how to interface with the wsgi server. It shows how to write an application with basic custom request handlers and file serving.

I have also addressed all PR comments with updated code or a follow up question/answer.

- Already in ap_mode branch

examples/esp32spi_wsgiserver.py

brentru · 2019-07-22T14:32:42Z

examples/esp32spi_wsgiserver.py

+wifi = wifimanager.ESPSPI_WiFiManager(esp, secrets, status_light)
+wifi.connect()
+
+class SimpleWSGIApplication:


Would it be better to make a library to handle this class, called CircuitPython_WSGI?

yep, absolutely. With a new repo/library there are probably a lot more features we could add to make building web apps really simple. This is something I'd like to contribute to.

This is just a simple example of how to use the wsgi server that the esp32SPI library provides. It's possible we remove this part of the example from here when we have a separate and more full featured wsgi web app library. Until we have our own solution built though, I think its important to at least illustrate how to write a wsgi app; show that its really not that complicated, here is something to use as is for simple use case, or here is a starter base to add your own features on top of.

To me, that's the best part about using an interface like wsgi to define communication between application and server.

you got it!
https://github.com/adafruit/Adafruit_CircuitPython_WSGI

examples/esp32spi_wsgiserver.py

brentru

@mscosti Thanks for implementing the changes. I have some feedback regarding the example.

mscosti · 2019-07-23T16:48:09Z

@brentru I have updated the PR with your feedback and is ready for re review whenever you have the chance

brentru · 2019-07-23T19:00:48Z

@mscosti Thank you for making the requested changes - the example looks good to me.

mscosti · 2019-07-23T20:09:16Z

Thanks @brentru !

Does anyone else have any additional feedback to add? @siddacious @sommersoft @ladyada ?
Do we need another round testing since a fair amount has changed since @arturo182 tested it?

I'm also curious what the process is for getting things squashed and merged, and then released?

brentru · 2019-07-23T20:38:03Z

@mscosti I'll do a test on a PyPortal later today with the current PR commit.

brentru · 2019-07-23T21:01:41Z

@mscosti Could you put the esp32spi_wsgiserver.py and /static/ into one folder in examples/server?

Works perfectly on a PyPortal running CircuitPython 4.0.2.

mscosti · 2019-07-23T23:11:42Z

@brentru Thanks for testing!

Moved static folder and example py script to a new folder located at /examples/server

brentru · 2019-07-24T14:17:32Z

@mscosti Looks OK to me for a merge into master if others agree. I was able to get the example up and running quickly on a PyPortal.

After this PR merges in, let's chat on discord about building out the WSGI library next (https://github.com/adafruit/Adafruit_CircuitPython_WSGI).

siddacious · 2019-07-24T16:42:52Z

Looks great! Merging!

Updating https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI to 1.7.0 from 1.6.0: > Merge pull request adafruit/Adafruit_CircuitPython_ESP32SPI#61 from brentru/switch-requests > Merge pull request adafruit/Adafruit_CircuitPython_ESP32SPI#59 from mscosti/server Updating https://github.com/adafruit/Adafruit_CircuitPython_PyPortal to 3.1.2 from 3.1.0: > Merge pull request adafruit/Adafruit_CircuitPython_PyPortal#50 from brentru/switch-requests > Merge pull request adafruit/Adafruit_CircuitPython_PyPortal#49 from brentru/fix-tft-in-use > Merge pull request adafruit/Adafruit_CircuitPython_PyPortal#47 from jerryneedell/jerryn_fixcursor > Merge pull request adafruit/Adafruit_CircuitPython_PyPortal#42 from adafruit/ladyada-patch-1

separate server code from AP code

5b28118

mscosti force-pushed the server branch from 130f966 to 5b28118 Compare July 13, 2019 13:49

Add server class to handle server start and get connected clients

5e09b7b

mscosti added 6 commits July 13, 2019 20:54

add simple request method handling

faf031a

- can register callbacks on specific HTTP methods and paths - Can call update_poll in event loop to check for new incoming requests - update server example script

py linting

8c4035d

more linting

b8fd02e

add in support for automatically serving files from a specified direc…

5bd6c86

…tory

add in example static asset serving with ajax call LED color changing

83d1525

fix spacing

f6d9415

sommersoft requested a review from a team July 14, 2019 22:30

mscosti marked this pull request as ready for review July 14, 2019 22:35

mscosti commented Jul 14, 2019

View reviewed changes

adafruit_esp32spi/adafruit_esp32spi.py Outdated Show resolved Hide resolved

sommersoft requested changes Jul 14, 2019

View reviewed changes

mscosti added 5 commits July 21, 2019 14:11

Remove server.py in favor of new wsgiserver.py

842ac46

Address PR comments for adafruit_esp32spi.py

c2b7cb0

fix file name. adafruite -> adafruit

4f3d07f

fix linting of example file

fc97dac

PATH_INFO shouldn't contain query params

41eab1c

sommersoft approved these changes Jul 21, 2019

View reviewed changes

mscosti added 2 commits July 21, 2019 22:31

remove wifimanager debug arg

e6bfd36

- Already in ap_mode branch

fix PATH_INFO when there are query params

473f850

brentru reviewed Jul 22, 2019

View reviewed changes

examples/esp32spi_wsgiserver.py Outdated Show resolved Hide resolved

brentru reviewed Jul 22, 2019

View reviewed changes

examples/esp32spi_wsgiserver.py Outdated Show resolved Hide resolved

brentru reviewed Jul 22, 2019

View reviewed changes

examples/esp32spi_wsgiserver.py Outdated Show resolved Hide resolved

brentru reviewed Jul 22, 2019

View reviewed changes

examples/esp32spi_wsgiserver.py Outdated Show resolved Hide resolved

brentru requested changes Jul 22, 2019

View reviewed changes

mscosti added 4 commits July 22, 2019 21:48

Address PR comments

a93c099

Use constants instead of magic numbers

fd4e191

fix import?

6951f8c

fix linting

a5d59f8

move all server example assets into examples/server

8dbeef0

siddacious merged commit c15e8fd into adafruit:master Jul 24, 2019

Add Server creation and management support #59

Add Server creation and management support #59

Uh oh!

Conversation

mscosti commented Jul 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Core Library additions

New example file

Todo outside of this PR

Uh oh!

siddacious commented Jul 11, 2019

Uh oh!

mscosti commented Jul 12, 2019

Uh oh!

ladyada commented Jul 12, 2019

Uh oh!

arturo182 commented Jul 13, 2019

Uh oh!

mscosti commented Jul 13, 2019

Uh oh!

mscosti commented Jul 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sommersoft left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

siddacious commented Jul 15, 2019

Uh oh!

mscosti commented Jul 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

siddacious commented Jul 15, 2019

Uh oh!

arturo182 commented Jul 15, 2019

Uh oh!

mscosti commented Jul 15, 2019

Uh oh!

arturo182 commented Jul 16, 2019

Uh oh!

mscosti commented Jul 17, 2019

Uh oh!

siddacious commented Jul 17, 2019

Uh oh!

mscosti commented Jul 21, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brentru left a comment

Choose a reason for hiding this comment

Uh oh!

mscosti commented Jul 23, 2019

Uh oh!

brentru commented Jul 23, 2019

Uh oh!

mscosti commented Jul 23, 2019

mscosti commented Jul 11, 2019 •

edited

Loading

mscosti commented Jul 15, 2019 •

edited

Loading