Skip to content

Commit 4e40848

Browse files
committed
Trim verbose code comments (refs #377)
Replace block comments that re-explained the what with terse single-line notes focused on the why. PR description retains the full context for reviewers.
1 parent 42c9763 commit 4e40848

2 files changed

Lines changed: 17 additions & 81 deletions

File tree

js/common/ble-file-transfer.js

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,9 @@
11
import {FileTransferClient as BLEFileTransferClient} from '@adafruit/ble-file-transfer-js';
22
//import {FileTransferClient as BLEFileTransferClient} from '../../../ble-file-transfer-js/adafruit-ble-file-transfer.js';
33

4-
// Wrapper for BLEFileTransferClient to add additional functionality.
5-
// Optionally accepts a workflow reference so that mutating ops can notify
6-
// the workflow about the impending firmware autoreload (see
7-
// circuitpython/web-editor#377).
8-
//
9-
// Mutating ops (write/move/delete/mkdir) trigger a CircuitPython VM
10-
// autoreload, which kills the GATT connection. We hold the op's promise
11-
// open until either (a) the connection is restored, or (b) the silent
12-
// reconnect window expires, so callers like FileDialog can chain
13-
// `await fileHelper.move(...); await this._openFolder();` without
14-
// blowing up on a torn-down GATT in the second await.
4+
// Wrapper that holds mutating-op promises open across the firmware
5+
// autoreload + silent reconnect, so callers see a live GATT on return.
6+
// See circuitpython/web-editor#377.
157
class FileTransferClient extends BLEFileTransferClient {
168
constructor(bleDevice, bufferSize, workflow = null) {
179
super(bleDevice, bufferSize);

js/workflows/ble.js

Lines changed: 14 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,12 @@ const BYTES_PER_WRITE = 20;
1717
// Tunables for silent auto-reconnect after firmware autoreload.
1818
// CircuitPython's BLE file transfer triggers an autoreload after every
1919
// mutating op (write/move/delete/mkdir), which tears down the GATT
20-
// connection. We wait briefly, then reconnect to the same already-paired
21-
// device without a user gesture. See circuitpython/web-editor#377.
20+
// Silent reconnect after firmware autoreload. See #377.
2221
const RECONNECT_DELAYS_MS = [1500, 2500, 4000];
2322
const POST_OP_RECONNECT_WINDOW_MS = 8000;
24-
// How long awaitPostOpReconnect() waits for the disconnect to fire after
25-
// a mutating op completes. Observed up to ~2s on CLUE / nRF52840 between
26-
// MOVE_STATUS=OK arriving over BLE and the firmware's VM reset actually
27-
// tearing down the GATT server. If no disconnect happens by then, assume
28-
// the firmware didn't autoreload (e.g. some future CP version) and proceed.
23+
// How long to wait for the post-op disconnect to fire (~2s observed).
2924
const POST_OP_DISCONNECT_GRACE_MS = 4000;
30-
// After GATT reconnects post-autoreload, the firmware VM is still
31-
// booting. boot.py runs, code.py may start, BLE service finishes
32-
// initializing, and the CIRCUITPY filesystem mount/lock settles. If we
33-
// let the next mutating op fire too early, the firmware returns
34-
// STATUS_ERROR_READONLY because filesystem_lock() can't be acquired
35-
// during this window. Empirically ~1.5s is enough on CLUE / nRF52840;
36-
// budget 2s for slower boards.
25+
// Wait after GATT reconnects so the VM finishes booting before the next op.
3726
const POST_RECONNECT_SETTLE_MS = 2000;
3827

3928
let btnRequestBluetoothDevice, btnReconnect;
@@ -56,16 +45,9 @@ class BLEWorkflow extends Workflow {
5645
{reconnect: false, request: true},
5746
{reconnect: true, request: true},
5847
];
59-
// Tracks when we last issued a mutating BLE-FT op. If we lose the
60-
// GATT connection within POST_OP_RECONNECT_WINDOW_MS after one of
61-
// those, treat it as an expected firmware autoreload and silently
62-
// reconnect. See circuitpython/web-editor#377.
48+
// Mutating-op disconnects within this window trigger silent reconnect.
6349
this._lastMutatingOpAt = 0;
6450
this._silentReconnectInFlight = false;
65-
// Resolved by _attemptSilentReconnect; awaited by
66-
// awaitPostOpReconnect() so callers can block on the reconnect
67-
// before issuing follow-up BLE-FT calls (e.g. listDir to refresh
68-
// the file dialog).
6951
this._silentReconnectPromise = null;
7052
}
7153

@@ -81,26 +63,12 @@ class BLEWorkflow extends Workflow {
8163
return (Date.now() - this._lastMutatingOpAt) < POST_OP_RECONNECT_WINDOW_MS;
8264
}
8365

84-
// Called by the FileTransferClient wrapper after a mutating op's
85-
// upstream call resolves. Waits long enough for the disconnect to
86-
// fire (it usually arrives ~10-100ms after MOVE_STATUS), then for
87-
// the silent reconnect to complete, so any follow-up BLE-FT call
88-
// (e.g. listDir for file-dialog refresh) sees a live GATT.
89-
//
90-
// If no disconnect fires within POST_OP_DISCONNECT_GRACE_MS, assume
91-
// the firmware did NOT autoreload and return immediately. This keeps
92-
// the wrapper safe against future firmware that fixes the autoreload
93-
// behavior on its own.
66+
// Awaited by mutating-op wrappers so callers see a live GATT before proceeding.
9467
async awaitPostOpReconnect() {
9568
const startedAt = Date.now();
9669
while (Date.now() - startedAt < POST_OP_DISCONNECT_GRACE_MS) {
97-
// bleDevice.gatt.connected flips false BEFORE the
98-
// gattserverdisconnected event fires on the next tick. Catch
99-
// that earlier so we never let the caller continue on a dead
100-
// GATT while we wait for the event-loop hop.
70+
// gatt.connected flips false before gattserverdisconnected fires.
10171
if (this.bleDevice && this.bleDevice.gatt && !this.bleDevice.gatt.connected) {
102-
// Wait up to grace window for the connect() flow to
103-
// wire up _silentReconnectPromise.
10472
const waitForPromise = Date.now();
10573
while (!this._silentReconnectPromise && Date.now() - waitForPromise < POST_OP_DISCONNECT_GRACE_MS) {
10674
await sleep(25);
@@ -110,7 +78,6 @@ class BLEWorkflow extends Workflow {
11078
if (this._silentReconnectPromise) {
11179
break;
11280
}
113-
// gattserverdisconnected hasn't fired yet. Yield briefly.
11481
await sleep(25);
11582
}
11683
if (this._silentReconnectPromise) {
@@ -334,20 +301,13 @@ class BLEWorkflow extends Workflow {
334301
}
335302

336303
async connect() {
337-
// Note: parentheses fix an operator-precedence bug from the original
338-
// code, where `instanceof` bound tighter than `=`, leaving `result`
339-
// as a boolean rather than the actual super.connect() return value.
340304
const result = await super.connect();
341305
if (result instanceof Error) {
342306
return result;
343307
}
344308

345-
// Unexpected disconnect right after a mutating BLE-FT op: this is
346-
// almost certainly the firmware autoreload. Reconnect silently to
347-
// the same device handle without prompting the user.
309+
// Disconnect right after a mutating op = firmware autoreload. Reconnect silently.
348310
if (this.bleDevice && this._wasMutatingOpRecent()) {
349-
// Expose the in-flight reconnect to awaitPostOpReconnect so
350-
// mutating-op promises chain correctly through the reload.
351311
this._silentReconnectPromise = this._attemptSilentReconnect();
352312
let ok = false;
353313
try {
@@ -358,8 +318,7 @@ class BLEWorkflow extends Workflow {
358318
if (ok) {
359319
return;
360320
}
361-
// Fell through: silent reconnect failed. Fall back to the
362-
// normal reconnect-to-permitted-devices path below.
321+
// Silent reconnect failed; fall through to normal reconnect.
363322
}
364323

365324
// Is this a new connection?
@@ -376,17 +335,9 @@ class BLEWorkflow extends Workflow {
376335
}
377336
}
378337

379-
// Try to reconnect to the same already-paired device, after a brief
380-
// delay to let the firmware finish its autoreload. We skip the
381-
// watchAdvertisements dance because CircuitPython starts advertising
382-
// immediately on boot and we already have a permitted device handle.
383-
//
384-
// Unlike switchToDevice(), this path REUSES the existing
385-
// FileTransferClient / FileHelper so that callers holding bound
386-
// references (e.g. FileDialog grabbed fileHelper.move at open time)
387-
// keep working. The upstream FileTransferClient.checkConnection()
388-
// method re-fetches its characteristics on the next op when its
389-
// internal _transfer is null (which onDisconnected sets it to).
338+
// Reconnect to the same paired device after firmware autoreload.
339+
// Reuses the existing FileTransferClient so FileDialog bindings stay live;
340+
// upstream checkConnection() re-fetches characteristics on next op.
390341
async _attemptSilentReconnect() {
391342
if (this._silentReconnectInFlight) {
392343
return false;
@@ -415,22 +366,15 @@ class BLEWorkflow extends Workflow {
415366
}
416367
}
417368

418-
// Re-establish characteristic references AFTER a silent reconnect,
419-
// without rebuilding fileHelper / FileTransferClient. Caller has
420-
// already confirmed this.bleServer.connected is true.
369+
// Rebind characteristics after silent reconnect without rebuilding fileHelper.
421370
async _rebindAfterSilentReconnect() {
422-
// Re-attach the disconnect listener (some browsers detach it on
423-
// GATT teardown, others don't — idempotent re-add is safe).
371+
// Re-attach disconnect listener (idempotent).
424372
this.bleDevice.removeEventListener('gattserverdisconnected', this.onDisconnected.bind(this));
425373
this.bleDevice.addEventListener('gattserverdisconnected', this.onDisconnected.bind(this));
426374

427-
// Re-fetch the NUS serial characteristics. The BLE file-transfer
428-
// client's characteristics get re-fetched lazily by its own
429-
// checkConnection() on next op, so we don't touch fileHelper.
375+
// NUS serial chars need re-fetch; BLE-FT chars re-fetched lazily by checkConnection().
430376
await this.connectToSerial();
431377

432-
// Mark the workflow as connected so the UI clears any
433-
// "disconnected" flicker without closing the active dialog.
434378
this.updateConnected(CONNSTATE.connected);
435379
}
436380

0 commit comments

Comments
 (0)