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 88f2f4d..b610dcf 100644 --- a/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/DeviceTest.java +++ b/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/DeviceTest.java @@ -36,6 +36,7 @@ import com.hoho.android.usbserial.util.UsbWrapper; import org.junit.After; import org.junit.AfterClass; +import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Rule; @@ -775,6 +776,102 @@ 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! + // + // 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. + if(usb.serialDriver instanceof CdcAcmSerialDriver) + return; // arduino sends each byte individually, so not testable here + byte[] data; + boolean purge = true; + + usb.open(EnumSet.of(UsbWrapper.OpenCloseFlags.NO_IOMANAGER_START)); + usb.ioManager.setReadBufferSize(8); + usb.startIoManager(); + usb.setParameters(115200, 8, 1, UsbSerialPort.PARITY_NONE); + telnet.setParameters(115200, 8, 1, UsbSerialPort.PARITY_NONE); + try { usb.serialPort.purgeHwBuffers(true, true); } catch(Exception ignored) { purge = false; } + + telnet.write("1aaa".getBytes()); + 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 + } + } + + usb.close(); + usb.open(EnumSet.of(UsbWrapper.OpenCloseFlags.NO_IOMANAGER_THREAD)); + usb.setParameters(115200, 8, 1, UsbSerialPort.PARITY_NONE); + telnet.setParameters(115200, 8, 1, UsbSerialPort.PARITY_NONE); + + try { + usb.serialPort.read(new byte[0], 0); + fail("IllegalArgumentException expected"); + } catch (IllegalArgumentException ignored) {} + try { + usb.serialPort.read(new byte[0], 100); + fail("IllegalArgumentException expected"); + } catch (IllegalArgumentException ignored) {} + if (usb.serialDriver instanceof FtdiSerialDriver) { + try { + usb.serialPort.read(new byte[2], 0); + fail("IllegalArgumentException expected"); + } catch (IllegalArgumentException ignored) {} + try { + usb.serialPort.read(new byte[2], 100); + fail("IllegalArgumentException expected"); + } catch (IllegalArgumentException ignored) {} + } + + telnet.write("2aaa".getBytes()); + data = usb.read(4, 8); + Assert.assertThat(data, equalTo("2aaa".getBytes())); + telnet.write(new byte[16]); + data = usb.read(16, 8); + 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("2ccc".getBytes()); + data = usb.read(4); + // Assert.assertThat(data, equalTo("1ccc".getBytes())); // unpredictable here. typically '2ccc' 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 + } + } + } + @Test // provoke data loss, when data is not read fast enough public void readBufferOverflow() throws Exception { @@ -841,13 +938,13 @@ public class DeviceTest { // Android is not a real time OS, so there is no guarantee that the USB thread is scheduled, or it might be blocked by Java garbage collection. // Using SERIAL_INPUT_OUTPUT_MANAGER_THREAD_PRIORITY=THREAD_PRIORITY_URGENT_AUDIO sometimes reduced errors by factor 10, sometimes not at all! // - int diffLen = readSpeedInt(5, 0); + int diffLen = readSpeedInt(5, -1, 0); if(usb.serialDriver instanceof Ch34xSerialDriver && diffLen == -1) diffLen = 0; // todo: investigate last packet loss assertEquals(0, diffLen); } - private int readSpeedInt(int writeSeconds, int readTimeout) throws Exception { + private int readSpeedInt(int writeSeconds, int readBufferSize, int readTimeout) throws Exception { int baudrate = 115200; if(usb.serialDriver instanceof Ch34xSerialDriver) baudrate = 38400; @@ -855,7 +952,11 @@ public class DeviceTest { if(usb.serialDriver instanceof CdcAcmSerialDriver) writeAhead = 50; - usb.open(EnumSet.noneOf(UsbWrapper.OpenCloseFlags.class), readTimeout); + usb.open(EnumSet.of(UsbWrapper.OpenCloseFlags.NO_IOMANAGER_START)); + usb.ioManager.setReadTimeout(readTimeout); + if(readBufferSize > 0) + usb.ioManager.setReadBufferSize(readBufferSize); + usb.startIoManager(); usb.setParameters(baudrate, 8, 1, UsbSerialPort.PARITY_NONE); telnet.setParameters(baudrate, 8, 1, UsbSerialPort.PARITY_NONE); @@ -1119,7 +1220,9 @@ public class DeviceTest { usb.close(); // with timeout: write after timeout - usb.open(EnumSet.noneOf(UsbWrapper.OpenCloseFlags.class), 100); + usb.open(EnumSet.of(UsbWrapper.OpenCloseFlags.NO_IOMANAGER_START)); + usb.ioManager.setReadTimeout(100); + usb.startIoManager(); usb.setParameters(19200, 8, 1, UsbSerialPort.PARITY_NONE); telnet.setParameters(19200, 8, 1, UsbSerialPort.PARITY_NONE); usb.ioManager.writeAsync(buf); @@ -1206,13 +1309,13 @@ public class DeviceTest { int diffLen; usb.close(); // no issue with high transfer rate and long read timeout - diffLen = readSpeedInt(5, longTimeout); + diffLen = readSpeedInt(5, -1, longTimeout); if(usb.serialDriver instanceof Ch34xSerialDriver && diffLen == -1) diffLen = 0; // todo: investigate last packet loss assertEquals(0, diffLen); usb.close(); // date loss with high transfer rate and short read timeout !!! - diffLen = readSpeedInt(5, shortTimeout); + diffLen = readSpeedInt(5, -1, shortTimeout); assertNotEquals(0, diffLen); 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 a2e1739..6060cb9 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 @@ -37,7 +37,7 @@ public class UsbWrapper implements SerialInputOutputManager.Listener { private final static int USB_WRITE_WAIT = 500; private static final String TAG = UsbWrapper.class.getSimpleName(); - public enum OpenCloseFlags { NO_IOMANAGER_THREAD, NO_CONTROL_LINE_INIT, NO_DEVICE_CONNECTION }; + public enum OpenCloseFlags { NO_IOMANAGER_THREAD, NO_IOMANAGER_START, NO_CONTROL_LINE_INIT, NO_DEVICE_CONNECTION }; // constructor final Context context; @@ -138,14 +138,10 @@ public class UsbWrapper implements SerialInputOutputManager.Listener { } public void open() throws Exception { - open(EnumSet.noneOf(OpenCloseFlags.class), 0); + open(EnumSet.noneOf(OpenCloseFlags.class)); } public void open(EnumSet flags) throws Exception { - open(flags, 0); - } - - public void open(EnumSet flags, int ioManagerTimeout) throws Exception { if(!flags.contains(OpenCloseFlags.NO_DEVICE_CONNECTION)) { UsbManager usbManager = (UsbManager) context.getSystemService(Context.USB_SERVICE); deviceConnection = usbManager.openDevice(serialDriver.getDevice()); @@ -157,9 +153,8 @@ public class UsbWrapper implements SerialInputOutputManager.Listener { } if(!flags.contains(OpenCloseFlags.NO_IOMANAGER_THREAD)) { ioManager = new SerialInputOutputManager(serialPort, this); - ioManager.setReadTimeout(ioManagerTimeout); - ioManager.setWriteTimeout(ioManagerTimeout); - Executors.newSingleThreadExecutor().submit(ioManager); + if(!flags.contains(OpenCloseFlags.NO_IOMANAGER_START)) + Executors.newSingleThreadExecutor().submit(ioManager); } synchronized (readBuffer) { readBuffer.clear(); @@ -167,6 +162,10 @@ public class UsbWrapper implements SerialInputOutputManager.Listener { readError = null; } + public void startIoManager() { + Executors.newSingleThreadExecutor().submit(ioManager); + } + public void waitForIoManagerStarted() throws IOException { for (int i = 0; i < 100; i++) { if (SerialInputOutputManager.State.STOPPED != ioManager.getState()) return; @@ -180,11 +179,10 @@ public class UsbWrapper implements SerialInputOutputManager.Listener { } // wait full time - public byte[] read() throws Exception { - return read(-1); - } + public byte[] read() throws Exception { return read(-1, -1); } + public byte[] read(int expectedLength) throws Exception { return read(expectedLength, -1); } - public byte[] read(int expectedLength) throws Exception { + public byte[] read(int expectedLength, int readBufferSize) throws Exception { long end = System.currentTimeMillis() + USB_READ_WAIT; ByteBuffer buf = ByteBuffer.allocate(16*1024); if(ioManager != null) { @@ -201,7 +199,7 @@ public class UsbWrapper implements SerialInputOutputManager.Listener { } } else { - byte[] b1 = new byte[256]; + byte[] b1 = new byte[readBufferSize > 0 ? readBufferSize : 256]; while (System.currentTimeMillis() < end) { int len = serialPort.read(b1, USB_READ_WAIT); if (len > 0) 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 1248ca1..9134991 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 @@ -134,6 +134,9 @@ public abstract class CommonUsbSerialPort implements UsbSerialPort { if(mConnection == null) { throw new IOException("Connection closed"); } + if(dest.length <= 0) { + throw new IllegalArgumentException("read buffer to small"); + } final int nread; if (timeout != 0) { // bulkTransfer will cause data loss with short timeout + high baud rates + continuous transfer 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 5292ccd..54f6903 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 @@ -137,6 +137,12 @@ public class FtdiSerialDriver implements UsbSerialDriver { @Override public int read(final byte[] dest, final int timeout) throws IOException { + if(dest.length <= READ_HEADER_LENGTH) { + throw new IllegalArgumentException("read buffer to small"); + // could allocate larger buffer, including space for 2 header bytes, but this would + // result in buffers not being 64 byte aligned any more, causing data loss at continuous + // data transfer at high baud rates when buffers are fully filled. + } int nread; if (timeout != 0) { long endTime = System.currentTimeMillis() + timeout;