Skip to content

Commit 10d6584

Browse files
haslinghuiscoderabbitai[bot]
haslinghuis
and
coderabbitai[bot]
authored
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>
1 parent 9b8da64 commit 10d6584

File tree

12 files changed

+3611
-48
lines changed

12 files changed

+3611
-48
lines changed

‎src/js/main.js‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,16 @@ import { updateTabList } from "./utils/updateTabList.js";
1818
import * as THREE from "three";
1919
import NotificationManager from "./utils/notifications.js";
2020

21+
import("./msp/debug/msp_debug_tools.js")
22+
.then(() => {
23+
console.log("🔧 MSP Debug Tools loaded for development environment");
24+
console.log("• Press Ctrl+Shift+M to toggle debug dashboard");
25+
console.log("• Use MSPTestRunner.help() for all commands");
26+
})
27+
.catch((err) => {
28+
console.warn("Failed to load MSP debug tools:", err);
29+
});
30+
2131
if (typeof String.prototype.replaceAll === "undefined") {
2232
String.prototype.replaceAll = function (match, replace) {
2333
return this.replace(new RegExp(match, "g"), () => replace);
@@ -111,7 +121,8 @@ function startProcess() {
111121
console.log(`Libraries: jQuery - ${$.fn.jquery}, three.js - ${THREE.REVISION}`);
112122

113123
// Check if this is the first visit
114-
if (getConfig("firstRun").firstRun === undefined) {
124+
const firstRunCfg = getConfig("firstRun") ?? {};
125+
if (firstRunCfg.firstRun === undefined) {
115126
setConfig({ firstRun: true });
116127
import("./tabs/static_tab.js").then(({ staticTab }) => {
117128
staticTab.initialize("options", () => {

‎src/js/msp.js‎

Lines changed: 28 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -51,23 +51,18 @@ const MSP = {
5151
message_buffer: null,
5252
message_buffer_uint8_view: null,
5353
message_checksum: 0,
54-
messageIsJumboFrame: false,
5554
crcError: false,
5655

5756
callbacks: [],
5857
packet_error: 0,
5958
unsupported: 0,
6059

61-
MIN_TIMEOUT: 200,
62-
MAX_TIMEOUT: 2000,
63-
timeout: 200,
60+
TIMEOUT: 1000,
6461

6562
last_received_timestamp: null,
6663
listeners: [],
6764

68-
JUMBO_FRAME_SIZE_LIMIT: 255,
69-
70-
cli_buffer: [], // buffer for CLI charactor output
65+
cli_buffer: [], // buffer for CLI character output
7166
cli_output: [],
7267
cli_callback: null,
7368

@@ -280,7 +275,6 @@ const MSP = {
280275
// Reset variables
281276
this.message_length_received = 0;
282277
this.state = 0;
283-
this.messageIsJumboFrame = false;
284278
this.crcError = false;
285279
},
286280
notify() {
@@ -289,7 +283,7 @@ const MSP = {
289283
});
290284
},
291285
listen(listener) {
292-
if (this.listeners.indexOf(listener) == -1) {
286+
if (this.listeners.indexOf(listener) === -1) {
293287
this.listeners.push(listener);
294288
}
295289
},
@@ -318,8 +312,8 @@ const MSP = {
318312
const dataLength = data ? data.length : 0;
319313
// always reserve 6 bytes for protocol overhead !
320314
const bufferSize = dataLength + 6;
321-
let bufferOut = new ArrayBuffer(bufferSize);
322-
let bufView = new Uint8Array(bufferOut);
315+
const bufferOut = new ArrayBuffer(bufferSize);
316+
const bufView = new Uint8Array(bufferOut);
323317

324318
bufView[0] = 36; // $
325319
bufView[1] = 77; // M
@@ -377,70 +371,64 @@ const MSP = {
377371

378372
serial.send(bufferOut);
379373
},
380-
send_message(code, data, callback_sent, callback_msp, doCallbackOnError) {
381-
const connected = serial.connected;
382-
383-
if (code === undefined || !connected || CONFIGURATOR.virtualMode) {
374+
send_message(code, data, callback_sent, callback_msp) {
375+
if (code === undefined || !serial.connected || CONFIGURATOR.virtualMode) {
384376
if (callback_msp) {
385377
callback_msp();
386378
}
387379
return false;
388380
}
389381

390-
let requestExists = false;
391-
for (const instance of this.callbacks) {
392-
if (instance.code === code) {
393-
requestExists = true;
394-
395-
break;
396-
}
397-
}
398-
399382
const bufferOut = code <= 254 ? this.encode_message_v1(code, data) : this.encode_message_v2(code, data);
383+
const view = new Uint8Array(bufferOut);
384+
const keyCrc = this.crc8_dvb_s2_data(view, 0, view.length);
385+
const requestExists = this.callbacks.some(
386+
(i) =>
387+
i.code === code &&
388+
i.requestBuffer?.byteLength === bufferOut.byteLength &&
389+
this.crc8_dvb_s2_data(new Uint8Array(i.requestBuffer), 0, i.requestBuffer.byteLength) === keyCrc,
390+
);
400391

401392
const obj = {
402-
code: code,
393+
code,
403394
requestBuffer: bufferOut,
404395
callback: callback_msp,
405-
callbackOnError: doCallbackOnError,
406396
start: performance.now(),
407397
};
408398

409399
if (!requestExists) {
410400
obj.timer = setTimeout(() => {
411401
console.warn(
412-
`MSP: data request timed-out: ${code} ID: ${serial.connectionId} TAB: ${GUI.active_tab} TIMEOUT: ${
413-
this.timeout
414-
} QUEUE: ${this.callbacks.length} (${this.callbacks.map((e) => e.code)})`,
402+
`MSP: data request timed-out: ${code} ID: ${serial.connectionId} TAB: ${GUI.active_tab} QUEUE: ${this.callbacks.length} (${this.callbacks.map((e) => e.code)})`,
415403
);
416404
serial.send(bufferOut, (_sendInfo) => {
417405
obj.stop = performance.now();
418406
const executionTime = Math.round(obj.stop - obj.start);
419-
this.timeout = Math.max(this.MIN_TIMEOUT, Math.min(executionTime, this.MAX_TIMEOUT));
407+
// We should probably give up connection if the request takes too long ?
408+
if (executionTime > 5000) {
409+
console.warn(
410+
`MSP: data request took too long: ${code} ID: ${serial.connectionId} TAB: ${GUI.active_tab} EXECUTION TIME: ${executionTime}ms`,
411+
);
412+
}
413+
414+
clearTimeout(obj.timer); // prevent leaks
420415
});
421-
}, this.timeout);
416+
}, this.TIMEOUT);
422417
}
423418

424419
this.callbacks.push(obj);
425420

426421
// always send messages with data payload (even when there is a message already in the queue)
427422
if (data || !requestExists) {
428-
if (this.timeout > this.MIN_TIMEOUT) {
429-
this.timeout--;
430-
}
431-
432423
serial.send(bufferOut, (sendInfo) => {
433-
if (sendInfo.bytesSent === bufferOut.byteLength) {
434-
if (callback_sent) {
435-
callback_sent();
436-
}
424+
if (sendInfo.bytesSent === bufferOut.byteLength && callback_sent) {
425+
callback_sent();
437426
}
438427
});
439428
}
440429

441430
return true;
442431
},
443-
444432
/**
445433
* resolves: {command: code, data: data, length: message_length}
446434
*/

‎src/js/msp/MSPHelper.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1762,14 +1762,13 @@ MspHelper.prototype.process_data = function (dataHandler) {
17621762
if (dataHandler.callbacks[i]?.code === code) {
17631763
// save callback reference
17641764
const callback = dataHandler.callbacks[i].callback;
1765-
const callbackOnError = dataHandler.callbacks[i].callbackOnError;
17661765

17671766
// remove timeout
1768-
clearInterval(dataHandler.callbacks[i].timer);
1767+
clearTimeout(dataHandler.callbacks[i].timer);
17691768

17701769
// remove object from array
17711770
dataHandler.callbacks.splice(i, 1);
1772-
if (!crcError || callbackOnError) {
1771+
if (!crcError) {
17731772
// fire callback
17741773
if (callback) {
17751774
callback({ command: code, data: data, length: data.byteLength, crcError: crcError });

0 commit comments

Comments
 (0)