From 1efbcf0a385ff1ccd0230e1795e55fba588a0a36 Mon Sep 17 00:00:00 2001 From: Kai Morich Date: Sun, 23 Feb 2025 16:12:46 +0100 Subject: [PATCH] move setReadQueue to UsbSerialPort interface --- .../hoho/android/usbserial/DeviceTest.java | 48 ++++++++++--------- .../usbserial/driver/CommonUsbSerialPort.java | 18 +++---- .../usbserial/driver/UsbSerialPort.java | 17 +++++++ .../util/SerialInputOutputManager.java | 22 ++++----- .../driver/CommonUsbSerialPortTest.java | 3 +- 5 files changed, 62 insertions(+), 46 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 c99a991..0e0a6c4 100644 --- a/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/DeviceTest.java +++ b/usbSerialForAndroid/src/androidTest/java/com/hoho/android/usbserial/DeviceTest.java @@ -1207,52 +1207,54 @@ public class DeviceTest { @Override public Object getClientData() { count += 1; return super.getClientData(); } } - usb.open(EnumSet.of(UsbWrapper.OpenCloseFlags.NO_IOMANAGER_THREAD)); - int len = usb.serialPort.getReadEndpoint().getMaxPacketSize(); - usb.close(); CommonUsbSerialPortWrapper.setReadQueueRequestSupplier(usb.serialPort, CountingUsbRequest::new); - CommonUsbSerialPort port = (CommonUsbSerialPort) usb.serialPort; - - port.setReadQueue(2, len); + usb.serialPort.setReadQueue(2, 0); + assertEquals(0, usb.serialPort.getReadQueueBufferSize()); usb.open(EnumSet.of(UsbWrapper.OpenCloseFlags.NO_IOMANAGER_START)); - usb.setParameters(115200, 8, 1, UsbSerialPort.PARITY_NONE); - telnet.setParameters(115200, 8, 1, UsbSerialPort.PARITY_NONE); - assertEquals(2, port.getReadQueueBufferCount()); + int len = usb.serialPort.getReadEndpoint().getMaxPacketSize(); + assertEquals(len, usb.serialPort.getReadQueueBufferSize()); + assertEquals(2, usb.serialPort.getReadQueueBufferCount()); assertEquals(4, usb.ioManager.getReadQueueBufferCount()); // not set at port yet assertThrows(IllegalStateException.class, () -> usb.ioManager.setReadQueue(1)); // cannot reduce bufferCount usb.ioManager.setReadQueue(2); usb.ioManager.start(); - port.setReadQueue(4, len); + usb.serialPort.setReadQueue(3, 0); + usb.serialPort.setReadQueue(3, len); + usb.ioManager.setReadQueue(4); + usb.setParameters(115200, 8, 1, UsbSerialPort.PARITY_NONE); + telnet.setParameters(115200, 8, 1, UsbSerialPort.PARITY_NONE); // linux kernel does round-robin LinkedList requests = CommonUsbSerialPortWrapper.getReadQueueRequests(usb.serialPort); assertNotNull(requests); - for (int i=0; i<16; i++) { + for (int i=0; i<4*4; i++) { telnet.write(new byte[1]); usb.read(1); } - List requestCounts; - if(usb.serialDriver instanceof FtdiSerialDriver) { - for (UsbRequest request : requests) { - int count = ((CountingUsbRequest)request).count; + for (UsbRequest request : requests) { + int count = ((CountingUsbRequest)request).count; + if(usb.serialDriver instanceof FtdiSerialDriver) { assertTrue(String.valueOf(count), count >= 4); + } else { + assertEquals(String.valueOf(count), 4, count); } - } else { - requestCounts = requests.stream().map(r -> ((CountingUsbRequest)r).count).collect(Collectors.toList()); - assertThat(requestCounts, equalTo(Arrays.asList(4, 4, 4, 4))); } usb.ioManager.setReadQueue(6); - for (int i=0; i<18; i++) { + for (int i=0; i<3*6; i++) { telnet.write(new byte[1]); usb.read(1); } - requestCounts = requests.stream().map(r -> ((CountingUsbRequest)r).count).collect(Collectors.toList()); - if(!(usb.serialDriver instanceof FtdiSerialDriver)) { - assertThat(requestCounts, equalTo(Arrays.asList(7, 7, 7, 7, 3, 3))); + for (UsbRequest request : requests) { + int count = ((CountingUsbRequest)request).count; + if(usb.serialDriver instanceof FtdiSerialDriver) { + assertTrue(String.valueOf(count), count >= 3); + } else { + assertTrue(String.valueOf(count), count == 7 || count == 3); + } } usb.close(); usb.open(EnumSet.of(UsbWrapper.OpenCloseFlags.NO_IOMANAGER_START)); - port.setReadQueue(8, len); + usb.serialPort.setReadQueue(8, len); assertThrows(IllegalStateException.class, () -> usb.serialPort.read(new byte[len], 1) ); // cannot use timeout != 0 assertThrows(IllegalStateException.class, () -> usb.serialPort.read(new byte[4], 0) ); // cannot use different length assertThrows(IllegalStateException.class, () -> usb.ioManager.start()); // cannot reduce bufferCount 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 b35624f..9e761a4 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 @@ -117,24 +117,24 @@ public abstract class CommonUsbSerialPort implements UsbSerialPort { } } - /** - * for applications doing permanent read() with timeout=0, multiple buffers can be - * used to copy next data from Linux kernel, while the current data is processed. - * @param bufferCount number of buffers to use for readQueue - * disabled with 0 - * @param bufferSize size of each buffer - */ + @Override public void setReadQueue(int bufferCount, int bufferSize) { if (bufferCount < 0) { throw new IllegalArgumentException("Invalid bufferCount"); } - if (bufferCount > 0 && bufferSize <= 0) { + if (bufferSize < 0) { throw new IllegalArgumentException("Invalid bufferSize"); } if(isOpen()) { if (bufferCount < mReadQueueBufferCount) { throw new IllegalStateException("Cannot reduce bufferCount when port is open"); } + if (bufferSize == 0) { + bufferSize = mReadEndpoint.getMaxPacketSize(); + } + if (mReadQueueBufferSize == 0) { + mReadQueueBufferSize = mReadEndpoint.getMaxPacketSize(); + } if (mReadQueueBufferCount != 0 && bufferSize != mReadQueueBufferSize) { throw new IllegalStateException("Cannot change bufferSize when port is open"); } @@ -156,7 +156,9 @@ public abstract class CommonUsbSerialPort implements UsbSerialPort { mReadQueueBufferSize = bufferSize; } + @Override public int getReadQueueBufferCount() { return mReadQueueBufferCount; } + @Override public int getReadQueueBufferSize() { return mReadQueueBufferSize; } private boolean useReadQueue() { return mReadQueueBufferCount != 0; } diff --git a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/UsbSerialPort.java b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/UsbSerialPort.java index ebf96df..567e45a 100644 --- a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/UsbSerialPort.java +++ b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/driver/UsbSerialPort.java @@ -104,6 +104,23 @@ public interface UsbSerialPort extends Closeable { */ String getSerial(); + /** + * Applications doing permanent {@link #read} with timeout=0 can reduce data loss likelihood + * at high baud rate and continuous data transfer by using multiple buffers to copy next data + * from Linux kernel, while the current data is processed. + * When enabled, {@link #read} can not be called with timeout!=0 or different buffer size. + * + * @param bufferCount number of buffers to use for readQueue. + * Use 0 to disable. + * @param bufferSize size of each buffer. + * Use 0 for optimal size (= getReadEndpoint().getMaxPacketSize()). + * @throws IllegalStateException if port is open and buffer count should be lowered or + * buffer size should be changed. + */ + void setReadQueue(int bufferCount, int bufferSize); + int getReadQueueBufferCount(); + int getReadQueueBufferSize(); + /** * Opens and initializes the port. Upon success, caller must ensure that * {@link #close()} is eventually called. diff --git a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/util/SerialInputOutputManager.java b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/util/SerialInputOutputManager.java index e5909e6..f228453 100644 --- a/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/util/SerialInputOutputManager.java +++ b/usbSerialForAndroid/src/main/java/com/hoho/android/usbserial/util/SerialInputOutputManager.java @@ -39,7 +39,8 @@ public class SerialInputOutputManager { private int mReadTimeout = 0; private int mWriteTimeout = 0; - private int mReadQueueBufferCount; // = READ_QUEUE_BUFFER_COUNT + private int mReadQueueBufferCount = READ_QUEUE_BUFFER_COUNT; + // no mReadQueueBufferSize, using mReadBuffer.size instead private final Object mReadBufferLock = new Object(); private final Object mWriteBufferLock = new Object(); @@ -68,8 +69,6 @@ public class SerialInputOutputManager { public SerialInputOutputManager(UsbSerialPort serialPort) { mSerialPort = serialPort; mReadBuffer = ByteBuffer.allocate(serialPort.getReadEndpoint().getMaxPacketSize()); - mReadQueueBufferCount = serialPort instanceof CommonUsbSerialPort ? READ_QUEUE_BUFFER_COUNT : 0; - //readQueueBufferSize fixed to getMaxPacketSize() } public SerialInputOutputManager(UsbSerialPort serialPort, Listener listener) { @@ -150,17 +149,16 @@ public class SerialInputOutputManager { } /** - * Set read queue. Set buffer size before. - * @param bufferCount number of buffers to use for readQueue + * Set read queue, similar to {@link UsbSerialPort#setReadQueue} + * except buffer size to be set before with {@link #setReadBufferSize}. + * + * @param bufferCount number of buffers to use for readQueue, * disable with value 0, * default enabled as value 4 (READ_QUEUE_BUFFER_COUNT) */ public void setReadQueue(int bufferCount) { - if (!(mSerialPort instanceof CommonUsbSerialPort)) { - throw new IllegalArgumentException("only for CommonUsbSerialPort based drivers"); - } - mReadQueueBufferCount = bufferCount; - ((CommonUsbSerialPort) mSerialPort).setReadQueue(getReadQueueBufferCount(), getReadBufferSize()); + mSerialPort.setReadQueue(bufferCount, getReadBufferSize()); + mReadQueueBufferCount = bufferCount; // only store if set ok } public int getReadQueueBufferCount() { return mReadQueueBufferCount; } @@ -179,9 +177,7 @@ public class SerialInputOutputManager { * start SerialInputOutputManager in separate threads */ public void start() { - if(mSerialPort instanceof CommonUsbSerialPort) { - ((CommonUsbSerialPort) mSerialPort).setReadQueue(mReadQueueBufferCount, getReadBufferSize()); - } + mSerialPort.setReadQueue(mReadQueueBufferCount, getReadBufferSize()); if(mState.compareAndSet(State.STOPPED, State.STARTING)) { mStartuplatch = new CountDownLatch(2); new Thread(this::runRead, this.getClass().getSimpleName() + "_read").start(); diff --git a/usbSerialForAndroid/src/test/java/com/hoho/android/usbserial/driver/CommonUsbSerialPortTest.java b/usbSerialForAndroid/src/test/java/com/hoho/android/usbserial/driver/CommonUsbSerialPortTest.java index 267449d..cf20c1d 100644 --- a/usbSerialForAndroid/src/test/java/com/hoho/android/usbserial/driver/CommonUsbSerialPortTest.java +++ b/usbSerialForAndroid/src/test/java/com/hoho/android/usbserial/driver/CommonUsbSerialPortTest.java @@ -71,9 +71,8 @@ public class CommonUsbSerialPortTest { // set before open port.setReadQueue(0, 0); - port.setReadQueue(0, -1); assertThrows(IllegalArgumentException.class, () -> port.setReadQueue(-1, 1)); - assertThrows(IllegalArgumentException.class, () -> port.setReadQueue(1, 0)); + assertThrows(IllegalArgumentException.class, () -> port.setReadQueue(1, -1)); port.setReadQueue(2, 256); assertNull(port.mReadQueueRequests);