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

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

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

ccrighton
Copy link

  • Apps can run one at a time
  • Apps can be run concurrently
  • Examples applications updated
  • Existing web applications work correctly using server.py compatibility methods
  • Documentation updated

- Apps be run one at a time
- Apps can be run concurrently
@ccrighton
Copy link
Author

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
Copy link

devfle commented Jun 11, 2023

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
Copy link
Author

ccrighton commented Jun 11, 2023

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

@riffnshred
Copy link

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

@rkompass
Copy link

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

@ccrighton
Copy link
Author

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
Copy link

I had an issue with closing the server to do other stuff. [(https://github.com//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 ccrighton changed the title Refactor Phew as a class to support multiple apps on one device running singly or concurrently Refactor Phew as a class to support multiple apps on one device running singly or concurrently; Support SSL and Session Authentication. Jun 26, 2024
@ccrighton
Copy link
Author

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

@Gadgetoid
Copy link
Member

Gadgetoid commented Jun 26, 2024

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 ❇️ stuff ❇️ 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
Copy link
Member

@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
Copy link
Author

@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
Copy link
Member

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
Copy link
Member

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
Copy link

ukscone commented Jul 14, 2024

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
Copy link
Author

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
Copy link

ukscone commented Jul 15, 2024

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
Copy link
Author

ccrighton commented Jul 15, 2024 via email

@ukscone
Copy link

ukscone commented Jul 15, 2024

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
Copy link

optimal-system commented Jul 17, 2024

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
Copy link

ukscone commented Jul 17, 2024

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
Copy link

optimal-system commented Jul 17, 2024

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
Copy link

ukscone commented Jul 17, 2024

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
Copy link

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
Copy link

ukscone commented Jul 17, 2024

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
Copy link

ukscone commented Jul 17, 2024

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

@optimal-system
Copy link

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() 

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.

7 participants