Improve msp send (#4510)

* Improve msp send

* Fixes per review coderabbit

* Refactor

* Mitigate failed retry attempts

* More coderabbit improvements

* Improve timeout optimization

* Add dynamic retries

* Improve duplicate message detection by also checking payload

* Remove dynamic timeout overhead

* Fix sonar

* Add queue elements for debugging

* Centralize timer cleanup

* Add MSP debugging tools

* Suggestions per Coderabbit

* Sonar

* Add lazy init

* More coderabbit

* More coderabbit ...

* More coderabbit ......

* Make available

* Increase queue limit

* coderabbit again

* Review per coderabbit

* More

* More ...

* Prevent XSS attack

* Fix performance violations

* Prevent runtime errors

* More from coderabbit

* Use subpath for debug tools

* Update src/js/msp/debug/msp_queue_monitor.js

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* fix test

* jumbo was removed in 2defc90

* More coderabbit

* Remove unused callbackonerror

* Remove unused JUMBO size define

* Also check for payload when checking existing msp messages

* Remove static timeout

* Compare payload

* Fix per review coderabbit

* Clear timeout not interval

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

Author: haslinghuis

src/js/main.js

@@ -18,6 +18,16 @@ import { updateTabList } from "./utils/updateTabList.js";
 import * as THREE from "three";
 import NotificationManager from "./utils/notifications.js";
 
+import("./msp/debug/msp_debug_tools.js")
+    .then(() => {
+        console.log("🔧 MSP Debug Tools loaded for development environment");
+        console.log("• Press Ctrl+Shift+M to toggle debug dashboard");
+        console.log("• Use MSPTestRunner.help() for all commands");
+    })
+    .catch((err) => {
+        console.warn("Failed to load MSP debug tools:", err);
+    });
+
 if (typeof String.prototype.replaceAll === "undefined") {
     String.prototype.replaceAll = function (match, replace) {
         return this.replace(new RegExp(match, "g"), () => replace);
@@ -111,7 +121,8 @@ function startProcess() {
     console.log(`Libraries: jQuery - ${$.fn.jquery}, three.js - ${THREE.REVISION}`);
 
     // Check if this is the first visit
-    if (getConfig("firstRun").firstRun === undefined) {
+    const firstRunCfg = getConfig("firstRun") ?? {};
+    if (firstRunCfg.firstRun === undefined) {
         setConfig({ firstRun: true });
         import("./tabs/static_tab.js").then(({ staticTab }) => {
             staticTab.initialize("options", () => {

src/js/msp.js

@@ -51,23 +51,18 @@ const MSP = {
     message_buffer: null,
     message_buffer_uint8_view: null,
     message_checksum: 0,
-    messageIsJumboFrame: false,
     crcError: false,
 
     callbacks: [],
     packet_error: 0,
     unsupported: 0,
 
-    MIN_TIMEOUT: 200,
-    MAX_TIMEOUT: 2000,
-    timeout: 200,
+    TIMEOUT: 1000,
 
     last_received_timestamp: null,
     listeners: [],
 
-    JUMBO_FRAME_SIZE_LIMIT: 255,
-
-    cli_buffer: [], // buffer for CLI charactor output
+    cli_buffer: [], // buffer for CLI character output
     cli_output: [],
     cli_callback: null,
 
@@ -280,7 +275,6 @@ const MSP = {
         // Reset variables
         this.message_length_received = 0;
         this.state = 0;
-        this.messageIsJumboFrame = false;
         this.crcError = false;
     },
     notify() {
@@ -289,7 +283,7 @@ const MSP = {
         });
     },
     listen(listener) {
-        if (this.listeners.indexOf(listener) == -1) {
+        if (this.listeners.indexOf(listener) === -1) {
             this.listeners.push(listener);
         }
     },
@@ -318,8 +312,8 @@ const MSP = {
         const dataLength = data ? data.length : 0;
         // always reserve 6 bytes for protocol overhead !
         const bufferSize = dataLength + 6;
-        let bufferOut = new ArrayBuffer(bufferSize);
-        let bufView = new Uint8Array(bufferOut);
+        const bufferOut = new ArrayBuffer(bufferSize);
+        const bufView = new Uint8Array(bufferOut);
 
         bufView[0] = 36; // $
         bufView[1] = 77; // M
@@ -377,70 +371,64 @@ const MSP = {
 
         serial.send(bufferOut);
     },
-    send_message(code, data, callback_sent, callback_msp, doCallbackOnError) {
-        const connected = serial.connected;
-
-        if (code === undefined || !connected || CONFIGURATOR.virtualMode) {
+    send_message(code, data, callback_sent, callback_msp) {
+        if (code === undefined || !serial.connected || CONFIGURATOR.virtualMode) {
             if (callback_msp) {
                 callback_msp();
             }
             return false;
         }
 
-        let requestExists = false;
-        for (const instance of this.callbacks) {
-            if (instance.code === code) {
-                requestExists = true;
-
-                break;
-            }
-        }
-
         const bufferOut = code <= 254 ? this.encode_message_v1(code, data) : this.encode_message_v2(code, data);
+        const view = new Uint8Array(bufferOut);
+        const keyCrc = this.crc8_dvb_s2_data(view, 0, view.length);
+        const requestExists = this.callbacks.some(
+            (i) =>
+                i.code === code &&
+                i.requestBuffer?.byteLength === bufferOut.byteLength &&
+                this.crc8_dvb_s2_data(new Uint8Array(i.requestBuffer), 0, i.requestBuffer.byteLength) === keyCrc,
+        );
 
         const obj = {
-            code: code,
+            code,
             requestBuffer: bufferOut,
             callback: callback_msp,
-            callbackOnError: doCallbackOnError,
             start: performance.now(),
         };
 
         if (!requestExists) {
             obj.timer = setTimeout(() => {
                 console.warn(
-                    `MSP: data request timed-out: ${code} ID: ${serial.connectionId} TAB: ${GUI.active_tab} TIMEOUT: ${
-                        this.timeout
-                    } QUEUE: ${this.callbacks.length} (${this.callbacks.map((e) => e.code)})`,
+                    `MSP: data request timed-out: ${code} ID: ${serial.connectionId} TAB: ${GUI.active_tab} QUEUE: ${this.callbacks.length} (${this.callbacks.map((e) => e.code)})`,
                 );
                 serial.send(bufferOut, (_sendInfo) => {
                     obj.stop = performance.now();
                     const executionTime = Math.round(obj.stop - obj.start);
-                    this.timeout = Math.max(this.MIN_TIMEOUT, Math.min(executionTime, this.MAX_TIMEOUT));
+                    // We should probably give up connection if the request takes too long ?
+                    if (executionTime > 5000) {
+                        console.warn(
+                            `MSP: data request took too long: ${code} ID: ${serial.connectionId} TAB: ${GUI.active_tab} EXECUTION TIME: ${executionTime}ms`,
+                        );
+                    }
+
+                    clearTimeout(obj.timer); // prevent leaks
                 });
-            }, this.timeout);
+            }, this.TIMEOUT);
         }
 
         this.callbacks.push(obj);
 
         // always send messages with data payload (even when there is a message already in the queue)
         if (data || !requestExists) {
-            if (this.timeout > this.MIN_TIMEOUT) {
-                this.timeout--;
-            }
-
             serial.send(bufferOut, (sendInfo) => {
-                if (sendInfo.bytesSent === bufferOut.byteLength) {
-                    if (callback_sent) {
-                        callback_sent();
-                    }
+                if (sendInfo.bytesSent === bufferOut.byteLength && callback_sent) {
+                    callback_sent();
                 }
             });
         }
 
         return true;
     },
-
     /**
      * resolves: {command: code, data: data, length: message_length}
      */

src/js/msp/MSPHelper.js

@@ -1762,14 +1762,13 @@ MspHelper.prototype.process_data = function (dataHandler) {
         if (dataHandler.callbacks[i]?.code === code) {
             // save callback reference
             const callback = dataHandler.callbacks[i].callback;
-            const callbackOnError = dataHandler.callbacks[i].callbackOnError;
 
             // remove timeout
-            clearInterval(dataHandler.callbacks[i].timer);
+            clearTimeout(dataHandler.callbacks[i].timer);
 
             // remove object from array
             dataHandler.callbacks.splice(i, 1);
-            if (!crcError || callbackOnError) {
+            if (!crcError) {
                 // fire callback
                 if (callback) {
                     callback({ command: code, data: data, length: data.byteLength, crcError: crcError });