_webrepl: Remove the _webrepl module.

[felixdoerre]

When micropython/micropython-lib#814 is merged, all functionality form the _webrepl module that is still needed, is part of the python module. The special filesystem protocol is not needed anymore as the web client implements filetransfer exactly as mpremote does. Also the filesystem protocol was not safe to use while the repl is busy with other file I/O anyways.

While making way for more features in the webrepl, this change makes the micropython firmware smaller (see the numbers in #13540), the benefit only shows in builds with networking enabled.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.36%. Comparing base (678707c) to head (5fbbca2).
Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #13786   +/-   ##
=======================================
  Coverage   98.36%   98.36%           
=======================================
  Files         161      161           
  Lines       21091    21091           
=======================================
  Hits        20746    20746           
  Misses        345      345           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

[dpgeorge]

The special filesystem protocol is not needed anymore

The original idea for WebREPL (conceived when esp8266 was new and it needed a minimal way to get a remote REPL) was to combine both Telnet and FTP into one protocol (so only one port needed to be listening), and also support accessing the device from a browser (hence websocket instead of normal sockets).

Removing filesystem support is quite a big breaking change.

But I agree that it's a good thing to do! I think it makes more sense to have filesystem access done using the standard mpremote mechanism (even if that means filesystem acces can no longer be done in the background).

Also the filesystem protocol was not safe to use while the repl is busy with other file I/O anyways.

Is that really true? WebREPL callbacks are done at thread level and so filesystem operations are atomic.

One benefit of this PR is that it removes the large C-stack-allocated filebuf[512] variable.

[felixdoerre]

I am not sure that filesystem operations are atomic. See, for example, this stacktrace: #9297 (comment), showing a mp_handle_pending in rp2_flash_writeblocks. That particular problem was resolved by locking the scheduler in dupterm_rx_chr:

micropython/extmod/os_dupterm.c

Line 109 in bc424dd

mp_sched_lock();

But this does not address possible races of the interrupt with main code (I've not tried to reproduce such a race, but I am pretty sure this is possible). This might be resolvable, but I am actually not sure which guarantee exactly we would need to verify, in order to ensure that file-system operations in a foreground job and an "unsolicited" file-transfer from dupterm_notify are safe.

If background operations are actually needed/required, isn't a more stable way to use the async-repl package, and then use the regular transfer logic? (I haven't tested that one yet, but this seems plausible to me).

[dpgeorge]

If background operations are actually needed/required, isn't a more stable way to use the async-repl package, and then use the regular transfer logic?

Yes. Essentially your application is creating its own "background FTP" task (really an asyncio task that handles file transfer). That does seem more general than having the file transfer baked into the low-level _webrepl module.

[vshymanskyy]

While working on ViperIDE, I used the plain mpremote protocol for the File Management, and it turns out to work well.

I'm now trying to get WebREPL working as a WebSocket client, and bumped into these:

• extmod/modwebsocket: Add client support. #3406
• extmod/uwebsocket: Support masked transmissions and handling of PING requests #6732

[felixdoerre]

From what I see in your code, you don't use the "exact" fragments from mpremote but just something similar. In particular you hex-encode the files to download them, which is a workaround against the problem, that webrepl as-is cannot send arbitrary bytes on stdout, only utf-8 chars. The adjustments that I suggest to webrepl here make this unnecessary and allow a more efficient binary transfer.

[vshymanskyy]

@felixdoerre wow! could you describe briefly the mechanism behind this optimization?
From what I understand, we can't represent byte 0x04 in the RAW REPL data stream, as it's used to indicate the end of execution. The way to work around it, would be to:

• apply some kind of escaping or
• Consistent Overhead Byte Stuffing (COBS) or
• prefixing data chunks with a size field, with 0 indicating the end of data