From b627e00b1797305dd347f6135d8b2a7eb3ba0199 Mon Sep 17 00:00:00 2001 From: Sandeep Mistry Date: Thu, 14 Jul 2016 14:50:27 -0400 Subject: [PATCH 1/4] Add USB send timeout Timeout is checked on the next send attempt. Initial timeout is set to 70ms, after a time out occurs and the previous transfer is still not complete a send will report as failed. If a send times out, a ZLP will be in the send queue to check for future timeouts. --- cores/arduino/USB/USBCore.cpp | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/cores/arduino/USB/USBCore.cpp b/cores/arduino/USB/USBCore.cpp index e2a9600ff..eaeb0a157 100644 --- a/cores/arduino/USB/USBCore.cpp +++ b/cores/arduino/USB/USBCore.cpp @@ -38,6 +38,7 @@ static volatile uint8_t rxLEDPulse; /**< Milliseconds remaining for data Rx LED #endif static char isRemoteWakeUpEnabled = 0; static char isEndpointHalt = 0; +static char lastTransmitTimedOut = 0; extern void (*gpf_isr)(void); @@ -598,6 +599,9 @@ uint8_t USBDeviceClass::armRecv(uint32_t ep) return usbd.epBank0ByteCount(ep); } +// Timeout for sends +#define TX_TIMEOUT_MS 70 + // Blocking Send of data to an endpoint uint32_t USBDeviceClass::send(uint32_t ep, const void *data, uint32_t len) { @@ -619,6 +623,28 @@ uint32_t USBDeviceClass::send(uint32_t ep, const void *data, uint32_t len) // Flash area while (len != 0) { + if (usbd.epBank1IsReady(ep)) { + // previous transfer is still not complete + + // convert the timeout from microseconds to a number of times through + // the wait loop; it takes (roughly) 18 clock cycles per iteration. + uint32_t timeout = microsecondsToClockCycles(TX_TIMEOUT_MS * 1000) / 18; + + // Wait for (previous) transfer to complete + while (!usbd.epBank1IsTransferComplete(ep)) { + if (lastTransmitTimedOut || timeout-- == 0) { + lastTransmitTimedOut = 1; + + // set byte count to zero, so that ZLP is sent + // instead of stale data + usbd.epBank1SetByteCount(ep, 0); + return -1; + } + } + } + + lastTransmitTimedOut = 0; + if (len >= EPX_SIZE) { length = EPX_SIZE - 1; } else { @@ -637,10 +663,6 @@ uint32_t USBDeviceClass::send(uint32_t ep, const void *data, uint32_t len) // RAM buffer is full, we can send data (IN) usbd.epBank1SetReady(ep); - // Wait for transfer to complete - while (!usbd.epBank1IsTransferComplete(ep)) { - ; // need fire exit. - } written += length; len -= length; data = (char *)data + length; From 8afe969ed2ea361f1645c0db357683c522bed2e6 Mon Sep 17 00:00:00 2001 From: Sandeep Mistry Date: Thu, 14 Jul 2016 16:37:25 -0400 Subject: [PATCH 2/4] Make transmit timeout per endpoint --- cores/arduino/USB/USBCore.cpp | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/cores/arduino/USB/USBCore.cpp b/cores/arduino/USB/USBCore.cpp index eaeb0a157..5b961f105 100644 --- a/cores/arduino/USB/USBCore.cpp +++ b/cores/arduino/USB/USBCore.cpp @@ -38,7 +38,6 @@ static volatile uint8_t rxLEDPulse; /**< Milliseconds remaining for data Rx LED #endif static char isRemoteWakeUpEnabled = 0; static char isEndpointHalt = 0; -static char lastTransmitTimedOut = 0; extern void (*gpf_isr)(void); @@ -602,6 +601,16 @@ uint8_t USBDeviceClass::armRecv(uint32_t ep) // Timeout for sends #define TX_TIMEOUT_MS 70 +static char LastTransmitTimedOut[7] = { + 0, + 0, + 0, + 0, + 0, + 0, + 0 +}; + // Blocking Send of data to an endpoint uint32_t USBDeviceClass::send(uint32_t ep, const void *data, uint32_t len) { @@ -627,13 +636,13 @@ uint32_t USBDeviceClass::send(uint32_t ep, const void *data, uint32_t len) // previous transfer is still not complete // convert the timeout from microseconds to a number of times through - // the wait loop; it takes (roughly) 18 clock cycles per iteration. - uint32_t timeout = microsecondsToClockCycles(TX_TIMEOUT_MS * 1000) / 18; + // the wait loop; it takes (roughly) 23 clock cycles per iteration. + uint32_t timeout = microsecondsToClockCycles(TX_TIMEOUT_MS * 1000) / 23; // Wait for (previous) transfer to complete while (!usbd.epBank1IsTransferComplete(ep)) { - if (lastTransmitTimedOut || timeout-- == 0) { - lastTransmitTimedOut = 1; + if (LastTransmitTimedOut[ep] || timeout-- == 0) { + LastTransmitTimedOut[ep] = 1; // set byte count to zero, so that ZLP is sent // instead of stale data @@ -643,7 +652,7 @@ uint32_t USBDeviceClass::send(uint32_t ep, const void *data, uint32_t len) } } - lastTransmitTimedOut = 0; + LastTransmitTimedOut[ep] = 0; if (len >= EPX_SIZE) { length = EPX_SIZE - 1; From 8f7873cea58b02cbb91d2ec2170a488fb90b7367 Mon Sep 17 00:00:00 2001 From: Sandeep Mistry Date: Mon, 18 Jul 2016 11:30:45 -0400 Subject: [PATCH 3/4] Add comment on inspiration --- cores/arduino/USB/USBCore.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/cores/arduino/USB/USBCore.cpp b/cores/arduino/USB/USBCore.cpp index 5b961f105..14390bccc 100644 --- a/cores/arduino/USB/USBCore.cpp +++ b/cores/arduino/USB/USBCore.cpp @@ -640,6 +640,7 @@ uint32_t USBDeviceClass::send(uint32_t ep, const void *data, uint32_t len) uint32_t timeout = microsecondsToClockCycles(TX_TIMEOUT_MS * 1000) / 23; // Wait for (previous) transfer to complete + // inspired by Paul Stoffregen's work on Teensy while (!usbd.epBank1IsTransferComplete(ep)) { if (LastTransmitTimedOut[ep] || timeout-- == 0) { LastTransmitTimedOut[ep] = 1; From b2462a5a1a9f8fb86a7153d5d352e97c44392c02 Mon Sep 17 00:00:00 2001 From: Sandeep Mistry Date: Mon, 18 Jul 2016 11:31:53 -0400 Subject: [PATCH 4/4] Remove line state check in Serial_::write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s no longer needed with the timeout mechanism in USBCore. --- cores/arduino/USB/CDC.cpp | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/cores/arduino/USB/CDC.cpp b/cores/arduino/USB/CDC.cpp index c65c63850..3c795f18e 100644 --- a/cores/arduino/USB/CDC.cpp +++ b/cores/arduino/USB/CDC.cpp @@ -205,28 +205,14 @@ void Serial_::flush(void) size_t Serial_::write(const uint8_t *buffer, size_t size) { - /* only try to send bytes if the high-level CDC connection itself - is open (not just the pipe) - the OS should set lineState when the port - is opened and clear lineState when the port is closed. - bytes sent before the user opens the connection or after - the connection is closed are lost - just like with a UART. */ - - // TODO - ZE - check behavior on different OSes and test what happens if an - // open connection isn't broken cleanly (cable is yanked out, host dies - // or locks up, or host virtual serial port hangs) - if (_usbLineInfo.lineState > 0) // Problem with Windows(R) - { - uint32_t r = usb.send(CDC_ENDPOINT_IN, buffer, size); + uint32_t r = usb.send(CDC_ENDPOINT_IN, buffer, size); - if (r > 0) { - return r; - } else { - setWriteError(); - return 0; - } + if (r > 0) { + return r; + } else { + setWriteError(); + return 0; } - setWriteError(); - return 0; } size_t Serial_::write(uint8_t c) {