Fix duplicate BLE connect/disconnect messages and 3rd-reconnect failure#512
Merged
makermelissa merged 1 commit intoJun 8, 2026
Conversation
…loses circuitpython#410) Several BLE workflow bugs caused log lines to pile up across connect/disconnect cycles, and made subsequent reconnects fail after a few attempts: 1. addEventListener/removeEventListener pairs used inline .bind(this), which creates a new function each call. The remove was a no-op, so listeners accumulated on every cycle. Cache bound handlers on the instance so the remove actually matches. 2. disconnectButtonHandler explicitly called onDisconnected(e, false) AFTER gatt.disconnect(), which itself fires gattserverdisconnected. That meant two trips through onDisconnected per click (two 'disconnected' log lines, two UI updates). Only call onDisconnected manually when GATT was already torn down. 3. onAdvertisementReceived wasn't guarded against re-entry. BLE peripherals can deliver multiple ads in the same event-loop tick before abortController.abort() takes effect on the listener, so the handler ran more than once per device. 4. reconnectButtonHandler iterates all paired devices and starts watchAdvertisements on each. Only one wins; the others kept watching in the background. After a few reconnect cycles, Chrome's per-device watch quota was exhausted and no further advertisementreceived events fired -- which manifested as 'logged Attempting... but never connected' on ~3rd reconnect. Track every in-flight AbortController in a Set so the winner can cancel the others, and clear the Set on disconnect. 5. Add a BLE-specific onDisconnected override that detaches the gattserverdisconnected and characteristicvaluechanged listeners on tear-down, preventing stale device handles from firing onDisconnected later (the root cause of accumulating 'disconnected' lines in earlier sessions).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #410.
Symptoms
In the BLE workflow, repeated connect/disconnect cycles would:
Connected to ...anddisconnectedlines per single user action (1 → 2 → 3 → ... per cycle).Attempting to connect to ...for each paired device but never actually establish a connection, with no error.Reproduces on Feather S3 Reverse TFT (reporter's hardware) and CLUE nRF52840 (my test rig) running CircuitPython 10.x. Chrome latest.
Root causes
Five overlapping bugs in
js/workflows/ble.js:1.
.bind(this)returning a fresh function each callremoveEventListeneronly matches by reference, so the remove was a no-op. EveryswitchToDevice/_rebindAfterSilentReconnect/_loadDomcall added another listener on top of the old ones.Fix: Cache each bound handler on the instance in the constructor and reuse the same reference for add/remove. Pattern already used in
usb.js.2.
disconnectButtonHandlerdouble-firingonDisconnectedTwo trips through
onDisconnectedper disconnect click → two log lines, two UI updates, twoconnect()recursions.Fix: Only call
onDisconnectedmanually when the GATT layer was already torn down (!gatt.connected); otherwise let the GATT event drive cleanup.3.
onAdvertisementReceivednot guarded against re-entryBLE peripherals can deliver several ads in the same event-loop tick before
abortController.abort()actually detaches the listener. The handler would run multiple times per device, callingswitchToDeviceandonConnectedmultiple times.Fix: Closure-local
advHandledboolean; bail on subsequent entries.4. Stale
watchAdvertisementsexhausting Chrome's per-device quotareconnectButtonHandleriterates ALL paired devices and startswatchAdvertisementson each. Only one device responds; the others keep watching indefinitely. After several reconnect cycles, each device has multiple background watches, and Chrome silently stops deliveringadvertisementreceivedevents.This is why the 3rd reconnect failed: the watches were active per Chrome's bookkeeping, but no events were being routed to our handlers.
Fix: Track every in-flight
AbortControllerinthis._pendingAdvAborts(Set). When any device wins, abort all of them (winner + losers). Also clear the Set on disconnect.5. BLE-specific
onDisconnectedto detach lingering listenersEven with fix #1, the device handle itself persists across reconnect cycles, and stale
gattserverdisconnectedlisteners attached to old transitions could fire spuriously.Fix: Add a BLE-specific
onDisconnectedoverride that detachesgattserverdisconnectedfrombleDeviceandcharacteristicvaluechangedfromtxCharacteristicbefore delegating tosuper.onDisconnected.Test protocol
On a CLUE running CircuitPython 10.0.0, with 3 paired BLE devices visible to Chrome:
Before:
After (verified by @makermelissa on Feather S3 Reverse TFT, 4+ cycles):
(The N "Attempting" lines per cycle are expected:
reconnectButtonHandlerchecks every paired device because we can't know in advance which one is on. Only the responsive one produces aConnected.)Out of scope
connect()after disconnect is a no-op whenbleDeviceis still set" path. ThereconnectButtonHandlerflow works because it doesn't depend on that path. A future PR could refactor to clearbleDeviceon disconnect so auto-reconnect viaconnect()also works, but that interacts with the silent-reconnect path from Silently reconnect BLE after autoreload-induced disconnect #511 and deserves its own change.this._resolve(...)calls during reconnect (separate issue noted in Silently reconnect BLE after autoreload-induced disconnect #511 work).cc @makermelissa