CDC hardware flow control can not be disabled

[robert-rozee]

in a particular user case i need to be able to disable CDC hardware flow control at the device end. it is not practical for the host end to simply assert DTR due to the need for backwards compatibility with other software. the feature i'm requesting is a flag to disable hardware flow control that is exposed via the api.

fyi, the hardware i working with is an RP2040.

my proposal is a bit of a hack, i'd welcome better suggestions. in class/cdc/cdc_device.c make the following changes:

line 116
return tud_ready() && (tu_bit_test(_cdcd_itf[itf].line_state, 0) || flag_ignore_DTR);

line 394
if (!flag_ignore_DTR) tu_fifo_set_overwritable(&p_cdc->tx_ff, !dtr);

where flag_ignore_DTR is exposed through the api.

cheers,
rob :-)

[hathach]

The existing code still works without DTR. However due to no flow control, it wouldn't know if there is any terminal connected and therefore have no way to tell when the fifo will be drained. Therefore the fifo mode is set to overwritable.

[robert-rozee]

on the RP2040, after sending around 1000 characters from a (linux) host without DTR being asserted (characters which are being echoed back by the RP2040), the USB subsystem on the host locks up solidly for around 30 seconds before returning a 'try again' error message back to the user application. while this seems to be a problem with the host/linux, it would be good for the RP2040/tinyUSB to instead be configurable to ignore the state of DTR and just keep sending characters back to the host even when DTR is not asserted. i can think of few cases where the host would genuinely need to signal to tinyUSB to stop sending characters.

the test environment i'm using here now is micropython (RPi foundations, built less than a week back) running on the RP2040, and a terminal emulator on the linux host that is able to independently manipulate DTR.

surely if there is no USB connection present, the are less subtle signs than checking the state of DTR? such as missing heartbeat from the host, Vusb not detected, etc?

cheers,
rob :-)

[hathach]

what is the issue with current implementation that you want to change ?

[robert-rozee]

at the moment, with RP2040+tinyUSB, if DTR is not asserted by the host (in my case a desktop PC running linux), the RP2040 stops sending characters to the host. it looks like tinyUSB attempts to buffer these characters. after approximately 1000 characters have been buffered a malfunction occurs. this scenario can be demonstrated with micropython running on a RP2040 (micropython uses tiny USB).

what i would like to be able to do, is to configure tinyUSB so that it ignores the state of DTR and keeps sending characters irrespective of the current state of DTR. ie, i would like the connection to act as if hardware flow control is turned OFF.

in my case, there are good reasons why the terminal emulator can not alter the state of DTR. my application running on the RP2040 is written in C, however the behaviour of micropython is able to sufficiently reproduce and demonstrate the problem.

cheers,
rob :-)

[hathach]

at the moment, with RP2040+tinyUSB, if DTR is not asserted by the host (in my case a desktop PC running linux), the RP2040 stops sending characters to the host. it looks like tinyUSB attempts to buffer these characters. after approximately 1000 characters have been buffered a malfunction occurs. this scenario can be demonstrated with micropython running on a RP2040 (micropython uses tiny USB).

Have you tried to figured out why tinyusb stop sending data to host. Any log from stack ? It seems to be a bug to me.

what i would like to be able to do, is to configure tinyUSB so that it ignores the state of DTR and keeps sending characters irrespective of the current state of DTR. ie, i would like the connection to act as if hardware flow control is turned OFF.

Could you provide information as required by bug template, and steps to reproduce the issue. It should be done with one of modified stock example here instead of micropython to make sure it is not micropython integration issue. Once we figure out exactly what went wrong. We could decide which is the best fix.

[robert-rozee]

the software engineer developing for the RP2040 is located in the UK, the other side of the world to where i am (in New Zealand). his perspective is that it works ok for him (win10, using teraterm as a terminal emulator) and he has no interest in testing for anything other than his own setup; he is a 'challenging' person to deal with. the source code will only become available to me once his code is 'released to production'.

hence my reproducing the behaviour with micropython, where the source is readily to hand. i'm working at the host/pc end, with my area of expertise is there - not development on the PR2040. if a solution can be found within tinyUSB, this will flow through into the RP2040 SDK, which will then flow through into our project.

what i can do is ring-fence the behaviour as precisely as possible, so that others may reproduce it.

cheers,
rob :-)

[hathach]

I see and understand your problem since you are not familiar with mcu. However for this issue to be addressed, and if you still want it to. Please update your 1st post to provide all the setup information with specific version of each software (mp, picosdk, tinyusb) and step to reproduce issue. Then change the label to Bug. Though since I don't often use mp, this will have huge delay for me to even try to reproduce it.

[hathach]

Moved to discussion since it is more like a bug but lack information to reproduce. Feel free to open a new bug issue with more details

[kholia]

I ran into this issue recently I think.

Steps to reproduce:

Write https://github.com/kholia/YaesuEmulatorPico firmware to Pico (.uf2 file is provided).

Use WSJT-X software on Windows 10 to talk to the Pico (https://physics.princeton.edu/pulsar/k1jt/wsjtx.html).

You will see that it fails horribly fairly quickly.

Next, force the "DTR" line to stay "High" in WSJT-X. Once this is done, WSJT-X is able to communicate just fine with the Pico.

It would be awesome if we could workaround / fix this problem by considering the solution proposed in #932 (comment).

CC @lu7did @robert-rozee.

Related: earlephilhower/arduino-pico#718.

Thank you!

[pigrew]

Perhaps we should document the expected behavior of the CDC class with respect to flow control? This topic seems to come up every few weeks, and there isn't any documentation to point to.

My preferred behavior would be to give control to the application code. I see some use cases (or are there more?):

1. Host wants manual control of DTR/RTS, using them for auxiliary GPIO like the ESP32 WROOM boards do for IO0/nRST. There may be independent hardware UART flow control (or not).
2. Use device hardware flow control for UART (USB has its flow control, and UART has independent hardware flow control; DTR/RTS is set by application and/or hardware.), allowing override by host (AND logic of CDC line state with hardware state).
3. XON/XOFF flow control, perhaps the application would notice the XON/XOFF and do the right thing... This would be supplemented by standard USB flow control.

This choice is not specified by the CDC class documentation; the class should be able to allow for any of the above.

I realize that there is no way to tell if a terminal is connected on the host, but the class shouldn't discard data in normal operation, that should be left up to the application to decide.

---

I would propose the following:

The CDC class driver does not change its operation based on "line state".

For data OUT, USB perhipheral would NAK bulk packets if the FIFO is full. Application may (if it chooses) receive and discard data or set the FIFO's "overwriteable" flag.

For data IN, it's up to the application to fill the application FIFO as needed (or set the overwritable flag).

The host can clear the FIFO/device/event buffers: bus reset, set_config(n), clear_feature(endpoint_halt).

It's also desirable for the application to be able to be able to clear FIFO/device/event buffers at its discretion.

---

As a first step, I would move the calls to set FIFO "overwritable" from the CDC class to the CDC example application. I have not studied how this would influence (for example) Arduino wrappers. The flag_ignore_DTR method would also be a solution without changing behavior for applications.

[kholia]

Here is a rough patch which implements the flag_ignore_DTR idea:

diff --git a/src/class/cdc/cdc_device.c b/src/class/cdc/cdc_device.c
index fab6f0035..af8d37524 100644
--- a/src/class/cdc/cdc_device.c
+++ b/src/class/cdc/cdc_device.c
@@ -41,6 +41,8 @@ enum
   BULK_PACKET_SIZE = (TUD_OPT_HIGH_SPEED ? 512 : 64)
 };
 
+static uint8_t flag_ignore_dtr = 0;
+
 typedef struct
 {
   uint8_t itf_num;
@@ -115,7 +117,7 @@ static bool _prep_out_transaction (cdcd_interface_t* p_cdc)
 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);
+  return tud_ready() && (tu_bit_test(_cdcd_itf[itf].line_state, 0) || flag_ignore_dtr);
 }
 
 uint8_t tud_cdc_n_get_line_state (uint8_t itf)
@@ -123,6 +125,11 @@ uint8_t tud_cdc_n_get_line_state (uint8_t itf)
   return _cdcd_itf[itf].line_state;
 }
 
+void tud_cdc_n_set_ignore_dtr (uint8_t itf)
+{
+  flag_ignore_dtr = 1;
+}
+
 void tud_cdc_n_get_line_coding (uint8_t itf, cdc_line_coding_t* coding)
 {
   (*coding) = _cdcd_itf[itf].line_coding;
@@ -391,7 +398,7 @@ bool cdcd_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t
         p_cdc->line_state = (uint8_t) request->wValue;
         
         // Disable fifo overwriting if DTR bit is set
-        tu_fifo_set_overwritable(&p_cdc->tx_ff, !dtr);
+        if (!flag_ignore_dtr) tu_fifo_set_overwritable(&p_cdc->tx_ff, !dtr);
 
         TU_LOG2("  Set Control Line State: DTR = %d, RTS = %d\r\n", dtr, rts);
 
diff --git a/src/class/cdc/cdc_device.h b/src/class/cdc/cdc_device.h
index fbc7162a3..c84cbfb58 100644
--- a/src/class/cdc/cdc_device.h
+++ b/src/class/cdc/cdc_device.h
@@ -84,6 +84,9 @@ void     tud_cdc_n_read_flush      (uint8_t itf);
 // Get a byte from FIFO at the specified position without removing it
 bool     tud_cdc_n_peek            (uint8_t itf, uint8_t* ui8);
 
+// Set the DTR status ignore flag
+void     tud_cdc_n_set_ignore_dtr  (uint8_t itf);
+
 // Write bytes to TX FIFO, data may remain in the FIFO for a while
 uint32_t tud_cdc_n_write           (uint8_t itf, void const* buffer, uint32_t bufsize);
 
@@ -117,6 +120,7 @@ static inline int32_t  tud_cdc_read_char       (void);
 static inline uint32_t tud_cdc_read            (void* buffer, uint32_t bufsize);
 static inline void     tud_cdc_read_flush      (void);
 static inline bool     tud_cdc_peek            (uint8_t* ui8);
+static inline void     tud_cdc_set_ignore_dtr  (void);
 
 static inline uint32_t tud_cdc_write_char      (char ch);
 static inline uint32_t tud_cdc_write           (void const* buffer, uint32_t bufsize);
@@ -171,6 +175,11 @@ static inline bool tud_cdc_connected (void)
   return tud_cdc_n_connected(0);
 }
 
+static inline void tud_cdc_set_ignore_dtr (void)
+{
+  tud_cdc_n_set_ignore_dtr(0);
+}
+
 static inline uint8_t tud_cdc_get_line_state (void)
 {
   return tud_cdc_n_get_line_state(0);

I also have corresponding patches for pico-sdk and arduino-core.

[caizikun]

Most of serial port don't use DTR , just TX RX and GND. If some instruments need DTR, vendor will point out especially. I had use many Agilent, Tek and other test instruments, all are this situation.