check read buffer size

pull/330/head
kai-morich 2020-09-06 09:11:35 +02:00
rodzic 6f4cd0313c
commit c53c3ed0ae
4 zmienionych plików z 130 dodań i 20 usunięć

Wyświetl plik

@ -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);

Wyświetl plik

@ -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<OpenCloseFlags> flags) throws Exception {
open(flags, 0);
}
public void open(EnumSet<OpenCloseFlags> 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)

Wyświetl plik

@ -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

Wyświetl plik

@ -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;