diff --git a/.gitignore b/.gitignore index 19af7ef..0bd3eef 100644 --- a/.gitignore +++ b/.gitignore @@ -40,4 +40,4 @@ build/ local.properties *.DS_Store proguard/ - +jacoco.exec diff --git a/build.gradle b/build.gradle index a5a05dc..dc0dd31 100644 --- a/build.gradle +++ b/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' } } diff --git a/usbSerialForAndroid/build.gradle b/usbSerialForAndroid/build.gradle index 5cd351c..d0a334b 100644 --- a/usbSerialForAndroid/build.gradle +++ b/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' } diff --git a/usbSerialForAndroid/coverage.gradle b/usbSerialForAndroid/coverage.gradle index 4e915e8..f149c40 100644 --- a/usbSerialForAndroid/coverage.gradle +++ b/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' , 'createDebugCoverageReport'*/]) { 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' ]) } diff --git a/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/DeviceTest.java b/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/DeviceTest.java index d7b2bd4..9388b86 100644 --- a/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/DeviceTest.java +++ b/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) - // if the Java byte[] is shorter than the received result, it is silently lost! + // 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 with read timeout! // - // with infinite timeout, one can see that UsbSerialPort.read() returns byte[] of length 0 - // but there might be other reasons for [] return, so unclear if this should be returned as - // 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. + // for buffer > packet size, but not multiple of packet size, the same issue happens, but typically + // only the last (partly filled) packet 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); - if (usb.serialDriver instanceof Cp21xxSerialDriver && usb.serialDriver.getPorts().size() == 1) - Assert.assertNotEquals(0, data.length); // can be shorter or full length - else if (usb.serialDriver instanceof ProlificSerialDriver) - Assert.assertTrue("expected > 0 and < 16 byte, got " + data.length, data.length > 0 && data.length < 16); - else // ftdi, ch340, cp2105 - Assert.assertEquals(0, data.length); - telnet.write("1ccc".getBytes()); - data = usb.read(4); - // Assert.assertThat(data, equalTo("1ccc".getBytes())); // unpredictable here. typically '1ccc' but sometimes '' or byte[16] - if(data.length != 4) { - if (purge) { - usb.serialPort.purgeHwBuffers(true, true); - } else { - usb.close(); - usb.open(); - Thread.sleep(500); // try to read missing value by iomanager to avoid garbage in next test - } + try { + data = usb.read(16); + if (usb.serialDriver instanceof Cp21xxSerialDriver && usb.serialDriver.getPorts().size() == 1) + Assert.assertNotEquals(0, data.length); // can be shorter or full length + else if (usb.serialDriver instanceof ProlificSerialDriver) + Assert.assertTrue("expected > 0 and < 16 byte, got " + data.length, data.length > 0 && data.length < 16); + else // ftdi, ch340, cp2105 + fail("buffer to small exception expected"); + } catch (IOException ignored) { + } + if (purge) { + usb.serialPort.purgeHwBuffers(true, true); + } else { + usb.close(); + usb.open(); + Thread.sleep(100); // try to read remaining data 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() - assertEquals(0, len); + try { + 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++) { diff --git a/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/util/UsbWrapper.java b/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/util/UsbWrapper.java index b9aa63d..1a40a83 100644 --- a/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/util/UsbWrapper.java +++ b/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()); diff --git a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/CommonUsbSerialPort.java b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/CommonUsbSerialPort.java index 3dc625d..62ba500 100644 --- a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/CommonUsbSerialPort.java +++ b/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 nread; - else - return 0; + return Math.max(nread, 0); } @Override diff --git a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/FtdiSerialDriver.java b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/FtdiSerialDriver.java index 14f4ec9..239355d 100644 --- a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/FtdiSerialDriver.java +++ b/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) { diff --git a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/ProlificSerialDriver.java b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/ProlificSerialDriver.java index 60aab27..3659612 100644 --- a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/ProlificSerialDriver.java +++ b/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; diff --git a/usbSerialForAndroid/src/test/java/com/hoho/android/usbserial/driver/FtdiSerialDriverTest.java b/usbSerialForAndroid/src/test/java/com/hoho/android/usbserial/driver/FtdiSerialDriverTest.java new file mode 100644 index 0000000..eeda9dc --- /dev/null +++ b/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 {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)); + } +} \ No newline at end of file