-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
RFC: Webrepl feature parity with mpremote #13540
Comments
Just to give an overview of the related PRs and their dependencies.
Regarding compatibility:
|
This looks fantastic, thanks for your work! I've been super keen to make mpremote work over network to a webrepl device but with 2 kids under 2 my project time has reduced pretty much to nil :-) Just reading your description so far, I like how you've organised your charges. I'll review them in more detail when I can and hopefully poke the right people to get this merged (eventually, these things take time) because I think this is really important to have - network connected devices are more and more the norm now. |
Sounds great, my own attempts at improving this have long stalled (I was trying to keep it backward compatible). I would be a bit unhappy to lose the ability to transfer files while a program is running though. I never had any filesystem corruption with it. Maybe having all mpremote functionality will make up for it in practice, who knows. I haven’t read the code carefully enough yet to tell: Are you putting just the stdio content into the binary websocket messages, or are the messages tagged with some kind of “opcode” field that would keep the protocol extensible, so that file transfer or other features that need a separate channel from stdio could be added back in the future? Lack of such extensibility was my main gripe with the original WebREPL protocol and I eventually gave up trying to think of convoluted ways to work around it. If we’re reimplementing things from scratch, it would be nice not to repeat that design flaw. |
Previously the stdio content was in websocket "text" message (and "binary" message were used for that file transfer protocol), which does not work well, if stdio does not contain valid utf8 (as can be the case by the file transfer logic implemented in mpremote). Essentially, this change makes the websocket "binary" messages behave identical to the repl that you can get via uart or usb. In theory you could use the (now unused) text segments again to transfer some other data, but that feels more like a hassle.
I haven't personally played with
Which other features for an additional channel would you want? In principle Having another "channel" towards the device is probably more tricky, as the stdin buffer might get filled up and block other channels if it is not read. Using asyncio with aiorepl that could proably be avoided, as this logic would make sure, that stdin is checked regularly. |
Good idea. I assume that only works in asyncio applications though. So that wouldn’t have helped me in the situations where I found the background file transfer handy. But maybe it’s not a big deal.
I don’t have any particular ones in mind (other than file transfer), I’m more thinking of future updates that we don’t know about yet. Like when I was trying to solve the same problem you did (inability to transfer non-UTF-8 data) and was hitting a brick wall because the protocol was not extensible to do that in a backward-compatible way without jumping through ugly hoops.
Hmm, |
You can confuse
I believe that one reason, that you didn't observe this, is that your applications only write to flash very rarely, and you only upload files very rarely, so the chance of those two operations (writes on both sides) interfere is pretty low. |
The current webrepl-code seems to be a bit outdated lack behind the feature progress of
mpremote
. Additionally, the current file-transfer logic that aims to work while the REPL is blocked, however this can lead to filesystem corruption if the blocking code accesses the filesystem at the same time.I propose the following:
_webrepl
towebrepl.py
_webrepl
module.mpremote
uses for file transfers in the webrepl client.Removing
_webrepl
and moving the password logic towebrepl.py
even seems to make the firmware smaller:I've implemented a proof-of-concept here: https://github.com/felixdoerre/webreplv2 together with a new webrepl client.
The benefits over the current webrepl are:
mpremote
instead of a serial device)mpremote
mip
to install packages directly from the webrepl-clientDo you want to include this into micropython, replacing the current webrepl module?
The text was updated successfully, but these errors were encountered: