read w/o timeout now throws exception on connection lost or buffer to small

SerialInputOutputManager already returned connection lost exception, as the next read failed

.gitignore

@@ -40,4 +40,4 @@ build/
 local.properties
 *.DS_Store
 proguard/
-
+jacoco.exec

build.gradle

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

usbSerialForAndroid/build.gradle

@@ -23,9 +23,10 @@ android {
 
 dependencies {
     implementation "androidx.annotation:annotation:1.1.0"
-    testImplementation 'junit:junit:4.13'
+    testImplementation 'junit:junit:4.13.2'
+    testImplementation 'org.mockito:mockito-core:1.10.19'
     androidTestImplementation 'com.android.support.test:runner:1.0.2'
-    androidTestImplementation 'commons-net:commons-net:3.6'
+    androidTestImplementation 'commons-net:commons-net:3.8.0'
     androidTestImplementation 'org.apache.commons:commons-lang3:3.11'
 }
 

usbSerialForAndroid/coverage.gradle

@@ -40,7 +40,7 @@ android {
 
     buildTypes {
         debug {
-            testCoverageEnabled true
+            testCoverageEnabled true // disable for testAnyDeviceDebugUnitTest
         }
     }
 }
@@ -53,7 +53,7 @@ project.gradle.taskGraph.whenReady {
 }
 
 task jacocoTestReport(type: JacocoReport ,
-        dependsOn: ['compileAnyDeviceDebugSources' /*'testDebugUnitTest', 'createAnyDeviceDebugCoverageReport'*/]) {
+        dependsOn: ['compileAnyDeviceDebugSources' /*'testAnyDeviceDebugUnitTest' , 'create<device>DebugCoverageReport'*/]) {
     reports {
         xml.enabled = false
         html.enabled = true
@@ -66,6 +66,6 @@ task jacocoTestReport(type: JacocoReport ,
     sourceDirectories.from files([mainSrc])
     classDirectories.from files([debugTree])
     executionData.from fileTree(dir: project.buildDir, includes: [
-            /*'jacoco/testDebugUnitTest.exec',*/ 'outputs/code_coverage/*AndroidTest/connected/*.ec'
+            'jacoco/testAnyDeviceDebugUnitTest.exec', 'outputs/code_coverage/*AndroidTest/connected/*.ec'
     ])
 }

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

@@ -984,15 +984,11 @@ public class DeviceTest {
 
     @Test
     public void readBufferSize() throws Exception {
-        // looks like most devices perform USB read with full mReadEndpoint.getMaxPacketSize() size (= 64)
+        // looks like devices perform USB read with full mReadEndpoint.getMaxPacketSize() size (32, 64, 512)
-        // if the Java byte[] is shorter than the received result, it is silently lost!
+        // if the Java byte[] is shorter than the received result, it is silently lost with read timeout!
         //
-        // with infinite timeout, one can see that UsbSerialPort.read() returns byte[] of length 0
+        // for buffer > packet size, but not multiple of packet size, the same issue happens, but typically
-        // but there might be other reasons for [] return, so unclear if this should be returned as
+        // only the last (partly filled) packet is lost.
-        // error exception.
-        //
-        // for buffer > 64 byte, but not multiple of 64 byte, the same issue happens, but typically
-        // only the last (partly filled) 64 byte fragment is lost.
         if(usb.serialDriver instanceof CdcAcmSerialDriver)
             return; // arduino sends each byte individually, so not testable here
         byte[] data;
@@ -1009,24 +1005,22 @@ public class DeviceTest {
         data = usb.read(4);
         Assert.assertThat(data, equalTo("1aaa".getBytes()));
         telnet.write(new byte[16]);
-        data = usb.read(16);
+        try {
-        if (usb.serialDriver instanceof Cp21xxSerialDriver && usb.serialDriver.getPorts().size() == 1)
+            data = usb.read(16);
-            Assert.assertNotEquals(0, data.length); // can be shorter or full length
+            if (usb.serialDriver instanceof Cp21xxSerialDriver && usb.serialDriver.getPorts().size() == 1)
-        else if (usb.serialDriver instanceof ProlificSerialDriver)
+                Assert.assertNotEquals(0, data.length); // can be shorter or full length
-            Assert.assertTrue("expected > 0 and < 16 byte, got " + data.length, data.length > 0 && data.length < 16);
+            else if (usb.serialDriver instanceof ProlificSerialDriver)
-        else // ftdi, ch340, cp2105
+                Assert.assertTrue("expected > 0 and < 16 byte, got " + data.length, data.length > 0 && data.length < 16);
-            Assert.assertEquals(0, data.length);
+            else // ftdi, ch340, cp2105
-        telnet.write("1ccc".getBytes());
+                fail("buffer to small exception expected");
-        data = usb.read(4);
+        } catch (IOException ignored) {
-        // Assert.assertThat(data, equalTo("1ccc".getBytes())); // unpredictable here. typically '1ccc' but sometimes '' or byte[16]
+        }
-        if(data.length != 4) {
+        if (purge) {
-            if (purge) {
+            usb.serialPort.purgeHwBuffers(true, true);
-                usb.serialPort.purgeHwBuffers(true, true);
+        } else {
-            } else {
+            usb.close();
-                usb.close();
+            usb.open();
-                usb.open();
+            Thread.sleep(100); // try to read remaining data by iomanager to avoid garbage in next test
-                Thread.sleep(500); // try to read missing value by iomanager to avoid garbage in next test
-            }
         }
 
         usb.close();
@@ -1073,7 +1067,7 @@ public class DeviceTest {
             } else {
                 usb.close();
                 usb.open();
-                Thread.sleep(500); // try to read missing value by iomanager to avoid garbage in next test
+                Thread.sleep(100); // try to read remaining data by iomanager to avoid garbage in next test
             }
         }
     }
@@ -1326,6 +1320,7 @@ public class DeviceTest {
 
     @Test
     public void IoManager() throws Exception {
+        SerialInputOutputManager.DEBUG = true;
         usb.ioManager = new SerialInputOutputManager(null);
         assertNull(usb.ioManager.getListener());
         usb.ioManager.setListener(usb);
@@ -1414,6 +1409,8 @@ public class DeviceTest {
         assertThat(usb.read(1), equalTo("c".getBytes()));
         telnet.write("d".getBytes());
         assertThat(usb.read(1), equalTo("d".getBytes()));
+
+        SerialInputOutputManager.DEBUG = false;
     }
 
     @Test
@@ -1484,8 +1481,10 @@ public class DeviceTest {
         time = System.currentTimeMillis();
         closed[0] = false;
         Executors.newSingleThreadExecutor().submit(closeThread);
-        len = usb.serialPort.read(readBuf, 0); // blocking until close()
+        try {
-        assertEquals(0, len);
+            usb.serialPort.read(readBuf, 0); // blocking until close()
+            fail("read error expected");
+        } catch (IOException ignored) {}
         assertTrue(System.currentTimeMillis()-time >= 100);
         // wait for usbClose
         for(i=0; i<100; i++) {

usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/util/UsbWrapper.java

@@ -194,7 +194,7 @@ public class UsbWrapper implements SerialInputOutputManager.Listener {
         if(ioManager != null) {
             while (System.currentTimeMillis() < end) {
                 if(readError != null)
-                    throw readError;
+                    throw new IOException(readError);
                 synchronized (readBuffer) {
                     while(readBuffer.peek() != null)
                         buf.put(readBuffer.remove());

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

@@ -155,7 +155,7 @@ public abstract class CommonUsbSerialPort implements UsbSerialPort {
         byte[] buf = new byte[2];
         int len = mConnection.controlTransfer(0x80 /*DEVICE*/, 0 /*GET_STATUS*/, 0, 0, buf, buf.length, 200);
         if(len < 0)
-            throw new IOException("USB get_status request failed");
+            throw new IOException("Connection lost, USB get_status request failed");
     }
 
     @Override
@@ -183,7 +183,7 @@ public abstract class CommonUsbSerialPort implements UsbSerialPort {
             long endTime = testConnection ? MonotonicClock.millis() + timeout : 0;
             int readMax = Math.min(dest.length, MAX_READ_SIZE);
             nread = mConnection.bulkTransfer(mReadEndpoint, dest, readMax, timeout);
-            // Android error propagation is improvable, nread == -1 can be: timeout, connection lost, buffer undersized, ...
+            // Android error propagation is improvable, nread == -1 can be: timeout, connection lost, buffer to small
             if(nread == -1 && testConnection && MonotonicClock.millis() < endTime)
                 testConnection();
 
@@ -197,11 +197,15 @@ public abstract class CommonUsbSerialPort implements UsbSerialPort {
                 throw new IOException("Waiting for USB request failed");
             }
             nread = buf.position();
+            if(nread == 0) {
+                if(dest.length % mReadEndpoint.getMaxPacketSize() != 0) {
+                    throw new IOException("Connection lost or buffer to small");
+                } else {
+                    throw new IOException("Connection lost");
+                }
+            }
         }
-        if (nread > 0)
+        return Math.max(nread, 0);
-            return nread;
-        else
-            return 0;
     }
 
     @Override

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

@@ -162,7 +162,7 @@ public class FtdiSerialDriver implements UsbSerialDriver {
             return readFilter(dest, nread);
         }
 
-        private int readFilter(byte[] buffer, int totalBytesRead) throws IOException {
+        protected int readFilter(byte[] buffer, int totalBytesRead) throws IOException {
             final int maxPacketSize = mReadEndpoint.getMaxPacketSize();
             int destPos = 0;
             for(int srcPos = 0; srcPos < totalBytesRead; srcPos += maxPacketSize) {

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

@@ -217,7 +217,7 @@ public class ProlificSerialDriver implements UsbSerialDriver {
             IOException readStatusException = mReadStatusException;
             if (mReadStatusException != null) {
                 mReadStatusException = null;
-                throw readStatusException;
+                throw new IOException(readStatusException);
             }
 
             return mStatus;

usbSerialForAndroid/src/test/java/com/hoho/android/usbserial/driver/FtdiSerialDriverTest.java

@@ -0,0 +1,89 @@
+package com.hoho.android.usbserial.driver;
+
+import android.hardware.usb.UsbDevice;
+import android.hardware.usb.UsbEndpoint;
+
+import org.junit.Test;
+
+import java.io.IOException;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class FtdiSerialDriverTest {
+
+    private final UsbDevice usbDevice = mock(UsbDevice.class);
+    private final UsbEndpoint readEndpoint = mock(UsbEndpoint.class);
+
+    private void initBuf(byte[] buf) {
+        for(int i=0; i<buf.length; i++)
+            buf[i] = (byte) i;
+    }
+    private boolean testBuf(byte[] buf, int len) {
+        byte j = 2;
+        for(int i=0; i<len; i++) {
+            if(buf[i]!=j)
+                return false;
+            j++;
+            if(j % 64 == 0)
+                j+=2;
+        }
+        return true;
+    }
+
+    @Test
+    public void readFilter() throws Exception {
+        byte[] buf = new byte[2048];
+        int len;
+
+        when(usbDevice.getInterfaceCount()).thenReturn(1);
+        when(readEndpoint.getMaxPacketSize()).thenReturn(64);
+        FtdiSerialDriver driver = new FtdiSerialDriver(usbDevice);
+        FtdiSerialDriver.FtdiSerialPort port = (FtdiSerialDriver.FtdiSerialPort) driver.getPorts().get(0);
+        port.mReadEndpoint = readEndpoint;
+
+        len = port.readFilter(buf, 0);
+        assertEquals(len, 0);
+
+        assertThrows(IOException.class, () -> {port.readFilter(buf, 1);});
+
+        initBuf(buf);
+        len = port.readFilter(buf, 2);
+        assertEquals(len, 0);
+
+        initBuf(buf);
+        len = port.readFilter(buf, 3);
+        assertEquals(len, 1);
+        assertTrue(testBuf(buf, len));
+
+        initBuf(buf);
+        len = port.readFilter(buf, 4);
+        assertEquals(len, 2);
+        assertTrue(testBuf(buf, len));
+
+        initBuf(buf);
+        len = port.readFilter(buf, 64);
+        assertEquals(len, 62);
+        assertTrue(testBuf(buf, len));
+
+        assertThrows(IOException.class, () -> {port.readFilter(buf, 65);});
+
+        initBuf(buf);
+        len = port.readFilter(buf, 66);
+        assertEquals(len, 62);
+        assertTrue(testBuf(buf, len));
+
+        initBuf(buf);
+        len = port.readFilter(buf, 68);
+        assertEquals(len, 64);
+        assertTrue(testBuf(buf, len));
+
+        initBuf(buf);
+        len = port.readFilter(buf, 16*64+11);
+        assertEquals(len, 16*62+9);
+        assertTrue(testBuf(buf, len));
+    }
+}