TL;DR : I propose an application-side fix for when the CDC Device
Class hangs when trying to send data after a while if the host never
reads it.
Long version:
The CDC Device Class hangs when trying to send data after a while if
the host never reads it.
Similar issues have been talked about a few times, among which:
http://code.google.com/p/lufa-lib/issues/detail?id=6
https://groups.google.com/group/lufa-support/browse_thread/thread/9310cc73b2f8c700/524d7eb4f5b6ef26
https://groups.google.com/group/lufa-support/browse_thread/thread/7f7b4568f5e997e2/5ce0f7e095d98bb3
and the problem is acknowledge in the documentation (LUFA/Drivers/USB/
Class/Device/CDCClassDevice.h):
> Another major oversight is that there is no mechanism for the host to notify the
> device that there is a data sink on the host side ready to accept data. This
> means that the device may try to send data while the host isn't listening, causing
> lengthy blocking timeouts in the transmission routines. To combat this, it is
> recommended that the virtual serial line DTR (Data Terminal Ready) be used where
> possible to determine if a host application is ready for data.
Yet as the USBtoSerial project in LUFA 120219 suffers from it, and the
firmware in the ATMega8u2 in my Arduino Uno too, it might be
interesting to talk a little more about the subject.
These blocking timeouts were particularly annoying for my application
(
usb2ax.com , firmware is based on LUFA USBtoSerial), where it's
common to have a lot of data coming back from the serial port to the
host that we just ignore.
Having degraded sending performances when the host receive buffer is
full is not very satisfactory compared to FTDI chips (yeah I know,
they have their own USB driver so it's not the same).
And since I wanted my board to be a drop-in replacement for a product
that uses an FTDI, and still be compatible with all existing software
libraries, using DTR was not an option.
Here is a potential fix I use, and basically I wonder if it's any good
and if it might be of use to anybody else.
Still, I reckon this fix does things quite differently than it's done
in LUFA, hence I'm not suggesting it as a potential fix of LUFA CDC
Device Class. Think of it as a workaround that applications could use
to get rid of the blocking behavior of the CDC Class in simple cases.
Right now, as far as I can tell, it works in my application on my test
setup (Win7 64bit and Linux, LUFA 120219, board with ATMega32u2,
serial baud rates up to 1Mbaud). I have witnessed the original problem
on MacOS too, but haven't had the opportunity to test the fix on this
OS.
I'll post the full code/patch to the USBtoSerial project soon (still
have to cleanup stuff to do before I can present a minimal diff), in
the meantime here's the basic idea:
uint8_t needEmptyPacket = false; // flag used when an additional,
empty packet needs to be sent to properly conclude an USB transfer
void cdc_send_byte(uint8_t data){
RingBuffer_Insert(&ToUSB_Buffer, data);
TIFR0 |= (1 << TOV0); // Clear timer
}
void cdc_send_USB_data( USB_ClassInfo_CDC_Device_t* const
CDCInterfaceInfo ){
// process outgoing USB data
Endpoint_SelectEndpoint(CDCInterfaceInfo-
>Config.DataINEndpointNumber); // select IN endpoint to restore its
registers
if ( Endpoint_IsINReady() ){ // if we can write on the outgoing
data bank
uint8_t BufferCount = RingBuffer_GetCount(&ToUSB_Buffer);
if (BufferCount) {
uint8_t bank_size = CDCInterfaceInfo-
>Config.DataINEndpointSize;
// if there are more bytes in the buffer than what can be
put in the data bank OR there are a few bytes and they have been
waiting for too long
if ( BufferCount >= bank_size || (TIFR0 & (1 << TOV0)) ){
// Clear flush timer expiry flag
TIFR0 |= (1 << TOV0);
// load the IN data bank until full or until we loaded
all the bytes we know we have
uint8_t nb_to_write = min(BufferCount,
bank_size );
while (nb_to_write--){
uint8_t Data = RingBuffer_Remove(&ToUSB_Buffer);
Endpoint_Write_8(Data);
}
// if the bank is full (== we can't write to it
anymore), we might need an empty packet after this one
needEmptyPacket = ! Endpoint_IsReadWriteAllowed();
Endpoint_ClearIN(); // allow the hardware to send the
content of the bank
}
} else if (needEmptyPacket) {
// send an empty packet to end the transfer
needEmptyPacket = false;
Endpoint_ClearIN(); // allow the hardware to send the
content of the bank
}
}
}
/** ISR to manage the reception of data from the serial port, placing
received bytes into a circular buffer
* for later transmission to the host.
*/
ISR(USART1_RX_vect, ISR_BLOCK)
{
uint8_t ReceivedByte = UDR1;
if (USB_DeviceState == DEVICE_STATE_Configured)
cdc_send_byte(ReceivedByte);
}
In a nutshell, it works like that:
- Don't use CDC_Device_SendByte() or similar CDC Device functions
dedicated to sending stuff to the host, nor CDC_Device_USBTask();
Everything about receiving data from USB is fine though, as far as I
can tell.
- Instead of calling CDC_Device_SendByte() to try to send data to the
host,data is stored in a ring buffer using cdc_send_byte().
cdc_send_byte() also resets the timer counter, since I wanted the
timer to overflow a fixed amount of time after the last byte has been
inserted, not a fixed amount of time after data has been sent last
(not important).
In the USBtoSerial project, the only place where cdc_send_byte() would
be called would be in the RX ISR (in my code however I needed to be
able to call it from other places).
- cdc_send_USB_data() is in charge of unloading data from the buffer
and sending them to the host.
It checks if the endpoint is ready to accept data with
Endpoint_IsINReady(), and only then pushes it.
So Endpoint_IsINReady() behaves like this (as far as I can tell):
* there's a little delay between Endpoint_ClearIN() and
Endpoint_IsINReady() being true again,
* when the endpoint is not polled, Endpoint_IsINReady() stays
false after the last Endpoint_ClearIN().
This makes checking Endpoint_IsINReady() first a good preliminary
test: if it's not true, then there's no way we can write data right
now. Better leave it to that (return without blocking) and come back
later to see if it's back up.
After checking if data can be given to the endpoint, it verifies if
there is enough data to send to fill a bank or if the available data
has been sitting in the buffer for too long. If either of these
conditions is true, then as much bytes as possible are put into the
endpoint bank and the bank is marked as "clear to send". The bank is
filled (or not in the case of an empty packet) and immediately
"cleared to send", so we don't have to check the amount of free space
in the bank, nor to loop to wait for free space to put our bytes.
If the bank has been completely filled, then a flag is set to remember
that an additional empty packet might be needed. This flag will be
used next time the function is called if there is no data in the
buffer.
This function has to be called regularly and often, similarly to the
way CDC_Device_USBTask() was called.
What will happen if the incoming data is never read by the application
that opened the port on the host, with the fix:
- The OS will regularly poll the IN endpoint and store everything the
device has to send in a buffer.
- Once this buffer is full (12300 or 12356 bytes on my Win7 64bits
Home Edition), it will stop polling the endpoint (that's when the
delays used to happen).
- calls to cdc_send_USB_data() will simply find that the endpoint is
not ready to receive new data and do nothing, without blocking. Calls
to cdc_send_byte() will continue to add data to the ring buffer, which
by its very nature will never block, just silently discard older bytes
when adding new ones.
- If the host reads data from its receive buffer, then
cdc_send_USB_data() will progressively send the content of the buffer,
and resume normal operation.
To compare with what is currently done in LUFA, in CDCDeviceClass.c,
there is:
uint8_t CDC_Device_SendByte(USB_ClassInfo_CDC_Device_t* const
CDCInterfaceInfo,
const uint8_t Data)
{
if ((USB_DeviceState != DEVICE_STATE_Configured) || !
(CDCInterfaceInfo->State.LineEncoding.BaudRateBPS))
return ENDPOINT_RWSTREAM_DeviceDisconnected;
Endpoint_SelectEndpoint(CDCInterfaceInfo-
>Config.DataINEndpoint.Address);
if (!(Endpoint_IsReadWriteAllowed()))
{
Endpoint_ClearIN();
uint8_t ErrorCode;
if ((ErrorCode = Endpoint_WaitUntilReady()) !=
ENDPOINT_READYWAIT_NoError)
return ErrorCode;
}
Endpoint_Write_8(Data);
return ENDPOINT_READYWAIT_NoError;
}
Endpoint_IsReadWriteAllowed() tests if the endpoint bank is NOT full.
Endpoint_IsINReady() tests if the bank is ready to accept data.
Basically, if the bank is currently full ( !
Endpoint_IsReadWriteAllowed()), it tells the hardware to send the
packet, then wait until it's ready to accept data
(Endpoint_WaitUntilReady()) before writing the byte.
In this case, Endpoint_WaitUntilReady() waits for Endpoint_IsINReady()
to be true, and tries to make a diagnostic of why it's not ready, and
that's where it hangs since Endpoint_IsINReady() will not become true
as long as the host does not poll the endpoint again.
I don't think the fix I propose can be easily integrated into LUFA
because it requires a buffer to gracefully manage the case where
cdc_send_USB_data() is called when Endpoint_IsINReady() does not
return true yet because it's currently processing data (as opposed to
because it has not been polled for a long time).
Ok, sorry for the rambling, I hope I make some sense...
So, to those who read all of this: any obvious mistakes, horrible
error, excruciating blunder? Any interest?
Best,
Xevel