RFC: Webrepl feature parity with mpremote

[felixdoerre]

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:

• move the password-checking logic from _webrepl to webrepl.py
• remove the current file-transfer protocol, and with it the _webrepl module.
• move the repl interaction from websocket "text" packages to "binary" packages, as the output from the repl is not required to be valid UTF-8, but just a byte sequence.
• implement the same logic that mpremote uses for file transfers in the webrepl client.

Removing _webrepl and moving the password logic to webrepl.py even seems to make the firmware smaller:

   text    data     bss     dec     hex filename                                                                       
 606100       0  381308  987408   f1110 /home/jenkins/tmp/micropython/ports/rp2/build-RPI_PICO_W/firmware.elf
 605668       0  381292  986960   f0f50 /home/jenkins/tmp/micropython/ports/rp2/build-RPI_PICO_W/firmware.elf

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:

• Allow binary data transfers over the websocket (this also helps other usages of the webrepl, like Thonny, or using the websocket as backend for mpremote instead of a serial device)
• Implement a File-Browser in the webrepl-client, with file up- and download using the same primitives as mpremote
• Implement mip to install packages directly from the webrepl-client
• Implement the filesystem-mounting feature
• Implement direct mounting of a local directory through the browser (without uploading). Due to webapi limitations this is only a readonly mount.
• Allow browser-saving of the webrepl password
• Auto-connect the webrepl, if the target (and password) is known.

Do you want to include this into micropython, replacing the current webrepl module?

[felixdoerre]

Just to give an overview of the related PRs and their dependencies.

• (No PR yet, I'm not sure if I should open it against micropython/webrepl or micropython-lib or somewhere else): Deploy webrepl_v2.js (https://github.com/felixdoerre/webreplv2) to https://micropython.org/webrepl/webrepl_v2.js
  • webrepl: Changes for more webrepl features while making it smaller. micropython-lib#814
    • _webrepl: Remove the _webrepl module. #13786
    • tools/mpremote: Add ws:// and wss:// endpoints. #14141

Regarding compatibility:

• As implemented, the current webrepl-client is not compatible with webreplv2, as webreplv2 will send REPL output via websocket-binary packages, and the webrepl-client ignores them. This could be extended, so that the webrepl client will accept binary packages as well. File transfer with the old webrepl client will not work. Not that this is only a problem if a user navigates to http://micropython.org/webrepl manually and enters the IP. If a user navigates to the device directly, it will serve the correct webrepl client in any case
• webrepl_cli (https://github.com/micropython/webrepl/blob/master/webrepl_cli.py) should work with webreplv2 for REPL, file transfer does not. I would suggest it to be superseeded by mpremote where (with tools/mpremote: Add ws:// and wss:// endpoints. #14141) file transfer and mounting works.

[andrewleech]

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.

[cwalther]

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.

[felixdoerre]

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

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 would be a bit unhappy to lose the ability to transfer files while a program is running though.

I haven't personally played with aiorepl, but I think that this might be the safer way to transfer files "in the background" (and would also work via usb/uart). That way, the transfer (blocks) only happen in between running application code and can not interrupt/interleave with another file system operation. I haven't tested this yet, but I might.

other features that need a separate channel from stdio could be added back in the future

Which other features for an additional channel would you want? In principle mpremote mount needs and establishes such a separate "channel", by putting an escape character before all mpremote mount messages. In principle nothing is preventing us from adding more such escape characters, or even nest them. However this technique only works will as long as the application does not output the escape character on stdout. One could start escaping the application's stdout, but I am not sure that this is wanted for the sake of keeping things simple.

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.

[cwalther]

I haven't personally played with aiorepl, but I think that this might be the safer way to transfer files "in the background" (and would also work via usb/uart).

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.

other features that need a separate channel from stdio could be added back in the future

Which other features for an additional channel would you want?

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.

In principle mpremote mount needs and establishes such a separate "channel", by putting an escape character before all mpremote mount messages. In principle nothing is preventing us from adding more such escape characters, or even nest them. However this technique only works will as long as the application does not output the escape character on stdout. One could start escaping the application's stdout, but I am not sure that this is wanted for the sake of keeping things simple.

Hmm, mpremote mount does not do that already? (I have never studied how exactly it works.) So you could confuse it by outputting certain bytes today? That sounds like a design flaw (similar to the one in the original WebREPL that we’re trying to fix here).

[felixdoerre]

You can confuse mpremote mount by printing the mount escape char: sys.stdout.write("\x18"). So I am arguing that mpremote over uart/usb faces the same challenge.

I never had any filesystem corruption with it

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.