Problem with mpremote

[shariltumin]

Problem with mpremote.

$ mpremote --version
mpremote 1.23.0
$ mpremote mount .
Local directory . is mounted at /remote
Connected to MicroPython at /dev/ttyUSB0
Use Ctrl-] or Ctrl-x to exit this shell
>
MicroPython v1.24.0-preview.409.g82e69df33 on 2024-10-10; Generic ESP32 module with ESP32
Type "help()" for more information.
>>> import hi
Traceback (most recent call last):
  File "/home/sharil/opt/bin/mpremote", line 8, in <module>
    sys.exit(main())
  File "/home/sharil/opt/lib/python3.10/site-packages/mpremote/main.py", line 536, in main
    do_repl(state, argparse_repl().parse_args([]))
  File "/home/sharil/opt/lib/python3.10/site-packages/mpremote/repl.py", line 89, in do_repl
    do_repl_main_loop(
  File "/home/sharil/opt/lib/python3.10/site-packages/mpremote/repl.py", line 35, in do_repl_main_loop
    n = state.transport.serial.inWaiting()
  File "/home/sharil/opt/lib/python3.10/site-packages/mpremote/transport_serial.py", line 1207, in inWaiting
    self._check_input(False)
  File "/home/sharil/opt/lib/python3.10/site-packages/mpremote/transport_serial.py", line 1188, in _check_input
    PyboardCommand.cmd_table[c](self.cmd)
  File "/home/sharil/opt/lib/python3.10/site-packages/mpremote/transport_serial.py", line 1019, in do_stat
    path = self.root + self.rd_str()
  File "/home/sharil/opt/lib/python3.10/site-packages/mpremote/transport_serial.py", line 981, in rd_str
    n = self.rd_s32()
  File "/home/sharil/opt/lib/python3.10/site-packages/mpremote/transport_serial.py", line 974, in rd_s32
    return struct.unpack("<i", self.fin.read(4))[0]
struct.error: unpack requires a buffer of 4 bytes

[shariltumin]

Putting a few "print" traces in the rd_s32 of transport_serial.py I saw the data coming through as:

MicroPython v1.24.0-preview.409.g82e69df33 on 2024-10-10; Generic ESP32 module with ESP32

b'\x03\x00\x00\x00'
b'\x06\x00\x00\x00'
b''

MicroPython v1.24.0-preview.315.g8feb714b4.dirty on 2024-10-10; Generic ESP32 module with ESP32

b'\x03\x00\x00\x00'
b'\x06\x00\x00\x00'
b'\x06\x00\x00\x00'
b'\x02\x00\x00\x00'
b'\xf9\x00\x00\x00'

MicroPython v1.24.0-preview.315.g8feb714b4.dirty works while the latest does not.

I replaced mphalport.c and mphalport.h from "409.g82e69df33" with those from "315.g8feb714b4" and recompiled. And things work after that. So it seems to me that there is something wrong with the new mphalport code.

[shariltumin]

Just found out what the problem was. Just a misplaced "#endif" in mphalport.c in the mp_hal_stdio_poll function for micropython/ports/esp32.

$ diff mphalport.c-ORIG mphalport.c
111a112
>     #endif
118d118
<     #endif

[andrewleech]

Oh that doesn't sound right!
Looking at

• https://github.com/micropython/micropython/blob/8feb714b4/ports/esp32/mphalport.c
• https://github.com/micropython/micropython/blob/82e69df33/ports/esp32/mphalport.c
  and
• https://github.com/micropython/micropython/commits/82e69df33e379bf491bea647e217d6d56c5b8090/ports/esp32/mphalport.c

The commits you're referring to look like:

• Working: esp32: Add MICROPY_HW_USB_CDC macro for native USB-CDC serial.
• Not working: esp32: Fix hang in taskYIELD() on riscv CPUs when IRQs disabled.

Which line/s in particular is the #endif on?

[shariltumin]

20241010/micropython/ports/esp32$ diff -u mphalport.c-ORIG mphalport.c
--- mphalport.c-ORIG    2024-10-10 14:34:04.591970165 +0200
+++ mphalport.c 2024-10-11 13:52:34.672699961 +0200
@@ -109,13 +109,13 @@
     uintptr_t ret = 0;
     #if MICROPY_HW_ESP_USB_SERIAL_JTAG
     usb_serial_jtag_poll_rx();
+    #endif
     if ((poll_flags & MP_STREAM_POLL_RD) && ringbuf_peek(&stdin_ringbuf) != -1) {
         ret |= MP_STREAM_POLL_RD;
     }
     if (poll_flags & MP_STREAM_POLL_WR) {
         ret |= MP_STREAM_POLL_WR;
     }
-    #endif
     #if MICROPY_HW_USB_CDC
     ret |= mp_usbd_cdc_poll_interfaces(poll_flags);
     #endif

I just moved the "#endif" at line number 118 up and placed it below line 111, so the "fixed" code has these lines:

    110     #if MICROPY_HW_ESP_USB_SERIAL_JTAG
    111     usb_serial_jtag_poll_rx();
    112     #endif

~

[andrewleech]

Oh wow, yeah pretty sure that one is my fault sorry! I read that master version of the function a few times thinking "no, that looks correct to me just like it is".

My thinking was that dupterm and cdc poll functions just below handle the poll flags internally so shouldn't need the "stdin ringbuffer / poll handling" in this function; only the serial_jtag poll needs it.

I completely forgot that UART repl uses the stdin ringbuffer handling also needs that same stdin ringbuffer / poll handling!

I was testing my recent changes on a bunch of newer esp boards but don't have an original esp32 currently and forgot to get that tested.

Would you be able to submit that endif change to a PR? I'm certainly happy to review it if you can. Thanks!

[shariltumin]

There's no need for a PR! It was so small. If you think this will fix the problem, I'm just happy if you do the necessary change. You and the core team were doing an incredible job. Thanks so much!

[Josverl]

I would argue that it is a common scenario on a common board and should be fixed before a release is cut

[andrewleech]

Yes I raised it here with an additional comment in the code. I referenced this thread there and thought it'd automatically back link from here; seems that only works for issues not discussions

#16006

[andrewleech]

The fix has been merged in #16006 so should now be working in master.

Thanks for the report and description of the fix @shariltumin !