Skip to content

Revert "fix: ble write handling"#3725

Merged
mdmohsin7 merged 1 commit into
mainfrom
revert-3471-fix/ble-transport
Dec 11, 2025
Merged

Revert "fix: ble write handling"#3725
mdmohsin7 merged 1 commit into
mainfrom
revert-3471-fix/ble-transport

Conversation

@mdmohsin7
Copy link
Copy Markdown
Member

Reverts #3471

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The pull request reverts a previous change related to BLE write handling. The original comment highlighting critical issues with the reverted BLE write functionality, including lack of long write support, writeWithoutResponse optimization, concurrency issues, and retries, has been kept as it does not contradict any existing rules and provides valuable feedback for robust BLE communication. The comment suggests re-introducing a more robust write mechanism.

_writeQueue = task.catchError((_) {});
await task;
try {
await characteristic.write(data);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This revert simplifies the write logic, but it re-introduces several critical issues that the previous implementation was trying to solve:

  • Lack of long write support: The call to characteristic.write(data) doesn't set allowLongWrite: true. This will cause writes larger than the MTU (typically 20 bytes) to fail on Android.
  • No writeWithoutResponse optimization: The previous code would use write without response if the characteristic supported it, which improves throughput. This implementation will always use write with response, which is slower.
  • Concurrency issues: Without a write queue (like the _writeQueue that was removed), concurrent calls to writeCharacteristic can lead to race conditions and GATT_BUSY errors, making the BLE communication unreliable.
  • No retries: The lack of a retry mechanism for transient errors makes the connection more fragile.

While I understand the reverted code may have had issues, simply reverting it makes the BLE write functionality significantly less robust. I'd strongly recommend re-introducing a more robust write mechanism that handles these cases. At a minimum, please consider enabling long writes and using write without response where appropriate.

      await characteristic.write(
        data,
        withoutResponse: characteristic.properties.writeWithoutResponse,
        allowLongWrite: true,
      );

@mdmohsin7 mdmohsin7 merged commit 9e141ec into main Dec 11, 2025
1 check passed
@mdmohsin7 mdmohsin7 deleted the revert-3471-fix/ble-transport branch December 11, 2025 19:29
Glucksberg pushed a commit to Glucksberg/omi-local that referenced this pull request Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant