-
Notifications
You must be signed in to change notification settings - Fork 1k
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
webrepl: Changes for more webrepl features while making it smaller. #814
base: master
Are you sure you want to change the base?
Conversation
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]>
e100da0
to
5aab2d1
Compare
@felixdoerre this could be a good opportunity |
9fafda2
to
a46c2fd
Compare
I've pushed a few adjustments and an (optional) bonus commit to allow specifying an ssl context. |
micropython/net/webrepl/webrepl.py
Outdated
|
||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, let's keep it as the same URL.
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
start_foreground()
needs to be changed to work with this new signature.
dff0f56
to
2b39a77
Compare
Signed-off-by: Felix Dörre <[email protected]>
2b39a77
to
772d240
Compare
This change:
mpremote
to 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 frommpremote
that 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
mount
and themip
implementation into separate PRs, but the file-browser and this PR are dependent on each other.With this change the
_webrepl
module from micropython is not needed anymore and can be removed.