ksksue/PhysicaloidLibrary

Asynchronous reading from the Arduino

iumez opened this issue · 16 comments

I'd noticed that while my writes to an arduino seemed to be pretty quick, my reads are not. It seems that I needed to somewhere around 100ms before I consistently have good read results. Since I'm working on an app that needs bidirectional communication with the arduino at around 20Hz (or faster), this is an issue.

Looking at the cdc driver, it seems like you implement reading in a thread which does the usb bulk transfer every 50ms. Is there a reason 50ms was chosen? Would I run into issues if I modified it to run faster? Where is the com.ftdi.j2xx driver referenced in physicaloid? Why have reads by synchronous (though hidden from the user) while writes are truly asynchronous?

Having a 50ms wait time for a read seems at odds with the idea of providing an interface to physical computing boards.

Don't worry about the ftdi driver issue. I found it at ftdi's website then realized that I forgot to look in your libs folder!

I've been having the same problem than iumez with the libray.

My app sends a command to the Arduino One rev. 3 and is waiting for the Arduino's answer. I am tryng to adjust some delay to adjust but it is just trial/error. I've been working with the previous version of the library but even when I've updated to the latest one, in this last version is even worse, I don't receive the status (RX/TX) comm lights blinking. Would you mind to give me some tip?

Hi,

So, I dug into the library and saw that it is written so that reads are synchronous and occur in a thread that runs every 50ms. So you can imagine, depending on the timing, it can take up to two read cycles to get a full message back (100ms). I modified the library so that reads are asynchronous (like the writes). In my testing, this reduced the read latency down to about 3ms which is significantly better.

Hi,

Would you mind to give me more details about how to get the library reads asynchronously?

I modified the read() method in UartCdcAcm.java (at
com/physicaloid/lib/usb/driver/uart/UardCdcAcm.java). I've attached my
version so you can diff against the original source. Here are the key
changes:

private void startRead() {
    if(mReadThreadStop) {
        mReadThreadStop = false;
        //ICU commented out for async read
        //new Thread(mLoop).start();
    }
}

Above, I commented out the creation of the read thread that the library
originally used to do reads in the background (every 50ms).

//ICU for async read
private static byte[] rbuf = new byte[USB_READ_BUFFER_SIZE];

@Override
public int read(byte[] buf, int size) {
//ICU added following code for async read
int len = 0;
try {
        len = mConnection.bulkTransfer(mEndpointIn,
                rbuf, rbuf.length, 2);
    } catch(Exception e) {
        Log.e(TAG, e.toString());
    }

    if (len > 0) {
        mBuffer.add(rbuf, len);
        onRead(len);
    }
//ICU end added code
    return mBuffer.get(buf, size);
}

Above, I modified the read() method to perform a read immediately when
called rather than simply reading from the buffer. Originally, this would
only return what's the in buffer and the buffer would only get filled when
the read thread ran (which was every 50ms). So the fastest you could
theoretically read would be every 50ms. With my changes, you're just
limited to how fast it takes for data to become available over usb.

Below is the code for the read thread (which now doesn't run since I
commented out the call in startRead()):

private Runnable mLoop = new Runnable() {
    @Override
    public void run() {
        int len=0;
        byte[] rbuf = new byte[USB_READ_BUFFER_SIZE];
        for (;;) {// this is the main loop for transferring

            try {
                len = mConnection.bulkTransfer(mEndpointIn,
                        rbuf, rbuf.length, 2);
            } catch(Exception e) {
                Log.e(TAG, e.toString());
            }

            if (len > 0) {
                mBuffer.add(rbuf, len);
                onRead(len);
            }

            if (mReadThreadStop) {
                return;
            }

            try {
                Thread.sleep(50);
            } catch (InterruptedException e) {
            }

        }
    } // end of run()
}; // end of runnable

I'd initially experimented with reducing the sleep interval then decided to
bypass the thread altogether. Note, I did not modify the ftdi or other
drivers (arduino uno uses cdcAcm). However, you can modify them in exactly
the same way if necessary.

-Iheanyi

On Tue, May 6, 2014 at 2:29 PM, elahera notifications@github.com wrote:

Hi,

Would you mind to give me more details about how to get the library reads
asynchronously?


Reply to this email directly or view it on GitHubhttps://github.com//issues/7#issuecomment-42354500
.

I just realized that replying via email probably discards my attachment. If you still need a copy of my modified UartCdcAcm.java, send me an email: iheanyi (dot) umez (dot) eronini (at) gmail (dot) com

How about a pull request?:)

Sure :). I'll be able to get on that tonight. I'm still a git and github infant...

@iumez @cody82 @elahera @Lauszus @ksksue Hi guys I found that a delay is necessary for the read method to work properly. If you eliminate the delay between each poll to the internal usb buffer, you will notice that after a certain amount of read times it will return -1 randomly. At least it happened to me. The root causes of this issue are unknown to me.
I just would like to know if some of you noticed this problem or just happened to me.

Umm, that is the read method working properly. The code as originally
written buffers reads for you, so application code doesn't notice that
sometimes, there is no data available. If you bypass that and remove
all delay, then you have to write your code to handle the case where
no data is available for read to return.

Essentially, by removing code to buffer the read data, you're taking
full responsibility for read timing and handling no data available
cases. I would not suggest this if you don't have control (or know
ahead of time) what the timing of serial data transmissions looks like
on both ends.

-Iheanyi

On 8/23/14, pablichjenkov notifications@github.com wrote:

Hi guys I was working on building a usb library in android too. I found that
a delay is necessary for the read method to work properly. If you eliminate
the delay between each poll to the internal usb buffer, you will notice that
after a certain amount of read times it will return -1 randomly. At least it
happened to me. The root causes of this issue are unknown to me.
I just would like to know if some of you noticed this problem or just
happened to me.


Reply to this email directly or view it on GitHub:
#7 (comment)

@iumez . Thanks for the response, I perfectly understand what you said. The read method return -1 when input buffer is empty. The problem is that it is returning -1 even when there is data in the buffer. I can assure there is data in the buffer because I have a USB annalizer Beagle 480, and it shows data pass from my board to android. However android does not pass it to me. It happens very random, once every 80 reads.

If you are constantly calling read, then it can happen at a period of
time when a bulk transfer has not completed between the Beagle and the
android phone. Data transfer is not instantaneous. It takes time to
happen. The phone is doing more than just running your code and
handling USB. There is no guarantee, even if you were constantly
sending data to the phone, that at any one point in time, data will be
available to be read if you are constantly reading.

On 8/23/14, pablichjenkov notifications@github.com wrote:

@iumez . Thanks for the response, I perfectly understand what you said. The
read method return -1 when input buffer is empty. The problem is that it is
returning -1 even when there is data in the buffer. I can assure there is
data in the buffer because I have a USB annalizer Beagle 480, and it shows
data pass from my board to android. However android does not pass it to me.
It happens very random, once every 80 reads.


Reply to this email directly or view it on GitHub:
#7 (comment)

@iumez Yeah I get the point where I could call the read method while bulktransfer is in process and then receive -1. However the next times when I read the Android internal buffer, I should get the data that was previously transmitted. In my case I don't see the data at all, seems like it were lost by the system.
I was thinking that probably calling bulktransfer(...) without a delay could overhead the system or something like that. The truth is that when I add the delay I don't miss any packet.
Even the Google example MissileLauncher has a delay of 50 ms between each usb request. See my post on stackoverflow: http://stackoverflow.com/questions/25464531/android-usb-host-input-bulktransfer-fails-to-read-randomly-when-data-available . I would like to add that I am communicating over bulktransfer directly with no cdc-acm interface.

Not necessarily. Depending on how fast you're reading, it might be a while
before data is available. According to the stackoverflow question, you're
sending data once a second. So, if you're reading faster than that, you may
have to wait until the next second for data.

On Sat, Aug 23, 2014 at 2:26 PM, pablichjenkov notifications@github.com
wrote:

@iumez https://github.com/iumez Yeah I get the point where I could call
the read method while bulktransfer is in process and then receive -1.
However the next times when I read the Android internal buffer, I should
get the data that was previously transmitted. In my case I don't see the
data at all, seems like it were lost by the system.
I was thinking that probably calling bulktransfer(...) without a delay
could overhead the system or something like that. The truth is that when I
add the delay I don't miss any packet.
Even the Google example MissileLauncher has a delay of 50 ms between each
usb request. See my post on stackoverflow:
http://stackoverflow.com/questions/25464531/android-usb-host-input-bulktransfer-fails-to-read-randomly-when-data-available
. I would like to add that I am communicating over bulktransfer directly
with no cdc-acm interface.


Reply to this email directly or view it on GitHub
#7 (comment)
.

I do, I wait untill the next read call and nothing happens. Seems like android does not keep it for me.
Sent via phone

iumez notifications@github.com wrote:

Not necessarily. Depending on how fast you're reading, it might be a while
before data is available. According to the stackoverflow question, you're
sending data once a second. So, if you're reading faster than that, you may
have to wait until the next second for data.

On Sat, Aug 23, 2014 at 2:26 PM, pablichjenkov notifications@github.com
wrote:

@iumez https://github.com/iumez Yeah I get the point where I could call
the read method while bulktransfer is in process and then receive -1.
However the next times when I read the Android internal buffer, I should
get the data that was previously transmitted. In my case I don't see the
data at all, seems like it were lost by the system.
I was thinking that probably calling bulktransfer(...) without a delay
could overhead the system or something like that. The truth is that when I
add the delay I don't miss any packet.
Even the Google example MissileLauncher has a delay of 50 ms between each
usb request. See my post on stackoverflow:
http://stackoverflow.com/questions/25464531/android-usb-host-input-bulktransfer-fails-to-read-randomly-when-data-available
. I would like to add that I am communicating over bulktransfer directly
with no cdc-acm interface.


Reply to this email directly or view it on GitHub
#7 (comment)
.


Reply to this email directly or view it on GitHub:
#7 (comment)

Can you please push these changes to github? For my purposes I have near continuous 115200 data stream and need high efficiency and low latency. This sounds like a good direction to move in.