Improve events

haslinghuis · haslinghuis · commit 1bbf39d10b45 · 2025-10-08T04:11:45.000+02:00
diff --git a/src/js/msp/MSPHelper.js b/src/js/msp/MSPHelper.js
@@ -1779,13 +1779,17 @@ MspHelper.prototype.process_data = function (dataHandler) {
 
             // remove object from array
             dataHandler.callbacks.splice(i, 1);
-            if (!crcError) {
-                // fire callback
-                if (callback) {
+            // Always invoke callback and pass crcError flag. Previously callbacks were skipped
+            // when crcError was true; tests and callers expect the callback to still be notified.
+            if (callback) {
+                try {
                     callback({ command: code, data: data, length: data.byteLength, crcError: crcError });
+                } catch (e) {
+                    console.error(`callback for code ${code} threw:`, e);
                 }
-            } else {
-                console.warn(`code: ${code} - crc failed. No callback`);
+            }
+            if (crcError) {
+                console.warn(`code: ${code} - callback invoked despite crc failure`);
             }
         }
     }
diff --git a/src/js/serial.js b/src/js/serial.js
@@ -191,6 +191,16 @@ class Serial extends EventTarget {
             return this._protocol.connect(path, options, callback);
         }
 
+        // If a connect/open is already in progress on the protocol, avoid starting
+        // another connect call which can lead to "open() already in progress" errors
+        // in browser implementations (WebSerial). Return true to indicate a
+        // connection attempt is already underway and listeners (connect event)
+        // will still fire when it completes.
+        if (this._protocol.openRequested) {
+            console.warn(`${this.logHead} Protocol connection already in progress, skipping duplicate connect`);
+            return true;
+        }
+
         console.log(`${this.logHead} Connecting to port:`, path, "with options:", options);
         return this._protocol.connect(path, options, callback);
     }
diff --git a/src/js/tabs/firmware_flasher.js b/src/js/tabs/firmware_flasher.js
@@ -21,6 +21,9 @@ import { EventBus } from "../../components/eventBus";
 import { ispConnected } from "../utils/connection.js";
 import FC from "../fc";
 
+const PORT_CHANGE_DEBOUNCE_MS = 500;
+const AUTO_DETECT_DELAY_MS = 1000;
+
 const firmware_flasher = {
     targets: null,
     buildApi: new BuildApi(),
@@ -40,6 +43,51 @@ const firmware_flasher = {
     // Properties to preserve firmware state during flashing
     preFlashingMessage: null,
     preFlashingMessageType: null,
+    // Minimal module-scoped handler references for cleanup to use
+    // Module-level handler implementations so tests and early callers can
+    // invoke them before `initialize()` binds the full DOM-aware versions.
+    detectedUsbDevice: function () {
+        // If a reboot-resume or flash-on-connect flow is active, we may need to
+        // resume flashing. Otherwise USB attaches don't auto-detect a board.
+        const isFlashOnConnect = $("input.flash_on_connect").is(":checked") || false;
+        if (STM32.rebootMode || isFlashOnConnect) {
+            STM32.rebootMode = 0;
+            GUI.connect_lock = false;
+            firmware_flasher?.startFlashing();
+        }
+    },
+    detectedSerialDevice: function () {
+        // Serial device connect should trigger auto-detect
+        AutoDetect.verifyBoard();
+    },
+    onPortChange: function (port) {
+        // Debounce rapid port events
+        if (firmware_flasher.portChangeTimer) {
+            clearTimeout(firmware_flasher.portChangeTimer);
+            firmware_flasher.portChangeTimer = null;
+        }
+
+        if (GUI.connect_lock) return;
+
+        if (port && port !== "0" && !STM32?.rebootMode) {
+            firmware_flasher.portChangeTimer = setTimeout(() => {
+                firmware_flasher.portChangeTimer = null;
+                AutoDetect.verifyBoard();
+            }, PORT_CHANGE_DEBOUNCE_MS);
+        } else if (!port || port === "0") {
+            $('select[name="board"]').val("0").trigger("change");
+        }
+    },
+    onDeviceRemoved: function () {
+        // Avoid clearing when removal is expected during flashing/reboot
+        if (GUI.connect_lock || STM32.rebootMode) {
+            return;
+        }
+        $('select[name="board"]').val("0").trigger("change");
+        firmware_flasher?.clearBufferedFirmware();
+    },
+    // Single debounce timer shared across instances
+    portChangeTimer: null,
 };
 
 firmware_flasher.initialize = async function (callback) {
@@ -615,6 +663,10 @@ firmware_flasher.initialize = async function (callback) {
             self.filename = null;
         }
 
+        // Expose the implementation to the module so module-scoped handlers that
+        // may run before initialize() completes can call it safely.
+        firmware_flasher.clearBufferedFirmware = clearBufferedFirmware;
+
         $('select[name="board"]').select2();
         $('select[name="osdProtocols"]').select2();
         $('select[name="radioProtocols"]').select2();
@@ -742,20 +794,91 @@ firmware_flasher.initialize = async function (callback) {
             return output.join("").split("\n");
         }
 
-        function detectedUsbDevice(device) {
+        firmware_flasher.detectedUsbDevice = function (device) {
             const isFlashOnConnect = $("input.flash_on_connect").is(":checked");
 
             console.log(`${self.logHead} Detected USB device:`, device);
             console.log(`${self.logHead} Reboot mode: %s, flash on connect`, STM32.rebootMode, isFlashOnConnect);
 
+            // If another operation is in progress, ignore port events (unless we're resuming from a reboot)
+            if (GUI.connect_lock && !STM32.rebootMode) {
+                console.log(`${self.logHead} Port event ignored due to active operation (connect_lock)`);
+                return;
+            }
+
+            // Proceed if we're resuming a reboot sequence or if flash-on-connect is enabled and no operation is active
             if (STM32.rebootMode || isFlashOnConnect) {
+                const wasReboot = !!STM32.rebootMode;
                 STM32.rebootMode = 0;
-                GUI.connect_lock = false;
+                // Only clear the global connect lock when we are resuming from a reboot
+                // so we don't accidentally interrupt another active operation.
+                if (wasReboot) {
+                    GUI.connect_lock = false;
+                }
                 startFlashing();
             }
-        }
+        };
+
+        firmware_flasher.detectedSerialDevice = function () {
+            AutoDetect.verifyBoard();
+        };
+
+        firmware_flasher.onPortChange = function (port) {
+            // Clear any pending debounce timer so rapid port events don't re-enter the handler.
+            if (firmware_flasher.portChangeTimer) {
+                clearTimeout(firmware_flasher.portChangeTimer);
+                firmware_flasher.portChangeTimer = null;
+            }
+
+            console.log(`${self.logHead} Port changed to:`, port);
+
+            if (GUI.connect_lock) {
+                console.log(`${self.logHead} Port change ignored during active operation (connect_lock set)`);
+                return;
+            }
 
-        EventBus.$on("port-handler:auto-select-usb-device", detectedUsbDevice);
+            // Auto-detect board when port changes and we're on firmware flasher tab
+            if (port && port !== "0" && $("input.flash_on_connect").is(":checked") === false && !STM32.rebootMode) {
+                console.log(`${self.logHead} Auto-detecting board for port change (debounced)`);
+
+                // Debounced verification: re-check connect lock when the timeout fires
+                firmware_flasher.portChangeTimer = setTimeout(() => {
+                    firmware_flasher.portChangeTimer = null;
+                    if (GUI.connect_lock) {
+                        console.log(`${self.logHead} Skipping auto-detect due to active operation at timeout`);
+                        return;
+                    }
+                    AutoDetect.verifyBoard();
+                }, PORT_CHANGE_DEBOUNCE_MS); // Small delay to ensure port is ready
+            } else if (!port || port === "0") {
+                if (!GUI.connect_lock) {
+                    console.log(`${self.logHead} Clearing board selection - no port selected`);
+                    $('select[name="board"]').val("0").trigger("change");
+                } else {
+                    console.log(`${self.logHead} Not clearing board selection because operation in progress`);
+                }
+            }
+        };
+
+        firmware_flasher.onDeviceRemoved = function (devicePath) {
+            console.log(`${self.logHead} Device removed:`, devicePath);
+
+            // If a flashing operation or reboot is in progress, the device may be
+            // removed intentionally (it reboots into DFU). Don't clear selection or
+            // buffered firmware in that case as it would prevent the flashing flow.
+            if (GUI.connect_lock || STM32.rebootMode) {
+                console.log(`${self.logHead} Device removal during active operation/reboot; skipping clear`);
+                return;
+            }
+
+            $('select[name="board"]').val("0").trigger("change");
+            clearBufferedFirmware();
+        };
+
+        EventBus.$on("port-handler:auto-select-usb-device", firmware_flasher.detectedUsbDevice);
+        EventBus.$on("port-handler:auto-select-serial-device", firmware_flasher.detectedSerialDevice);
+        EventBus.$on("ports-input:change", firmware_flasher.onPortChange);
+        EventBus.$on("port-handler:device-removed", firmware_flasher.onDeviceRemoved);
 
         async function saveFirmware() {
             const fileType = self.firmware_type;
@@ -1389,6 +1512,11 @@ firmware_flasher.initialize = async function (callback) {
             }
         }
 
+        // Expose the local startFlashing implementation to module callers/tests so
+        // module-scoped handlers can safely call firmware_flasher.startFlashing()
+        // even if those callers ran before initialize() completed.
+        firmware_flasher.startFlashing = startFlashing;
+
         $("a.flash_firmware").on("click", async function () {
             if (GUI.connect_lock) {
                 return;
@@ -1472,6 +1600,17 @@ firmware_flasher.initialize = async function (callback) {
             $("a.exit_dfu").removeClass("disabled");
         }
 
+        // Auto-detect board if drone is already connected when tab becomes active
+        if (
+            (PortHandler.portAvailable && !$('select[name="board"]').val()) ||
+            $('select[name="board"]').val() === "0"
+        ) {
+            console.log(`${self.logHead} Auto-detecting board for already connected device`);
+            setTimeout(() => {
+                AutoDetect.verifyBoard();
+            }, AUTO_DETECT_DELAY_MS); // Small delay to ensure tab is fully loaded
+        }
+
         GUI.content_ready(callback);
     }
 
@@ -1490,6 +1629,24 @@ firmware_flasher.cleanup = function (callback) {
     $(document).unbind("keypress");
     $(document).off("click", "span.progressLabel a");
 
+    const cleanupHandler = (evt, property) => {
+        const handler = firmware_flasher[property];
+        if (handler) {
+            EventBus.$off(evt, handler);
+        }
+    };
+
+    cleanupHandler("port-handler:auto-select-usb-device", "detectedUsbDevice");
+    cleanupHandler("port-handler:auto-select-serial-device", "detectedSerialDevice");
+    cleanupHandler("ports-input:change", "onPortChange");
+    cleanupHandler("port-handler:device-removed", "onDeviceRemoved");
+
+    // Clear any pending debounce timer so it cannot fire after cleanup
+    if (firmware_flasher.portChangeTimer) {
+        clearTimeout(firmware_flasher.portChangeTimer);
+        firmware_flasher.portChangeTimer = null;
+    }
+
     if (callback) callback();
 };
 
diff --git a/test/js/MSPHelper.test.js b/test/js/MSPHelper.test.js
@@ -1,9 +1,9 @@
 import { beforeEach, describe, expect, it } from "vitest";
-import MspHelper from "../../../src/js/msp/MSPHelper";
-import MSPCodes from "../../../src/js/msp/MSPCodes";
-import "../../../src/js/injected_methods";
-import FC from "../../../src/js/fc";
-import { API_VERSION_1_47 } from "../../../src/js/data_storage";
+import MspHelper from "../../src/js/msp/MSPHelper";
+import MSPCodes from "../../src/js/msp/MSPCodes";
+import "../../src/js/injected_methods";
+import FC from "../../src/js/fc";
+import { API_VERSION_1_47 } from "../../src/js/data_storage";
 
 describe("MspHelper", () => {
     const mspHelper = new MspHelper();
@@ -35,6 +35,61 @@ describe("MspHelper", () => {
 
             expect(callbackCalled).toEqual(true);
         });
+
+        it("passes crcError flag to callbacks when crcError is set", () => {
+            let received = null;
+
+            mspHelper.process_data({
+                code: MSPCodes.MSP_BOARD_INFO,
+                dataView: new DataView(new Uint8Array([]).buffer),
+                crcError: true,
+                callbacks: [
+                    {
+                        callback: (item) => {
+                            received = item;
+                        },
+                        code: MSPCodes.MSP_BOARD_INFO,
+                    },
+                ],
+            });
+
+            expect(received).not.toEqual(null);
+            expect(received.crcError).toEqual(true);
+            expect(received.command).toEqual(MSPCodes.MSP_BOARD_INFO);
+        });
+
+        it("passes full data and length to callbacks when crcError is set", () => {
+            let received = null;
+
+            const payload = new Uint8Array([1, 2, 3, 4]);
+            const dv = new DataView(payload.buffer);
+
+            mspHelper.process_data({
+                code: MSPCodes.MSP_BOARD_INFO,
+                dataView: dv,
+                crcError: true,
+                callbacks: [
+                    {
+                        callback: (item) => {
+                            received = item;
+                        },
+                        code: MSPCodes.MSP_BOARD_INFO,
+                    },
+                ],
+            });
+
+            expect(received).not.toEqual(null);
+            expect(received.length).toEqual(dv.byteLength);
+            expect(received.data instanceof DataView).toEqual(true);
+
+            const receivedBytes = new Uint8Array(
+                received.data.buffer,
+                received.data.byteOffset,
+                received.data.byteLength,
+            );
+            expect(Array.from(receivedBytes)).toEqual(Array.from(payload));
+        });
+
         it("handles MSP_API_VERSION correctly", () => {
             let randomValues = crypto.getRandomValues(new Uint8Array(3));
             const [mspProtocolVersion, apiVersionMajor, apiVersionMinor] = randomValues;
@@ -48,6 +103,7 @@ describe("MspHelper", () => {
             expect(FC.CONFIG.mspProtocolVersion).toEqual(mspProtocolVersion);
             expect(FC.CONFIG.apiVersion).toEqual(`${apiVersionMajor}.${apiVersionMinor}.0`);
         });
+
         it("handles MSP_PIDNAMES correctly", () => {
             let pidNamesCount = 1 + crypto.getRandomValues(new Uint8Array(1))[0];
             let expectedNames = Array.from({ length: pidNamesCount }).map((_) => generateRandomString());
@@ -64,6 +120,7 @@ describe("MspHelper", () => {
 
             expect(FC.PID_NAMES).toEqual(expectedNames);
         });
+
         it("handles MSP_MOTOR correctly", () => {
             let motorCount = crypto.getRandomValues(new Uint8Array(1))[0] % 8;
             let motorBytes = crypto.getRandomValues(new Uint16Array(motorCount));
@@ -77,6 +134,7 @@ describe("MspHelper", () => {
             expect(new Uint16Array(FC.MOTOR_DATA).slice(0, motorCount)).toEqual(motorBytes);
             expect(FC.MOTOR_DATA.slice(motorCount, 8)).toContain(undefined);
         });
+
         it("handles MSP_BOARD_INFO correctly for API version", () => {
             FC.CONFIG.apiVersion = API_VERSION_1_47;
             let infoBuffer = [];
diff --git a/test/js/cli.test.js b/test/js/cli.test.js
diff --git a/test/js/firmware_flasher.test.js b/test/js/firmware_flasher.test.js

Original file line number	Diff line number	Diff line change
`@@ -1779,13 +1779,17 @@ MspHelper.prototype.process_data = function (dataHandler) {`
`1779`	`1779`
`1780`	`1780`	`// remove object from array`
`1781`	`1781`	`dataHandler.callbacks.splice(i, 1);`
`1782`		`- if (!crcError) {`
`1783`		`- // fire callback`
`1784`		`- if (callback) {`
	`1782`	`+ // Always invoke callback and pass crcError flag. Previously callbacks were skipped`
	`1783`	`+ // when crcError was true; tests and callers expect the callback to still be notified.`
	`1784`	`+ if (callback) {`
	`1785`	`+ try {`
`1785`	`1786`	`callback({ command: code, data: data, length: data.byteLength, crcError: crcError });`
	`1787`	`+ } catch (e) {`
	`1788`	+ console.error(`callback for code ${code} threw:`, e);
`1786`	`1789`	`}`
`1787`		`- } else {`
`1788`		- console.warn(`code: ${code} - crc failed. No callback`);
	`1790`	`+ }`
	`1791`	`+ if (crcError) {`
	`1792`	+ console.warn(`code: ${code} - callback invoked despite crc failure`);
`1789`	`1793`	`}`
`1790`	`1794`	`}`
`1791`	`1795`	`}`