pimoroni / phew

MIT License
204 stars 44 forks source link

Refactor Phew as a class to support multiple apps on one device running singly or concurrently; Support SSL and Session Authentication. #53

Open ccrighton opened 1 year ago

ccrighton commented 1 year ago
ccrighton commented 1 year ago

Currently, the run method takes control of the asyncio loop. That means nothing else can be added as a task. As a result, it's not possible to run anything but a web app. I've introduced a run_as_task method that doesn't ceed control of the loop. That means that Phew can be run alongside other functions such as an mqtt client, interrupt handlers and so on.

devfle commented 1 year ago

Very cool! If im not wrong, this would bring asyncio support for the Webserver 😄 ? This is exactly what I need, and I think a lots of other projects with an config webpage too. Nice work!

ccrighton commented 1 year ago

Thanks :-). Phew already supported asyncio under the covers but didn't support a way of calling it as a asyncio task.

riffnshred commented 1 year ago

If not merged, I think it stands well as its own fork. Love it !!!

rkompass commented 1 year ago

Thank you for this PR. For novices like me it would be helpful, now to have an example like this included.

ccrighton commented 1 year ago

If not merged, I think it stands well as its own fork. Love it !!!

I've now released a version that also fixes a critical bug.

https://pypi.org/project/micropython-ccrighton-phew/

optimal-system commented 6 months ago

I had an issue with closing the server to do other stuff. [(https://github.com/pimoroni/phew/issues/49#issuecomment-2134021937 )] Then I found your fork (v0.0.4) and it's indeed much cleaner to run Phew in asyncio loop. I managed to get one task running the Phew server and another one running a small part of my main program... It's not fully integrated yet so I'm still cautious about it.

ccrighton commented 5 months ago

I've now added support for TLS and cookie based session authentication.

Gadgetoid commented 5 months ago

Will give this a shot, thank you! Have a little project to test it with.

Edit: Right out of the gate it seems to work, but I've got some :sparkle: stuff :sparkle: to implement and see how it goes.

Also I know it's probably out of scope, but phew probably needs to report the IP address when it's connected (I don't think there's another PR for this):

diff --git a/phew/__init__.py b/phew/__init__.py
index 4131b0f..6ff2e25 100644
--- a/phew/__init__.py
+++ b/phew/__init__.py
@@ -34,12 +34,12 @@ def connect_to_wifi(ssid, password, timeout_seconds=30):
   import network, time

   statuses = {
-    network.STAT_IDLE: "idle",
-    network.STAT_CONNECTING: "connecting",
-    network.STAT_WRONG_PASSWORD: "wrong password",
-    network.STAT_NO_AP_FOUND: "access point not found",
-    network.STAT_CONNECT_FAIL: "connection failed",
-    network.STAT_GOT_IP: "got ip address"
+    network.STAT_IDLE: "> idle",
+    network.STAT_CONNECTING: "> connecting",
+    network.STAT_WRONG_PASSWORD: "> wrong password",
+    network.STAT_NO_AP_FOUND: "> access point not found",
+    network.STAT_CONNECT_FAIL: "> connection failed",
+    network.STAT_GOT_IP: "> got ip address: {ip}"
   }

   wlan = network.WLAN(network.STA_IF)
@@ -48,11 +48,14 @@ def connect_to_wifi(ssid, password, timeout_seconds=30):
   start = time.ticks_ms()
   status = wlan.status()

-  logging.debug(f"  - {statuses[status]}")
+  ip = wlan.ifconfig()[0] if status == network.STAT_GOT_IP else None
+  logging.debug(statuses[status].format(ip=ip))
+
   while not wlan.isconnected() and (time.ticks_ms() - start) < (timeout_seconds * 1000):
     new_status = wlan.status()
     if status != new_status:
-      logging.debug(f"  - {statuses[status]}")
+      ip = wlan.ifconfig()[0] if status == network.STAT_GOT_IP else None
+      logging.debug(statuses[status].format(ip=ip))
       status = new_status
     time.sleep(0.25)
Gadgetoid commented 5 months ago

@ccrighton since you probably have a lot more experience with this project than me, do you have any impressions on the other open PRs here? Looks like there are some valid fixes that could be swept up in one big merge 'n' release to get us to a decent version. Specifically #41 #54 #21 and #43.

Also if we merge here, it may or may not overshadow your fork- I don't want to downplay your huge contributions and think you should be credited more explicitly than just appearing in license headers and the list of maintainers. Any thoughts?

I'm aware we need some upstream attention here- Phew was written more or less explicitly for Enviro and has got more love than anyone expected!

ccrighton commented 5 months ago

@Gadgetoid I'll take a look. I've had a quick look at the logging truncation problem #41. The patch still doesn't behave as expected, although it is better. It doesn't always truncate at the closest newline. I've got a patch that only needs one find that fixes the issue. I'll commit it once I've tidied it up.

Still haven't had time to look at the other pull requests #54, #21, #43. I will review as soon as I can.

I don't have any immediate thoughts on credit. I appreciate that you are thinking about it :-). It would be good to have some more explicit credit if possible.

Regarding the pull request, I don't see a major issue if you merge the other pull requests. I may have create a branch and rebase from pimoroni/phew. Hopefully, not a big issue.

Gadgetoid commented 4 months ago

I don't have any immediate thoughts on credit. I appreciate that you are thinking about it :-). It would be good to have some more explicit credit if possible.

My biggest concern is that we've neglected Phew for at least a year and your fork clearly has a life of its own. I don't want to stamp out the flame, as it were, by coming in here a day late and a dollar short and merging your efforts without some consideration, credit or similar accommodations.

Thanks for casting your eye over the patches!

Gadgetoid commented 4 months ago

I'm also having thoughts along these lines:

diff --git a/phew/server.py b/phew/server.py
index 4c445be..20d70da 100644
--- a/phew/server.py
+++ b/phew/server.py
@@ -1,6 +1,7 @@
 import binascii
 import gc
 import random
+import builtins

 import uasyncio, os, time
 from . import logging
@@ -131,6 +132,14 @@ class Route:
     for part, compare in zip(self.path_parts, compare_parts):
       if not part.startswith("<") and part != compare:
         return False
+      if part.startswith("<") and ":" in part:
+        _, ptype = part[1:-1].split(":")
+        ptype = builtins.__dict__.get(ptype)
+        try:
+          _ = ptype(compare)
+        except ValueError as e:
+          return False
+
     return True

   # call the route handler passing any named parameters in the path
@@ -139,7 +148,12 @@ class Route:
     for part, compare in zip(self.path_parts, request.path.split("/")):
       if part.startswith("<"):
         name = part[1:-1]
-        parameters[name] = compare
+        ptype = str
+        if ":" in name:
+            name, ptype = name.split(":")
+            ptype = builtins.__dict__.get(ptype)
+
+        parameters[name] = ptype(compare)

     return self.handler(request, **parameters)

The first thing I did when firing up some code was footgun myself into trying to use a route with a number, since it had no automatic coercion to int. This patch - in theory - forces routes to (optionally) be typed so you either get a valid int, or a 404.

This does the same thing as Flask, effectively, though since I didn't look at how Flask does it (and forgot the pattern) I got the type/name "backwards."

ukscone commented 4 months ago

switched to ccrighton's fork of phew as it lets me run a function as an asyncio task and phew at the same time. i have pretty much everything sussed so far other than cleanly shutting down phew so i don't need to reset the picow between runs.

similar code that worked when i was trying to do this without using phew

async def shutdown():
    global rfid
    phew_app.stop()
    phew_app.close()

    rfid.cancel()
    await uasyncio.sleep(1)  # Give some time for the tasks to be canceled

try:       
  rfid = uasyncio.create_task(rfid_reader())
  loop = uasyncio.get_event_loop()
  phew_app.run_as_task(loop, host="0.0.0.0", port=80)
  loop.run_forever()
except KeyboardInterrupt:
    print("Shutting down...")
    loop.run_until_complete(shutdown())
finally:
    loop.close()
    print("Shutdown complete")

generates OSError: [Errno 98] EADDRINUSE

that i get elsewhere when a server doesn't exit cleanly and leaves the sockets open.

does anyone know how to stop & close the phew app cleanly?

ccrighton commented 4 months ago

Hi @ukscone,

You have made a good point. This is a gap either in the implementation or the documentation.

Try this after you end the asyncio loop:

import network
wlan = network.WLAN(network.STA_IF)
wlan.disconnect()

See network.WLAN docs.

ukscone commented 4 months ago

thanks @ccrighton doesn't seem to work (yet) but i'll keep trying it in various places in my code. Screenshot 2024-07-14 200421

ccrighton commented 4 months ago

Hi Russell,

You can try to disconnect from wifi after you have left the asyncio loop. I don't think you need to run it as an asyncio coroutine.

Cheers,

Charlie

On Mon, 15 Jul 2024 at 12:07, Russell Davis @.***> wrote:

thanks @ccrighton https://github.com/ccrighton doesn't seem to work (yet) but i'll keep trying it in various places in my code. Screenshot.2024-07-14.200421.png (view on web) https://github.com/user-attachments/assets/bdaef575-1c67-42fc-a85d-0d358835495f

— Reply to this email directly, view it on GitHub https://github.com/pimoroni/phew/pull/53#issuecomment-2227529239, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABUNQSURC6LFQZDZDYV43NTZMMHCLAVCNFSM6AAAAAAY6W73EOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRXGUZDSMRTHE . You are receiving this because you were mentioned.Message ID: @.***>

ukscone commented 4 months ago

Hi Charlie Got the same error when the wireless stuff was in the finally block too which was a bit of a surprise. In the grand scheme of things it's not a big deal as I can work around but it is annoying especially as I can't see why it's happening as i'm sure i've stopped or closed everything i could possibly stop or close including the door to my room but something is not being freed.

oh well machine.reset() it is

optimal-system commented 4 months ago

Hi Charlie I've tried your new version and it's much faster, very good. My code is running fine and I got 2 tasks running on my Pico : a main task involving a captors and a web server running with Phew. There is still a small problem. When the router is reset or the Internet connection fails (too often) the web server task does not restart automatically. I tried to reset in my main loop but it's not working. I think the Phew server should to that check but I'm not sure about it. Any idea ?

Here is the code of the main loop :


     async def captor() :
      this is not important for this problem

      async def webserver() : 
            while True :
                sw.check_connection()    # this routine should check if wifi is on and restart it if needed
                sw.loop()
                await asyncio.sleep(10)

        async def main():
            asyncio.create_task(captor())
            asyncio.create_task(webserver())
            await asyncio.sleep(100)

        asyncio.run(main())`

and the code in the sw.loop (the web server) :

    def loop(self): 
        server.add_route("/", handler = self.index, methods = ["GET"]) 
         server.add_route("/temperature", handler = self.get_temperature, methods = ["GET"]) 
        server.add_route("/toggle", handler = self.toggle_led, methods = ["GET"]) 
        server.set_callback(self.catch_all) 
        loop = uasyncio.get_event_loop() 
        server.run_as_task(loop, host="0.0.0.0", port=80) 
        loop.run_forever()           # the wifi check is never done :(  
`
ukscone commented 4 months ago

Not a problem but a comment as it took me ages to workout how to do it (by mixing several ideas and distilling down to a single function) and others might find it useful.

i couldn't get fonts working in css files no matter what I did or where I placed them 404s all over the place until I stumbled accross serve_file() and it seems to work nicely which is good as my other ideas (that almost worked) all required patching server.py

/* oxygen-regular - latin-ext_latin */
@font-face {
  font-family: 'Oxygen';
  font-style: normal;
  font-weight: 400;
  src: url('/static/fonts/oxygen-v15-latin-ext_latin-regular.eot');
  /* IE9 Compat Modes */
  src: url('/static/fonts/oxygen-v15-latin-ext_latin-regular.eot?#iefix') format('embedded-opentype'),
    /* IE6-IE8 */
    url('/static/fonts/oxygen-v15-latin-ext_latin-regular.woff2') format('woff2'),
    /* Super Modern Browsers */
    url('/static/fonts/oxygen-v15-latin-ext_latin-regular.woff') format('woff'),
    /* Modern Browsers */
    url('/static/fonts/oxygen-v15-latin-ext_latin-regular.ttf') format('truetype'),
    /* Safari, Android, iOS */
    url('/static/fonts/oxygen-v15-latin-ext_latin-regular.svg#Oxygen') format('svg');
  /* Legacy iOS */
}
<head>
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <meta http-equiv="refresh" content="5">
    <title>TAGS</title>
    <style>{{render_template("static/css/style.css")}}</style>
</head>
<body>
    <center>
        <h1> {{cnt}} Tags Scanned</h1>
        <table>
            {{''.join([f'<tr><td>{entry}</td></tr>' for entry in tags])}}
        </table>
    </center>
</body>`

@phew_app.route('/static/fonts/<fn>')
def fonts(req, fn):
    return phew_app.serve_file("/static/fonts/{}".format(fn))```
optimal-system commented 4 months ago

Hi ukscone, I gave up with CSS because it takes too much memory. I got error messages when I tried fancy formatting although the Pico had plenty of free mem. I tried gc.collect but it's not enough. So back to basic HTML and cutting my long output in several strings. It seems that memory allocation is not very efficient in micropython on Pico...

ukscone commented 4 months ago

Hi ukscone, I gave up with CSS because it takes too much memory. I got error messages when I tried fancy formatting although the Pico had plenty of free mem. I tried gc.collect but it's not enough. So back to basic HTML and cutting my long output in several strings. It seems that memory allocation is not very efficient in micropython on Pico...

I'm just doing some simple CSS, a bit of table styling, some colour and a bit of prettying. the big thing for me was the fonts. So far I seem to be doing ok memory-wise as i had it running overnight with an autorefreshing webpage with an every growing table and it was still ok when I checked it when I woke up. it does get scarily don't to 90ishK free quite often but on the whole it hovers around 130K free

optimal-system commented 4 months ago

Hi Russel I received your comment by email but I cannot see it here, strange!

"not sure if this would work but wouldn't putting the wifi check/restart as a separate async task a la the captor do what you need/want?"

This is a possible solution but it's awkward. The Phew server should check for itself if the connection is good. Maybe it's uasyncio problem with lines 422 of server.py...

def run_as_task(self, loop, host = "0.0.0.0", port = 80, ssl=None): loop.create_task(uasyncio.start_server(self._handle_request, host, port, ssl=ssl))

ukscone commented 4 months ago

Hi Russel I received your comment by email but I cannot see it here, strange!

"not sure if this would work but wouldn't putting the wifi check/restart as a separate async task a la the captor do what you need/want?"

i replied and then had a think about it an realised it probably wouldn't workfor what you want. what might work would be a test similar to below in the Phew class in server.py

 def is_server_running(self):
        try:
            if self.loop.is_running():
                return True
            else:
                return False
        except:
            return False

that you then run in an async task along with a wifi check and if something has failed either restart it or if you don't need persistance of some value just do a machine.reset()

ukscone commented 4 months ago

n/m ignore that as there doesn't seem to be an is an is_running() function for asyncio loops

optimal-system commented 3 months ago

that you then run in an async task along with a wifi check and if something has failed either restart it or if you don't need persistance of some value just do a machine.reset()

That's my way to reconnect. In the main program I added this coroutine :


        async def wifi_cnx() :
            while True :
                sw.check_connection()
                await asyncio.sleep(10)

and in the server web (sw) module :

    def check_connection(self) :
        if DEBUG : print('check_connection :',is_connected_to_wifi())
        if not is_connected_to_wifi():
            connect_wifi()