Skip to content

Conversation

@Scavanger
Copy link
Contributor

@Scavanger Scavanger commented Dec 15, 2024

Actually it should be ‘only’ due to support from ESM, but somehow it has become a major maintenance update, the Configurator should now really be fit for the next few years.

  • Complete switch to ESM modules
  • Vite as module bundler and dev server
  • Assets partially renamed, moved and converted to be compatible with vite
  • iOS7 switch library ‘switchery’ replaced by a pure CSS variant, because not ESM compatible.
  • Updated some npm modules
  • Removal of (almost) all static libraries and replacement by corresponding npm modules
  • ‘Accidentally’ fixed minor bugs / layout problems ;)

Bonus: The packed installers/archives are now about 40 Mb smaller.

Known issues:
Elevation graph in Misson control not working, graph lib incompatible with vite.
Will be fixed in Missionplanner rewrite.

@mmosca mmosca added this to the 9.0 milestone Dec 15, 2024
@Scavanger
Copy link
Contributor Author

Tested under Windows and Linux, looks good so far.
Maybe someone can take another look and check if I have overlooked something. :)

@Scavanger Scavanger marked this pull request as ready for review December 18, 2024 01:09
@stronnag
Copy link
Collaborator

Lightly "tested" on Arch Linux

@Scavanger
Copy link
Contributor Author

Scavanger commented Dec 18, 2024

@stronnag

Lightly "tested" on Arch Linux

  • Neccesary to run npm install --save-dev @electron-forge/plugin-vite`

Make sure to do a npm update after checkout the new branch

Works for me on Kubuntu 24.10/X11 and propetary NVIDIA drivers.
Do you have any usefull error messages when running in dev mode?
Edit: I disabled 3D acceleration for linux, should work on all linux systems now, but makes the 3D animations sluggish.
electron/electron#32760

  • Mission control does not load at all (may be part of "Known issues")

Fixed, (wrong filename of an asset)

@mmosca
Copy link
Collaborator

mmosca commented Jan 21, 2025

9.0 (master) is ready for submissions, if you want to get this merged now :)

@Scavanger
Copy link
Contributor Author

Bump ;)

@Scavanger
Copy link
Contributor Author

No idea, the ARM64 Action hangs on the SITL executable, even though it is no longer present.

@sensei-hacker
Copy link
Member

Thanks for your work on this!

If this is ready to be included in NAV 9.0RC1 tomorrow, please resolve the merge conflicts.

@sensei-hacker
Copy link
Member

/review

@qodo-merge-pro
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 Security concerns

External resource handling:
The new dynamic imports for images/models (e.g., in gps and magnetometer tabs) rely on runtime asset resolution. While generally safe, ensure imported paths are controlled constants and not influenced by user input to prevent path traversal or bundling of unintended assets. Additionally, IPC handlers in js/main/main.js expose filesystem operations (readFile, writeFile, chmod, rm). Validate inputs on the main process side to avoid accidental destructive operations or privilege escalation from renderer-originated requests.

⚡ Recommended focus areas for review

Possible Issue

Comparisons against NaN using '!=' will always be true; use Number.isNaN after parseInt to validate numeric input (e.g., port and baud rate handling). This could allow invalid values into profiles and process args.

    if (!currentSim.isPortFixed) {
        var port = parseInt(port_e.val());
        if (port != NaN)
            currentProfile.port = parseInt(port_e.val());
    } 
});

simIp_e.on('change', () => {
    currentProfile.ip = simIp_e.val();
});

useImu_e.on('change', () => {
    currentProfile.useImu = useImu_e.is(':checked');
});

$('.sitlStart').on('click', ()=> {
    $('.sitlStart').addClass('disabled');
    $('.sitlStop').removeClass('disabled');

    var sim, simPort, simIp, channelMap = "";

    if (enableSim_e.is(':checked')) {
        switch(currentSim.name) {
            case 'X-Plane':
                sim = 'xp';
                break;
            case 'RealFlight':
                sim = 'rf'
                break;
        }
    }

    if (port_e.val() !== "") {
        simPort = port_e.val();
    }

    if (simIp_e.val() !== "") {
        simIp = simIp_e.val();
    }

    const zeroPad = (num, places) => String(num).padStart(places, '0');

    for (let i = 0; i < currentSim.inputChannels; i++) {
        var inavChan = mapping[i];
        if (inavChan == 0) {
            continue;
        } else if (inavChan < 13) {
            channelMap += `M${zeroPad(inavChan, 2)}-${zeroPad(i + 1, 2)},`;
        } else {
            channelMap += `S${zeroPad(inavChan - 12, 2)}-${zeroPad(i + 1, 2)},`;
        }
    }
    channelMap = channelMap.substring(0, channelMap.length - 1);

    var serialOptions = null;
    if ( serialReceiverEnable_e.is(':checked') && !!serialPorts_e.val()) {
        var selectedProtocoll = protocollPreset_e.find(':selected').val();
        if (selectedProtocoll == "manual") {
            serialOptions = {
                protocollName: "manual",
                baudRate: baudRate_e.val() || currentProfile.baudRate || "115200",
                stopBits: stopBits_e.val() || currentProfile.stopBits || "One",
                parity: parity_e.val() || currentProfile.parity || "None",
                serialPort: serialPorts_e.val() || currentProfile.serialPort || "",
                serialUart: serialUart_e.val() || currentProfile.serialUart || -1
            }
        } else {;
            serialOptions = {
                protocollName: selectedProtocoll || "SBus",
                serialPort: serialPorts_e.val() || currentProfile.serialPort || "" ,
                serialUart: serialUart_e.val() || currentProfile.serialUart || -1
            }
        }
    }

    appendLog("\n");

    SITLProcess.start(currentProfile.eepromFileName, sim, useImu_e.is(':checked'), simIp, simPort, channelMap, serialOptions,result => {
        appendLog(result);
    });
});

$('.sitlStop').on('click', ()=> {
    $('.sitlStop').addClass('disabled');
    $('.sitlStart').removeClass('disabled');
    SITLProcess.stop();
    appendLog(i18n.getMessage('sitlStopped'));
});

profileSaveBtn_e.on('click', function () {
    saveProfiles();
});

profileNewBtn_e.on('click', function () {
    smalltalk.prompt(i18n.getMessage('sitlNewProfile'), i18n.getMessage('sitlEnterName')).then(name => {
        if (!name)
            return;

        if (profiles.find(e => { return 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",
                isStdProfile: false,
                simEnabled: false,
                eepromFileName: eerpromName,
                port: 49001,
                ip: "127.0.0.1",
                useImu: false,
                channelMap: [ 1, 13, 14, 0, 0, 0, 0, 0, 0, 0, 0, 0],
                useSerialReceiver: true,
                serialPort: serialPorts_e.val(),
                serialUart: 3,
                serialProtocol: "SBus",
                baudRate: false,
                stopBits: false,
                parity: false
        }
        profiles.push(profile);
        profiles_e.append(`<option value="${name}">${name}</option>`)
        profiles_e.val(name);
        updateCurrentProfile();
        saveProfiles();
    }).catch(() => {} );
});

profileDeleteBtn_e.on('click', function () {

    if (currentProfile.isStdProfile) {
        dialog.alert(i18n.getMessage('sitlStdProfileCantDeleted'));            
        return;
    }

    var selected = profiles_e.find(':selected').val();
    profiles = profiles.filter(profile => {
        return profile.name != selected;
    });

    SITLProcess.deleteEepromFile(currentProfile.eepromFileName);
    profiles_e.find('*').remove();

    initElements(false);
});

serialReceiverEnable_e.on('change', () => {
currentProfile.useSerialReceiver = serialReceiverEnable_e.is(':checked');
});

protocollPreset_e.on('change', () => {
    var selectedProtocoll = protocollPreset_e.find(':selected').val();

    var protocoll = serialProtocolls.find(protocoll => {
        return protocoll.name == selectedProtocoll;
    });

    if (selectedProtocoll != 'manual'){  
        baudRate_e.prop('disabled', true);
        baudRate_e.val(protocoll.baudRate);
        stopBits_e.prop('disabled', true);
        stopBits_e.val(protocoll.stopBits);
        parity_e.prop('disabled', true);
        parity_e.val(protocoll.parity);
        serialUart_e.prop('disabled', selectedProtocoll == "Flight Controller Proxy");
    } else {
        baudRate_e.prop('disabled', false);
        stopBits_e.prop('disabled', false);
        parity_e.prop('disabled', false);
        serialUart_e.prop('disabled', false);
    }

    currentProfile.serialProtocol = selectedProtocoll;
});

serialPorts_e.on('change', () => {
    currentProfile.serialPort = serialPorts_e.val();
});

serialUart_e.on('change', () => {
    currentProfile.serialUart = parseInt(serialUart_e.val());
});

baudRate_e.on('change', () => {
    var baud = parseInt(baudRate_e.val());
    if (baud != NaN)
        currentProfile.baudRate = baud
});
Resource Loading

Dynamic imports for 3D assets inside loops can create many parallel requests; consider preloading or batching and add error handling to avoid partial UI if a single model fails to load.

//Load the models
const manager = new THREE.LoadingManager();
const loader = new GLTFLoader(manager);

const magModelNames = ['xyz', 'ak8963c', 'ak8963n', 'ak8975', 'ak8975c', 'bn_880', 'diatone_mamba_m10_pro', 'flywoo_goku_m10_pro_v3', 'foxeer_m10q_120', 'foxeer_m10q_180', 'foxeer_m10q_250', 
    'geprc_gep_m10_dq', 'gy271', 'gy273', 'hglrc_m100', 'qmc5883', 'holybro_m9n_micro', 'holybro_m9n_micro', 'ist8308', 'ist8310', 'lis3mdl', 
    'mag3110', 'matek_m8q', 'matek_m9n', 'matek_m10q', 'mlx90393', 'mp9250', 'qmc5883', 'flywoo_goku_m10_pro_v3', 'ws_m181'];
magModels = [];
//Load the UAV model
import(`./../resources/models/model_${model_file}.gltf`).then(({default: model}) => {
loader.load(model, (obj) => {
        const model = obj.scene;
        const scaleFactor = 15;
        model.scale.set(scaleFactor, scaleFactor, scaleFactor);
        modelWrapper.add(model);

        const gpsOffset = getDistanceByModelName(model_file);

        magModelNames.forEach( (name, i) => 
        {
            import(`./../resources/models/model_${name}.glb`).then(({default: magModel}) => {
                loader.load(magModel, (obj) => {
                    const gps = obj.scene;
                    const scaleFactor = i==0 ? 0.03 : 0.04;
                    gps.scale.set(scaleFactor, scaleFactor, scaleFactor);
                    gps.position.set(gpsOffset[0], gpsOffset[1] + 0.5, gpsOffset[2]);
                    gps.traverse(child => {
                    if (child.material) child.material.metalness = 0;
                    });
                    gps.rotation.y = 3 * Math.PI / 2;
                    model.add(gps);
                    magModels[i]=gps;
                    this.resize3D();
                });
            });
        });

        //Load the FC model
        import('./../resources/models/model_fc.gltf').then(({default: fcModel}) => {
            loader.load(fcModel, (obj) => {
                fc = obj.scene;
                const scaleFactor = 0.04;
                fc.scale.set(scaleFactor, scaleFactor, scaleFactor);
                fc.position.set(gpsOffset[0], gpsOffset[1] - 0.5, gpsOffset[2]);
                fc.rotation.y = 3 * Math.PI / 2;
                model.add(fc);
                this.render3D();
            });
        });

    });
    this.render3D();
    this.resize3D();
});
Async Init Risk

$.flightIndicator is now async and returns a promise; any existing callers expecting a synchronous return may break. Ensure all call sites await the initializer and handle errors if imports fail.

	return attitude;
};

async function getImages()  {
	var images = {};
	for (const image of imageNames) {
		const svg = (await import(`./../../images/flightindicators/fi_${image}.svg`)).default;
		const name = image.split('.')[0];
		images[name] = svg;
	}
	return images;
}

// Extension to jQuery
$.flightIndicator = async function(placeholder, type, options){
	var images = await getImages();
	var flightIndicator = new FlightIndicator($(placeholder), type, options, images)
	return flightIndicator;
}

@sensei-hacker
Copy link
Member

/improve

@qodo-merge-pro
Copy link

qodo-merge-pro bot commented Nov 18, 2025

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix UDP event binding and window guard
Suggestion Impact:The commit changed event bindings from this._socket to socket for both 'error' and 'message' events, aligning with the suggestion.

code diff:

-                this._socket.on('error', error => {
+                socket.on('error', error => {
                     if (!window.isDestroyed()) {
                         window.webContents.send('udpError', error); 
                     }
                 });
 
-                this._socket.on('message', (message, _rinfo) => {
+                socket.on('message', (message, _rinfo) => {

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.

js/main/udp.js [3-34]

 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 });
             }
         });
     },

[To ensure code accuracy, apply this suggestion manually]

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.

High
Fix undefined handler target
Suggestion Impact:The Drag constructor was modified to pass arrow functions for handleDownEvent/handleDragEvent/handleMoveEvent/handleUpEvent instead of direct app method references, addressing the undefined handler issue. However, the handlers were wired to app.handle... within the arrow functions rather than to this methods, and the additional instance method definitions were not added. Some constructor property initializations remained, but comments were removed.

code diff:

-        class Drag extends PointerInteraction{
+        class Drag extends PointerInteraction {
             constructor() {
                 super ({
-                    handleDownEvent: app.handleDownEvent,
-                    handleDragEvent: app.handleDragEvent,
-                    handleMoveEvent: app.handleMoveEvent,
-                    handleUpEvent: app.handleUpEvent
+                    handleDownEvent: (evt) => app.handleDownEvent(evt),
+                    handleDragEvent: (evt) => app.handleDragEvent(evt),
+                    handleMoveEvent: (evt) => app.handleMoveEvent(evt),
+                    handleUpEvent: (evt) => app.handleUpEvent(evt)
                 });
 
-                /**
-                 * @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]

-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);
+    }
+}

[To ensure code accuracy, apply this suggestion manually]

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.

High
Correct OpenLayers Feature import
Suggestion Impact:The commit updated the import from 'ol/format/Feature' to 'ol/Feature', correcting the module source for Feature.

code diff:

-import Feature from 'ol/format/Feature';
+import Feature from 'ol/Feature';

Change the import for Feature from ol/format/Feature to ol/Feature.js to
correctly import the feature constructor.

js/groundstation.js [13-175]

-import Feature from 'ol/format/Feature';
+import Feature from 'ol/Feature.js';
 ...
 privateScope.cursorFeature = new Feature({
     geometry: privateScope.cursorPosition
 });

[To ensure code accuracy, apply this suggestion manually]

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.

High
Fix mismatched DOM id selector
Suggestion Impact:The commit updated the jQuery selectors from #armedicon to #armedIcon in both branches, matching the suggested fix.

code diff:

         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');

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');
 }

[To ensure code accuracy, apply this suggestion manually]

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.

High
Normalize and validate icon key mapping
Suggestion Impact:The commit replaced the icons object initialization with Object.create(null), introduced an iconKey function, normalized dynamic imports to use the base filename, threw an error when icon URL is missing, and updated all icon accesses to use bracket notation with normalized keys.

code diff:

+const icons = Object.create(null)
+
 ////////////////////////////////////
 //
 // Tab mission control block
@@ -181,19 +183,26 @@
         }
     }
     
+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('.');
+        for (const fname of iconNames) {
             // 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;
-            }
-            if (iconUrl) {
-                icons[nameSplit[0]] = iconUrl;
-            }
+            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) {
+               throw new Error(`Missing icon URL for ${fname}`);
+            }
+            icons[base] = iconUrl;
         }
     }
 
@@ -281,7 +290,7 @@
                           anchor: [0.5, 0.5],
                           opacity: 1,
                           scale: 0.6,
-                          src: icons.icon_mission_airplane
+                          src: icons['icon_mission_airplane']
                       }))
                   });
 
@@ -308,7 +317,7 @@
                           anchor: [0.5, 1.0],
                           opacity: 1,
                           scale: 0.5,
-                          src: icons.icon_RTH
+                          src: icons['icon_RTH']
                       }))
                   });
 
@@ -621,7 +630,7 @@
                 anchor: [0.5, 1],
                 opacity: 1,
                 scale: 0.5,
-                src: safehome.isUsed() ? icons.icon_safehome_used : icons.icon_safehome
+                src: safehome.isUsed() ? icons['icon_safehome_used'] : icons['icon_safehome']
             })),
             text: new Text(({
                 text: String(Number(safehome.getNumber())+1),
@@ -841,7 +850,7 @@
                 anchor: [0.5, 1],
                 opacity: 1,
                 scale: 0.5,
-                src: geozone.getType() == GeozoneType.EXCULSIVE ? icons.icon_geozone_excl : icons.icon_geozone_incl
+                src: geozone.getType() == GeozoneType.EXCULSIVE ? icons['icon_geozone_excl'] : icons['icon_geozone_incl']
             })),
             text: new Text(({
                 text: String(number + 1),
@@ -1178,7 +1187,7 @@
                 anchor: [0.5, 1],
                 opacity: 1,
                 scale: 0.5,
-                src: icons.icon_home
+                src: icons['icon_home']
             })),
         });
     }
@@ -1641,7 +1650,7 @@
             featureArrow.setStyle(
                 new Style({
                     image: new Icon({
-                        src: icons.icon_arrow,
+                        src: icons['icon_arrow'],
                         scale: 0.3,
                         anchor: [0.5, 0.5],
                         rotateWithView: true,
@@ -1997,7 +2006,7 @@
         //      Drag behavior definition
         //////////////////////////////////////////////////////////////////////////////////////////////
 
-        class Drag extends PointerInteraction{
+        class Drag extends PointerInteraction {
             constructor() {
                 super ({
                     handleDownEvent: app.handleDownEvent,
@@ -2006,32 +2015,12 @@
                     handleUpEvent: app.handleUpEvent
                 });
 
-                /**
-                 * @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]

 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...)]
+

[To ensure code accuracy, apply this suggestion manually]

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.

Medium
Fix async HTML/icon load order
Suggestion Impact:The function load_html was converted to async, awaited the HTML import and loadIcons(), and then called GUI.load with process_html, matching the suggested change.

code diff:

+    async function load_html() {
+        const { default: html } = await import('./gps.html?raw');
+        await loadIcons();
+        GUI.load(html, Settings.processHtml(process_html));

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.

tabs/gps.js [117-119]

-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));
 }

[To ensure code accuracy, apply this suggestion manually]

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.

Medium
Prevent variable shadowing
Suggestion Impact:The commit renamed the inner 'model' to 'modelScene' and updated usages, addressing variable shadowing. It also changed the loader call to use 'modelUrl', partially aligning with the suggestion, though the destructuring change isn't shown here.

code diff:

-    loader.load(model, (obj) => {
-            const model = obj.scene;
+    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);

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);

[To ensure code accuracy, apply this suggestion manually]

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.

Low
Remove duplicate API property
Suggestion Impact:The commit removed the duplicated storeSet entry, leaving a single definition.

code diff:

-  storeSet: (key, value) => ipcRenderer.send('storeSet', key, value),
   storeSet: (key, value) => ipcRenderer.send('storeSet', key, value),

Remove the duplicate storeSet property from the object exposed via contextBridge
to improve code quality and prevent potential future errors.

js/main/preload.js [4-10]

 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)),
 });

[To ensure code accuracy, apply this suggestion manually]

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.

Low
Security
Sanitize user-provided profile name
Suggestion Impact:The commit implements input trimming, adds HTML-escaping via safeText when appending the option, renames eeprom variable, and applies some stylistic changes. This addresses the XSS concern.

code diff:

-            smalltalk.prompt(i18n.getMessage('sitlNewProfile'), i18n.getMessage('sitlEnterName')).then(name => {
+            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";
+                const safeText = $('<div>').text(name).html(); // escape
+                const eepromName = name.replace(/[^a-z0-9]/gi, '_').toLowerCase() + '.bin';;
                 var profile = {
                         name: name,
-                        sim: "RealFlight",
+                        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],
                         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(() => {});

Sanitize the user-provided profile name before inserting it into the DOM to
prevent a potential cross-site scripting (XSS) vulnerability.

tabs/sitl.js [258-292]

 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(() => {});
 });

[To ensure code accuracy, apply this suggestion manually]

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.

Medium
  • Update

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This is a major infrastructure upgrade transitioning the INAV Configurator from CommonJS modules to modern ESM modules, with Vite replacing the previous build system. The PR modernizes the development toolchain while maintaining functionality, except for a known issue with the elevation graph in Mission Control.

Key Changes:

  • Complete migration from CommonJS (require/module.exports) to ESM (import/export)
  • Integration of Vite as the module bundler and development server
  • Replacement of the iOS7-style Switchery library with pure CSS implementation
  • Update of npm dependencies and removal of static libraries in favor of npm-managed packages

Reviewed Changes

Copilot reviewed 124 out of 306 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
package.json Updated dependencies, added Vite plugin configuration, changed main entry point to Vite build output, and set module type to "module"
js/**/*.js Converted module syntax from CommonJS to ESM across all JavaScript files
js/main/main.js New main process file with Electron IPC handlers and window management
js/main/preload.js New preload script exposing IPC communication to renderer process
js/main/serial.js New serial port communication handler for main process
js/main/tcp.js New TCP connection handler for main process
js/main/udp.js New UDP connection handler for main process
js/main/child_process.js New child process management for SITL demo mode
js/store.js New store wrapper exposing electron-store via IPC
resources/models/*.gltf Removed GLTF model files (likely moved or handled differently by Vite)
js/libraries/* Removed static library files now managed through npm

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sensei-hacker
Copy link
Member

Note the comments from the bots likely include some false positives. Maybe even mostly false positives. But a couple of the points look like they may be valid.

@Scavanger
Copy link
Contributor Author

Scavanger commented Nov 19, 2025

High
Fix undefined handler target
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]

-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);
+    }
+}

This will not work, but will only lead to a “range error” and cause the call stack to overflow.
Solution: Use the arrow operator, but get rid of the member functions.

@sensei-hacker
Copy link
Member

sensei-hacker commented Nov 20, 2025

I'm having a couple issues with sensor reading when testing on Ubuntu 22.04.

But first, I learned t run it at all I needed to upgrade node to a higher version that what is available via apt:

curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.4/install.sh | bash ; nvm install 20.19 ; nvm use 20.19

npm update

yarn add yarn start
yarn start

After I got it to run, I found that on two of the three flight controllers I tested, this version of Configurator was not reading accelerometer data and therefore not updating the 3D model. The Sensors tab doesn't show any changes to accelerometer data.

The third FC did show the data changing in the Sensors tab, but then the 3D model wasn't visible at all.
I don't see any error messages that are clearly relevant.

UPDATE - the accelerometer sensor reading issue continues after I reverted to the master branch or earlier. So the problem may have been caused by my attempt to update npm, rather than the code itself. Now I just need to figure out how to un-screw my system, with limited knowledge about nodejs. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants