move setReadQueue to UsbSerialPort interface

kai-morich · kai-morich · commit 1efbcf0a385f · 2025-02-23T17:05:21.000+01:00
diff --git 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 @@ class CountingUsbRequest extends UsbRequest {
             @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<UsbRequest> 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<Integer> 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
@@ -117,24 +117,24 @@ public final void setWriteBufferSize(int bufferSize) {
         }
     }
 
-    /**
-     * 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 void setReadQueue(int bufferCount, int bufferSize) {
         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
@@ -104,6 +104,23 @@ enum FlowControl { NONE, RTS_CTS, DTR_DSR, XON_XOFF, XON_XOFF_INLINE }
      */
     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
@@ -39,7 +39,8 @@ public enum State {
 
     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 interface Listener {
     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 int getWriteBufferSize() {
     }
 
     /**
-     * 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 void writeAsync(byte[] data) {
      * 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
@@ -71,9 +71,8 @@ public void readQueue() throws Exception {
 
         // 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);
 

Original file line number	Diff line number	Diff line change
`@@ -117,24 +117,24 @@ public final void setWriteBufferSize(int bufferSize) {`
`117`	`117`	`}`
`118`	`118`	`}`
`119`	`119`
`120`		`- /**`
`121`		`- * for applications doing permanent read() with timeout=0, multiple buffers can be`
`122`		`- * used to copy next data from Linux kernel, while the current data is processed.`
`123`		`- * @param bufferCount number of buffers to use for readQueue`
`124`		`- * disabled with 0`
`125`		`- * @param bufferSize size of each buffer`
`126`		`- */`
	`120`	`+ @Override`
`127`	`121`	`public void setReadQueue(int bufferCount, int bufferSize) {`
`128`	`122`	`if (bufferCount < 0) {`
`129`	`123`	`throw new IllegalArgumentException("Invalid bufferCount");`
`130`	`124`	`}`
`131`		`- if (bufferCount > 0 && bufferSize <= 0) {`
	`125`	`+ if (bufferSize < 0) {`
`132`	`126`	`throw new IllegalArgumentException("Invalid bufferSize");`
`133`	`127`	`}`
`134`	`128`	`if(isOpen()) {`
`135`	`129`	`if (bufferCount < mReadQueueBufferCount) {`
`136`	`130`	`throw new IllegalStateException("Cannot reduce bufferCount when port is open");`
`137`	`131`	`}`
	`132`	`+ if (bufferSize == 0) {`
	`133`	`+ bufferSize = mReadEndpoint.getMaxPacketSize();`
	`134`	`+ }`
	`135`	`+ if (mReadQueueBufferSize == 0) {`
	`136`	`+ mReadQueueBufferSize = mReadEndpoint.getMaxPacketSize();`
	`137`	`+ }`
`138`	`138`	`if (mReadQueueBufferCount != 0 && bufferSize != mReadQueueBufferSize) {`
`139`	`139`	`throw new IllegalStateException("Cannot change bufferSize when port is open");`
`140`	`140`	`}`
`@@ -156,7 +156,9 @@ public void setReadQueue(int bufferCount, int bufferSize) {`
`156`	`156`	`mReadQueueBufferSize = bufferSize;`
`157`	`157`	`}`
`158`	`158`
	`159`	`+ @Override`
`159`	`160`	`public int getReadQueueBufferCount() { return mReadQueueBufferCount; }`
	`161`	`+ @Override`
`160`	`162`	`public int getReadQueueBufferSize() { return mReadQueueBufferSize; }`
`161`	`163`
`162`	`164`	`private boolean useReadQueue() { return mReadQueueBufferCount != 0; }`