improve write timeout handling

Return type of write() method changed to void. The return value was redundant before, as it always was the request length or an exception was thrown.

If timeout is reached, write() now throws a SerialTimeoutException with ex.bytesTransferred filled with known transferred bytes.

Added CommonUsbSerialPort.getReadEndpoint() and .getWriteEndpoint() to assist in setting the optimal write buffer size with port.setWriteBufferSize(port.getWriteEndpoint().getMaxPacketSize()).

By default the write buffer size is > MaxPacketSize and the Linux kernel splits writes in chunks. When the timeout occurs, it's unknown how many chunks have already been transferred and the exception typically stores 0. With optimal write buffer size, this value is known and stored in SerialTimeoutException, but due to more kernel round trips write() might take slightly longer().

build.gradle

@@ -6,7 +6,7 @@ buildscript {
         google()
     }
     dependencies {
-        classpath 'com.android.tools.build:gradle:4.1.1'
+        classpath 'com.android.tools.build:gradle:4.1.2'
     }
 }
 

usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/DeviceTest.java

@@ -26,6 +26,7 @@ import com.hoho.android.usbserial.driver.Cp21xxSerialDriver;
 import com.hoho.android.usbserial.driver.FtdiSerialDriver;
 import com.hoho.android.usbserial.driver.ProbeTable;
 import com.hoho.android.usbserial.driver.ProlificSerialDriver;
+import com.hoho.android.usbserial.driver.SerialTimeoutException;
 import com.hoho.android.usbserial.driver.UsbId;
 import com.hoho.android.usbserial.driver.UsbSerialDriver;
 import com.hoho.android.usbserial.driver.UsbSerialPort;
@@ -37,6 +38,7 @@ import com.hoho.android.usbserial.util.UsbWrapper;
 import org.junit.After;
 import org.junit.AfterClass;
 import org.junit.Assert;
+import org.junit.Assume;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Rule;
@@ -236,6 +238,21 @@ public class DeviceTest {
         assertThat(reason, data, equalTo(buf2));
     }
 
+    private void purgeWriteBuffer(int timeout) throws Exception {
+        try {
+            Log.d(TAG, " purge begin");
+            usb.serialPort.purgeHwBuffers(true, false);
+        } catch(UnsupportedOperationException ignored) {}
+        byte[] data = telnet.read(-1, timeout);
+        int len = 0;
+        while(data.length != 0) {
+            len += data.length;
+            Log.d(TAG, " purge read " + data.length);
+            data = telnet.read(-1, timeout);
+        }
+        Log.d(TAG, " purge end " + len);
+    }
+
     @Test
     public void openClose() throws Exception {
         usb.open();
@@ -294,8 +311,7 @@ public class DeviceTest {
 
     @Test
     public void prolificBaudRate() throws Exception {
-        if(!(usb.serialDriver instanceof ProlificSerialDriver))
+        Assume.assumeTrue("only for Prolific", usb.serialDriver instanceof ProlificSerialDriver);
-            return;
 
         int[] baudRates = {
                 75, 150, 300, 600, 1200, 1800, 2400, 3600, 4800, 7200, 9600, 14400, 19200,
@@ -357,8 +373,7 @@ public class DeviceTest {
 
     @Test
     public void ftdiBaudRate() throws Exception {
-        if (!(usb.serialDriver instanceof FtdiSerialDriver))
+        Assume.assumeTrue("only for FTDI", usb.serialDriver instanceof FtdiSerialDriver);
-            return;
 
         usb.open();
         try {
@@ -768,65 +783,129 @@ public class DeviceTest {
     @Test
     public void writeTimeout() throws Exception {
         usb.open();
-        usb.setParameters(115200, 8, 1, UsbSerialPort.PARITY_NONE);
+        int baudRate = 300;
-        telnet.setParameters(115200, 8, 1, UsbSerialPort.PARITY_NONE);
+        if(usb.serialDriver instanceof Cp21xxSerialDriver && usb.serialDriver.getPorts().size() > 1)
-
+            baudRate = 2400;
-        // Basically all devices have a UsbEndpoint.getMaxPacketSize() 64. When the timeout
+        usb.setParameters(baudRate, 8, 1, UsbSerialPort.PARITY_NONE);
-        // in usb.serialPort.write() is reached, some packets have been written and the rest
+        telnet.setParameters(baudRate, 8, 1, UsbSerialPort.PARITY_NONE);
-        // is discarded. bulkTransfer() does not return the number written so far, but -1.
+        int purgeTimeout = 250;
-        // With 115200 baud and 1/2 second timeout, typical values are:
+        if(usb.serialDriver instanceof CdcAcmSerialDriver)
-        //   ch340    6080 of 6144
+            purgeTimeout = 500;
-        //   pl2302   5952 of 6144
+        purgeWriteBuffer(purgeTimeout);
-        //   cp2102   6400 of 7168
-        //   cp2105   6272 of 7168
-        //   ft232    5952 of 6144
-        //   ft2232   9728 of 10240
-        //   arduino   128 of 144
-        int timeout = 500;
-        int len = 0;
-        int startLen = 1024;
-        int step = 1024;
-        int minLen = 4069;
-        int maxLen = 12288;
-        int bufferSize = 511;
-        TestBuffer buf = new TestBuffer(len);
-        if(usb.serialDriver instanceof CdcAcmSerialDriver) {
-            startLen = 16;
-            step = 16;
-            minLen = 128;
-            maxLen = 256;
-            bufferSize = 31;
-        }
 
+        // determine write buffer size
+        int writePacketSize = ((CommonUsbSerialPort)usb.serialPort).getWriteEndpoint().getMaxPacketSize();
+        byte[] pbuf = new byte[writePacketSize];
+        int writePackets = 0;
         try {
-            for (len = startLen; len < maxLen; len += step) {
+            for (writePackets = 0; writePackets < 64; writePackets++)
-                buf = new TestBuffer(len);
+                usb.serialPort.write(pbuf, 1);
-                Log.d(TAG, "write buffer size " + len);
+            fail("write error expected");
-                usb.serialPort.write(buf.buf, timeout);
+        } catch(IOException ignored) {}
-                while (!buf.testRead(telnet.read(-1)))
+        purgeWriteBuffer(purgeTimeout);
-                    ;
+
+        int writeBufferSize = writePacketSize * writePackets;
+        Log.d(TAG, "write packet size = " + writePacketSize + ", write buffer size = " + writeBufferSize);
+        if(usb.serialDriver instanceof Cp21xxSerialDriver) {
+            switch(usb.serialDriver.getPorts().size()*0x10 + usb.serialPort.getPortNumber()) {
+                case 0x10: assertEquals("write packet size", 64, writePacketSize); assertEquals("write buffer size", 576, writeBufferSize); break;
+                case 0x20: assertEquals("write packet size", 64, writePacketSize); assertEquals("write buffer size", 320, writeBufferSize); break;
+                case 0x21: assertEquals("write packet size", 32, writePacketSize); assertTrue("write buffer size", writeBufferSize == 128 || writeBufferSize == 160); break;
+                default: fail("unknown port number");
             }
-            fail("write timeout expected between " + minLen + " and " + maxLen + ", is " + len);
+        } else if(usb.serialDriver instanceof Ch34xSerialDriver) {
-        } catch (IOException e) {
+            assertEquals("write packet size", 32, writePacketSize); assertEquals("write buffer size", 64, writeBufferSize);
-            Log.d(TAG, "usbWrite failed", e);
+        } else if(usb.serialDriver instanceof ProlificSerialDriver) {
-            while (true) {
+            assertEquals("write packet size", 64, writePacketSize); assertEquals("write buffer size", 256, writeBufferSize);
-                byte[] data = telnet.read(-1);
+        } else if(usb.serialDriver instanceof FtdiSerialDriver) {
-                if (data.length == 0) break;
+            switch(usb.serialDriver.getPorts().size()) {
-                if (buf.testRead(data)) break;
+                case 1: assertEquals("write packet size", 64, writePacketSize);  assertEquals("write buffer size", 128, writeBufferSize); break;
+                case 2: assertEquals("write packet size", 512, writePacketSize); assertEquals("write buffer size", 4096, writeBufferSize); break;
+                case 4: assertEquals("write packet size", 512, writePacketSize); assertEquals( "write buffer size", 2048, writeBufferSize); break;
+                default: fail("unknown port number");
             }
-            Log.d(TAG, "received " + buf.len + " of " + len + " bytes of failing usbWrite");
+        } else if(usb.serialDriver instanceof CdcAcmSerialDriver) {
-            assertTrue("write timeout expected between " + minLen + " and " + maxLen + ", is " + len, len > minLen);
+            assertEquals(64, writePacketSize); assertEquals(128, writeBufferSize); // values for ATMega32U4
+        } else {
+            fail("unknown driver " + usb.serialDriver.getClass().getSimpleName());
+        }
+        purgeWriteBuffer(purgeTimeout);
+        if(usb.serialDriver instanceof CdcAcmSerialDriver)
+            return; // serial processing to slow for tests below, but they anyway only check shared code in CommonUsbSerialPort
+        if(usb.serialDriver instanceof Cp21xxSerialDriver && usb.serialDriver.getPorts().size() > 1)
+            return; // write buffer size detection unreliable as baud rate to high
+
+        usb.setParameters(9600, 8, 1, UsbSerialPort.PARITY_NONE);
+        telnet.setParameters(9600, 8, 1, UsbSerialPort.PARITY_NONE);
+        TestBuffer tbuf;
+
+        // compare write time
+        //   full write @ 1-2 msec
+        //   packet write @ 5-10 msec
+        if(false) {
+            tbuf = new TestBuffer(writeBufferSize);
+            ((CommonUsbSerialPort) usb.serialPort).setWriteBufferSize(tbuf.buf.length);
+            Log.d(TAG, "full write begin");
+            long begin = System.currentTimeMillis();
+            usb.serialPort.write(tbuf.buf, 0);
+            Log.d(TAG, "full write end, duration=" + (System.currentTimeMillis() - begin));
+            purgeWriteBuffer(purgeTimeout);
+            ((CommonUsbSerialPort) usb.serialPort).setWriteBufferSize(writePacketSize);
+            Log.d(TAG, "packet write begin");
+            begin = System.currentTimeMillis();
+            usb.serialPort.write(tbuf.buf, 0);
+            Log.d(TAG, "packet write end, duration=" + (System.currentTimeMillis() - begin));
+            purgeWriteBuffer(purgeTimeout);
+            Assume.assumeTrue(false);
         }
 
-        // With smaller writebuffer, the timeout is used per bulkTransfer and each call 'fits'
+        // total write timeout
-        // into this timout, but shouldn't further calls only use the remaining timeout?
+        tbuf = new TestBuffer(writeBufferSize + writePacketSize);
-        ((CommonUsbSerialPort) usb.serialPort).setWriteBufferSize(bufferSize);
+        int timeout = writePacketSize / 32 * 50; // time for 1.5 packets. write 48 byte in 50 msec at 9600 baud
-        len = maxLen;
+        ((CommonUsbSerialPort)usb.serialPort).setWriteBufferSize(writePacketSize);
-        buf = new TestBuffer(len);
+        usb.serialPort.write(tbuf.buf, timeout);
-        Log.d(TAG, "write buffer size " + len);
+        purgeWriteBuffer(purgeTimeout);
-        usb.serialPort.write(buf.buf, timeout);
+        tbuf = new TestBuffer(writeBufferSize + 2*writePacketSize);
-        while (!buf.testRead(telnet.read(-1)))
+        try {
-            ;
+            usb.serialPort.write(tbuf.buf, timeout); // would not fail if each block has own timeout
+            fail("write error expected");
+        } catch(SerialTimeoutException ignored) {}
+        purgeWriteBuffer(purgeTimeout);
+
+        // infinite wait
+        //((CommonUsbSerialPort)usb.serialPort).setWriteBufferSize(writePacketSize);
+        usb.serialPort.write(tbuf.buf, 0);
+        purgeWriteBuffer(purgeTimeout);
+
+        // SerialTimeoutException.bytesTransferred
+        int readWait = writePacketSize > 64 ? 250 : 50;
+        ((CommonUsbSerialPort)usb.serialPort).setWriteBufferSize(tbuf.buf.length);
+        try {
+            usb.serialPort.write(tbuf.buf, timeout);
+            fail("write error expected");
+        } catch(SerialTimeoutException ex) {
+            for(byte[] data = telnet.read(-1, readWait); data.length != 0;
+                       data = telnet.read(-1, readWait)) {
+                tbuf.testRead(data);
+            }
+            assertEquals(0, ex.bytesTransferred);
+            assertEquals(writeBufferSize + writePacketSize, tbuf.len);
+        }
+        purgeWriteBuffer(purgeTimeout);
+        ((CommonUsbSerialPort)usb.serialPort).setWriteBufferSize(writePacketSize);
+        tbuf.len = 0;
+        try {
+            usb.serialPort.write(tbuf.buf, timeout);
+            fail("write error expected");
+        } catch(SerialTimeoutException ex) {
+            for(byte[] data = telnet.read(-1, readWait); data.length != 0;
+                       data = telnet.read(-1, readWait)) {
+                tbuf.testRead(data);
+            }
+            assertEquals(writeBufferSize + writePacketSize, ex.bytesTransferred);
+            assertEquals(writeBufferSize + writePacketSize, tbuf.len);
+        }
+        purgeWriteBuffer(purgeTimeout);
     }
 
     @Test
@@ -1872,6 +1951,8 @@ public class DeviceTest {
         usb.open();
         assertTrue(usb.serialPort.isOpen());
 
+        assertEquals(((CommonUsbSerialPort)usb.serialPort).getWriteEndpoint().getMaxPacketSize(),
+                     ((CommonUsbSerialPort)usb.serialPort).getReadEndpoint().getMaxPacketSize());
         s = usb.serialPort.getSerial();
         // with target sdk 29 can throw SecurityException before USB permission dialog is confirmed
         // not all devices implement serial numbers. some observed values are:
@@ -1898,8 +1979,8 @@ public class DeviceTest {
 
     @Test
     public void ftdiMethods() throws Exception {
-        if(!(usb.serialDriver instanceof FtdiSerialDriver))
+        Assume.assumeTrue("only for FTDI", usb.serialDriver instanceof FtdiSerialDriver);
-            return;
+
         byte[] b;
         usb.open();
         usb.setParameters(115200, 8, 1, UsbSerialPort.PARITY_NONE);

usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/CommonUsbSerialPort.java

@@ -63,7 +63,19 @@ public abstract class CommonUsbSerialPort implements UsbSerialPort {
     public int getPortNumber() {
         return mPortNumber;
     }
-    
+
+    /**
+     * Returns the write endpoint.
+     * @return write endpoint
+     */
+    public UsbEndpoint getWriteEndpoint() { return mWriteEndpoint; }
+
+    /**
+     * Returns the read endpoint.
+     * @return read endpoint
+     */
+    public UsbEndpoint getReadEndpoint() { return mReadEndpoint; }
+
     /**
      * Returns the device serial number
      *  @return serial number
@@ -191,39 +203,55 @@ public abstract class CommonUsbSerialPort implements UsbSerialPort {
     }
 
     @Override
-    public int write(final byte[] src, final int timeout) throws IOException {
+    public void write(final byte[] src, final int timeout) throws IOException {
         int offset = 0;
+        int requestTimeout = timeout;
 
         if(mConnection == null) {
             throw new IOException("Connection closed");
         }
         while (offset < src.length) {
-            final int writeLength;
+            final int requestLength;
-            final int amtWritten;
+            final int actualLength;
+            final int requestDuration;
 
             synchronized (mWriteBufferLock) {
                 final byte[] writeBuffer;
 
-                writeLength = Math.min(src.length - offset, mWriteBuffer.length);
+                requestLength = Math.min(src.length - offset, mWriteBuffer.length);
                 if (offset == 0) {
                     writeBuffer = src;
                 } else {
                     // bulkTransfer does not support offsets, make a copy.
-                    System.arraycopy(src, offset, mWriteBuffer, 0, writeLength);
+                    System.arraycopy(src, offset, mWriteBuffer, 0, requestLength);
                     writeBuffer = mWriteBuffer;
                 }
-
+                if (requestTimeout < 0) {
-                amtWritten = mConnection.bulkTransfer(mWriteEndpoint, writeBuffer, writeLength, timeout);
+                    actualLength = -2;
+                    requestDuration = 0;
+                } else {
+                    final long startTime = System.currentTimeMillis();
+                    actualLength = mConnection.bulkTransfer(mWriteEndpoint, writeBuffer, requestLength, requestTimeout);
+                    requestDuration = (int) (System.currentTimeMillis() - startTime);
+                }
             }
-            if (amtWritten <= 0) {
+            Log.d(TAG, "Wrote " + actualLength + "/" + requestLength + " offset " + offset + "/" + src.length + " timeout " + requestTimeout);
-                throw new IOException("Error writing " + writeLength
+            if (requestTimeout > 0) {
-                        + " bytes at offset " + offset + " length=" + src.length);
+                requestTimeout -= requestDuration;
+                if (requestTimeout == 0)
+                    requestTimeout = -1;
             }
-
+            if (actualLength <= 0) {
-            Log.d(TAG, "Wrote amt=" + amtWritten + " attempted=" + writeLength);
+                if(requestTimeout < 0) {
-            offset += amtWritten;
+                    SerialTimeoutException ex = new SerialTimeoutException("Error writing " + requestLength + " bytes at offset " + offset + " of total " + src.length);
+                    ex.bytesTransferred = offset;
+                    throw ex;
+                } else {
+                    throw new IOException("Error writing " + requestLength + " bytes at offset " + offset + " of total " + src.length);
+                }
+            }
+            offset += actualLength;
         }
-        return offset;
     }
 
     @Override

usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/SerialTimeoutException.java

@@ -0,0 +1,15 @@
+package com.hoho.android.usbserial.driver;
+
+import java.io.InterruptedIOException;
+
+/**
+ * Signals that a timeout has occurred on serial write.
+ * Similar to SocketTimeoutException.
+ *
+ * {@see InterruptedIOException#bytesTransferred} may contain bytes transferred
+ */
+public class SerialTimeoutException extends InterruptedIOException {
+    public SerialTimeoutException(String s) {
+        super(s);
+    }
+}

usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/UsbSerialPort.java

@@ -114,10 +114,11 @@ public interface UsbSerialPort extends Closeable {
      *
      * @param src the source byte buffer
      * @param timeout the timeout for writing in milliseconds, 0 is infinite
-     * @return the actual number of bytes written
+     * @throws SerialTimeoutException if timeout reached before sending all data.
+     *                                ex.bytesTransferred may contain bytes transferred
      * @throws IOException if an error occurred during writing
      */
-    int write(final byte[] src, final int timeout) throws IOException;
+    void write(final byte[] src, final int timeout) throws IOException;
 
     /**
      * Sets various serial port parameters.