earlephilhower / arduino-pico

Raspberry Pi Pico Arduino core, for all RP2040 and RP2350 boards
GNU Lesser General Public License v2.1
2.03k stars 422 forks source link

USB Serial seems not to send data in some weird cases #1620

Closed marklinmax closed 1 year ago

marklinmax commented 1 year ago

I am running a rather simple code on my raspberry pi pico (RP2040), and testing the communication with the Arduino IDE serial monitor is working great. Problem is, when I run the communication in the actual system, I can receive data sent to the pico, but the data sent from the pico seems lost somewhere in its entirety.

The target for communication is an application on which I have zero control over, so I did debugging by sniffing the COM port it is connected to with another software.

Funny thing is, the exact same code runs just fine on an Arduino UNO, so I guess this is an issue related to either how the Serial is implemented using the USB CDC library, or the USB CDC library itself (but I find it hard to believe).

maxgerhardt commented 1 year ago

Does the program in your actual system set DTR = 1 in the COM settings? This virtual data-terminal-ready line needs to be asserted, the TinyUSB CDC implementation checks for this.

Specifically, I'm talking about

https://github.com/adafruit/Adafruit_TinyUSB_Arduino/blob/4a93c7d869852c03b198d74cefe87de4ba071eca/src/class/cdc/cdc_device.h#L168-L171

https://github.com/adafruit/Adafruit_TinyUSB_Arduino/blob/4a93c7d869852c03b198d74cefe87de4ba071eca/src/class/cdc/cdc_device.c#L113-L117

https://github.com/earlephilhower/arduino-pico/blob/3cc5ac14ff5ab9fd2d0399c8d4a99c3451c5e818/cores/rp2040/SerialUSB.cpp#L125-L133

marklinmax commented 1 year ago

Does the program in your actual system set DTR = 1 in the COM settings? This virtual data-terminal-ready line needs to be asserted, the TinyUSB CDC implementation checks for this.

Specifically, I'm talking about

https://github.com/adafruit/Adafruit_TinyUSB_Arduino/blob/4a93c7d869852c03b198d74cefe87de4ba071eca/src/class/cdc/cdc_device.h#L168-L171

https://github.com/adafruit/Adafruit_TinyUSB_Arduino/blob/4a93c7d869852c03b198d74cefe87de4ba071eca/src/class/cdc/cdc_device.c#L113-L117

https://github.com/earlephilhower/arduino-pico/blob/3cc5ac14ff5ab9fd2d0399c8d4a99c3451c5e818/cores/rp2040/SerialUSB.cpp#L125-L133

Seems like that was the case... I simply bypassed the _tud_cdcconnected() test in SerialUSB.cpp and now it works like a charm.

Maybe there should be an option to ignore it, as it seems some weird implementations on the application side can be found and will mess with writing on the com port.

Also, this confirms that Arduino implementation just does not care about it either.

Thanks a lot, I would never have found this one alone.

maxgerhardt commented 1 year ago

I see, then my guess was right. (Same issue as in https://github.com/earlephilhower/arduino-pico/issues/718)

Indeed the Arduino Uno boards, whether they are the original one with the USB-to-UART converter being the ATMega16u2, or a CH340, or an FTDI chip, do not care whether DTR is asserted or not. But many of the Arduino cores for boards in which the target MCU does the USB communication itself (such as for the Pico, STM32 chips if configured so) do need DTR asserted. I guess a workaround is always to either comment that out in the core / library code or let the chip output via UART unconditionally and slap an extra USB-UART converter on top.

marklinmax commented 1 year ago

I see, then my guess was right. (Same issue as in #718)

Indeed the Arduino Uno boards, whether they are the original one with the USB-to-UART converter being the ATMega16u2, or a CH340, or an FTDI chip, do not care whether DTR is asserted or not. But many of the Arduino cores for boards in which the target MCU does the USB communication itself (such as for the Pico, STM32 chips if configured so) do need DTR asserted. I guess a workaround is always to either comment that out in the core / library code or let the chip output via UART unconditionally and slap an extra USB-UART converter on top.

Oh, I didn't see that issue.

Wouldn't it be possible to just not check if the device is connected at all when writing (or at least have an option to do so)? We could just write the data and do not care if it is lost or not in the end. As I said, this is what I did to fix my issue, and it seems to work just fine. Of course, the protected way should still be the default one, however, having to modify the lib locally is far from ideal.

I though, maybe adding an #ifndef statement with an ignore flag which defaults to false and can be set to true in the user's code might do the job. Something like that:

...
if (tud_cdc_connected() || IGNORE_CONNECTED) { 
...

That way we bypass the test, and this does not require modifying the Pico-SDK code.

earlephilhower commented 1 year ago

I'm not a great fan of #define hell, but if you'd like to add a SerialUSB::ignoreFlowControl(bool ignore = true) method that does this programmatically I'd be fine with a PR.