CDC doesn't respect no flow control setting (DTR control needed to receive data)

MrGreensWorkshop commented

2022-07-07 21:16:56 -05:00

(Migrated from github.com)

Operating System

Windows 10, Mac OS, I think in all OS es.

Board

Raspberry pi pico

Firmware

just simple app to test cdc behavior.

RasPiPicoSDK_cdc_test-main.zip

What happened ?

When handshake is set to none, to receive data we need to enable DTR. We shouldn't be force to enable DTR to receive data. (other flow control status shouldn't be changed in cdc side if it is the case.)

I believe this caused by tinyusb lib. That's why I open this issue also in tinyusb repo.

please watch the gif file.

How to reproduce ?

RasPiPicoSDK_cdc_test-main.zip

Download
Compile and run. (or just use binary named PicoTest.uf2)
Connect to Raspberry pi pico to USB thats all. (No usb serial converters attached.)
Use Terminal.exe or CoolTerm or any other terminal let you set DTR.
Check DTR behavior.

Expected behavior

This the demonstration that 2 FDTI USB serial converter connected each other with 3 pins.

Rx <- Tx
Tx -> Rx
GND <-> GND

It shouldn't be forced to enable DTR to receive data either sides. Please watch the gif file.

Cause

This happens when raspberry pi usb cdc is used.
Discussed in here https://github.com/hathach/tinyusb/issues/1548 and https://github.com/hathach/tinyusb/discussions/932

Please check this file.

https://github.com/hathach/tinyusb/blob/master/src/class/cdc/cdc_device.c

it is checking DTR if it's on or not regardless checking flow control.

bool tud_cdc_n_connected(uint8_t itf)
{
  // DTR (bit 0) active  is considered as connected
  return tud_ready() && tu_bit_test(_cdcd_itf[itf].line_state, 0);
}

### Operating System Windows 10, Mac OS, I think in all OS es. ### Board Raspberry pi pico ### Firmware just simple app to test cdc behavior. [RasPiPicoSDK_cdc_test-main.zip](https://github.com/raspberrypi/pico-sdk/files/10067696/RasPiPicoSDK_cdc_test-main.zip) ### What happened ? When handshake is set to none, to receive data we need to enable DTR. We shouldn't be force to enable DTR to receive data. (other flow control status shouldn't be changed in cdc side if it is the case.) I believe this caused by tinyusb lib. That's why I open this issue also in [tinyusb repo](https://github.com/hathach/tinyusb/issues/1548). please watch the gif file. ![raspi pico serial dtr](https://user-images.githubusercontent.com/76992231/177780477-e542724c-630b-4e83-a234-a48680f823a0.gif) ### How to reproduce ? [RasPiPicoSDK_cdc_test-main.zip](https://github.com/raspberrypi/pico-sdk/files/10067696/RasPiPicoSDK_cdc_test-main.zip) - Download - Compile and run. (or just use binary named PicoTest.uf2) - Connect to Raspberry pi pico to USB thats all. (No usb serial converters attached.) - Use Terminal.exe or [CoolTerm](https://freeware.the-meiers.org/) or any other terminal let you set DTR. - Check DTR behavior. ### Expected behavior This the demonstration that 2 FDTI USB serial converter connected each other with 3 pins. ``` Rx <- Tx Tx -> Rx GND <-> GND ``` It shouldn't be forced to enable DTR to receive data either sides. Please watch the gif file. ![usual usb serial](https://user-images.githubusercontent.com/76992231/177780995-f4d3242d-b360-488a-8ac6-d4a83e4ffb36.gif) ### Cause This happens when raspberry pi usb cdc is used. Discussed in here https://github.com/hathach/tinyusb/issues/1548 and https://github.com/hathach/tinyusb/discussions/932 Please check this file. https://github.com/hathach/tinyusb/blob/master/src/class/cdc/cdc_device.c it is checking DTR if it's on or not regardless checking flow control. ``` bool tud_cdc_n_connected(uint8_t itf) { // DTR (bit 0) active is considered as connected return tud_ready() && tu_bit_test(_cdcd_itf[itf].line_state, 0); } ```

lurch commented

2022-07-08 03:07:02 -05:00

(Migrated from github.com)

Are you sure that this isn't a quirk of your particular serial terminal program? I never had to fiddle with any DTR settings when using PuTTY as detailed in https://datasheets.raspberrypi.com/pico/getting-started-with-pico.pdf

MrGreensWorkshop commented

2022-07-08 03:22:17 -05:00

(Migrated from github.com)

yes i'm sure. Check this out. This is with CoolTerm on MacOS.
data only received if only DTR is enabled.

yes i'm sure. Check this out. This is with [CoolTerm](https://freeware.the-meiers.org/) on MacOS. data only received if only DTR is enabled. ![CoolTermMac_term](https://user-images.githubusercontent.com/76992231/177949694-c8a83947-5dc3-46d0-8d29-4776fbe68a84.gif)

MrGreensWorkshop commented

2022-07-08 04:22:21 -05:00

(Migrated from github.com)

PuTTY or Teraterm terminals are made for VT100 etc., not exactly plain text transmission. You can use if you want. But those are uses other pins too. DTR -> DSR and DSR <- DTR. That's the problem. Asking host application for DSR on is reduce the compatibility with other apps if user is explicitly select no flow control. I'm saying this because if you check 2 FDTI USB serial converter example gif, it doesn't uses DTR. If flow control set to none most 3rd party apps only uses RX, TX, GND.

aallan commented

2022-07-08 04:41:01 -05:00

(Migrated from github.com)

I've never used any flow control when talking to an RP2040-based board. I've never even connected up the CTS/RTS or DSR/DTR pins when using UART adaptors, just RX/TX and GND. I can't replicate this problem. Downloading the UF2 binary in the OP's repo it works as expected on my machine.

I've never used any flow control when talking to an RP2040-based board. I've never even connected up the CTS/RTS or DSR/DTR pins when using UART adaptors, just RX/TX and GND. I can't replicate this problem. Downloading the UF2 binary in the OP's repo it works as expected on my machine. ![IMG_2376](https://user-images.githubusercontent.com/53625/177964426-897c04be-8443-4b83-893c-3efc1798588d.jpg) ![Screenshot 2022-07-08 at 10 37 20](https://user-images.githubusercontent.com/53625/177964438-926e2600-6896-49f1-aaf1-abaa5fe9edd3.jpg)

MrGreensWorkshop commented

2022-07-08 04:56:08 -05:00

(Migrated from github.com)

you used screen command right? its also uses VT100.

I'm talking about CDC not UART. I can see that you connect the serial over rx,tx,gnd pins.

you used screen command right? its also uses VT100. I'm talking about CDC not UART. I can see that you connect the serial over rx,tx,gnd pins.

aallan commented

2022-07-08 05:02:48 -05:00

(Migrated from github.com)

you used screen command right? its also uses VT100

I did not use the screen command.

> you used screen command right? its also uses VT100 I did not use the `screen` command. <img width="696" alt="Screenshot 2022-07-08 at 11 00 12" src="https://user-images.githubusercontent.com/53625/177968207-a64bc4c5-0e83-493a-b11e-ba45ee659dff.png">

MrGreensWorkshop commented

2022-07-08 05:15:39 -05:00

(Migrated from github.com)

This is my setup.

Raspberry pi pico -> USB thats all. No usb serial converters attached.

This happens when raspberry pi usb cdc is used.

This is my setup. Raspberry pi pico -> USB thats all. No usb serial converters attached. This happens when raspberry pi usb cdc is used.

aallan commented

2022-07-08 05:32:49 -05:00

(Migrated from github.com)

~~The UF2 binary you provided doesn't have CDC enabled, only UART?~~

Oops. No, I'm wrong. I wasn't paying enough attention. Sorry.

However, it turns out I am connected via CDC via USB, not UART serial at all. 🤦‍♂️

~The UF2 binary you provided doesn't have CDC enabled, only UART?~ Oops. No, I'm wrong. I wasn't paying enough attention. Sorry. However, it turns out I *am* connected via CDC via USB, not UART serial at all. 🤦‍♂️ ![IMG_2377](https://user-images.githubusercontent.com/53625/177976752-88df8798-7e45-401c-bddd-354b41aad08f.jpg) <img width="965" alt="Screenshot 2022-07-08 at 11 36 57" src="https://user-images.githubusercontent.com/53625/177976783-7e247fcf-1f70-4011-8d99-91ce2ad986d6.png">

MrGreensWorkshop commented

2022-07-08 05:40:49 -05:00

(Migrated from github.com)

Yes, I checked again just to be 100% sure. I downloaded the file and write to my pico. Yes I only check CDC over USB. I didn't even check uart output because you can clearly see my setting that I disabled UART I'm not talking about uart.

https://github.com/MrGreensWorkshop/RasPiPicoSDK_cdc_test/blob/main/CMakeLists.txt

# Enable USB CDC
pico_enable_stdio_usb(PicoTest 1)

# Disable UART
pico_enable_stdio_uart(PicoTest 0)
 
# Enable extra outputs (bins)
pico_add_extra_outputs(PicoTest)

Yes, I checked again just to be 100% sure. I downloaded the file and write to my pico. Yes I only check CDC over USB. I didn't even check uart output because you can clearly see my setting that I disabled UART I'm not talking about uart. https://github.com/MrGreensWorkshop/RasPiPicoSDK_cdc_test/blob/main/CMakeLists.txt ``` # Enable USB CDC pico_enable_stdio_usb(PicoTest 1) # Disable UART pico_enable_stdio_uart(PicoTest 0) # Enable extra outputs (bins) pico_add_extra_outputs(PicoTest) ```

MrGreensWorkshop commented

2022-07-08 05:45:17 -05:00

(Migrated from github.com)

As I mentioned earlier here if you use VT100 terminal you can't catch it.

May I kindly ask you, please check with Coolterm as I mentioned here

Please make sure initial line states for DTR, RTS is off.

Thank you.

As I mentioned [earlier here](https://github.com/raspberrypi/pico-sdk/issues/906#issuecomment-1178758803) if you use VT100 terminal you can't catch it. May I kindly ask you, please check with [Coolterm](https://freeware.the-meiers.org/CoolTermMac.zip) as I mentioned [here](https://github.com/raspberrypi/pico-sdk/issues/906#issuecomment-1178700704) Please make sure initial line states for DTR, RTS is off. ![CoolTermMac_term](https://user-images.githubusercontent.com/76992231/177949694-c8a83947-5dc3-46d0-8d29-4776fbe68a84.gif) Thank you.

JamesH65 commented

2022-07-08 07:44:33 -05:00

(Migrated from github.com)

Might be worth flagging this up on the tinyUSB GitHub issue tracker, since it is their code.

MrGreensWorkshop commented

2022-07-09 06:01:38 -05:00

(Migrated from github.com)

"can't reproduce" feels sad.

MrGreensWorkshop commented

2022-07-18 20:53:42 -05:00

(Migrated from github.com)

@JamesH65

Might be worth flagging this up on the tinyUSB GitHub issue tracker, since it is their code.

The problem is how tinyUSB implemented in pico-sdk source code.
https://github.com/raspberrypi/pico-sdk/blob/master/src/rp2_common/pico_stdio_usb/stdio_usb.c

tud_cdc_connected is used for checking connection but under the hood its checking DTR. I don't think using tud_cdc_connected is appropriate way to check the connection.

~~Could you please consider to reopen the issue? Thank you.~~

src/class/cdc/cdc_device.h

...
static inline bool tud_cdc_connected (void)
{
  return tud_cdc_n_connected(0);
}
...

src/class/cdc/cdc_device.h

...
bool tud_cdc_n_connected(uint8_t itf)
{
  // DTR (bit 0) active  is considered as connected
  return tud_ready() && tu_bit_test(_cdcd_itf[itf].line_state, 0);
}
...

I made a PR #932 which gives user option to disable DTR check.

@JamesH65 > Might be worth flagging this up on the tinyUSB GitHub issue tracker, since it is their code. The problem is how tinyUSB implemented in pico-sdk source code. https://github.com/raspberrypi/pico-sdk/blob/master/src/rp2_common/pico_stdio_usb/stdio_usb.c `tud_cdc_connected` is used for checking connection but under the hood its checking DTR. I don't think using `tud_cdc_connected` is appropriate way to check the connection. ~~Could you please consider to reopen the issue? Thank you.~~ [src/class/cdc/cdc_device.h](https://github.com/hathach/tinyusb/blob/master/src/class/cdc/cdc_device.h) ``` ... static inline bool tud_cdc_connected (void) { return tud_cdc_n_connected(0); } ... ``` [src/class/cdc/cdc_device.h](https://github.com/hathach/tinyusb/blob/master/src/class/cdc/cdc_device.c) ``` ... bool tud_cdc_n_connected(uint8_t itf) { // DTR (bit 0) active is considered as connected return tud_ready() && tu_bit_test(_cdcd_itf[itf].line_state, 0); } ... ``` I made a PR #932 which gives user option to disable DTR check.

JamesH65 commented

2022-07-19 04:00:19 -05:00

(Migrated from github.com)

Not my call to reopen, @kilograham?

MrGreensWorkshop commented

2022-07-19 04:12:06 -05:00

(Migrated from github.com)

@JamesH65 Things are more clear now. You don't need to reopen the issue. I made a PR #932, but label can't reproduce could be removed.

@JamesH65 Things are more clear now. You don't need to reopen the issue. I made a PR #932, but label `can't reproduce` could be removed.

Rows
Columns

CDC doesn't respect no flow control setting (DTR control needed to receive data) #906

Operating System

Board

Firmware

What happened ?

How to reproduce ?

Expected behavior

Cause