readBytes for usb_serial_class

Post Reply
victor_pv
Posts: 1750
Joined: Mon Apr 27, 2015 12:12 pm

readBytes for usb_serial_class

Post by victor_pv » Wed Jul 05, 2017 4:14 pm

During the discussion on some enhancements Steve made to the USB library, Jared shared a link to a test Paul had been doing to enhance the serial speed in Teensy.
This is the comment:
https://github.com/rogerclarkmelbourne/ ... -255538645

Basically the Stream class has a function, readBytes, to read a number of bytes to a buffer, which should be more efficient than reading bytes 1 by 1, but actually the Stream class implementation does read the bytes 1 by 1. What Paul did for the teensy was write a readBytes implementation for the serialUSB peripheral that's more efficient, reading as many bytes are in the buffer in a single call, rather than a call and return for each single byte.

In our usb_serial_class we have a read function that can read a number of bytes to a buffer, but it's not the standard Arduino one for that, which is readyBytes. Also readBytes has a timeout, so the code won't block forever waiting for data, only for 1 second, while the read (buf, len) function in our USB driver does not provide any timeout and would block forever.

The result is that at the moment any sketch doing a readBytes from the SerialUSB uses an inefficient implementation.

I propose to add one like this, unless someone can suggest a better faster implementation:

Code: Select all

size_t usb_serial_class::readBytes(char *buf, const size_t& len)
{
    size_t rxed=0;
    unsigned long startMillis;
    startMillis = millis();
    if (len <= 0) return 0;
    do {
        rxed += usb_cdcacm_rx((uint8 *)buf + rxed, len - rxed);
        if (rxed == len) return rxed;
    } while(millis() - startMillis < _timeout);
    return rxed;
}
If the host can send data at a high speed, this will dump that data to the buffer faster. I will run some test later today to confirm it works and try to get speed numbers.

I would also suggest that we do the same for the USARTs since they also have a buffer but not an implementation of readBytes.

EDIT:
Test results with a Windows 10 laptop:
Results with read():

Code: Select all

ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
Bytes per second = 307520
Bytes per second = 307431
Bytes per second = 307509
Bytes per second = 305907
Bytes per second = 307443
Bytes per second = 307473
Bytes per second = 307519
Bytes per second = 307432
Bytes per second = 307509
Bytes per second = 305916
Bytes per second = 307473
Bytes per second = 307465
Bytes per second = 307487
Bytes per second = 307431
Bytes per second = 307520
Average bytes per second = 307269
Results with readBytes:

Code: Select all

port COM3 opened
ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
Bytes per second = 507997
Bytes per second = 512457
Bytes per second = 508238
Bytes per second = 512457
Bytes per second = 508113
Bytes per second = 508121
Bytes per second = 512435
Bytes per second = 512474
Bytes per second = 508001
Bytes per second = 512461
Bytes per second = 512457
Bytes per second = 508117
Bytes per second = 512584
Bytes per second = 507988
Bytes per second = 512466
Average bytes per second = 510424
Last edited by victor_pv on Sat Jul 08, 2017 3:45 pm, edited 1 time in total.

User avatar
RogerClark
Posts: 7489
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: readBytes for usb_serial_class

Post by RogerClark » Wed Jul 05, 2017 10:41 pm

Thanks Victor

victor_pv
Posts: 1750
Joined: Mon Apr 27, 2015 12:12 pm

Re: readBytes for usb_serial_class

Post by victor_pv » Wed Jul 05, 2017 11:15 pm

victor_pv wrote:
Wed Jul 05, 2017 4:14 pm
During the discussion on some enhancements Steve made to the USB library, Jared shared a link to a test Paul had been doing to enhance the serial speed in Teensy.
This is the comment:
https://github.com/rogerclarkmelbourne/ ... -255538645

Basically the Stream class has a function, readBytes, to read a number of bytes to a buffer, which should be more efficient than reading bytes 1 by 1, but actually the Stream class implementation does read the bytes 1 by 1. What Paul did for the teensy was write a readBytes implementation for the serialUSB peripheral that's more efficient, reading as many bytes are in the buffer in a single call, rather than a call and return for each single byte.

In our usb_serial_class we have a read function that can read a number of bytes to a buffer, but it's not the standard Arduino one for that, which is readyBytes. Also readBytes has a timeout, so the code won't block forever waiting for data, only for 1 second, while the read (buf, len) function in our USB driver does not provide any timeout and would block forever.

The result is that at the moment any sketch doing a readBytes from the SerialUSB uses an inefficient implementation.

I propose to add one like this, unless someone can suggest a better faster implementation:
size_t usb_serial_class::readBytes(char *buf, const size_t& len)

Code: Select all

size_t USBSerial::readBytes(char *buf, const size_t& len)
{
    size_t rxed=0;
    unsigned long startMillis;
    startMillis = millis();
    if (len <= 0) return 0;
    do {
        rxed += usb_cdcacm_rx((uint8 *)buf + rxed, len - rxed);
        if (rxed == len) return rxed;
    } while(millis() - startMillis < _timeout);
    return rxed;
}
If the host can send data at a high speed, this will dump that data to the buffer faster. I will run some test later today to confirm it works and try to get speed numbers.

I would also suggest that we do the same for the USARTs since they also have a buffer but not an implementation of readBytes.

EDIT:
Test results with a Windows 10 laptop:
Results with read():

Code: Select all

ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
Bytes per second = 307520
Bytes per second = 307431
Bytes per second = 307509
Bytes per second = 305907
Bytes per second = 307443
Bytes per second = 307473
Bytes per second = 307519
Bytes per second = 307432
Bytes per second = 307509
Bytes per second = 305916
Bytes per second = 307473
Bytes per second = 307465
Bytes per second = 307487
Bytes per second = 307431
Bytes per second = 307520
Average bytes per second = 307269
Results with readBytes:

Code: Select all

port COM3 opened
ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
ignoring startup buffering...
Bytes per second = 507997
Bytes per second = 512457
Bytes per second = 508238
Bytes per second = 512457
Bytes per second = 508113
Bytes per second = 508121
Bytes per second = 512435
Bytes per second = 512474
Bytes per second = 508001
Bytes per second = 512461
Bytes per second = 512457
Bytes per second = 508117
Bytes per second = 512584
Bytes per second = 507988
Bytes per second = 512466
Average bytes per second = 510424

stevestrong
Posts: 1831
Joined: Mon Oct 19, 2015 12:06 am
Location: Munich, Germany

Re: readBytes for usb_serial_class

Post by stevestrong » Wed Jul 05, 2017 11:17 pm

Victor, good job,
Do we need a PR?

victor_pv
Posts: 1750
Joined: Mon Apr 27, 2015 12:12 pm

Re: readBytes for usb_serial_class

Post by victor_pv » Wed Jul 05, 2017 11:38 pm

stevestrong wrote:
Wed Jul 05, 2017 11:17 pm
Victor, good job,
Do we need a PR?
I haven't sent a PR yet, in case someone wants to suggest a more efficient code, or any other change.

victor_pv
Posts: 1750
Joined: Mon Apr 27, 2015 12:12 pm

Re: readBytes for usb_serial_class

Post by victor_pv » Sat Jul 08, 2017 4:10 pm

Since there is no more comments, I have submitted a PR with the above code as tested.
I have not measured CPU time saving, but I believe the top speed is achieved because of reduced CPU usage, so at lower speeds it should reduce CPU usage and leave more time available for the user application.

User avatar
RogerClark
Posts: 7489
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: readBytes for usb_serial_class

Post by RogerClark » Sat Jul 08, 2017 9:40 pm

Thanks, I will try to merge it today

User avatar
RogerClark
Posts: 7489
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: readBytes for usb_serial_class

Post by RogerClark » Sat Jul 15, 2017 10:08 pm

Guys

I am going to try to manually merge @stevestrong's code today, which includes some USB stuff.

I can't merge his PR as it changes loads of unrelated stuff in one hit, some of which has already been fixed in other ways.

After the dust settles we can see what else is left to merge

Post Reply