Refactor USB Communication: Implement Multiple Buffers and Remove Read Timeouts #614

dkaukov · 2025-01-16T02:58:12Z

Refactored SerialInputOutputManager:

Implemented multiple read buffers to improve USB reading efficiency and reduce latency.
Removed read timeouts, as they are not necessary for event-driven data reading.

These changes enhance the responsiveness, reliability, and robustness of USB communication.

Now it just shutting down itself, like before.

kai-morich · 2025-01-19T15:00:58Z

I wasn't aware that one can queue multiple requests on the same endpoint. Do you have numbers for the performance improvement?

I ran tests against real devices. These showed that SerialInputOutputManager.stop() does not return as .requestWait() is blocking. Please have a look into CommonUsbSerialPort. There read has some additional error checks if length 0 is returned and close cancels the request. Your logic should probably be moved there, with a list of UsbRequest instead of single one.

On recent Android versions I could observe that buffers are used round-robin. I will check with older versions too.

kai-morich · 2025-01-19T18:34:33Z

crashes on Android 5.0 at
private Supplier mRequestSupplier = UsbRequest::new;
with java.lang.NoClassDefFoundError: com.hoho.android.usbserial.util.SerialInputOutputManager$$ExternalSyntheticLambda0
at com.hoho.android.usbserial.util.SerialInputOutputManager.(SerialInputOutputManager.java:57)
at de.kai_morich.serial_usb_terminal.SerialSocket.connect(SerialSocket.java:49)

More classic approach to inject mocked `UsbRequest`

added some additional error checks if length 0 is returned

dkaukov · 2025-01-20T02:33:10Z

Hi @kai-morich,

Thanks for reviewing the changes! The refactor was driven by my work transferring audio through USB serial in this project, where we faced issues with USB stack inserting zeros, especially with CP2102 "S1LABS" variants. Despite using a 16k buffer and separate thread processing, sporadic zero-filled chunks persisted. Implementing multiple small buffers fixed the issue and reduced latency, which is crucial for audio.

I’ve also addressed your suggestions: removed the Supplier for older Android compatibility and added error checks for length 0 returns.

Regarding the suggestion to move the logic to UsbSerialPort, I believe keeping SerialInputOutputManager is more appropriate. Since SerialInputOutputManager provides async functionality through callbacks, it is more logical to use the Android UsbRequest.queue(...) API there. UsbSerialPort provides blocking operations via .read() and .write(), so using UsbConnection.bulkTransfer(...) there is more appropriate. Mixing these would reduce clarity, so separating them maintains a clear distinction between async and blocking operations.

Make .stop() async

dkaukov · 2025-01-20T07:17:52Z

Hi @kai-morich
I took a careful look into the current implementation and noticed an asymmetry: .start() is blocking while .stop() is asynchronous. It's not immediately clear why this design choice was made, but I adjusted the PR to maintain this logic for consistency.

By aligning the behavior of .stop() with the existing design, the issue of .stop() hanging should now be addressed.

Regarding the zero-length checks, I found them likely redundant from this perspective. However, I decided to retain them in the PR to ensure consistency with the rest of the implementation and to safeguard against any unexpected edge cases.

BTW: I also kept mShutdownLatch in place. Let me know if you’d like to clean that up too!

kai-morich · 2025-01-22T14:32:25Z

please split the PR

multithreading should be easy
buffers has to be moved into UsbSerialPort, as FTDI postprocessing, error checking, ... would be missing. There could be similar method setReadBufferCount() which should be set by applications that do permanent read and should be called by default at SerialInputOutputManager.

If called by SerialInputOutputManager very likely has to be reconfigurable while the connection is open. Adding requests could be done immediately, for removing might be better to not re-queue them at the end of read().

I always prefer if everybody could benefit from optimizations without having to enable explicitly. Would a default re-queue at the end of read() have impact on applications only doing ocasional reads, e.g. then returning oldest instead of newest data?

dkaukov · 2025-01-22T21:42:01Z

Hi @kai-morich

Thanks for the suggestions! I have a concern about pre-filling the request queue in UsbSerialPort. Doing it in .open() could interfere with control requests, and lazily in .read() seems messy.

I was thinking about adding a dedicated async API in UsbSerialPort for SerialInputOutputManager (like .asyncReadBegin(), .asyncRead(), .asyncWrite()), but that seems like mixing concerns without much gain. (something like: https://github.com/dkaukov/usb-serial-for-android/pull/1/files)

What would you recommend for managing the queue without causing issues with control requests or making things too complicated?

dkaukov · 2025-01-22T21:46:06Z

Also, you mentioned FTDI post-processing and error checks—could you clarify which checks you're referring to and why they’re important to include in this flow?

kai-morich · 2025-01-25T07:44:32Z

I have an idea for lightweight integration into CommonUsbSerialPort. Let me try out and send you branch for review

dkaukov · 2025-01-28T22:49:31Z

Hey @kai-morich ,

I’ve pulled in the latest upstream changes to keep things up-to-date. Can you also share how to run the integration tests for this? Would be great to verify everything’s working as expected.

kai-morich · 2025-01-29T06:47:53Z

here you can find instructions to run the integration tests

dkaukov · 2025-01-30T02:22:27Z

@kai-morich
It is not working very-well for me. ;( This is master branch:

The "wrongDriver()" actually crashing usb stack and "dataBits()" crashing rfc2217_server.py. Any advice?

kai-morich · 2025-01-30T07:01:26Z

Doesn't look that bad. A few tests are always flaky. I enhanced the wiki page with missing control line loop-back connection.

What does wrongDriver crash? USB stack of Android or serial device. As some serial devices do not recover from this test I already excluded some combinations.

Where do you run rfc2217 server? I use Linux on Raspi. What is the actual error?

dkaukov · 2025-01-30T09:53:13Z

Hi @kai-morich
Every test after wrongDriver() was reporting "no device found". Can fix only by unplugging (an plugging back) adapter. Had to disable it. I was using Intel linux for rfc2217 server. Will post log output tomorrow. ;)

dkaukov · 2025-01-30T10:30:55Z

INFO:rfc2217.server:set baud rate: 19200
INFO:rfc2217.server:set data size: 8
INFO:rfc2217.server:set stop bits: 1
INFO:rfc2217.server:set parity: N
INFO:rfc2217.server:set baud rate: 9600
INFO:rfc2217.server:set data size: 8
INFO:rfc2217.server:set stop bits: 1
INFO:rfc2217.server:set parity: N
WARNING:rfc2217.server:ignoring Telnet command: b'\xf6'
INFO:rfc2217.server:purge both
WARNING:rfc2217.server:ignoring Telnet command: b'\xf6'
INFO:rfc2217.server:purge both
INFO:rfc2217.server:set baud rate: 19200
INFO:root:Disconnected
Traceback (most recent call last):
  File "/usr/share/doc/python3-serial/examples/./rfc2217_server.py", line 172, in <module>
    r.shortcircuit()
  File "/usr/share/doc/python3-serial/examples/./rfc2217_server.py", line 49, in shortcircuit
    self.writer()
  File "/usr/share/doc/python3-serial/examples/./rfc2217_server.py", line 79, in writer
    self.serial.write(b''.join(self.rfc2217.filter(data)))
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/serial/rfc2217.py", line 1123, in filter
    self._telnet_process_subnegotiation(bytes(self.suboption))
  File "/usr/lib/python3/dist-packages/serial/rfc2217.py", line 1189, in _telnet_process_subnegotiation
    self.serial.bytesize = datasize
    ^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/serial/serialutil.py", line 313, in bytesize
    self._reconfigure_port()
  File "/usr/lib/python3/dist-packages/serial/serialposix.py", line 517, in _reconfigure_port
    termios.tcsetattr(
termios.error: (22, 'Invalid argument')

dkaukov · 2025-01-30T10:34:46Z

Package: python3-serial
Version: 3.5-2
Priority: optional
Section: python
Source: pyserial
Origin: Ubuntu
Maintainer: Ubuntu Developers <[email protected]>
Original-Maintainer: Matthias Klose <[email protected]>

kai-morich · 2025-01-30T10:47:15Z

you might have to change this setting:

usb-serial-for-android/usbSerialForAndroid/build.gradle

Line 17 in 1501745

'rfc2217_server_nonstandard_baudrates': 'true', // true false false

dkaukov · 2025-02-02T01:10:49Z

Hi @kai-morich,

Looks like I've fixed the integration tests to the same level of flakiness as before. Any updates on your idea of lightweight integration?

dkaukov added 2 commits January 16, 2025 13:34

Multi buff read

611be4c

better status check

851a169

dkaukov changed the title ~~Refactor USB Communication: Implement Multiple Buffers, Separate Threads, and Remove Read TimeoutsMulti buff read~~ Refactor USB Communication: Implement Multiple Buffers, Separate Threads, and Remove Read Timeouts Jan 16, 2025

dkaukov added 7 commits January 16, 2025 14:07

cleanup

2bfe23b

docs update

f84de35

Cosmetics

ac461cd

Cosmetics

9c3c7b5

Cosmetics

582fa5a

Better state management

c2fc0eb

Better tolerance of users calling start() on closed port

a93423b

Now it just shutting down itself, like before.

dkaukov added 2 commits January 20, 2025 12:45

PR feedback

8ab3ce1

More classic approach to inject mocked `UsbRequest`

PR feedback

ffbcedf

added some additional error checks if length 0 is returned

PR feedback

e1437d4

Make .stop() async

dkaukov mentioned this pull request Jan 25, 2025

USB Serial driver patch VanceVagell/kv4p-ht#192

Merged

dkaukov added 2 commits January 29, 2025 09:40

Merge branch 'master' into multy-buff-read

537b9f3

cleanup.

412e9bc

dkaukov changed the title ~~Refactor USB Communication: Implement Multiple Buffers, Separate Threads, and Remove Read Timeouts~~ Refactor USB Communication: Implement Multiple Buffers and Remove Read Timeouts Jan 29, 2025

dkaukov added 2 commits January 29, 2025 20:07

Merge branch 'master' into multy-buff-read

6a953b9

cleanup

616c19c

Fixed tests

ebad847

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor USB Communication: Implement Multiple Buffers and Remove Read Timeouts #614

Refactor USB Communication: Implement Multiple Buffers and Remove Read Timeouts #614

dkaukov commented Jan 16, 2025 •

edited

Loading

kai-morich commented Jan 19, 2025

kai-morich commented Jan 19, 2025

dkaukov commented Jan 20, 2025 •

edited

Loading

dkaukov commented Jan 20, 2025 •

edited

Loading

kai-morich commented Jan 22, 2025

dkaukov commented Jan 22, 2025 •

edited

Loading

dkaukov commented Jan 22, 2025

kai-morich commented Jan 25, 2025

dkaukov commented Jan 28, 2025

kai-morich commented Jan 29, 2025

dkaukov commented Jan 30, 2025

kai-morich commented Jan 30, 2025

dkaukov commented Jan 30, 2025

dkaukov commented Jan 30, 2025

dkaukov commented Jan 30, 2025 •

edited

Loading

kai-morich commented Jan 30, 2025

dkaukov commented Feb 2, 2025

Refactor USB Communication: Implement Multiple Buffers and Remove Read Timeouts #614

Are you sure you want to change the base?

Refactor USB Communication: Implement Multiple Buffers and Remove Read Timeouts #614

Conversation

dkaukov commented Jan 16, 2025 • edited Loading

kai-morich commented Jan 19, 2025

kai-morich commented Jan 19, 2025

dkaukov commented Jan 20, 2025 • edited Loading

dkaukov commented Jan 20, 2025 • edited Loading

kai-morich commented Jan 22, 2025

dkaukov commented Jan 22, 2025 • edited Loading

dkaukov commented Jan 22, 2025

kai-morich commented Jan 25, 2025

dkaukov commented Jan 28, 2025

kai-morich commented Jan 29, 2025

dkaukov commented Jan 30, 2025

kai-morich commented Jan 30, 2025

dkaukov commented Jan 30, 2025

dkaukov commented Jan 30, 2025

dkaukov commented Jan 30, 2025 • edited Loading

kai-morich commented Jan 30, 2025

dkaukov commented Feb 2, 2025

dkaukov commented Jan 16, 2025 •

edited

Loading

dkaukov commented Jan 20, 2025 •

edited

Loading

dkaukov commented Jan 20, 2025 •

edited

Loading

dkaukov commented Jan 22, 2025 •

edited

Loading

dkaukov commented Jan 30, 2025 •

edited

Loading