webrepl: Changes for more webrepl features while making it smaller. #814
Conversation
felixdoerre
force-pushed
the
webreplv2
branch
2 times, most recently
from
e100da0
to
5aab2d1
Compare
|
@felixdoerre this could be a good opportunity |
This change: - Moves the password checking to python - Removes the special file transfer protocol - Moves the REPL data to websocket binary packages Signed-off-by: Felix Dörre <[email protected]>
Signed-off-by: Felix Dörre <[email protected]>
|
I've pushed a few adjustments and an (optional) bonus commit to allow specifying an ssl context. |
|
|
||
| listen_s = None | ||
| client_s = None | ||
|
|
||
| DEBUG = 0 | ||
|
|
||
| _DEFAULT_STATIC_HOST = const("https://micropython.org/webrepl/") | ||
| _DEFAULT_STATIC_HOST = const("https://felix.dogcraft.de/webrepl/") |
There was a problem hiding this comment.
To maintain the old webrepl webpage for older devices, does it make sense to change this URL to something like https://micropython.org/webrepl2/? Or is it enough to just specify different JS code as you've done below with the line webreplv2_content.js? Ie, can both the old and new web clients exist at this same URL?
There was a problem hiding this comment.
For people who open the webrepl web-client like I do, by navigating to the micropython-device and then being served this webpage, the micropython device can select the correct file and from there the correct dependencies. These clients can choose the correct files and exist at the same URL.
It does make a difference for people who navigate to https://micropython.org/webrepl/ manually and then enter their device's IP address in the text field. https://micropython.org/webrepl/ can only serve one version. How many people do that? I am not directly aware of documentation that sends people directly to that URL. Do you have some access statistics of how often https://micropython.org/webrepl/ is accessed vs https://micropython.org/webrepl/webrepl_content.js? Also those people who manually navigate to the website would need to be aware that there are two different versions and then choose the correct one.
I would argue that we recommend people to navigate to their device directly so it can choose the correct version and don't recommend to navigate to the page on micropython.org manually.
The main "pro"-argument for keeping the same base url would be, that we can "eventually" switch the version served there to show webreplv2, so we start catching those people who navigate to the URL (from wherever they might have found the link).
So that's the rationale for why I chose this variant, but I have no strong preference at all for either.
There was a problem hiding this comment.
Thinking of it: another reason to keep the URL and change the filename is to make sure we signal a different protocol version to whatever REPL-client is under that URL.
If you have any other WebREPL-client, the existance if webrepl_content.js indicates that it has support for the "old" webrepl protocol, and webreplv2_content.js signals, that it supports the new webrepl protocol.
There was a problem hiding this comment.
OK, let's keep it as the same URL.
| cl, remote_addr = listen_sock.accept() | ||
| req = cl.makefile("rwb", 0) |
There was a problem hiding this comment.
Is this makefile necessary? On bare-metal ports it just returns cl. It's really only there for CPython compatibility, and I don't think this code here needs to be compatible with CPython.
There was a problem hiding this comment.
I've moved the code here from the old line 22, and yes it seems this is a no-op. I'll test this code with makefile removed.
| @@ -146,11 +200,12 @@ def stop(): | |||
| listen_s.close() | |||
|
|
|||
|
|
|||
| def start(port=8266, password=None, accept_handler=accept_conn): | |||
| global static_host | |||
| def start(port=8266, password=None, ssl_context=None, accept_handler=accept_conn): | |||
There was a problem hiding this comment.
start_foreground() needs to be changed to work with this new signature.
This change:
mpremoteto support webrepl just like a serial port)The change should be compatible with the current micropython version, however a new webrepl client needs to be deployed under https://micropython.org/webrepl/
Currently, to help you testing, I've adjusted the default URL to https://felix.dogcraft.de/webrepl/, where I host the corresponding draft "new" webrepl client. I've pushed the modified js code here, if you want to take a look: https://github.com/felixdoerre/webreplv2. I'm not completely sure, if I should open a PR in https://github.com/micropython/webrepl, as this seems to be the authoritative source of the client, but the repo seems to be abandoned for some time now.
The new webrepl client currently features (See also #13540):
mpremote.mount-feature frommpremotethat works with the same injected code. It allows mounting files from the browser's local storage (which can host some python scripts that one wants to bring for debugging), or from a local folder, that is drag-and-dropped into the webrepl (read-only), this is useful for rapid development/testing.Currently the "mount" feature has to be activated "manually" (by calling
hookme()from the developer tools, and then executing__mount()in the repl manually). Also the UI for the file browser might not be super intuitive yet (the + for upload and mounting a local folder might be too small and not clear enough). So I'm hoping for UI improvement suggestions.If you want, I can split the
mountand themipimplementation into separate PRs, but the file-browser and this PR are dependent on each other.With this change the
_webreplmodule from micropython is not needed anymore and can be removed.