.pr_agent_accepted_suggestions
| PR 2436 (2025-11-26) |
[learned best practice] Guard modal open call
✅ Guard modal open call
Ensure the modal instance is valid and not destroyed before calling open to prevent runtime errors if initialization failed or it was disposed.
function showMSPWarning() {
- if (mspWarningModal) {
+ if (mspWarningModal && typeof mspWarningModal.open === 'function') {
mspWarningModal.open();
}
}Suggestion importance[1-10]: 6
__
Why: Relevant best practice - Gate UI behavior with guards and validate DOM elements before use to avoid runtime errors.
| PR 2434 (2025-11-26) |
[possible issue] Fix silent error handling bug
✅ Fix silent error handling bug
Refactor the appendFile IPC handler to be a simple async function that properly rejects on error, instead of resolving with an error object, to enable correct error handling in the renderer process.
-return new Promise(async resolve => {
- try {
- await appendFile(filename, data);
- resolve(false)
- } catch (err) {
- resolve(err);
- }
-});
+try {
+ await appendFile(filename, data);
+ return false;
+} catch (err) {
+ // Re-throwing the error will cause the promise on the renderer side to be rejected.
+ throw err;
+}Suggestion importance[1-10]: 8
__
Why: The suggestion correctly identifies a promise anti-pattern in the new appendFile handler that would lead to silent failures and provides the correct fix.
| PR 2433 (2025-11-25) |
[possible issue] Guard and sanitize reboot polling loop
✅ Guard and sanitize reboot polling loop
Refactor pollForRebootCompletion to correctly use the promise-based ConnectionSerial.getDevices API. Additionally, add a guard flag to prevent race conditions and ensure the polling interval is always cleared.
STM32_protocol.prototype.pollForRebootCompletion = function(port, hex, options, onSuccess, onTimeout) {
var self = this;
var intervalMs = 200;
var retries = 0;
var maxRetries = 50; // timeout after intervalMs * 50 (10 seconds)
+ var finished = false;
+
+ var finishOnce = function(fn) {
+ if (!finished) {
+ finished = true;
+ clearInterval(pollInterval);
+ fn && fn();
+ }
+ };
var pollInterval = setInterval(function() {
+ if (finished) return;
retries++;
if (retries > maxRetries) {
- clearInterval(pollInterval);
- onTimeout();
+ finishOnce(onTimeout);
return;
}
// Check for DFU devices first
PortHandler.check_usb_devices(function(dfu_available) {
+ if (finished) return;
if (dfu_available) {
- clearInterval(pollInterval);
- STM32DFU.connect(usbDevices, hex, options);
+ finishOnce(function() {
+ STM32DFU.connect(usbDevices, hex, options);
+ });
return;
}
- // Check for serial port
- ConnectionSerial.getDevices(function(devices) {
- if (devices && devices.includes(port)) {
- // Serial port reappeared - try to connect
- CONFIGURATOR.connection.connect(port, {bitrate: self.baud, parityBit: 'even', stopBits: 'one'}, function (openInfo) {
- if (openInfo) {
- clearInterval(pollInterval);
- onSuccess();
- } else {
- GUI.connect_lock = false;
+ // Check for serial port (promise-based)
+ ConnectionSerial.getDevices().then(function(devices) {
+ if (finished) return;
+ if (Array.isArray(devices) && devices.includes(port)) {
+ CONFIGURATOR.connection.connect(
+ port,
+ { bitrate: self.baud, parityBit: 'even', stopBits: 'one' },
+ function(openInfo) {
+ if (finished) return;
+ if (openInfo) {
+ finishOnce(onSuccess);
+ } else {
+ GUI.connect_lock = false;
+ }
}
- });
+ );
}
+ }).catch(function() {
+ // ignore errors and keep polling
});
});
}, intervalMs);
};Suggestion importance[1-10]: 9
__
Why: The suggestion correctly identifies a critical bug where the pollForRebootCompletion function uses ConnectionSerial.getDevices with a callback, but the function was changed in the PR to be async and no longer accepts one, which would break the serial reconnection logic.
[incremental [*]] Scope and harden response listener
✅ Scope and harden response listener
Improve the waitForResponse function by ensuring the receive listener only processes data for the current connection and by using a more robust method for string conversion.
js/protocols/stm32.js [101-139]
-// Waits for a specific response from the serial connection with timeout
STM32_protocol.prototype.waitForResponse = function(expectedString, timeoutMs, callback) {
var receivedData = '';
var timeoutHandle = null;
var onReceiveListener = null;
+ var connectionId = CONFIGURATOR.connection && CONFIGURATOR.connection._connectionId;
var cleanup = function() {
if (timeoutHandle) {
clearTimeout(timeoutHandle);
timeoutHandle = null;
}
- if (onReceiveListener) {
+ if (onReceiveListener && CONFIGURATOR.connection) {
CONFIGURATOR.connection.removeOnReceiveCallback(onReceiveListener);
onReceiveListener = null;
}
};
- // Set up timeout
+ // Bail out early if no active connection
+ if (!connectionId) {
+ callback(false, receivedData);
+ return;
+ }
+
timeoutHandle = setTimeout(function() {
cleanup();
console.log('Timeout waiting for response:', expectedString);
callback(false, receivedData);
}, timeoutMs);
- // Set up receive listener
onReceiveListener = function(info) {
+ // Ignore data from other connections
+ if (info.connectionId && info.connectionId !== connectionId) {
+ return;
+ }
var data = new Uint8Array(info.data);
- var str = String.fromCharCode.apply(null, data);
+ // Robust conversion without apply to avoid call-stack issues on large buffers
+ var decoder = new TextDecoder('utf-8');
+ var str = decoder.decode(data);
receivedData += str;
- // Check if we received the expected string
- if (receivedData.includes(expectedString)) {
+ if (receivedData.indexOf(expectedString) !== -1) {
cleanup();
callback(true, receivedData);
}
};
CONFIGURATOR.connection.addOnReceiveCallback(onReceiveListener);
};Suggestion importance[1-10]: 7
__
Why: The suggestion correctly identifies a potential race condition where data from a different connection could be processed, and proposes a robust fix by checking connectionId, which improves the reliability of the response handling logic.
[learned best practice] Guard and cleanup listener once
✅ Guard and cleanup listener once
Add a one-time guard to ensure the callback fires once and the listener is always removed even on rapid multiple matches.
js/protocols/stm32.js [102-139]
STM32_protocol.prototype.waitForResponse = function(expectedString, timeoutMs, callback) {
var receivedData = '';
var timeoutHandle = null;
var onReceiveListener = null;
+ var done = false;
- var cleanup = function() {
+ var finalize = function(success) {
+ if (done) return;
+ done = true;
if (timeoutHandle) {
clearTimeout(timeoutHandle);
timeoutHandle = null;
}
if (onReceiveListener) {
CONFIGURATOR.connection.removeOnReceiveCallback(onReceiveListener);
onReceiveListener = null;
}
+ callback(success, receivedData);
};
- // Set up timeout
timeoutHandle = setTimeout(function() {
- cleanup();
console.log('Timeout waiting for response:', expectedString);
- callback(false, receivedData);
+ finalize(false);
}, timeoutMs);
- // Set up receive listener
onReceiveListener = function(info) {
var data = new Uint8Array(info.data);
var str = String.fromCharCode.apply(null, data);
receivedData += str;
-
- // Check if we received the expected string
if (receivedData.includes(expectedString)) {
- cleanup();
- callback(true, receivedData);
+ finalize(true);
}
};
CONFIGURATOR.connection.addOnReceiveCallback(onReceiveListener);
};Suggestion importance[1-10]: 6
__
Why: Relevant best practice - Avoid leaking listeners; always remove event handlers after use to prevent broken bindings and memory leaks.
[possible issue] Prevent multiple callback invocations
✅ Prevent multiple callback invocations
To prevent a race condition in waitForResponse, add a flag to ensure the callback is only executed once, even if multiple data chunks are received in rapid succession.
js/protocols/stm32.js [114-151]
STM32_protocol.prototype.waitForResponse = function(expectedString, timeoutMs, callback) {
var receivedData = '';
var timeoutHandle = null;
var onReceiveListener = null;
+ var callbackFired = false;
var cleanup = function() {
if (timeoutHandle) {
clearTimeout(timeoutHandle);
timeoutHandle = null;
}
if (onReceiveListener) {
CONFIGURATOR.connection.removeOnReceiveCallback(onReceiveListener);
onReceiveListener = null;
}
};
+ var executeCallback = function(success, data) {
+ if (!callbackFired) {
+ callbackFired = true;
+ cleanup();
+ callback(success, data);
+ }
+ };
+
// Set up timeout
timeoutHandle = setTimeout(function() {
- cleanup();
console.log('Timeout waiting for response:', expectedString);
- callback(false, receivedData);
+ executeCallback(false, receivedData);
}, timeoutMs);
// Set up receive listener
onReceiveListener = function(info) {
var data = new Uint8Array(info.data);
var str = String.fromCharCode.apply(null, data);
receivedData += str;
// Check if we received the expected string
if (receivedData.includes(expectedString)) {
- cleanup();
- callback(true, receivedData);
+ executeCallback(true, receivedData);
}
};
CONFIGURATOR.connection.addOnReceiveCallback(onReceiveListener);
};Suggestion importance[1-10]: 6
__
Why: The suggestion correctly identifies a potential race condition in the new waitForResponse function where the callback could be invoked multiple times, and proposes a standard and effective fix using a flag to ensure it only runs once.
[learned best practice] Use proper async handling
✅ Use proper async handling
ConnectionSerial.getDevices() is async and returns a promise per its definition; use then/catch or await instead of a callback and guard devices shape before includes.
js/protocols/stm32.js [91-103]
-ConnectionSerial.getDevices(function(devices) {
- if (devices && devices.includes(port)) {
- // Serial port reappeared - try to connect
+ConnectionSerial.getDevices().then(devices => {
+ if (Array.isArray(devices) && devices.includes(port)) {
CONFIGURATOR.connection.connect(port, {bitrate: self.baud, parityBit: 'even', stopBits: 'one'}, function (openInfo) {
if (openInfo) {
clearInterval(pollInterval);
onSuccess();
} else {
GUI.connect_lock = false;
}
});
}
+}).catch(() => {
+ // ignore and keep polling
});Suggestion importance[1-10]: 6
__
Why: Relevant best practice - Gate asynchronous flows with explicit guards and correct API usage; validate objects and await/handle promises before use.
[learned best practice] Fix listener array mismatch
✅ Fix listener array mismatch
Register receive callbacks to the correct listener array and remove from the same array to avoid broken bindings.
js/connection/connectionUdp.js [105-111]
addOnReceiveCallback(callback){
- this._onReceiveErrorListeners.push(callback);
+ this._onReceiveListeners.push(callback);
}
removeOnReceiveCallback(callback){
- this._onReceiveListeners = this._onReceiveErrorListeners.filter(listener => listener !== callback);
+ this._onReceiveListeners = this._onReceiveListeners.filter(listener => listener !== callback);
}Suggestion importance[1-10]: 5
__
Why: Relevant best practice - Ensure event handler registration uses correct identifiers/instances to avoid mismatches.
| PR 2416 (2025-10-19) |
[possible issue] Hide FrSky options for non-serial receivers
✅ Hide FrSky options for non-serial receivers
Hide the #frSkyOptions section when the receiver mode is changed to a non-SERIAL type to prevent it from being displayed incorrectly.
$receiverMode.on('change', function () {
if ($(this).find("option:selected").text() == "SERIAL") {
$serialWrapper.show();
} else {
$serialWrapper.hide();
+ $("#frSkyOptions").hide();
}
});Suggestion importance[1-10]: 7
__
Why: The suggestion correctly identifies a UI logic bug where the frSkyOptions section remains visible after switching from a SERIAL receiver to a non-serial one, and provides the correct fix.
[possible issue] Fix incorrect help icon association
✅ Fix incorrect help icon association
Update the for attribute on the help icon div for the fuel unit dropdown from frSkyPitchRoll to frSkyFuelUnit to correctly associate it.
-<div for="frSkyPitchRoll" class="helpicon cf_tip" data-i18n_title="serialrx_frSkyFuelUnitHelp"></div>
+<div for="frSkyFuelUnit" class="helpicon cf_tip" data-i18n_title="serialrx_frSkyFuelUnitHelp"></div>Suggestion importance[1-10]: 5
__
Why: The suggestion correctly identifies a copy-paste error in the for attribute of a help icon div, which should be associated with frSkyFuelUnit instead of frSkyPitchRoll.
| PR 2414 (2025-10-15) |
[possible issue] Correct the data unit mismatch
✅ Correct the data unit mismatch
Update the data-unit attribute for the osdPanServoPwm2decadeg input from centideg-deg to decadeg-deg. This ensures the UI sends the correct value to the firmware, matching the unit change implied by the setting's new name.
-<input id="osdPanServoPwm2decadeg" data-setting="osd_pan_servo_pwm2decadeg" data-unit="centideg-deg" data-setting-multiplier="1" type="number" data-step="1" />
+<input id="osdPanServoPwm2decadeg" data-setting="osd_pan_servo_pwm2decadeg" data-unit="decadeg-deg" data-setting-multiplier="1" type="number" data-step="1" />Suggestion importance[1-10]: 10
__
Why: The suggestion correctly identifies a critical bug where the UI data-unit (centideg-deg) was not updated to match the renamed setting (osd_pan_servo_pwm2decadeg), which would cause incorrect values to be sent to the firmware.
| PR 2285 (2024-12-15) |
[possible issue] Fix UDP event binding and window guard
✅ Fix UDP event binding and window guard
Correct the UDP connection logic by attaching event listeners to the correct socket object and adding a guard to prevent calling methods on a non-window object.
const socket = dgram.createSocket('udp4');
const udp = {
_id: 1,
- _ip: false,
- _port: false,
- connect: function(ip, port, window = true) {
+ _ip: null,
+ _port: null,
+ connect: function(ip, port, window) {
return new Promise(resolve => {
try {
- socket.bind(port, () => {
+ socket.once('listening', () => {
this._ip = ip;
this._port = port;
+ resolve({ error: false, id: this._id++ });
});
- this._socket.on('error', error => {
- if (!window.isDestroyed()) {
+ socket.on('error', error => {
+ if (window && typeof window.isDestroyed === 'function' && !window.isDestroyed()) {
window.webContents.send('udpError', error);
}
});
- this._socket.on('message', (message, _rinfo) => {
- if (!window.isDestroyed()) {
+ socket.on('message', (message, _rinfo) => {
+ if (window && typeof window.isDestroyed === 'function' && !window.isDestroyed()) {
window.webContents.send('udpMessage', message);
}
});
- resolve({error: false, id: this._id++});
+
+ socket.bind(port);
} catch (err) {
- resolve ({error: true, errorMsg: err});
+ resolve({ error: true, errorMsg: err });
}
});
},Suggestion importance[1-10]: 10
__
Why: The suggestion correctly identifies two critical bugs in the new udp.connect function that would cause it to fail at runtime and prevent any UDP communication.
[possible issue] Fix undefined handler target
✅ Fix undefined handler target
**
-
* @type {ol.Pixel} -
* @private -
*/ this.coordinate_ = null; -
/** -
* @type {string|undefined} -
* @private -
*/ this.cursor_ = 'pointer'; -
/** -
* @type {Feature} -
* @private -
*/ this.feature_ = null; -
/** -
* @type {string|undefined} -
* @private -
*/ this.previousCursor_ = undefined; } -
};
-
}
**Fix a reference error in the Drag class constructor by correctly referencing event handlers. The app object's methods are not defined at the time of instantiation.**
[tabs/mission_control.js [2000-2014]](https://github.com/iNavFlight/inav-configurator/pull/2285/files#diff-54d84cb8c735be5909a6f1d56d93c5033b9884a1e6182fa5ac1eb619a8443786R2000-R2014)
```diff
-class Drag extends PointerInteraction{
+class Drag extends PointerInteraction {
constructor() {
- super ({
- handleDownEvent: app.handleDownEvent,
- handleDragEvent: app.handleDragEvent,
- handleMoveEvent: app.handleMoveEvent,
- handleUpEvent: app.handleUpEvent
+ super({
+ handleDownEvent: (evt) => this.handleDownEvent(evt),
+ handleDragEvent: (evt) => this.handleDragEvent(evt),
+ handleMoveEvent: (evt) => this.handleMoveEvent(evt),
+ handleUpEvent: (evt) => this.handleUpEvent(evt),
});
+ this.coordinate_ = null;
+ this.cursor_ = 'pointer';
+ this.feature_ = null;
+ this.previousCursor_ = undefined;
+ }
- /**
- * @type {ol.Pixel}
- * @private
- */
- this.coordinate_ = null;
- ...
+ handleDownEvent(evt) {
+ return app.handleDownEvent(evt);
}
-};
+ handleDragEvent(evt) {
+ return app.handleDragEvent(evt);
+ }
+ handleMoveEvent(evt) {
+ return app.handleMoveEvent(evt);
+ }
+ handleUpEvent(evt) {
+ return app.handleUpEvent(evt);
+ }
+}
Suggestion importance[1-10]: 9
__
Why: The suggestion correctly identifies that app.handle... functions are undefined when new Drag() is called, which will cause a runtime error.
[possible issue] Correct OpenLayers Feature import
✅ Correct OpenLayers Feature import
Change the import for Feature from ol/format/Feature to ol/Feature.js to correctly import the feature constructor.
-import Feature from 'ol/format/Feature';
+import Feature from 'ol/Feature.js';
...
privateScope.cursorFeature = new Feature({
geometry: privateScope.cursorPosition
});Suggestion importance[1-10]: 9
__
Why: This suggestion correctly identifies a critical bug where an incorrect module is imported, which would cause a runtime error when new Feature() is called.
[possible issue] Fix mismatched DOM id selector
✅ Fix mismatched DOM id selector
Fix the jQuery selector for the armed status icon by changing #armedicon to #armedIcon to match the element's ID in the HTML.
js/periodicStatusUpdater.js [42-48]
if (FC.isModeEnabled('ARM')) {
- $("#armedicon").removeClass('armed');
- $("#armedicon").addClass('armed-active');
+ $("#armedIcon").removeClass('armed');
+ $("#armedIcon").addClass('armed-active');
} else {
- $("#armedicon").removeClass('armed-active');
- $("#armedicon").addClass('armed');
+ $("#armedIcon").removeClass('armed-active');
+ $("#armedIcon").addClass('armed');
}Suggestion importance[1-10]: 9
__
Why: The suggestion correctly identifies a bug where the jQuery selector armedicon does not match the HTML element ID armedIcon, which would prevent the armed status icon from updating.
[possible issue] Normalize and validate icon key mapping
✅ Normalize and validate icon key mapping
**
-
* @type {ol.Pixel} -
* @private -
*/ this.coordinate_ = null; -
/** -
* @type {string|undefined} -
* @private -
*/ this.cursor_ = 'pointer'; -
/** -
* @type {Feature} -
* @private -
*/ this.feature_ = null; -
/** -
* @type {string|undefined} -
* @private -
*/ this.previousCursor_ = undefined; } -
};
-
} app.ConvertCentimetersToMeters = function (val) { return parseInt(val) / 100;
@@ -2044,7 +2033,7 @@ var button = document.createElement('button');
button.innerHTML = ' ';
-
button.style = `background: url("${icons.settings_white}") no-repeat 1px -1px;background-color: rgba(0,60,136,.5);`;
-
button.style = `background: url("${icons['settings_white']}") no-repeat 1px -1px;background-color: rgba(0,60,136,.5);`; var handleShowSettings = function () {
@@ -2073,7 +2062,7 @@ var button = document.createElement('button');
button.innerHTML = ' ';
-
button.style = `background: url("${icons.icon_safehome_white}") no-repeat 1px -1px;background-color: rgba(0,60,136,.5);`;
-
button.style = `background: url("${icons['icon_safehome_white']}") no-repeat 1px -1px;background-color: rgba(0,60,136,.5);`; var handleShowSafehome = function () { $('#missionPlannerSafehome').fadeIn(300);
@@ -2105,7 +2094,7 @@ var button = document.createElement('button');
button.innerHTML = ' ';
-
button.style = `background: url("${icons.icon_geozone_white}") no-repeat 1px -1px;background-color: rgba(0,60,136,.5);`;
-
button.style = `background: url("${icons['icon_geozone_white']}") no-repeat 1px -1px;background-color: rgba(0,60,136,.5);`; var handleShowGeozoneSettings = function () { $('#missionPlannerGeozones').fadeIn(300);
@@ -2138,7 +2127,7 @@ var button = document.createElement('button');
button.innerHTML = ' ';
-
button.style = `background: url("${icons.icon_elevation_white}") no-repeat 1px -1px;background-color: rgba(0,60,136,.5);`;
-
button.style = `background: url("${icons['icon_elevation_white']}") no-repeat 1px -1px;background-color: rgba(0,60,136,.5);`; var handleShowSettings = function () { $('#missionPlannerHome').fadeIn(300);
@@ -2171,7 +2160,7 @@ var button = document.createElement('button');
button.innerHTML = ' ';
-
button.style = `background: url("${icons.icon_multimission_white}") no-repeat 1px -1px;background-color: rgba(0,60,136,.5);`;
-
button.style = `background: url("${icons['icon_multimission_white']}") no-repeat 1px -1px;background-color: rgba(0,60,136,.5);`;
**Normalize the icon key mapping to prevent broken image URLs. The dynamic import path is inconsistent with how icons are accessed, and the iconNames array contains a duplicate entry.**
[tabs/mission_control.js [70-198]](https://github.com/iNavFlight/inav-configurator/pull/2285/files#diff-54d84cb8c735be5909a6f1d56d93c5033b9884a1e6182fa5ac1eb619a8443786R70-R198)
```diff
const iconNames = [
'icon_mission_airplane.png',
- ...
- 'icon_multimission_white.svg'
+ 'icon_RTH.png',
+ 'icon_safehome.png',
+ 'icon_safehome_used.png',
+ 'icon_geozone_excl.png',
+ 'icon_geozone_incl.png',
+ 'icon_home.png',
+ 'icon_position_edit.png',
+ 'icon_position_head.png',
+ 'icon_position_LDG_edit.png',
+ 'icon_position_LDG.png',
+ 'icon_position_PH_edit.png',
+ 'icon_position_PH.png',
+ 'icon_position_POI.png',
+ 'icon_position_POI_edit.png',
+ 'icon_position_WP_edit.png',
+ 'icon_position_WP.png',
+ 'icon_arrow.png',
+ 'settings_white.svg',
+ 'icon_safehome_white.svg',
+ 'icon_geozone_white.svg',
+ 'icon_elevation_white.svg',
+ 'icon_multimission_white.svg'
];
-var icons = {};
-...
+const icons = Object.create(null);
+
+function iconKey(filename) {
+ // drop extension, keep base name (e.g., "icon_RTH")
+ return filename.replace(/\.(png|svg)$/i, '');
+}
+
async function loadIcons() {
- for (const icon of iconNames) {
- const nameSplit = icon.split('.');
- // Vites packager needs a bit help
- var iconUrl;
- if (nameSplit[1] == 'png') {
- iconUrl = (await import(`./../images/icons/map/cf_${nameSplit[0]}.png?inline`)).default;
- } else if (nameSplit[1] == 'svg') {
- iconUrl = (await import(`./../images/icons/map/cf_${nameSplit[0]}.svg?inline`)).default;
+ for (const fname of iconNames) {
+ const base = iconKey(fname);
+ const ext = fname.split('.').pop();
+ let iconUrl;
+ if (ext === 'png') {
+ iconUrl = (await import(`./../images/icons/map/cf_${base}.png?inline`)).default;
+ } else if (ext === 'svg') {
+ iconUrl = (await import(`./../images/icons/map/cf_${base}.svg?inline`)).default;
}
- if (iconUrl) {
- icons[nameSplit[0]] = iconUrl;
+ if (!iconUrl) {
+ throw new Error(`Missing icon URL for ${fname}`);
}
+ icons[base] = iconUrl;
}
}
+// usage examples adjusted:
+// src: icons['icon_RTH']
+// src: icons['icon_position' + (suffixes...)]
+
Suggestion importance[1-10]: 8
__
Why: The suggestion correctly points out that the dynamic import path cf_${nameSplit[0]} is inconsistent with how icons are accessed later, which will lead to broken image URLs.
[possible issue] Fix async HTML/icon load order
✅ Fix async HTML/icon load order
Refactor load_html to be an async function. Await the completion of loadIcons() before calling GUI.load to ensure icons are loaded before process_html is executed.
-function load_html() {
- import('./gps.html?raw').then(({default: html}) => GUI.load(html, Settings.processHtml(loadIcons().then(process_html))));
+async function load_html() {
+ const { default: html } = await import('./gps.html?raw');
+ await loadIcons();
+ GUI.load(html, Settings.processHtml(process_html));
}Suggestion importance[1-10]: 8
__
Why: The suggestion correctly identifies a critical race condition where process_html could execute before loadIcons completes, leading to runtime errors.
[possible issue] Prevent variable shadowing
✅ Prevent variable shadowing
Rename the model variable inside the loader.load callback to avoid shadowing the model variable from the outer scope, which holds the imported URL.
tabs/magnetometer.js [741-746]
-import(`./../resources/models/model_${model_file}.gltf`).then(({default: model}) => {
-loader.load(model, (obj) => {
- const model = obj.scene;
+import(`./../resources/models/model_${model_file}.gltf`).then(({ default: model: modelUrl }) => {
+ loader.load(modelUrl, (obj) => {
+ const modelScene = obj.scene;
const scaleFactor = 15;
- model.scale.set(scaleFactor, scaleFactor, scaleFactor);
- modelWrapper.add(model);
+ modelScene.scale.set(scaleFactor, scaleFactor, scaleFactor);
+ modelWrapper.add(modelScene);Suggestion importance[1-10]: 4
__
Why: The suggestion correctly identifies variable shadowing which is poor practice, but the proposed improved_code contains a syntax error in the destructuring assignment.
[possible issue] Remove duplicate API property
✅ Remove duplicate API property
Remove the duplicate storeSet property from the object exposed via contextBridge to improve code quality and prevent potential future errors.
contextBridge.exposeInMainWorld('electronAPI', {
listSerialDevices: () => ipcRenderer.invoke('listSerialDevices'),
storeGet: (key, defaultValue) => ipcRenderer.sendSync('storeGet', key, defaultValue),
storeSet: (key, value) => ipcRenderer.send('storeSet', key, value),
- storeSet: (key, value) => ipcRenderer.send('storeSet', key, value),
storeDelete: (key) => ipcRenderer.send('storeDelete', key),
- ...
+ appGetPath: (name) => ipcRenderer.sendSync('appGetPath', name),
+ appGetVersion: () => ipcRenderer.sendSync('appGetVersion'),
+ appGetLocale: () => ipcRenderer.sendSync('appGetLocale'),
+ showOpenDialog: (options) => ipcRenderer.invoke('dialog.showOpenDialog', options),
+ showSaveDialog: (options) => ipcRenderer.invoke('dialog.showSaveDialog', options),
+ alertDialog: (message) => ipcRenderer.sendSync('dialog.alert', message),
+ confirmDialog: (message) => ipcRenderer.sendSync('dialog.confirm', message),
+ tcpConnect: (host, port) => ipcRenderer.invoke('tcpConnect', host, port),
+ tcpClose: () => ipcRenderer.send('tcpClose'),
+ tcpSend: (data) => ipcRenderer.invoke('tcpSend', data),
+ onTcpError: (callback) => ipcRenderer.on('tcpError', (_event, error) => callback(error)),
+ onTcpData: (callback) => ipcRenderer.on('tcpData', (_event, data) => callback(data)),
+ onTcpEnd: (callback) => ipcRenderer.on('tcpEnd', (_event) => callback()),
+ serialConnect: (path, options) => ipcRenderer.invoke('serialConnect', path, options),
+ serialClose: () => ipcRenderer.invoke('serialClose'),
+ serialSend: (data) => ipcRenderer.invoke('serialSend', data),
+ onSerialError: (callback) => ipcRenderer.on('serialError', (_event, error) => callback(error)),
+ onSerialData: (callback) => ipcRenderer.on('serialData', (_event, data) => callback(data)),
+ onSerialClose: (callback) => ipcRenderer.on('serialClose', (_event) => callback()),
+ udpConnect: (ip, port) => ipcRenderer.invoke('udpConnect', ip, port),
+ udpClose: () => ipcRenderer.invoke('udpClose'),
+ udpSend: (data) => ipcRenderer.invoke('udpSend', data),
+ onUdpError: (callback) => ipcRenderer.on('udpError', (_event, error) => callback(error)),
+ onUdpMessage: (callback) => ipcRenderer.on('udpMessage', (_event, data) => callback(data)),
+ writeFile: (filename, data) => ipcRenderer.invoke('writeFile', filename, data),
+ readFile: (filename, encoding = 'utf8') => ipcRenderer.invoke('readFile', filename, encoding),
+ rm: (path) => ipcRenderer.invoke('rm', path),
+ chmod: (path, mode) => ipcRenderer.invoke('chmod', path, mode),
+ startChildProcess: (command, args, opts) => ipcRenderer.send('startChildProcess', command, args, opts),
+ killChildProcess: () => ipcRenderer.send('killChildProcess'),
+ onChildProcessStdout: (callback) => ipcRenderer.on('onChildProcessStdout', (_event, data) => callback(data)),
+ onChildProcessStderr: (callback) => ipcRenderer.on('onChildProcessStderr', (_event, data) => callback(data)),
+ onChildProcessError: (callback) => ipcRenderer.on('onChildProcessError', (_event, error) => callback(error)),
});Suggestion importance[1-10]: 3
__
Why: The suggestion correctly identifies a duplicate key storeSet which, while not causing a functional bug here, is a code quality issue that should be fixed.
[security] Sanitize user-provided profile name
✅ Sanitize user-provided profile name
Sanitize the user-provided profile name before inserting it into the DOM to prevent a potential cross-site scripting (XSS) vulnerability.
profileNewBtn_e.on('click', function () {
- smalltalk.prompt(i18n.getMessage('sitlNewProfile'), i18n.getMessage('sitlEnterName')).then(name => {
- if (!name)
- return;
+ smalltalk.prompt(i18n.getMessage('sitlNewProfile'), i18n.getMessage('sitlEnterName')).then(rawName => {
+ const name = (rawName || '').trim();
+ if (!name) return;
- if (profiles.find(e => { return e.name == name })) {
+ if (profiles.find(e => e.name === name)) {
dialog.alert(i18n.getMessage('sitlProfileExists'));
return;
}
- var eerpromName = name.replace(/[^a-z0-9]/gi, '_').toLowerCase() + ".bin";
- var profile = {
- name: name,
- sim: "RealFlight",
+ const safeText = $('<div>').text(name).html(); // escape
+ const eepromName = name.replace(/[^a-z0-9]/gi, '_').toLowerCase() + '.bin';
+ const profile = {
+ name,
+ sim: 'RealFlight',
isStdProfile: false,
simEnabled: false,
- eepromFileName: eerpromName,
+ eepromFileName: eepromName,
port: 49001,
- ip: "127.0.0.1",
+ ip: '127.0.0.1',
useImu: false,
- channelMap: [ 1, 13, 14, 0, 0, 0, 0, 0, 0, 0, 0, 0],
+ channelMap: [1, 13, 14, 0, 0, 0, 0, 0, 0, 0, 0, 0],
useSerialReceiver: true,
serialPort: serialPorts_e.val(),
serialUart: 3,
- serialProtocol: "SBus",
+ serialProtocol: 'SBus',
baudRate: false,
stopBits: false,
parity: false
- }
+ };
profiles.push(profile);
- profiles_e.append(`<option value="${name}">${name}</option>`)
+ profiles_e.append(`<option value="${safeText}">${safeText}</option>`);
profiles_e.val(name);
updateCurrentProfile();
saveProfiles();
- }).catch(() => {} );
+ }).catch(() => {});
});Suggestion importance[1-10]: 8
__
Why: The suggestion correctly identifies a cross-site scripting (XSS) vulnerability by using unescaped user input to construct HTML, which is a critical security issue.
The latest version of INAV Configurator can be found in the Releases section of this repository.
For more information about the features of the INAV firmware. Check out the firmware Wiki or technical documents. For supported flight controllers, take a look here and here.