Skip to content

Fix duplicate BLE connect/disconnect messages and 3rd-reconnect failure #512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
makermelissa merged 1 commit into from
Jun 8, 2026
Merged

Fix duplicate BLE connect/disconnect messages and 3rd-reconnect failure #512

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 73 additions & 16 deletions js/workflows/ble.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,19 @@ class BLEWorkflow extends Workflow {
this._lastMutatingOpAt = 0;
this._silentReconnectInFlight = false;
this._silentReconnectPromise = null;

// Cached bound handlers so add/remove use the same reference.
// Without this, .bind() returns a fresh function every call and
// removeEventListener becomes a no-op, leaking listeners on every
// connect/reconnect cycle. See #410.
this._onRequestBluetoothDeviceClick = this.onRequestBluetoothDeviceButtonClick.bind(this);
this._onReconnectClick = this.reconnectButtonHandler.bind(this);
this._onSerialReceiveBound = this.onSerialReceive.bind(this);
this._onDisconnectedBound = this.onDisconnected.bind(this);

// Track in-flight watchAdvertisements abort controllers so we can
// cancel them when any device wins or when we tear down (#410).
this._pendingAdvAborts = new Set();
}

// Called by the FileTransferClient wrapper right before any mutating
Expand Down Expand Up @@ -93,14 +106,38 @@ class BLEWorkflow extends Workflow {
async disconnectButtonHandler(e) {
await super.disconnectButtonHandler(e);
if (this.connectionStatus()) {
// Disconnect BlueTooth and Reset things
if (this.bleDevice !== undefined && this.bleDevice.gatt.connected) {
// gatt.disconnect() fires gattserverdisconnected which calls
// onDisconnected via our listener — don't call onDisconnected
// again here or we double-log and double-fire UI updates.
// See #410.
this.bleDevice.gatt.disconnect();
} else {
// Already torn down at the GATT layer; run our cleanup directly.
await this.onDisconnected(e, false);
}
await this.onDisconnected(e, false);
}
}

async onDisconnected(e, reconnect = true) {
// Detach the gattserverdisconnected listener so a stale device handle
// can't fire onDisconnected later (the cause of the duplicate log
// accumulation in #410).
if (this.bleDevice) {
this.bleDevice.removeEventListener('gattserverdisconnected', this._onDisconnectedBound);
}
if (this.txCharacteristic) {
this.txCharacteristic.removeEventListener('characteristicvaluechanged', this._onSerialReceiveBound);
}
// Cancel any in-flight watchAdvertisements so a subsequent reconnect
// doesn't pile up Chrome's per-device watch quota (#410).
for (const ctrl of this._pendingAdvAborts) {
ctrl.abort();
}
this._pendingAdvAborts.clear();
await super.onDisconnected(e, reconnect);
}

async showConnect(documentState) {
let p = this.connectDialog.open();
let modal = this.connectDialog.getModal();
Expand All @@ -114,8 +151,10 @@ class BLEWorkflow extends Workflow {
request: btnRequestBluetoothDevice
};

btnRequestBluetoothDevice.addEventListener('click', this.onRequestBluetoothDeviceButtonClick.bind(this));
btnReconnect.addEventListener('click', this.reconnectButtonHandler.bind(this));
btnRequestBluetoothDevice.removeEventListener('click', this._onRequestBluetoothDeviceClick);
btnRequestBluetoothDevice.addEventListener('click', this._onRequestBluetoothDeviceClick);
btnReconnect.removeEventListener('click', this._onReconnectClick);
btnReconnect.addEventListener('click', this._onReconnectClick);

// Check if Web Bluetooth is available
if (!(await this.available() instanceof Error)) {
Expand Down Expand Up @@ -155,9 +194,9 @@ class BLEWorkflow extends Workflow {
this.txCharacteristic = await this.serialService.getCharacteristic(bleNusCharTXUUID);
this.rxCharacteristic = await this.serialService.getCharacteristic(bleNusCharRXUUID);

// Remove any existing event listeners to prevent multiple reads
this.txCharacteristic.removeEventListener('characteristicvaluechanged', this.onSerialReceive.bind(this));
this.txCharacteristic.addEventListener('characteristicvaluechanged', this.onSerialReceive.bind(this));
// Use cached bound handler so removeEventListener actually matches.
this.txCharacteristic.removeEventListener('characteristicvaluechanged', this._onSerialReceiveBound);
this.txCharacteristic.addEventListener('characteristicvaluechanged', this._onSerialReceiveBound);
await this.txCharacteristic.startNotifications();
return true;
} catch (e) {
Expand Down Expand Up @@ -197,11 +236,25 @@ class BLEWorkflow extends Workflow {

async connectToBluetoothDevice(device) {
const abortController = new AbortController();
this._pendingAdvAborts.add(abortController);
let advHandled = false;

async function onAdvertisementReceived(event) {
// Multiple ads can land in the same event-loop tick before
// abortController.abort() takes effect on the listener. Guard
// so we only run the connect flow once per device. See #410.
if (advHandled) {
return;
}
advHandled = true;
console.log('> Received advertisement from "' + device.name + '"...');
// Stop watching advertisements to conserve battery life.
abortController.abort();
// This device won. Abort ALL pending watchAdvertisements
// (including this one) so other paired devices stop scanning
// and don't pile up Chrome's per-device watch quota.
for (const ctrl of this._pendingAdvAborts) {
ctrl.abort();
}
this._pendingAdvAborts.clear();
console.log('Connecting to GATT Server from "' + device.name + '"...');
try {
this.bleServer = await device.gatt.connect();
Expand All @@ -220,8 +273,12 @@ class BLEWorkflow extends Workflow {
}
}

device.removeEventListener('advertisementreceived', onAdvertisementReceived.bind(this));
device.addEventListener('advertisementreceived', onAdvertisementReceived.bind(this));
// Use the abortController signal so we don't need to manage the
// handler reference manually — the listener is auto-removed when
// onAdvertisementReceived calls abortController.abort().
device.addEventListener('advertisementreceived',
onAdvertisementReceived.bind(this),
{signal: abortController.signal});

this.debugLog("Attempting to connect to " + device.name + "...");
try {
Expand Down Expand Up @@ -253,8 +310,8 @@ class BLEWorkflow extends Workflow {

async switchToDevice(device) {
this.bleDevice = device;
this.bleDevice.removeEventListener("gattserverdisconnected", this.onDisconnected.bind(this));
this.bleDevice.addEventListener("gattserverdisconnected", this.onDisconnected.bind(this));
this.bleDevice.removeEventListener("gattserverdisconnected", this._onDisconnectedBound);
this.bleDevice.addEventListener("gattserverdisconnected", this._onDisconnectedBound);
console.log("connected", this.bleServer);

try {
Expand Down Expand Up @@ -368,9 +425,9 @@ class BLEWorkflow extends Workflow {

// Rebind characteristics after silent reconnect without rebuilding fileHelper.
async _rebindAfterSilentReconnect() {
// Re-attach disconnect listener (idempotent).
this.bleDevice.removeEventListener('gattserverdisconnected', this.onDisconnected.bind(this));
this.bleDevice.addEventListener('gattserverdisconnected', this.onDisconnected.bind(this));
// Re-attach disconnect listener (idempotent thanks to cached bound ref).
this.bleDevice.removeEventListener('gattserverdisconnected', this._onDisconnectedBound);
this.bleDevice.addEventListener('gattserverdisconnected', this._onDisconnectedBound);

// NUS serial chars need re-fetch; BLE-FT chars re-fetched lazily by checkConnection().
await this.connectToSerial();
Expand Down