From 5f94a47b6356104677869a8044fd7385f5dc9632 Mon Sep 17 00:00:00 2001 From: kai-morich Date: Sun, 18 Apr 2021 19:53:44 +0200 Subject: [PATCH] read w/o timeout now only throws exception on connection lost partly revert f4166f34, as there might be unkown reasons for empty response --- .../hoho/android/usbserial/DeviceTest.java | 55 ++++++++----------- .../android/usbserial/util/UsbWrapper.java | 2 +- .../usbserial/driver/CommonUsbSerialPort.java | 13 ++--- 3 files changed, 31 insertions(+), 39 deletions(-) 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 aed23459..c02c7f2d 100644 --- a/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/DeviceTest.java +++ b/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/DeviceTest.java @@ -54,6 +54,9 @@ import java.util.EnumSet; import java.util.List; import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; import static org.hamcrest.CoreMatchers.anyOf; import static org.hamcrest.CoreMatchers.equalTo; @@ -985,7 +988,7 @@ public void writeFragments() throws Exception { @Test public void readBufferSize() throws Exception { // 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! + // if the buffer is smaller than the received result, it is silently 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. @@ -1012,7 +1015,7 @@ public void readBufferSize() throws Exception { 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"); + Assert.assertEquals(0, data.length); } catch (IOException ignored) { } if (purge) { @@ -1474,22 +1477,8 @@ public void writeAsync() throws Exception { @Test public void readTimeout() throws Exception { - final Boolean[] closed = {Boolean.FALSE}; - - Runnable closeThread = () -> { - try { - Thread.sleep(100); - } catch (InterruptedException e) { - e.printStackTrace(); - } - usb.close(); - closed[0] = true; - }; - - usb.open(EnumSet.of(UsbWrapper.OpenCloseFlags.NO_IOMANAGER_THREAD)); - usb.setParameters(19200, 8, 1, UsbSerialPort.PARITY_NONE); - telnet.setParameters(19200, 8, 1, UsbSerialPort.PARITY_NONE); - + ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1); + ScheduledFuture future; byte[] writeBuf = new byte[]{1}; byte[] readBuf = new byte[1]; if (usb.serialDriver instanceof FtdiSerialDriver) @@ -1497,25 +1486,29 @@ public void readTimeout() throws Exception { int len,i,j; long time; + usb.open(EnumSet.of(UsbWrapper.OpenCloseFlags.NO_IOMANAGER_THREAD)); + usb.setParameters(19200, 8, 1, UsbSerialPort.PARITY_NONE); + telnet.setParameters(19200, 8, 1, UsbSerialPort.PARITY_NONE); + // w/o timeout telnet.write(writeBuf); len = usb.serialPort.read(readBuf, 0); // not blocking because data is available assertEquals(1, len); time = System.currentTimeMillis(); - closed[0] = false; - Executors.newSingleThreadExecutor().submit(closeThread); + future = scheduler.schedule(() -> usb.close(), 100, TimeUnit.MILLISECONDS); 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++) { - if(closed[0]) break; - Thread.sleep(1); + len = usb.serialPort.read(readBuf, 0); // blocking until close() + assertEquals(0, len); + } catch (IOException ignored) { + // typically no exception as read request canceled at the beginning of close() + // and most cases the connection is still valid in testConnection() + } catch (Exception ignored) { + // can fail with NPE if connection is closed between closed check and queueing/waiting for request } - assertTrue("not closed in time", closed[0]); + assertTrue(System.currentTimeMillis()-time >= 100); + future.get(); // wait until close finished + scheduler.shutdown(); // with timeout usb.open(EnumSet.of(UsbWrapper.OpenCloseFlags.NO_IOMANAGER_THREAD)); @@ -2083,8 +2076,8 @@ public void ftdiMethods() throws Exception { b = usb.read(1); long t3 = System.currentTimeMillis(); ftdiSerialPort.setLatencyTimer(lt); - assertEquals("latency 1", 99, Math.max(t2-t1, 99)); // looks strange, but shows actual value - assertEquals("latency 100", 99, Math.min(t3-t2, 99)); + assertTrue("latency 1: expected < 100, got "+ (t2-t1), (t2-t1) < 100); + assertTrue("latency 100: expected > 100, got " + (t3-t2), (t3-t2) > 100); usb.deviceConnection.close(); try { 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 a2a7c153..c62f655f 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 @@ -152,7 +152,7 @@ public void open(EnumSet flags) throws Exception { if(!flags.contains(OpenCloseFlags.NO_IOMANAGER_THREAD)) { ioManager = new SerialInputOutputManager(serialPort, this); if(!flags.contains(OpenCloseFlags.NO_IOMANAGER_START)) - Executors.newSingleThreadExecutor().submit(ioManager); + ioManager.start(); } synchronized (readBuffer) { readBuffer.clear(); 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 354712db..278cc4de 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 @@ -149,7 +149,7 @@ protected void testConnection() throws IOException { 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("Connection lost, USB get_status request failed"); + throw new IOException("USB get_status request failed"); } @Override @@ -177,7 +177,8 @@ protected int read(final byte[] dest, final int timeout, boolean testConnection) 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 to small + // Android error propagation is improvable: + // nread == -1 can be: timeout, connection lost, buffer to small, ??? if(nread == -1 && testConnection && MonotonicClock.millis() < endTime) testConnection(); @@ -191,12 +192,10 @@ protected int read(final byte[] dest, final int timeout, boolean testConnection) throw new IOException("Waiting for USB request failed"); } nread = buf.position(); + // Android error propagation is improvable: + // response != null & nread == 0 can be: connection lost, buffer to small, ??? if(nread == 0) { - if(dest.length % mReadEndpoint.getMaxPacketSize() != 0) { - throw new IOException("Connection lost or buffer to small"); - } else { - throw new IOException("Connection lost"); - } + testConnection(); } } return Math.max(nread, 0);