Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] [For discussion] Migrate from nw.js to Electron #1961

Merged
merged 18 commits into from
Apr 23, 2024

Conversation

Scavanger
Copy link
Contributor

@Scavanger Scavanger commented Feb 10, 2024

This is to discuss an track the migration to Electron

nw.js has had a lot of problems for some time now, it is now just a "one man project" and the bugs are piling up, bug reports are practically not processed, usually you don't even get an answer.
nw.js seems to be dying a slow death.
As already briefly discussed on Discord, it probably makes sense to switch to the much more active and better supported Electron.

Todo List:

  • Basic structure for Electron
  • Serial connection
  • BLE
  • TCP
  • UDP
  • SITL
  • Firmware flasher
  • CLI (uses bluebird.js, wich doesn't work with Electron)
  • translation (use i18n-next, translation is now just a stub)
  • check all paths (images, other resources)
  • new Window (MSP "transmitter" for example)
  • fix smaller issues with the stylesheets
  • Configs for forge: customize packages/installer (Icons, ...)
  • Update documentation
  • Convert to CommonJS Modules
  • Change depreached methods (.click() -> .on('click, ... )

To discuss:
With "Electron Forge" for creating packages/installers no Gulp or similar is necessary.
For Windows we could use Squirrel (https://github.com/Squirrel/Squirrel.Windows) a kind of "one click" installer or the traditional MSI installer.

@DzikuVx
Copy link
Member

DzikuVx commented Feb 11, 2024

That would be nice....

@Scavanger Scavanger marked this pull request as ready for review April 21, 2024 23:55
@Scavanger
Copy link
Contributor Author

Scavanger commented Apr 22, 2024

I think it's ready to be merged.

Since I have to touch almost all files anyway, I took the opportunity to modernize the code a bit:

  • replaced obsolete methods (e.g. .click() -> .on('click', ... )
  • conversion to CommonJS modules. This results in faster loading as you no longer have to load a huge javascript file but only the required code per tab and easier development. Editors like VSCode should now recognize most of the code and make appropriate suggestions.

Electron brings a build system with "Electron forge", the Gulp is completely removed.
The packages:

  • ZIP for all platforms
  • MSI for Windows
  • DMG for MacOS
  • DEB and RPM for Linux

@DzikuVx
Copy link
Member

DzikuVx commented Apr 22, 2024

@Scavanger there seem to be problems running in windows 11

PS C:\Users\Pawel\Projects\inav-configurator> npm start

> [email protected] start
> electron-forge start

✔ Checking your system
✔ Locating application
✔ Loading configuration
✔ Preparing native dependencies: 2 / 2 [0.4s]
✔ Running generateAssets hook


App threw an error during load
Error: "@electron/remote" cannot be required in the browser process. Instead require("@electron/remote/main").
    at Object.<anonymous> (C:\Users\Pawel\Projects\inav-configurator\node_modules\@electron\remote\dist\src\renderer\index.js:14:11)
    at Module._compile (node:internal/modules/cjs/loader:1271:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1326:10)
    at Module.load (node:internal/modules/cjs/loader:1126:32)
    at Module._load (node:internal/modules/cjs/loader:967:12)
    at l._load (node:electron/js2c/asar_bundle:2:13642)
    at Module.require (node:internal/modules/cjs/loader:1150:19)
    at require (node:internal/modules/cjs/helpers:119:18)
    at Object.<anonymous> (C:\Users\Pawel\Projects\inav-configurator\node_modules\@electron\remote\renderer\index.js:1:18)
    at Module._compile (node:internal/modules/cjs/loader:1271:14)
    ```


@DzikuVx
Copy link
Member

DzikuVx commented Apr 22, 2024

image

@DzikuVx DzikuVx added this to the 8.0 milestone Apr 22, 2024
@Scavanger
Copy link
Contributor Author

@DzikuVx Should work now

@DzikuVx
Copy link
Member

DzikuVx commented Apr 22, 2024

Awesome work @Scavanger and I already love debugger ;)

image

@DzikuVx
Copy link
Member

DzikuVx commented Apr 22, 2024

But something changed with Serial connection. I basically can not connect to non-VCP serial port at the moment.
VCP work, COM on the ELRS TX is not responsive

From what I was able to quickly debug, no data is coming from the serial port to the app in this case. Will have to dig when find some time

@DzikuVx
Copy link
Member

DzikuVx commented Apr 23, 2024

I was able to find that non-VCP COM does not gets strange data in

this._serialport.on('data',

It receives 3 bytes of zeros

@DzikuVx
Copy link
Member

DzikuVx commented Apr 23, 2024

OK, I think we will just ignore the fact that it's not working with Express LRS Airport at all. Everything else seems to work just fine

@Scavanger
Copy link
Contributor Author

Scavanger commented Apr 23, 2024

OK, I think we will just ignore the fact that it's not working with Express LRS Airport at all. Everything else seems to work just fine

Unfortunately I don't have any ELRS hardware here to test to help.

Fixed a issue with BLE device chooser and windows installer.

@DzikuVx
Copy link
Member

DzikuVx commented Apr 23, 2024

@Scavanger it seems like it's something ELRS related. TX-RX loose sync when serial connection is opened. It reads like 3 bytes and then they disconnect. Why? Don't know!

@DzikuVx DzikuVx merged commit 82ede9c into iNavFlight:master Apr 23, 2024
@MrD-RC MrD-RC modified the milestones: 8.0, 7.1.1 Apr 24, 2024
@MrD-RC
Copy link
Collaborator

MrD-RC commented Apr 24, 2024

The OSD page preview seems to be broken
image

@MrD-RC
Copy link
Collaborator

MrD-RC commented Apr 24, 2024

There is also a crash in Mission Control when you try to add options to a waypoint.

Mission.Control.Crash.mp4

@breadoven
Copy link
Collaborator

Well I couldn't get it to work at all on Windows 10 after updating from current master.

Ran npm install then npm start and got:

> [email protected] start /mnt/f/INAVDev/inav-configurator
> electron-forge start

/mnt/f/INAVDev/inav-configurator/node_modules/listr2/dist/index.cjs:298
      ...options ?? {}
                  ^

SyntaxError: Unexpected token ?
    at Module._compile (internal/modules/cjs/loader.js:723:23)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (/mnt/f/INAVDev/inav-configurator/node_modules/@electron-forge/cli/dist/electron-forge.js:10:18)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] start: `electron-forge start`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] start script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

Any ideas ?

@Scavanger
Copy link
Contributor Author

There is also a crash in Mission Control when you try to add options to a waypoint.

Confirmed.

The OSD page preview seems to be broken

Strange. Is displayed correctly for me.
Screenshot 2024-04-24 082233

@Scavanger
Copy link
Contributor Author

Scavanger commented Apr 24, 2024

Well I couldn't get it to work at all on Windows 10 after updating from current master.

What is your Node.js version?
It should work with Node v20 LTS.

Edit: See here:
listr2/listr2#689

@OptimusTi
Copy link
Contributor

The "Save and Reboot" button in not working for me in the GPS tab.

@breadoven
Copy link
Collaborator

Well I couldn't get it to work at all on Windows 10 after updating from current master.

What is your Node.js version? It should work with Node v20 LTS.

Edit: See here: listr2/listr2#689

OK I got it to work using Powershell rather than Ubuntu WSL CLI (usual cmd window used to compile INAV stuff). This was after updating to Node v20. Don't know what version was installed before, must have been older since it hasn't been updated for a while. Ubuntu is using Node v10 so needs updating although trying the obvious way threw windows access errors so I'll need to try something else where that's concerned.

Anyway, Powershell works fine which is good enough.

@Scavanger
Copy link
Contributor Author

The "Save and Reboot" button in not working for me in the GPS tab.

Unfortunately I cannot confirm this, it works as expected for me.

@OptimusTi
Copy link
Contributor

OptimusTi commented Apr 24, 2024

The "Save and Reboot" button in not working for me in the GPS tab.

Unfortunately I cannot confirm this, it works as expected for me.

Weird. Same issue in macOS. Ignore. I was using 7.1 hex.

@breadoven
Copy link
Collaborator

Some more apparent bugs (on Windows 10):

  1. It's not possible to enter text in any tab after getting message box appearing in Mission Control (related to altitude being below ground level). However, this is fixed when Configurator window loses then regains focus.

  2. Can't get into the Mixer, Tuning and LED tabs. Console is showing:

F:\INAVDev\inav-configurator\js\msp\MSPHelper.js:1545 Uncaught (in promise) RangeError: Offset is outside the bounds of the DataView
    at DataView.getUint8 (<anonymous>)
    at self.processData (F:\INAVDev\inav-configurator\js\msp\MSPHelper.js:1545:46)
    at Object._dispatch_message (F:\INAVDev\inav-configurator\js\msp.js:257:18)
    at Object.read (F:\INAVDev\inav-configurator\js\msp.js:236:26)
    at publicScope.read_serial (F:\INAVDev\inav-configurator\js\serial_backend.js:505:17)
    at F:\INAVDev\inav-configurator\js\connection\connectionSerial.js:38:17
    at Array.forEach (<anonymous>)
    at SerialPortStream.<anonymous> (F:\INAVDev\inav-configurator\js\connection\connectionSerial.js:37:38)
    at SerialPortStream.emit (node:events:517:28)
    at addChunk (node:internal/streams/readable:335:12)
F:\INAVDev\inav-configurator\js\serial_queue.js:162 MSP data request timed-out: 113
F:\INAVDev\inav-configurator\js\serial_queue.js:162 MSP data request timed-out: 8194
F:\INAVDev\inav-configurator\js\serial_queue.js:162 MSP data request timed-out: 151
F:\INAVDev\inav-configurator\js\serial_queue.js:162 MSP data request timed-out: 8192
F:\INAVDev\inav-configurator\js\serial_queue.js:162 MSP data request timed-out: 113
F:\INAVDev\inav-configurator\js\serial_queue.js:162 MSP data request timed-out: 8194

@Scavanger
Copy link
Contributor Author

Scavanger commented Apr 25, 2024

Some more apparent bugs (on Windows 10):

  1. It's not possible to enter text in any tab after getting message box appearing in Mission Control (related to altitude being below ground level). However, this is fixed when Configurator window loses then regains focus.

Strange error. I will have a look.

  1. Can't get into the Mixer, Tuning and LED tabs. Console is showing:

This happens when the Configurator 8 is connected to 7.X. Firmware.
No Electron error, the minimum supported firmware must be adapted.

@breadoven
Copy link
Collaborator

  1. Can't get into the Mixer, Tuning and LED tabs. Console is showing:

This happens when the Configurator 8 is connected to 7.X. Firmware. No Electron error, the minimum supported firmware must be adapted.

INAV version is 8 but not up to date with the current master:

version

INAV/HGLRCF722 8.0.0 Apr 20 2024 / 10:06:27 (f6386c33)

GCC-13.2.1 20231009

I'll update and see if that helps.

@Scavanger
Copy link
Contributor Author

Scavanger commented Apr 26, 2024

@breadoven

  1. It's not possible to enter text in any tab after getting message box appearing in Mission Control (related to altitude being below ground level). However, this is fixed when Configurator window loses then regains focus.

Found the issue: It`s a common bug in Electron.
Fix/Workaround: #2041

@breadoven
Copy link
Collaborator

I updated to the current master and I still have the same issue with the LED tab ... same Console error message. The other 2 tabs now work OK.

I've also noticed that the Blackbox log can't be downloaded, nothing happens when you click the button. You get the same Console error message as above.

When running from Windows Powershell script I had to use $env:NODE_ENV="development" to set developer mode, the method mentioned in the readme.md didn't work.

And there doesn't seem to be any mouse right click context menu in the CLI any more, cut/copy/paste etc. Pretty sure there used to be.

@Scavanger
Copy link
Contributor Author

Scavanger commented Apr 26, 2024

I updated to the current master and I still have the same issue with the LED tab ... same Console error message. The other 2 tabs now work OK.

Hmm, can you make a video of exactly what you are doing? Unfortunately I can't reproduce it.

I've also noticed that the Blackbox log can't be downloaded, nothing happens when you click the button. You get the same Console error message as above.

I'll have a look.

When running from Windows Powershell script I had to use $env:NODE_ENV="development" to set developer mode, the method mentioned in the readme.md didn't work.

I'll update the readme again anyway, I'll include it.

And there doesn't seem to be any mouse right click context menu in the CLI any more, cut/copy/paste etc. Pretty sure there used to be.

That's right, the context menu is generally deactivated in Electron. Let's see if it can be activated.

@Scavanger
Copy link
Contributor Author

@breadoven
I think I have found and fixed all the points you mentioned. #2041

@bkleiner bkleiner mentioned this pull request Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants