deps(deps): bump com.github.mik3y:usb-serial-for-android from 3.7.0 to v3.10.0#636
Conversation
Bumps com.github.mik3y:usb-serial-for-android from 3.7.0 to v3.10.0. --- updated-dependencies: - dependency-name: com.github.mik3y:usb-serial-for-android dependency-version: v3.10.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| // Use api() so KotlinUSBBridge types are accessible to app module | ||
| api("com.github.mik3y:usb-serial-for-android:3.7.0") | ||
| api("com.github.mik3y:usb-serial-for-android:v3.10.0") | ||
|
|
There was a problem hiding this comment.
Bug: The usb-serial-for-android library upgrade requires changing how SerialInputOutputManager is started. The current use of ioExecutor.submit(manager) will silently fail, breaking USB communication.
Severity: CRITICAL
Suggested Fix
In KotlinUSBBridge.kt, replace the deprecated ioExecutor.submit(manager) calls in both the connect() and disableRawMode() methods with the new API call, ioManager.start(), to correctly start the serial I/O manager.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: reticulum/build.gradle.kts#L70
Potential issue: The upgrade of the `usb-serial-for-android` library to v3.10.0
introduces a breaking change from v3.9.0 that is not accounted for. The
`SerialInputOutputManager` is initialized using `ioExecutor.submit(manager)` in the
`connect()` and `disableRawMode()` methods within `KotlinUSBBridge.kt`. This pattern is
deprecated and will cause the I/O manager to fail to start silently at runtime. As a
result, no data will be read from connected USB devices, breaking all RNode USB
communication which is a core feature of the application.
Did we get this right? 👍 / 👎 to inform future reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
SerialInputOutputManager no longer implements Runnable in v3.10.0. Replace ioExecutor.submit(manager) with manager.start() which now manages its own read/write threads internally. Remove the ioExecutor and ioManagerFuture fields that are no longer needed. Rewrite waitForManagerStop to poll manager.state instead of Future.get(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Greptile SummaryThis PR bumps Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant KotlinUSBBridge
participant SerialInputOutputManager
Note over KotlinUSBBridge,SerialInputOutputManager: connect() / disableRawMode()
KotlinUSBBridge->>SerialInputOutputManager: new SerialInputOutputManager(port, listener)
KotlinUSBBridge->>SerialInputOutputManager: manager.start()
Note right of SerialInputOutputManager: Manages own read/write threads internally (v3.10.0+)
SerialInputOutputManager-->>KotlinUSBBridge: onNewData(data) callbacks
Note over KotlinUSBBridge,SerialInputOutputManager: enableRawMode() / disconnect()
KotlinUSBBridge->>SerialInputOutputManager: manager.stop()
loop Poll every 10ms (up to 500ms)
KotlinUSBBridge->>SerialInputOutputManager: check manager.state
SerialInputOutputManager-->>KotlinUSBBridge: state (RUNNING | STOPPING | STOPPED)
end
KotlinUSBBridge->>KotlinUSBBridge: drainPortAfterManagerStop() [raw mode only]
Prompt To Fix All With AIThis is a comment left during a code review.
Path: reticulum/src/main/java/com/lxmf/messenger/reticulum/usb/KotlinUSBBridge.kt
Line: 1075-1085
Comment:
**`InterruptedException` not handled in polling loop**
`Thread.sleep(10)` can throw `InterruptedException`. If the calling thread is interrupted while waiting, the exception propagates uncaught out of `waitForManagerStop` and then out of `enableRawMode`, skipping both `drainPortAfterManagerStop()` and `readBuffer.clear()` — leaving the read buffer dirty and the thread's interrupt flag unrestored. The standard practice is to catch `InterruptedException` inside the loop and re-set the interrupt flag so callers can observe it.
```suggestion
private fun waitForManagerStop(manager: SerialInputOutputManager) {
val deadline = System.currentTimeMillis() + 500
while (manager.state != SerialInputOutputManager.State.STOPPED &&
System.currentTimeMillis() < deadline
) {
try {
Thread.sleep(10)
} catch (e: InterruptedException) {
Thread.currentThread().interrupt()
break
}
}
if (manager.state != SerialInputOutputManager.State.STOPPED) {
Log.w(TAG, "Timeout waiting for SerialInputOutputManager to stop (state=${manager.state})")
}
}
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: 1023371 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| private fun waitForManagerStop(manager: SerialInputOutputManager) { | ||
| val deadline = System.currentTimeMillis() + 500 | ||
| while (manager.state != SerialInputOutputManager.State.STOPPED && | ||
| System.currentTimeMillis() < deadline | ||
| ) { | ||
| Thread.sleep(10) | ||
| } | ||
| if (manager.state != SerialInputOutputManager.State.STOPPED) { | ||
| Log.w(TAG, "Timeout waiting for SerialInputOutputManager to stop (state=${manager.state})") | ||
| } | ||
| } |
There was a problem hiding this comment.
InterruptedException not handled in polling loop
Thread.sleep(10) can throw InterruptedException. If the calling thread is interrupted while waiting, the exception propagates uncaught out of waitForManagerStop and then out of enableRawMode, skipping both drainPortAfterManagerStop() and readBuffer.clear() — leaving the read buffer dirty and the thread's interrupt flag unrestored. The standard practice is to catch InterruptedException inside the loop and re-set the interrupt flag so callers can observe it.
| private fun waitForManagerStop(manager: SerialInputOutputManager) { | |
| val deadline = System.currentTimeMillis() + 500 | |
| while (manager.state != SerialInputOutputManager.State.STOPPED && | |
| System.currentTimeMillis() < deadline | |
| ) { | |
| Thread.sleep(10) | |
| } | |
| if (manager.state != SerialInputOutputManager.State.STOPPED) { | |
| Log.w(TAG, "Timeout waiting for SerialInputOutputManager to stop (state=${manager.state})") | |
| } | |
| } | |
| private fun waitForManagerStop(manager: SerialInputOutputManager) { | |
| val deadline = System.currentTimeMillis() + 500 | |
| while (manager.state != SerialInputOutputManager.State.STOPPED && | |
| System.currentTimeMillis() < deadline | |
| ) { | |
| try { | |
| Thread.sleep(10) | |
| } catch (e: InterruptedException) { | |
| Thread.currentThread().interrupt() | |
| break | |
| } | |
| } | |
| if (manager.state != SerialInputOutputManager.State.STOPPED) { | |
| Log.w(TAG, "Timeout waiting for SerialInputOutputManager to stop (state=${manager.state})") | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: reticulum/src/main/java/com/lxmf/messenger/reticulum/usb/KotlinUSBBridge.kt
Line: 1075-1085
Comment:
**`InterruptedException` not handled in polling loop**
`Thread.sleep(10)` can throw `InterruptedException`. If the calling thread is interrupted while waiting, the exception propagates uncaught out of `waitForManagerStop` and then out of `enableRawMode`, skipping both `drainPortAfterManagerStop()` and `readBuffer.clear()` — leaving the read buffer dirty and the thread's interrupt flag unrestored. The standard practice is to catch `InterruptedException` inside the loop and re-set the interrupt flag so callers can observe it.
```suggestion
private fun waitForManagerStop(manager: SerialInputOutputManager) {
val deadline = System.currentTimeMillis() + 500
while (manager.state != SerialInputOutputManager.State.STOPPED &&
System.currentTimeMillis() < deadline
) {
try {
Thread.sleep(10)
} catch (e: InterruptedException) {
Thread.currentThread().interrupt()
break
}
}
if (manager.state != SerialInputOutputManager.State.STOPPED) {
Log.w(TAG, "Timeout waiting for SerialInputOutputManager to stop (state=${manager.state})")
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
|
A newer version of com.github.mik3y:usb-serial-for-android exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged. |
This is interesting. Where? I just see the same version like before to be the latest one: https://github.com/mik3y/usb-serial-for-android/releases I have read a bit in the repo. It looks like the version currently in columba have known issues with serial data loss. The higher the serial speed, the more data is getting lost. This is one of the reasons why this new version got released to fix this issue. Maybe some of the inconsistencies there was, was exactly because of the serial data loss in the old version currently implemented. |
2 participants
Bumps com.github.mik3y:usb-serial-for-android from 3.7.0 to v3.10.0.
API Changes Required
SerialInputOutputManagerno longer implementsRunnablein v3.10.0. It now manages its own read/write threads internally viastart()/stop(). The following changes were made toKotlinUSBBridge.kt:ioExecutor.submit(manager)withmanager.start()(2 call sites)ioExecutor(SingleThreadExecutor) andioManagerFuturefields — no longer neededwaitForManagerStop()to pollmanager.stateinstead ofFuture.get()Manual USB Testing Required
This is a 3-minor-version jump (3.7.0 → 3.10.0) with a breaking API change in the threading model. Extensive manual testing with physical USB serial hardware is required before merge:
Original Dependabot description: