Sending files to esp32c6 boards fails almost 100% of the time.

UnexpectedMaker commented 4 months ago

CircuitPython version

Adafruit CircuitPython 9.0.5 on 2024-05-22; TinyC6 with ESP32C6

Code/REPL

No specific code.

Behavior

No specific behaviour.

Description

Working with a TinyC6 customer - both of us on MacOS - tried using Thonny, Ampy and mpremote to copy files to the TinyC6 and it would hang/fail 99% of the time between 10 and 12%.

Rebooting the board and listing the contents in Thonny would show a partial file there.

Even editing a file in Thonny and putting it back onto the TinyC6 would fail.

Example:

./mpremote connect /dev/cu.usbmodem2101 cp ~/src/esp32_c6/circuitpython_9/lib/adafruit_seesaw/seesaw.mpy :lib/adafruit_seesaw/seesaw.py yields  ... copying  17% [###-----------------]could not complete raw paste: b'\x01'

although sometimes it doesn't display copy progress at all, and sometimes it just hangs after making it through a chunk of the file.

I've tried changing boards as well, with no change in result.

Using a Microython build from an upcoming PR supporting esp32c6 and IDF 5.2.1 and 5.2.2 work correctly and file transfers with mpremote, Thonny, and rshell all work as expected. Even copying a folder of files at once, so that's a good sign.

Hopefully this is just an IDF version thing or something simple to fix, but unfortunately, I am shipping CP on my boards and more folks are hitting this than I'd like :(

Interestingly I see less (almost no) issues on my rPI test jigs.

Additional information

No response

UnexpectedMaker commented 4 months ago

Let me add that I am aware mpremote isn't supported - yes, a few things don't work, like ls but things like cp do and I use it on my test jigs to send files to TinyC6 boards being flashed and tested.

But it fails with Ampy and Thonny as well :)

UnexpectedMaker commented 4 months ago

I just tried the latest 9.1.0 Beta 3 and have the same issue with mpremote and Thonny on my Mac.

bablokb commented 4 months ago

You might want to try my tool "cpshell", see https://github.com/bablokb/cp-shell. It is basically a port of rshell which is based on similar code as mpremote and allows full scripting of the deployment of CP-code.

Since PR #9325 I experience similar issues as you describe here. But there is a new PR #9395 which might be the fix. I am about to test that.

UnexpectedMaker commented 4 months ago

https://github.com/bablokb/cp-shell

Oh, that looks interesting - I love rshell :)

I'll look into those PR's shortly, but I tried installing cpshell following your install section, but it's not working. I added an issue on your repo.

mattytrentini commented 4 months ago

You might want to try my tool "cpshell", see https://github.com/bablokb/cp-shell. It is basically a port of rshell which is based on similar code as mpremote and allows full scripting of the deployment of CP-code.

I don't suppose I could convince you to, instead, help update mpremote so that it also works with CircuitPython? :) I believe there are very, very few issues remaining for this to happen (main one: it currently uses the non-standard os.ilistdir() which CP has indicated they won't accept).

Performance is fast with mpremote (buffering was improved over rshell) and it has what I argue is the most productive workflow of all for development - using mpremote mount to have a PC filesystem appear on a connected device (no copies, files are transferred as imports require). CircuitPython users are currently missing out on this feature. Most importantly, the MicroPython core team are committed to maintaining and improving it.

Anyway, I know it's a tough ask (!) but the communities need fewer competing tools (mpremote, rshell, mpfshell, ampy+forks etc) that all-too-often become abandoned; the effort would be best to get behind one (or maybe a couple!).

bablokb commented 4 months ago

Good point, but I have seemed to miss mpremote when I was looking for a suitable tool. And since rshell almost worked, I first tried to fix the issues there. But the code is hard to maintain, so I refactored most of it.

Thanks for pointing me to mpremote, I will have a look at that tool as well and see if this fits my needs.

bablokb commented 4 months ago

@mattytrentini : I checked mpremote. It does not support rsync, so it is no option for me. Otherwise, it fails with CP for various reasons (same as original rshell), e.g. it only supports English versions of CP or it uses fixed chunk/delay when sending data (does not work for basic ESP32 devices). And I can't imagine that MP will accept PRs that are only needed to support CP.

tannewt commented 4 months ago

I wonder if we need the mutex lock like: https://github.com/micropython/micropython/blob/f61fac0ba6e2684b8bb4b11c7a768b0cb805c198/ports/esp32/usb_serial_jtag.c#L44-L67

Here is our current impl: https://github.com/adafruit/circuitpython/blob/06d653587be4bac9ccfedb0ebc1671f5b7b70fa8/ports/espressif/supervisor/usb_serial_jtag.c#L29-L50

We could also read the rxfifo len every time through the loop to get everything.

UnexpectedMaker commented 4 months ago

@tannewt Have you or anyone else on the dev team been able to replicate this on an Adafruit Feather ESP32-C6 dev board? I'm happy to send some TinyC6 boards over to help diagnose if you don't have HW there to test on. Let me know.

mattytrentini commented 4 months ago

@mattytrentini : I checked mpremote. It does not support rsync, so it is no option for me.

I'm assured that it will be added soon.

Otherwise, it fails with CP for various reasons (same as original rshell), e.g. it only supports English versions of CP or it uses fixed chunk/delay when sending data (does not work for basic ESP32 devices).

Can you help me understand the ESP32 issue you're referring to? I presume it's specific to CircuitPython since I use mpremote with ESP32's frequently?

And I can't imagine that MP will accept PRs that are only needed to support CP.

If it didn't affect MicroPython performance I'm quite sure they actually would get merged in. I'll certainly advocate for such fixes.

Perhaps we should be discussing this elsewhere...

bablokb commented 4 months ago

Perhaps we should be discussing this elsewhere...

@mattytrentini: I have created a discussion here: https://github.com/bablokb/cp-shell/discussions/2

vshymanskyy commented 4 months ago

Anyway, I know it's a tough ask (!) but the communities need fewer competing tools (mpremote, rshell, mpfshell, ampy+forks etc) that all-too-often become abandoned; the effort would be best to get behind one (or maybe a couple!).

I tested several boards with my ViperIDE and CircuitPython 9.0.5. It does look to be ESP32-C6-specific bug, rather than a general tool incompatibility issue. I'm using the ViperIDE benchmark tool as it streamlines the testing procedure.

[!NOTE] ViperIDE does not use the os.ilistdir(), and also uses some tweaks to work with CircuitPython. The difference is not too big, at least with the current functionality of the IDE.

Adafruit Feather STM32F405 Express

{"machine":"Adafruit Feather STM32F405 Express with STM32F405RG","release":"9.0.5","sysname":"stm32f4","version":"CircuitPython 9.0.5 on 2024-05-22","mpy_ver":6,"sys_path":["","/","/lib"],"connection":"usb"}
Writing ascii file 10240 bytes... 3118.2ms
Reading... 142.4ms
Writing binary file 10240 bytes... 3092.7ms
Reading... 143.9ms
Listing FS... 73.6ms
Cleanup... 505.8ms

micro:bit v2 with nRF52833

{"machine":"micro:bit v2 with nRF52833","release":"9.0.5","sysname":"nrf52","version":"CircuitPython 9.0.5 on 2024-05-22","mpy_ver":6,"sys_path":["","/","/lib"],"connection":"usb"}
Writing ascii file 10240 bytes... 11370.8ms
Reading... 5503.7ms
Writing binary file 10240 bytes... 15652.8ms
Reading... 5530.7ms
Listing FS... 125.2ms
Cleanup... 1212.1ms

ESP32-C6

{"machine":"ESP32-C6-DevKitM-1 with ESP32-C6N4","release":"9.0.5","sysname":"ESP32-C6N4","version":"CircuitPython 9.0.5 on 2024-05-22","mpy_ver":6,"sys_path":["","/","/lib"],"connection":"usb"}
Writing ascii file 10240 bytes...
 === FAILED ===
Error: Timeout

UnexpectedMaker commented 4 months ago

@vshymanskyy Thanks for confirming - and ViperIDE looks awesome!

mattytrentini commented 4 months ago

I tested several boards with my ViperIDE and CircuitPython 9.0.5. It does look to be ESP32-specific bug, rather than a general tool incompatibility issue. I'm using the ViperIDE benchmark tool as it streamlines the testing procedure.

Awesome work @vshymanskyy, thanks!

vshymanskyy commented 4 months ago

ViperIDE looks awesome!

Thank you. Please give it a GitHub star and spread the word! :grin:

I have also tried it on other ESP32 boards, and it works as expected. Did it multiple times, seems to be stable.

ESP32

{"machine":"ESP32 Devkit V1 with ESP32","release":"9.0.5","sysname":"ESP32","version":"CircuitPython 9.0.5 on 2024-05-22","mpy_ver":6,"sys_path":["","/","/lib"],"connection":"usb"}
Writing ascii file 10240 bytes... 2895.8ms
Reading... 1879.0ms
Writing binary file 10240 bytes... 3822.5ms
Reading... 1882.0ms
Listing FS... 63.1ms
Cleanup... 286.9ms

ESP32-C3

{"machine":"LILYGO TTGO T-OI PLUS with ESP32-C3","release":"9.0.5","sysname":"ESP32-C3","version":"CircuitPython 9.0.5 on 2024-05-22","mpy_ver":6,"sys_path":["","/","/lib"],"connection":"usb"}
Writing ascii file 10240 bytes... 3300.6ms
Reading... 1917.1ms
Writing binary file 10240 bytes... 4209.2ms
Reading... 1910.3ms
Listing FS... 73.7ms
Cleanup... 352.5ms

So, looks like this is a purely ESP32-C6 issue.

NateBank commented 4 months ago

I was the one originally running into these issues trying to upload the Adafruit seesaw lib to my C6 and UnexpectedMaker kindly helped me get MP running on my C6 so I may continue development. Here is a bit more data (sorry some of this has already been partially covered) from my testing. I am using the stock chip fresh out of the bag: Adafruit CircuitPython 9.0.5 on 2024-05-22; TinyC6 with ESP32C6

Thonny fails to connect to it about a third of the time requiring a chip reset and stop/start.
Thonny's saves to code.py fail about half the time and when it fails it leaves an empty code.py behind.
Thonny sometimes doesn't fully read the chip file system - it will say /lib is empty one time and has a nested directory the next reset.
Thonny took dozens of attempts to upload the seesaw.mpy library which at 4315 bytes is the largest file in the adafruit_seesaw collection and the one that fails the most.
In Thonny, Ampy, and mpremote the failure point is inconsistent. Sometimes it never starts, sometimes it makes it to 5%, 11%, 17%, 23%, 29%, 35%, 41%, 47%, 59%, 65%, 83%... several of these numbers come up repeatedly (11% and 17% especially) and then it will either hang indefinitely or within ~20 seconds (sometimes immediately) fail with could not complete raw paste: b'\x01'
Every once in a great while mpremote will respond with ... copying 41% [########------------]Traceback (most recent call last): File "<stdin>", line 1 SyntaxError: invalid syntax

vshymanskyy commented 4 months ago

Which all clearly indicate some low level issue on the serial transmission level.

dhalbert commented 4 months ago

Could someone try the 9.1.0-beta.4 builds? That includes #9400, which fixes something that sees how many bytes are available to read.

UnexpectedMaker commented 4 months ago

Could someone try the 9.1.0-beta.4 builds? That includes #9400, which fixes something that sees how many bytes are available to read.

Sure, I'll give it a go today and report back :)

NateBank commented 4 months ago

I'm running into the same issues with it, and Thonny seems to be having a harder time connecting to it. Adafruit CircuitPython 9.1.0-beta.4 on 2024-07-03; TinyC6 with ESP32C6

UnexpectedMaker commented 4 months ago

Yup, I can connect to it with mpremote (all I have here atm) but cannot copy anything to it at all...

seon@seons-m2-max-mbp-2 Downloads % mpremote connect /dev/cu.usbmodem101 cp profiles.csv :
cp profiles.csv :
 ... copying   0% [--------------------]could not complete raw paste: b'\x01'

Adafruit CircuitPython 9.1.0-beta.4 on 2024-07-03; TinyC6 with ESP32C6
>>> 
>>>

I feel like it's even worse now. Before I could occasionally get part way through something... now everything fails.

UnexpectedMaker commented 4 months ago

I wonder if we need the mutex lock like: https://github.com/micropython/micropython/blob/f61fac0ba6e2684b8bb4b11c7a768b0cb805c198/ports/esp32/usb_serial_jtag.c#L44-L67

Here is our current impl:

https://github.com/adafruit/circuitpython/blob/06d653587be4bac9ccfedb0ebc1671f5b7b70fa8/ports/espressif/supervisor/usb_serial_jtag.c#L29-L50

We could also read the rxfifo len every time through the loop to get everything.

I just tried adding the mutex lock and unlock to beta4 and it didn't change/improve anything.

I'm not sure what you mean by reading the rxfifo length in the loop as the loop length is determined by the length.

UnexpectedMaker commented 4 months ago

Ok, I've spent quite a few hours on this trying all sorts of things, and I've not made any progress :(

Things I've tried:

adding the mutex locking
removing the disable interrupt
increasing the supervisor_ticks_ms32 timeout from 200 to 2000 here
refactoring this function - seems quite messy tbh
tried a build with ESP-IDF v5.2.2-250-ga23f078905-dirty

Maybe there's just something at the IDF level for the C6 that's different than the C3 that we're missing.

dhalbert commented 4 months ago

I have also been trying some things. There are a few files that have CONFIG_IDF_TARGET_ESP32C3 preprocessor conditionals and should also have CONFIG_IDF_TARGET_ESP32C6 conditionals, but didn't. There were also some differences in mpconfigboard.* between a C3 board and a C6 board, and I tried to make the C6 more like the C3 options. So far I've only succeeded in getting some boot loops. I have further things to try.

UnexpectedMaker commented 4 months ago

I also found some places CONFIG_IDF_TARGET_ESP32C6 wasn't set but adding them in has not made any difference to the issue.

Still hunting.

dhalbert commented 4 months ago

I think I may have fixed this. I added interrupt disabling/enabling around the ringbuf operations, and I've been writing large files in thonny successfully. I need to clean it up a bit, and then I'll submit a PR you can test.

dhalbert commented 4 months ago

See #9409 for a possible fix and artifacts for you to test.

NateBank commented 4 months ago

Initial testing looks very promising! Thonny hasn't thrown any errors yet and I was able to transfer the entire seesaw library over in one go. It's a little unpredictable whether Thonny gives you the output of the code or the file system on each reset but that may be a Thonny thing? Regardless it's far more stable and having to reset it a few extra times is no hardship compared to before.

UnexpectedMaker commented 4 months ago

Looking good over here too, thanks Dan!

So all interrupts needed to go off during the check for length of data in the fifo? Seems extreme.

dhalbert commented 4 months ago

So all interrupts needed to go off during the check for length of data in the fifo? Seems extreme.

I was worried about a transfer being in progress and the number being not quite right. Yes, it may be overkill.

adafruit / circuitpython