External Routes, Server-Sent Events, Websockets #63
Conversation
There was a problem hiding this comment.
This is looking pretty good to me, but I did have a question about the copyright lines.
I tested the two new examples and related functionality successfully on a Feather TFT S3.
One extra touch I think would be nice, but not worth holding this up for is showing something visible on the page. Both new examples are console.log() output only which could be missed by someone without as much webdev experience. For the SSE example it could show the most recently recieved temperature in a or some other element on the page which would make it immediately visible to everyone.
| @@ -0,0 +1,82 @@ | |||
| # SPDX-FileCopyrightText: 2023 Dan Halbert for Adafruit Industries | |||
There was a problem hiding this comment.
Are you open to having your own name or github handle for the copyright notice in the new files that you've written. Or perhaps even added to the library components that you've made lots of modifications too?
There was a problem hiding this comment.
Of course. I was not available during your live, but I watched the video save after and i saw the DJDevon3's comment on how to specify the name in these lines. Is my understanding that it should be # SPDX-FileCopyrightText: 2023 XYZ for non-employees correct? How about when one employee and one non-employee are to be specified?
There was a problem hiding this comment.
As far as I am aware it is allowed to have comma separated lists of names for the copyright in the event that there are multiple authors
i.e.:
# SPDX-FileCopyrightText: 2023 Dan Halbert for Adafruit Industries, Michal Pokusa
Thank you for testing during livestream. I will make the examples more visual and then re-open the PR. |
examples/httpserver_neopixel.py
Outdated
| # You can always manually create a Route object and import or register it later. | ||
| # Using this approach you can also use the same handler for multiple routes. | ||
| post_json_route = Route( | ||
| "/change-neopixel-color/json", GET, change_neopixel_color_handler_post_json |
There was a problem hiding this comment.
Should this be registered as a POST handler instead of GET? The function name and docstring with it mention POST.
I was surprised to find that it does actually still function with a GET request though. I didn't realize before that a body could be sent along with a GET request, but I was able to successfully send the JSON object to change the neopixel color using this handler so it can work. There is some discussion here about the topic https://stackoverflow.com/questions/978061/http-get-with-request-body
There was a problem hiding this comment.
Should this be registered as a
POSThandler instead ofGET? The function name and docstring with it mention POST.I was surprised to find that it does actually still function with a GET request though. I didn't realize before that a body could be sent along with a GET request, but I was able to successfully send the JSON object to change the neopixel color using this handler so it can work. There is some discussion here about the topic https://stackoverflow.com/questions/978061/http-get-with-request-body
You are right, I must have missed that when I was doing the copy-pasting. Changed that in latest commit. I also restricted the Request.json() to POST requests only.
Updating https://github.com/adafruit/Adafruit_CircuitPython_MCP9808 to 3.3.21 from 3.3.20: > Merge pull request adafruit/Adafruit_CircuitPython_MCP9808#37 from jposada202020/updating_example Updating https://github.com/adafruit/Adafruit_CircuitPython_AdafruitIO to 5.7.2 from 5.7.1: > Merge pull request adafruit/Adafruit_CircuitPython_AdafruitIO#98 from ForgottenProgramme/mahe-typehint Updating https://github.com/adafruit/Adafruit_CircuitPython_HTTPServer to 4.2.0 from 4.1.0: > Merge pull request adafruit/Adafruit_CircuitPython_HTTPServer#63 from michalpokusa/external-routes-websockets-sse Updating https://github.com/adafruit/Adafruit_CircuitPython_NeoKey to 1.1.0 from 1.0.12: > Merge pull request adafruit/Adafruit_CircuitPython_NeoKey#11 from EAGrahamJr/all-the-buttons Updating https://github.com/adafruit/Adafruit_CircuitPython_RGBLED to 1.1.17 from 1.1.16: > Merge pull request adafruit/Adafruit_CircuitPython_RGBLED#23 from BiffoBear/Add_typing Updating https://github.com/adafruit/Adafruit_CircuitPython_TinyLoRa to 2.2.16 from 2.2.15: > Merge pull request adafruit/Adafruit_CircuitPython_TinyLoRa#50 from samatjain/type-annotations Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA: > Updated download stats for the libraries
PR does not contain any breaking changes. All is backwards compatible.
⭐ Added:
SSEResponseandWebsocketclasses for handling Server-Sent Events and Websocket requests. (maybe closes Yielding data from an async task to a chunked response #62)Server.add_routes()for addign routes that e.g. are defined in separate file and do not have access toServerobject.as_route()decorator for converting handler functions intoRouteobjects, that can be later imported and added to aServerinstance🛠️ Updated/Changed:
Server.poll()there is no more awithstatement, this is because it closes a socket every time, but we need it open for SSE and Websockets.Routeis now a public class and acceptshandleras argument in constructor. Also all validation and checks are moved to this class.🏗️ Refactor:
Responseclasses (exceptSSEResponseandWebsocket) explicitly close socket (see Updated/Changed second point) after sending data.