Improve events

haslinghuis · haslinghuis · commit b4f81dad1e5d · 2025-10-07T23:15:17.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} - crc failed`);
             }
         }
     }
diff --git a/src/js/tabs/firmware_flasher.js b/src/js/tabs/firmware_flasher.js
@@ -40,6 +40,94 @@ const firmware_flasher = {
     // Properties to preserve firmware state during flashing
     preFlashingMessage: null,
     preFlashingMessageType: null,
+    // Minimal module-scoped handler references for cleanup to use
+    detectedUsbDevice: null,
+    detectedSerialDevice: null,
+    onPortChange: null,
+    onDeviceRemoved: null,
+    // Single debounce timer shared across instances
+    _portChangeTimer: null,
+};
+
+// Move port/device handlers to module scope for easier maintenance and testing.
+// These handlers reference the `firmware_flasher` object and shared imports (STM32, AutoDetect, GUI).
+const PORT_CHANGE_DEBOUNCE_MS = 500;
+
+function handleDetectedDevice(device, isSerial) {
+    const isFlashOnConnect = $("input.flash_on_connect").is(":checked");
+
+    console.log(
+        `${firmware_flasher.logHead || "[FIRMWARE_FLASHER]"} Detected ${isSerial ? "serial" : "USB"} device:`,
+        device,
+    );
+    console.log(
+        `${firmware_flasher.logHead || "[FIRMWARE_FLASHER]"} Reboot mode: %s, flash on connect`,
+        STM32.rebootMode,
+        isFlashOnConnect,
+    );
+
+    if (STM32.rebootMode || isFlashOnConnect) {
+        STM32.rebootMode = 0;
+        GUI.connect_lock = false;
+        firmware_flasher.startFlashing();
+    } else {
+        if (isSerial) {
+            // Auto-detect board when firmware flasher tab is active and no flash-on-connect
+            console.log(`${firmware_flasher.logHead} Auto-detecting board for connected serial device`);
+        }
+        AutoDetect.verifyBoard();
+    }
+}
+
+firmware_flasher.detectedUsbDevice = function (device) {
+    handleDetectedDevice(device, false);
+};
+
+firmware_flasher.detectedSerialDevice = function (device) {
+    handleDetectedDevice(device, true);
+};
+
+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(`${firmware_flasher.logHead || "[FIRMWARE_FLASHER]"} Port changed to:`, port);
+
+    if (GUI.connect_lock) {
+        console.log(`${firmware_flasher.logHead} Port change ignored during active operation (connect_lock set)`);
+        return;
+    }
+
+    // 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(`${firmware_flasher.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(`${firmware_flasher.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(`${firmware_flasher.logHead} Clearing board selection - no port selected`);
+            $('select[name="board"]').val("0").trigger("change");
+        } else {
+            console.log(`${firmware_flasher.logHead} Not clearing board selection because operation in progress`);
+        }
+    }
+};
+
+firmware_flasher.onDeviceRemoved = function (devicePath) {
+    console.log(`${firmware_flasher.logHead || "[FIRMWARE_FLASHER]"} Device removed:`, devicePath);
+    $('select[name="board"]').val("0").trigger("change");
+    firmware_flasher.clearBufferedFirmware();
 };
 
 firmware_flasher.initialize = async function (callback) {
@@ -742,20 +830,16 @@ firmware_flasher.initialize = async function (callback) {
             return output.join("").split("\n");
         }
 
-        function detectedUsbDevice(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);
+        // Wire module-scoped handlers (defined above) into the instance once DOM is ready.
+        // Expose instance functions used by the module handlers.
+        firmware_flasher.clearBufferedFirmware = clearBufferedFirmware;
 
-            if (STM32.rebootMode || isFlashOnConnect) {
-                STM32.rebootMode = 0;
-                GUI.connect_lock = false;
-                startFlashing();
-            }
-        }
+        // startFlashing implementation will be registered after its declaration below.
 
-        EventBus.$on("port-handler:auto-select-usb-device", detectedUsbDevice);
+        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 +1473,11 @@ firmware_flasher.initialize = async function (callback) {
             }
         }
 
+        // Register the implementation so the module-level entrypoint can call it.
+        // Assign the concrete implementation directly (bound to this instance)
+        // so callers can invoke `firmware_flasher.startFlashing()` without a trampoline.
+        firmware_flasher.startFlashing = startFlashing.bind(self);
+
         $("a.flash_firmware").on("click", async function () {
             if (GUI.connect_lock) {
                 return;
@@ -1472,6 +1561,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();
+            }, 1000); // Small delay to ensure tab is fully loaded
+        }
+
         GUI.content_ready(callback);
     }
 
@@ -1490,6 +1590,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,60 @@ 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;
@@ -123,13 +177,9 @@ describe("MspHelper", () => {
 });
 
 /**
- * Appends given string to an array. If required, it will append length of the string by length.
- * @param destination array to which we append given string (and length if required)
- * @param source string to append to an array
- * @param prefixWithLength should we prefix the string by its length in the array
- * @returns {*} string that was requested to be inserted to the array
+ * Test helpers reused by MSP-related tests
  */
-function appendStringToArray(destination, source, prefixWithLength = false) {
+export function appendStringToArray(destination, source, prefixWithLength = false) {
     const size = source.length;
 
     if (prefixWithLength) {
@@ -143,12 +193,7 @@ function appendStringToArray(destination, source, prefixWithLength = false) {
     return source;
 }
 
-/**
- * Generates a random string of required length. If required length is -1, it will generate a random string of a random length.
- * @param length required random string length. If lower than 0, it will generate a string of random length.
- * @returns {string} random string (composed of letters [A-Za-z0-9])
- */
-function generateRandomString(length = -1) {
+export function generateRandomString(length = -1) {
     let result = "";
     const characters = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
     const charactersLength = characters.length;
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
@@ -0,0 +1,87 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
+import { firmware_flasher } from "../../src/js/tabs/firmware_flasher.js";
+import { EventBus } from "../../src/components/eventBus.js";
+
+// We'll use fake timers to test debounce and spy EventBus.$off calls
+
+describe("firmware_flasher port change debounce and cleanup", () => {
+    beforeEach(() => {
+        vi.useFakeTimers();
+        // ensure no handlers are set
+        firmware_flasher._portChangeTimer = null;
+        // keep module-scoped handlers intact so tests can call them directly
+        // firmware_flasher.detectedUsbDevice etc. are module-scoped and should not be nulled here
+    });
+
+    afterEach(() => {
+        vi.useRealTimers();
+        // restore any spies
+        vi.restoreAllMocks();
+    });
+
+    it("debounces rapid port changes and invokes AutoDetect.verifyBoard once", async () => {
+        // stub AutoDetect.verifyBoard
+        const autoDetect = await import("../../src/js/utils/AutoDetect.js");
+        const spy = vi.spyOn(autoDetect.default || autoDetect, "verifyBoard").mockImplementation(() => {});
+
+        // assign the module-level onPortChange to a local function so we can call it
+        // (the module already exported firmware_flasher with handlers)
+        firmware_flasher.onPortChange = firmware_flasher.onPortChange || (() => {});
+
+        // simulate three rapid port change events
+        firmware_flasher.onPortChange("COM3");
+        firmware_flasher.onPortChange("COM4");
+        firmware_flasher.onPortChange("COM5");
+
+        // advance timers by less than debounce (should not call verifyBoard yet)
+        vi.advanceTimersByTime(200);
+        expect(spy).not.toHaveBeenCalled();
+
+        // advance past debounce
+        vi.advanceTimersByTime(400);
+        expect(spy).toHaveBeenCalledTimes(1);
+
+        spy.mockRestore();
+    });
+
+    it("simulates USB attach/detach without physical cable by calling handlers", async () => {
+        const autoDetect = await import("../../src/js/utils/AutoDetect.js");
+        const spy = vi.spyOn(autoDetect.default || autoDetect, "verifyBoard").mockImplementation(() => {});
+
+        // Ensure logHead is present to avoid console warnings
+        firmware_flasher.logHead = firmware_flasher.logHead || "[FIRMWARE_FLASHER_TEST]";
+
+        // Call the module-scoped detection handler directly (simulates driver reporting a device)
+        if (typeof firmware_flasher.detectedUsbDevice === "function") {
+            firmware_flasher.detectedUsbDevice({ path: "usb:1" });
+        } else {
+            // fallback: emit the event the port handler would emit
+            EventBus.$emit("port-handler:auto-select-usb-device", { path: "usb:1" });
+        }
+
+        expect(spy).toHaveBeenCalled();
+        spy.mockRestore();
+    });
+
+    it("cleanup clears timer and unregisters EventBus handlers", () => {
+        const offSpy = vi.spyOn(EventBus, "$off");
+
+        // set a fake timer id and fake handlers
+        firmware_flasher._portChangeTimer = setTimeout(() => {}, 10000);
+        firmware_flasher.detectedUsbDevice = () => {};
+        firmware_flasher.detectedSerialDevice = () => {};
+        firmware_flasher.onPortChange = () => {};
+        firmware_flasher.onDeviceRemoved = () => {};
+
+        // call cleanup
+        firmware_flasher.cleanup();
+
+        // timer should be cleared
+        expect(firmware_flasher._portChangeTimer).toBeNull();
+
+        // EventBus.$off should be called for each handler
+        expect(offSpy).toHaveBeenCalled();
+
+        offSpy.mockRestore();
+    });
+});

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} - crc failed`);
`1789`	`1793`	`}`
`1790`	`1794`	`}`
`1791`	`1795`	`}`