chegewara / EspTinyUSB

ESP32S2 native USB library. Implemented few common classes, like MIDI, CDC, HID or DFU (update).
MIT License
488 stars 70 forks source link

USB CDC crashes when sending data too fast #107

Closed Steffen-W closed 2 years ago

Steffen-W commented 2 years ago

I have the problem that when I send too much data via USB CDC without delay, it happens that data is lost or the entire connection crashes. Apparently I also can not query if the buffer is full to send, so I could wait until I send more data. Apart from inserting delays, I don't have a solution to my problem.

The baud rate for the USB CDC connection does not seem to affect the problem. So the picture is the same at 2MBoud as well as 9600 baud. The Mircocontroller (ESP-S2) does not crash every time.

Code:

#include <Arduino.h>
#include "FFat.h"
#include "SD.h" //only because of a copiler error
#include "cdcusb.h"
CDCusb USBSerial;

bool MyUSBconnect = false;
class MyUSBCallbacks : public CDCCallbacks {
  void onCodingChange(cdc_line_coding_t const *p_line_coding)  {
    USBSerial.flush();
    int bitrate = USBSerial.getBitrate();
    log_d("new bitrate: %d\n", bitrate);
  }

  bool onConnect(bool dtr, bool rts)  {
    if (dtr & rts)
      MyUSBconnect = true;
    else
      MyUSBconnect = false;
    log_d("connection state changed, dtr: %d, rts: %d\n", dtr, rts);
    return true; // allow to persist reset, when Arduino IDE is trying to enter bootloader mode
  }
};

void setup() {
  Serial.begin(115200);

  USBSerial.setCallbacks(new MyUSBCallbacks());
  if (!USBSerial.begin())
    Serial.println("Failed to start CDC USB stack");
}

void loop() {
  if (MyUSBconnect)  {
    for (int i = 01; i <= 20; i++)  {
      USBSerial.print(i);
      USBSerial.write(' ');
    }
    Serial.print("+");
  }
}

Output USB CDC:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 2 3 4 5 6 7 8 9 10 11 12 1314 15 16 17 18 19 20 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18K]
???j??
h"?@?0??
#????XL? ?xV`4T ?

Output UART:

ESP-ROM:esp32s2-rc4-20191025<\r><\n>
Build:Oct 25 2019<\r><\n>
rst:0x1 (POWERON),boot:0x9 (SPI_FAST_FLASH_BOOT)<\r><\n>
SPIWP:0xee<\r><\n>
mode:DIO, clock div:1<\r><\n>
load:0x3ffe6100,len:0x524<\r><\n>
load:0x4004c000,len:0xa50<\r><\n>
load:0x40050000,len:0x28cc<\r><\n>
entry 0x4004c18c<\r><\n>
[   498][D][usb_descriptors.cpp:45] getConfigurationDescriptor(): descriptor length: 75<\r><\n>
<\r><\n>
[   498][D][esptinyusb.cpp:53] esptinyusbtask(): [esptinyusbtask] USB tud_task created<\r><\n>
[  1220][D][main.cpp:18] onCodingChange(): new bitrate: 9600<\r><\n>
<\r><\n>
[  5080][D][main.cpp:27] onConnect(): connection state changed, dtr: 1, rts: 1<\r><\n>
<\r><\n>
+++[  5084][D][main.cpp:18] onCodingChange(): new bitrate: 115200<\r><\n>
<\r><\n>
Guru Meditation Error: Core  0 panic'ed (LoadStoreError). Exception was unhandled.<\r><\n>
<\r><\n>
Core  0 register dump:<\r><\n>
PC      : 0x4001ab69  PS      : 0x00060730  A0      : 0x8008c956  A1      : 0x3ffcae80  <\r><\n>
A2      : 0x3ffc22f9  A3      : 0x40000000  A4      : 0x00000002  A5      : 0x3ffc22f9  <\r><\n>
A6      : 0x0000005e  A7      : 0x40000002  A8      : 0x8008c88c  A9      : 0x3ffcae30  <\r><\n>
A10     : 0x00000001  A11     : 0x3ffc23d0  A12     : 0x3ffc2384  A13     : 0x00000000  <\r><\n>
A14     : 0x3ffc23d0  A15     : 0x3ffc2384  SAR     : 0x00000018  EXCCAUSE: 0x00000003  <\r><\n>
EXCVADDR: 0x40000000  LBEG    : 0x3ffc2384  LEND    : 0x00000000  LCOUNT  : 0x40027141  <\r><\n>

`

chegewara commented 2 years ago

You should to decode backtrace first. Im not sure, but try to not call USBSerial.flush(); from withing callback.

Steffen-W commented 2 years ago

Hi thank you for the extremely fast feedback. Unfortunately it makes no difference if I call USBSerial.flush(); or not. Even if I don't define any callbacks myself the behavior is the same. I have tried to isolate the problem.

Using USBSerial.print(i); causes the ESP32-S2 to crash. USBSerial.print((char)i); as well as USBSerial.write(i); work as expected.

#include <Arduino.h>
#include "FFat.h"
#include "SD.h" //only because of a copiler error
#include "cdcusb.h"
CDCusb USBSerial;

void setup()
{
  if (!USBSerial.begin())
    return;
}

void loop() {
  for (int i = 48; i <= 90; i++)
  {
    // USBSerial.print((char)i); // is working good
    // USBSerial.print(i); // crash the ESP32-S2
    USBSerial.write(i); // is working good
  }
  USBSerial.write('\n');
}

Output USB CDC:

0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ
0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ
012345678VWXYZ
0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ
0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ
0123456789:;<=>?@ABCDEFGH;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ
0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ
0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVZ
0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ
0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ
0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ
DEFGHIJKLMNOPQRSTUVWXYZ
0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVW
0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ

Unfortunately I do not know how to solve the problem with the non-transmitted characters.

Steffen-W commented 2 years ago

As a small addition:

USBSerial.printf("%c", i); works very well in the output. There are not even missing characters in the output. The speed is also only about 60000 characters per second (before about twice as high). But only if a single character is to be output. If three or more characters are to be output USBSerial.printf("%c%c%c", i, i, i); there are considerable problems.

chegewara commented 2 years ago

USBSerial.write(i); is returning value, if its 0 then its not sent. Maybe try to check return value.

chegewara commented 2 years ago

The CDC class inherit from Stream: class EspTinyUSB : public Stream but i never been testing it actually, i just assumed it should works. It is possible that print, printf and other functions from Stream may cause the problems or slow down USB. I would suggest to use write() and to check return value.

Steffen-W commented 2 years ago

Thank you for the wonderful tip. I will have to find a better solution but it works without errors and lost characters. The speed is about 128000 characters per second.

#include <Arduino.h>
#include "FFat.h"
#include "SD.h" //only because of a copiler error
#include "cdcusb.h"
CDCusb USBSerial;

void setup()
{
  if (!USBSerial.begin())
    return;
}

inline void USBSerial_write(int c) { while (!USBSerial.write(c)); }

void loop()
{
  for (int i = 48; i <= 90; i++)
  {
    // USBSerial.print((char)i);     // works but characters are lost
    // USBSerial.print(i);           // crash the ESP32-S2
    // USBSerial.write(i);           // works but characters are lost
    // USBSerial.printf("%c", i);    // works
    // USBSerial.printf("%c%c%c", i);// crash the ESP32-S2
    USBSerial_write(i); // works best
  }
  USBSerial_write('\n');
}
Steffen-W commented 2 years ago

To prevent others from using my solution you should use the following function instead of USBSerial.write(c);: USBSerial.write(&i, 1);

The function is much safer to use because it checks if a serial connection exists and makes sure that the data is sent. What happens if the connection is disconnected while the data is being sent I don't know.

What I wonder now, however, if when using .write(c) the data is written into a buffer and only if too much data is written in is it also overwritten. Or what is probably the reason for the disappearance of the characters?

chegewara commented 2 years ago

Sorry, i am not working with USB for very long time now (over 12-18 months i think), so i cant answer your questions, especially that original tinyusb library is still evolving. I am wondering why you are sending only 1 byte at the time (i hope it is for tests purpose only), because with more bytes per transaction you will get higher bitrate.

Steffen-W commented 2 years ago

Thanks in any case for the help. But there seems to be a problem that I can't output so many characters (more than 4). My actual code also outputs several characters directly and not just one.

  uint8_t test[] = "\n0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
  USBSerial.write(test, 4);           // working good
  USBSerial.write(test, 5);           // crash the ESP32-S2
  USBSerial.write(test, sizeof(test) - 1); // crash the ESP32-S2
chegewara commented 2 years ago

https://github.com/espressif/arduino-esp32#decoding-exceptions

Steffen-W commented 2 years ago
  #0  0x4001aca2:0x3ffcaf80 in ?? ??:0
  #1  0x4008ca3f:0x3ffcaf90 in _ff_push_n at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/components/arduino_tinyusb/tinyusb/src/common/tusb_fifo.c:183
      (inlined by) _tu_fifo_write_n at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/components/arduino_tinyusb/tinyusb/src/common/tusb_fifo.c:488
  #2  0x4008cbd5:0x3ffcafd0 in _tu_fifo_write_n at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/components/arduino_tinyusb/tinyusb/src/common/tusb_fifo.c:461
      (inlined by) tu_fifo_write_n at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/components/arduino_tinyusb/tinyusb/src/common/tusb_fifo.c:784
  #3  0x4008c3fb:0x3ffcaff0 in tud_cdc_n_write at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/components/arduino_tinyusb/tinyusb/src/class/cdc/cdc_device.c:171
  #4  0x40081540:0x3ffcb010 in CDCusb::write(unsigned char const*, unsigned int) at .pio/libdeps/esp32-s2-saola-1/ESP32TinyUSB/src/device/cdc/cdcusb.cpp:115 (discriminator 4)
  #5  0x40081473:0x3ffcb030 in loop() at src/main.cpp:18
  #6  0x400831d0:0x3ffcb050 in loopTask(void*) at /home/steffen/.platformio/packages/framework-arduinoespressif32/cores/esp32/main.cpp:50
chegewara commented 2 years ago

Well, its not very helpful, but you could try to add delay(1) after this line: https://github.com/chegewara/EspTinyUSB/blob/master/src/device/cdc/cdcusb.cpp#L107

Steffen-W commented 2 years ago

With the delay, the error does not occur.

Steffen-W commented 2 years ago

Sorry, that was probably written too quickly. The error is less frequent but the message is the same.

Steffen-W commented 2 years ago

The following clearly shows the error. It seems that the limitation of the length does not work.

Code:

uint8_t test[] = "\n0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ please don't show!";
USBSerial.write(test, 37); // crash the ESP32-S2

Ouput:

0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ
0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ
0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ please don't show!:0K??0??@@????@??A??x?d????
?l??????@?@?A??@?@@A?@@|    @??4o@4o@y@D:?@4o@4o@4o@4o@4o@E@p?$p@4o@`@4o@
Steffen-W commented 2 years ago

I think I have found the error in the code. (size - d) > 0 this is always true when size > 0. Both are types of uint and the result is therefore also positive. In tud_cdc_n_write I have not been able to trace the set buffer size. In my opinion it must be (size - d).

https://github.com/chegewara/EspTinyUSB/blob/master/src/device/cdc/cdcusb.cpp#L106-L108

do {
  d += tud_cdc_n_write(_itf, buffer + d, size - d);
} while ((int)(size - d) > 0 || d == 0);