From b9b07616d846d08b33b4e5f467bc4626b76db1f0 Mon Sep 17 00:00:00 2001 From: makermelissa-piclaw Date: Wed, 6 May 2026 14:02:55 -0700 Subject: [PATCH] Fix Connect/Disconnect button behavior across all workflows Closes #373. Three related problems were causing the Connect/Disconnect button to get stuck in a wrong state when the underlying device went away: 1. The USB read loop / writer surfaced "NetworkError: The device has been lost." as unhandled promise rejections (usb.js:277, script.js:592 in the issue) when a board was unplugged or its USB grant was revoked. The read-loop catch path also called onDisconnected() with the default reconnect=true, which re-entered connect() on a now-dead device and triggered another NetworkError. - serialTransmit() now wraps the writer awaits in a single try/catch and treats any failure as a disconnect (reconnect=false) instead of letting the rejection escape. - The read loop's catch handler now passes reconnect=false. - onDisconnected() is now idempotent (guards against the gatt-style double-fire from "disconnect" event + read-loop rejection) and downgrades reconnect to false if any of the underlying close calls also throw NetworkError. 2. The Web workflow had its keepalive ping commented out and never reacted to websocket errors, so a silent network drop left the UI stuck on "Disconnect". - websocket.onerror now fires onDisconnected(event, false). - serialTransmit() bails early (and disconnects) if the socket is not in OPEN state. - The PING_INTERVAL_MS watchdog is re-enabled in onConnected(). 3. Clicking Connect after a disconnect always re-prompted for a workflow. - The most recently chosen backend is persisted in localStorage ("webeditor.lastBackend") whenever loadWorkflow() runs. - checkConnected() prefers that backend and goes straight to its showConnect() dialog. Each connect dialog now has a "Choose a different workflow" link that re-opens the workflow chooser, so the user is never trapped in the auto-selected workflow. The new chooser-back link is wired generically via Workflow._wireBack ToChooser(), called from each subclass's showConnect(); the markup lives in index.html with the .connect-back class and minimal styling in sass/layout/_layout.scss. --- index.html | 3 ++ js/script.js | 65 ++++++++++++++++++++++++++++++++++++++-- js/workflows/ble.js | 1 + js/workflows/usb.js | 65 +++++++++++++++++++++++++++++++++------- js/workflows/web.js | 40 ++++++++++++++++++++----- js/workflows/workflow.js | 30 +++++++++++++++++++ sass/layout/_layout.scss | 22 ++++++++++++++ 7 files changed, 205 insertions(+), 21 deletions(-) diff --git a/index.html b/index.html index e937f604..3d243ff3 100644 --- a/index.html +++ b/index.html @@ -224,6 +224,7 @@
+

Choose a different workflow

If you are connecting to a new device, follow the steps below. Otherwise click the Reconnect button.

@@ -268,6 +269,7 @@

Request Bluetooth Device

+

Choose a different workflow

@@ -309,6 +311,7 @@

Navigate to your Device

+

Choose a different workflow

diff --git a/js/script.js b/js/script.js index db7cc75e..016dde36 100644 --- a/js/script.js +++ b/js/script.js @@ -53,6 +53,34 @@ const messageDialog = new MessageModal("message"); const connectionType = new ButtonValueDialog("connection-type"); const settings = new Settings(); +// localStorage key used to remember the most recently chosen backend +// ("web" | "ble" | "usb"). When the user clicks Connect after a +// disconnect, we prefer the last backend over re-prompting for one. +const LAST_BACKEND_KEY = "webeditor.lastBackend"; + +function getLastBackend() { + try { + const name = window.localStorage.getItem(LAST_BACKEND_KEY); + if (name && isValidBackend(name)) { + return getBackendWorkflow(name); + } + } catch (e) { + // localStorage may be unavailable (privacy mode, etc.) — that's fine + } + return null; +} + +function rememberLastBackend(workflowType) { + try { + const name = getWorkflowBackendName(workflowType); + if (name) { + window.localStorage.setItem(LAST_BACKEND_KEY, name); + } + } catch (e) { + // ignore — non-fatal + } +} + const editorTheme = EditorView.theme({}, {dark: getCssVar('editor-theme-dark').trim() === '1'}); document.addEventListener('DOMContentLoaded', function() { @@ -218,9 +246,20 @@ function setSaved(saved) { async function checkConnected() { if (!workflow || !workflow.connectionStatus()) { - let connType = await chooseConnection(); - if (!connType) { - return false; + let connType; + + // Prefer the last backend the user successfully connected with + // (issue #373) so clicking Connect after a disconnect skips the + // chooser. The connect dialog itself has a "back" link that calls + // chooseAndShowConnect() if the user wants to switch workflows. + const lastBackend = getLastBackend(); + if (lastBackend) { + connType = lastBackend; + } else { + connType = await chooseConnection(); + if (!connType) { + return false; + } } await loadWorkflow(connType); @@ -243,6 +282,24 @@ async function checkConnected() { return true; } +// Closes whatever connect dialog is open and re-opens the workflow chooser, +// then loads/connects to whichever workflow the user picks. Used by the +// "back" button inside each connect dialog (issue #373). +async function chooseAndShowConnect() { + if (workflow && workflow.connectDialog && workflow.connectDialog.isOpen()) { + workflow.connectDialog.close(); + } + let connType = await chooseConnection(); + if (!connType) { + return false; + } + await loadWorkflow(connType); + if (!workflow.connectionStatus()) { + await workflow.showConnect(getDocState()); + } + return true; +} + function getDocState() { return workflow.makeDocState(editor.state.doc.sliceString(0), unchanged); } @@ -361,6 +418,7 @@ async function loadWorkflow(workflowType = null) { } } workflow = workflows[workflowType]; + rememberLastBackend(workflowType); // Initialize the workflow await workflow.init({ terminal: state.terminal, @@ -376,6 +434,7 @@ async function loadWorkflow(workflowType = null) { showMessageFunc: showMessage, currentFilename: currentFilename, showSerialFunc: showSerial, + chooseConnectionFunc: chooseAndShowConnect, }); } else { console.log("Reload workflow"); diff --git a/js/workflows/ble.js b/js/workflows/ble.js index c2ad3239..8f40b8fd 100644 --- a/js/workflows/ble.js +++ b/js/workflows/ble.js @@ -51,6 +51,7 @@ class BLEWorkflow extends Workflow { async showConnect(documentState) { let p = this.connectDialog.open(); let modal = this.connectDialog.getModal(); + this._wireBackToChooser(modal); btnRequestBluetoothDevice = modal.querySelector('#requestBluetoothDevice'); btnReconnect = modal.querySelector('#bleReconnect'); diff --git a/js/workflows/usb.js b/js/workflows/usb.js index 26532cfc..e5e2e106 100644 --- a/js/workflows/usb.js +++ b/js/workflows/usb.js @@ -52,11 +52,28 @@ class USBWorkflow extends Workflow { } async onDisconnected(e, reconnect = true) { + // Guard against the disconnect callbacks firing twice (e.g. the + // "disconnect" event AND the read-loop catch both call us when a + // device is unplugged — see issue #373). + if (this._isDisconnecting) { + return; + } + this._isDisconnecting = true; + + // If we got here because the underlying device was lost (NetworkError), + // the writable stream is errored and any further writes will throw + // unhandled NetworkErrors. Force reconnect=false so we don't loop + // back through connect() on a gone device; the user can click Connect + // (which now remembers the last backend — issue #373) to try again. + let safeReconnect = reconnect; + if (this.reader) { try { await this.reader.cancel(); } catch (error) { + // Reader cancel can reject with NetworkError on a lost device console.warn("Error calling reader.cancel:", error); + safeReconnect = false; } this.reader = null; } @@ -65,6 +82,7 @@ class USBWorkflow extends Workflow { await this.writer.releaseLock(); } catch (error) { console.warn("Error calling writer.releaseLock:", error); + safeReconnect = false; } this.writer = null; } @@ -74,24 +92,45 @@ class USBWorkflow extends Workflow { await this._serialDevice.close(); } catch (error) { console.warn("Error calling _serialDevice.close:", error); + safeReconnect = false; } this._serialDevice = null; } - super.onDisconnected(e, reconnect); + try { + await super.onDisconnected(e, safeReconnect); + } finally { + this._isDisconnecting = false; + } } async serialTransmit(msg) { const encoder = new TextEncoder(); - if (this.writer) { - const encMessage = encoder.encode(msg); - await this.writer.ready.catch((err) => { - console.error(`Ready error: ${err}`); - }); - await this.writer.write(encMessage).catch((err) => { - console.error(`Chunk error: ${err}`); - }); + if (!this.writer) { + return; + } + const encMessage = encoder.encode(msg); + + // Any of these awaits can reject with NetworkError if the device was + // unplugged or otherwise lost (#373). We treat any of those as a + // disconnect event and stop touching the writer instead of letting the + // unhandled rejection escape (which used to spam the console at + // script.js:592). + const handleTransmitError = async (err) => { + console.warn("Serial transmit error, treating as disconnect:", err); + // Don't try to reconnect automatically — the device is gone, and + // the user clicking Connect will pick up the last backend. + await this.onDisconnected(null, false); + }; + + try { + await this.writer.ready; + if (!this.writer) return; // disconnected during await + await this.writer.write(encMessage); + if (!this.writer) return; await this.writer.ready; + } catch (err) { + await handleTransmitError(err); } } @@ -166,6 +205,7 @@ class USBWorkflow extends Workflow { async showConnect(documentState) { let p = this.connectDialog.open(); let modal = this.connectDialog.getModal(); + this._wireBackToChooser(modal); btnRequestSerialDevice = modal.querySelector('#requestSerialDevice'); btnSelectHostFolder = modal.querySelector('#selectHostFolder'); btnUseHostFolder = modal.querySelector('#useHostFolder'); @@ -298,9 +338,14 @@ class USBWorkflow extends Workflow { console.log("switch to", this._serialDevice); await this._serialDevice.open({baudRate: 115200}); // Throws if something else is already connected or it isn't found. console.log("Starting Read Loop"); + // Pass reconnect=false: if the read loop bails out, the device is + // most likely gone (NetworkError: device has been lost). Trying to + // immediately connect() again would re-enter this method on a dead + // device and surface another unhandled NetworkError (#373). this._readLoopPromise = this._readSerialLoop().catch( async function(error) { - await this.onDisconnected(); + console.warn("Read loop ended with error:", error); + await this.onDisconnected(null, false); }.bind(this) ); diff --git a/js/workflows/web.js b/js/workflows/web.js index 7e0872ec..b03d38a0 100644 --- a/js/workflows/web.js +++ b/js/workflows/web.js @@ -37,13 +37,21 @@ class WebWorkflow extends Workflow { async serialTransmit(msg) { // Use an open web socket to transmit serial data - if (this.websocket) { - let value = decodeURIComponent(escape(msg)); - try { - this.websocket.send(value); - } catch (e) { - console.log("caught write error", e, e.stack); - } + if (!this.websocket) { + return; + } + // If the socket is closing/closed, don't even try — just clean up. + if (this.websocket.readyState !== WebSocket.OPEN) { + console.warn("Serial transmit on a non-open websocket; treating as disconnect"); + await this.onDisconnected(null, false); + return; + } + let value = decodeURIComponent(escape(msg)); + try { + this.websocket.send(value); + } catch (e) { + console.warn("caught write error, treating as disconnect:", e); + await this.onDisconnected(null, false); } } @@ -61,7 +69,13 @@ class WebWorkflow extends Workflow { async onConnected(e) { this.debugLog("connected"); await super.onConnected(e); - //this.connIntervalId = setInterval(this._checkConnection.bind(this), PING_INTERVAL_MS); + // Periodic ping so we notice silent network drops (issue #373). + // _checkConnection() calls onDisconnected(null, false) when a ping + // times out, which flips the UI back to "Connect". + if (this.connIntervalId) { + clearInterval(this.connIntervalId); + } + this.connIntervalId = setInterval(this._checkConnection.bind(this), PING_INTERVAL_MS); } async onDisconnected(e, reconnect = true) { @@ -85,6 +99,7 @@ class WebWorkflow extends Workflow { async showConnect(documentState) { const p = this.connectDialog.open(); const modal = this.connectDialog.getModal(); + this._wireBackToChooser(modal); const deviceLink = modal.querySelector("#device-link"); deviceLink.addEventListener("click", (event) => { event.preventDefault(); @@ -131,6 +146,15 @@ class WebWorkflow extends Workflow { this.websocket.onopen = this.onConnected.bind(this); this.websocket.onmessage = this.onSerialReceive.bind(this); this.websocket.onclose = this.onDisconnected.bind(this); + // A websocket error (network drop, device reboot) does not + // always fire onclose cleanly. Treat onerror as a hard + // disconnect so the UI flips back to "Connect" (issue #373). + this.websocket.onerror = (event) => { + console.warn("WebSocket error, treating as disconnect:", event); + this.onDisconnected(event, false).catch((err) => { + console.warn("onDisconnected after socket error failed:", err); + }); + }; return true; } catch (e) { //console.log(e, e.stack); diff --git a/js/workflows/workflow.js b/js/workflows/workflow.js index 1b05915f..e04be993 100644 --- a/js/workflows/workflow.js +++ b/js/workflows/workflow.js @@ -54,6 +54,9 @@ class Workflow { this.plotterChart = false; this.buttonStates = []; this.connectButtons = {}; + // Caller-supplied callback used by the "back to workflow chooser" + // button on each connect dialog (issue #373). Set in init(). + this.chooseConnection = null; } async init(params) { @@ -73,6 +76,9 @@ class Workflow { } this.currentFilename = params.currentFilename; this._showSerial = params.showSerialFunc; + if (params.chooseConnectionFunc) { + this.chooseConnection = params.chooseConnectionFunc; + } this.repl.setTitle = this.setTerminalTitle.bind(this); this.repl.writeToTerminal = this.writeToTerminal.bind(this); @@ -221,6 +227,30 @@ except ImportError: return await this.connectDialog.open(); } + // Wires up the "Choose a different workflow" link inside a connect + // dialog. Each subclass calls this from showConnect() after it has + // resolved its modal. The link/button is selected by the + // `.connect-back` class so the markup stays consistent across dialogs. + _wireBackToChooser(modal) { + if (!modal || !this.chooseConnection) { + return; + } + const backLinks = modal.querySelectorAll('.connect-back'); + backLinks.forEach((el) => { + const handler = async (e) => { + e.preventDefault(); + e.stopPropagation(); + await this.chooseConnection(); + }; + // Remove a previously attached handler (idempotent re-open) + if (el._connectBackHandler) { + el.removeEventListener('click', el._connectBackHandler); + } + el._connectBackHandler = handler; + el.addEventListener('click', handler); + }); + } + async runCurrentCode() { let path = this.currentFilename; diff --git a/sass/layout/_layout.scss b/sass/layout/_layout.scss index b9f808ec..6ab229fa 100644 --- a/sass/layout/_layout.scss +++ b/sass/layout/_layout.scss @@ -220,6 +220,28 @@ } } +// "Choose a different workflow" back link inside each connect dialog +// (issue #373). Sits above the existing instructions, indented in from +// the modal close button. +.connect-back-row { + margin: 0 0 1rem 0; + font-size: 0.9rem; + + a.connect-back { + text-decoration: none; + opacity: 0.8; + + &:hover { + opacity: 1; + text-decoration: underline; + } + + i { + margin-right: 0.35rem; + } + } +} + .loader { display: none; z-index: 9998;