felHR85 / UsbSerial

Usb serial controller for Android
MIT License
1.78k stars 582 forks source link

Data arrives out of order #280

Open shakram02 opened 4 years ago

shakram02 commented 4 years ago

Hello, thank you for making this library available, I'm having problems reading data. This is the sample code I'm using on Arduino

void write_serial_bytes(int count) {
  for (int i = 0; i < count ; i++) {
    Serial.write(random('A', 'Z'));
    if (i != count - 1)
    {
      Serial.write(",") ;
    }
  }
  Serial.write("&");
}

Problem

The data doesn't arrive in the order it's written by. This format sends a character followed by a comma but sometimes I either find 2 consecutive characters or two commas which means that data arrives out of order

I made a python program to verify if the problem is from Arduino but it didn't seem to be the case. Maybe that happens because I do multiple writes in the arduino and not send all data as a string?, also sometimes I receive empty data events (when I used the async api). It seems like a race problem (in the library) but I'm not sure

I tried using both sync and async APIs.

soreau commented 4 years ago

I am having a similar issue. I send 3 bytes with delimiter, always the same, as a test. On android, I receive the data but every once in awhile, data[318] == data[319]. As you can see in the comment, it has to skip the duplicate data when getting 3 characters in a row that are all not the delimiting character. This doesn't happen on linux using python serial.Serial.

sprintf(buf, "%c%c\n", 0x80, 0x90);
Serial.print(buf);

UsbSerialInterface.UsbReadCallback mCallback = (byte[] data) -> {
    int length = data.length;
    byte[] buf = new byte[2];
    boolean split = false;
    int size = 0, i;

    if (length == 0) {
        // This does happen
        return;
    }

    if (last_end_byte != '\n' && data[0] != '\n') {
        buf[size++] = last_end_byte;
    }

    last_end_byte = data[length - 1];

    for (i = 0; i < length; i++) {
        if (data[i] == '\n')
            continue;

        // HACK: Work around bug where we sometimes get duplicate data
        if ((i == 319) && (data[i - 1] == data[i]) && ((data[i - 2] != '\n') || (((i + 1) < length) && (data[i + 1] != '\n'))))
            continue;

        buf[size] = data[i];

        if ((size % 2) == 1)
            // Use buf

        size++;

        if (size > 1)
            size = 0;
    }
};
mtinnes commented 4 years ago

Same issue communicating with Arduino Uno (Possibly a CDC driver). Reads contain out of order and/or duplicated characters.

shakram02 commented 4 years ago

@felHR85 any clue on where a fix can be made? that'll be pretty helpful

mtinnes commented 4 years ago

I tracked it down to the read() method in the SerialInputStream.java. I don't think data should be added to the buffer if syncRead() returns 0;

    @Override
    public int read() {
        int value = checkFromBuffer();
        if (value >= 0)
            return value;

        int ret = device.syncRead(buffer, timeout);
        if (ret == 0) return 0;
        if (ret > 0) {
            bufferSize = ret;
            return buffer[pointer++] & 0xff;
        } else {
            return -1;
        }
    }
shakram02 commented 4 years ago

I sent felHR85 an email, he said that he'll check it out in the weekend

obbardc commented 4 years ago

Hi,

My project has a lot of data flowing over two serial ports, and I think my bug may be related. The serial port has messages send like ":test test test;", using the colons as a start & stop character. What I see received by the library is "::test test test;" in about 50% of all messages.

I've found using the asynchronous functions UsbSerialDevice.read() with a callback works perfectly fine, all of the data seems to be present. This would be fine normally, but the asynchronous functions can only be used with one port at a time.

So opening the serial port using the builder SerialPortBuilder.openSerialPorts() and the synchronous functions the following is true:-

So maybe SerialInputStream is not reading from the correctly? I think I have solved my issue using syncRead.

subvertallchris commented 3 years ago

Any chance this got fixed and the issue wasn't closed?