Make port_handler work with PWA #3958

McGiverGim · 2024-05-13T17:26:27Z

This makes port_handler work again. This fixes several problems with PWA, like the auto-connect, save and reboot and the list of ports in the combo. I'm sure some things can be done better but it's difficult maintaining nwjs compatibility.

Some things to note:

Only serial devices, the USB devices are disabled, so no DFU. I hope they will be fixed soon after Add DFU support for PWA #3949
The first time, we need to give "permission" to access the FC. I've added at the combo, that is where usually people search, an option "I don't find my device", when selected it will ask for the permission and since then, this will not be needed anymore (at least, for this kind of FCs). Maybe we can add a simple '+' button just next the combo, but for the moment this works.
The serial API don't have information about port or device name. I have added a simply counter like port, and "Betaflight Flight Controller" as name temporary. What we have is the manufacturer and productid, I can generate a more or less "correct" text from here. Another option, for the future, is forget about the Serial API and use the USB API to do the same, in this way we have a lot more information.
The nw.js version is currently broken. I have tried to maintain compatibility but I had no luck with it ;) I will fix it later.

For the rest, I pushed the PR to let test if this fixes the CLI and Presets problems.

netlify · 2024-05-13T17:26:54Z

✅ Deploy Preview for origin-betaflight-app ready!

Name	Link
🔨 Latest commit	`5c40efc`
🔍 Latest deploy log	https://app.netlify.com/sites/origin-betaflight-app/deploys/664659da6573520008d7f2ed
😎 Deploy Preview	https://deploy-preview-3958.dev.app.betaflight.com
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

src/js/port_handler.js

chmelevskij

Glad that this works, but targetting vue componets with jquery is a slippery slope and can lead to unexpected results.

In order to connect vue to the rest of the things it's better to use something like event bus.

We have a basic example with config storage
https://github.com/betaflight/betaflight-configurator/blob/18eb8dd1b20a721fd1ec082d1ac306bb1f9b6d6d/src/components/eventBus.js

betaflight-configurator/src/js/ConfigStorage.js

Line 48 in 18eb8dd

EventBus.$emit(`config-storage:set`, 'element');

betaflight-configurator/src/components/port-picker/PortsInput.vue

Lines 81 to 86 in 18eb8dd

    
               mounted() { 
        
                 EventBus.$on('config-storage:set', this.setShowVirtual); 
        
               }, 
        
               destroyed() { 
        
                 EventBus.$off('config-storage:set', this.setShowVirtual); 
        
               },

chmelevskij · 2024-05-13T18:53:31Z

src/js/webSerial.js

+        navigator.serial.addEventListener("connect", device => {
+            this.dispatchEvent(new CustomEvent("addedDevice", { detail: device.target }));
+        });
+
+        navigator.serial.addEventListener("disconnect", device => {
+            this.dispatchEvent(new CustomEvent("removedDevice", { detail: device.target }));
+        });


Wondering if we need this here? We are not using the event.detail later on and all this is doing is proxying the events.

Could we just use navigator.serial directly in the port_handler under the isWeb branch?

I think is good to "isolate" the webserial. If we have a webserial.js to isolate all the serial operations, then it's responsible of indicating all the important events, like sending datan, new devices, etc.
I think more, it will be good to send events like "connected", "disconnected", etc. Will make a lot more simple the rest of the code, but maybe for other PR.
About the event detail, I will remove it, now we don't use it, but maybe we can act different in a future depending on the type of device connected or similar. My idea was to "add" or "remove" the device from the list in the port handler without calling the "getPorts" again, but the code is not ready for that, at least while we maintain compatibility with nw.js.

McGiverGim · 2024-05-14T20:17:43Z

@chmelevskij two doubts about the EventBus code. The Config Storage is emitting the message, with the text 'element' and not the value of the variable. It's that right?
The listeners, will listen for all the changes in the Config Storage, not only for the show virtual element, so all the listeners will read the storage after any change.
It's that true?

McGiverGim · 2024-05-15T13:51:39Z

Pushed changes. I was trying to do a complete refactor, and I did it, but the serial_backend.js was not happy. I will continue working in the complete refactor that will make this code a lot simpler.
But now, I have pushed a version that more or less works, and breaks totally the nw.js connection. If the code is more or less ok, and the basic serial connection works, please merge this PR ASAP, is better to continue fixing things with little PRs than making this bigger.

nerdCopter · 2024-05-15T14:07:18Z

great work!

3f5c126d observations:
it auto-connects by default, maybe i don't want that.
and when it does, it shows manual connection before and after disconnect. then i have to select Betaflight STM Electronics for a second connect.

nerdCopter

approving on basis of future fixes/refactors.
will need rebase on the DFU mode fix.

McGiverGim · 2024-05-15T14:36:25Z

The auto-connect switch is missing. I don't have modified the behaviour, but of course is one of the things missing.
About the "manual"... If you disconnect your FC, or reset it, it shows manual because the device has disappear of the system, and it shows the default one (manual). Maybe in the future, we can add some "waiting" or similar when we are restarting the FC, but it's at the bottom of the list right now.

haslinghuis

Perhaps DEFAULT_PORT should be Make selection - cannot connect without choosing first.
If device already has permission it should be pre-selected (after disconnect - connect)
After selecting find my device it reverts to manual. So connect does not work.

McGiverGim · 2024-05-15T15:52:33Z

The default port, we can add what we want, it always tries to show the "best" device detected. For this reason if there is not device, manual is not a bad option.
In my tests, the device with permissions is preselected. Can you give some steps to see if I can reproduce?
@haslinghuis Fixed the problem when requesting permission.

McGiverGim · 2024-05-15T15:59:11Z

Fixed some sonar issues too...
The baud selection is broken too, but I think it's not important at this point.

src/js/port_handler.js

src/components/init.js

src/components/port-picker/PortsInput.vue

src/js/webSerial.js

McGiverGim · 2024-05-16T05:31:56Z

@haslinghuis rebased!!

McGiverGim · 2024-05-16T15:11:59Z

Added a little commit to hide the baud selection in port-picker when virtual is selected

McGiverGim · 2024-05-16T15:45:06Z

Added new commit to show the port override option for "manual". It does not use it, but now is ready for it.

McGiverGim · 2024-05-16T15:54:22Z

The latest one, the state of the port was not correct when loading the page, only reacted to changes in the options checkbox. Now it loads the value when loading the component too.

haslinghuis · 2024-05-16T16:50:01Z

Fresh build. After I can't find my device - get the request permission dialog.

After giving permission manual is selected. This should be select the board given permission to.
Portpicker shows:

Doing that again we get:

Note all Betaflight STMicro Electronics are one - i.e. all of them connect.
So guess this should not be added but replaced with new permission.

McGiverGim · 2024-05-16T16:58:41Z

Ok, I think I can look at it later...

McGiverGim · 2024-05-16T17:27:47Z

@haslinghuis can you test this new version?

haslinghuis · 2024-05-16T17:33:32Z

@McGiverGim yes that works. Only thing left is selecting the firmware port if detected instead of manual.

McGiverGim · 2024-05-16T17:37:43Z

I can't reproduce it. In my case, it detects the betaflight port perfectly, each time. Maybe you're using Linux? My VM with Linux it's dead, I can't try right now.

haslinghuis · 2024-05-16T17:43:13Z

On startup manual is selected - STM32 entry is in portpicker list.
When using can't find board - give permission - also selects manual. Not the STM32 entry.

Screencast.from.16-05-24.19.42.36.webm

McGiverGim · 2024-05-16T17:47:01Z

Yes, I understand the problem, but in my Windows it is detected correctly each time. I suspect it has something to do with Linux, I will try to recover the VM...

nerdCopter · 2024-05-16T18:28:22Z

Debian 12; yarn dev; eb8c0dd9.

what is the expected behavior here?
if i select can't find my device, then connect it, i get stuck at connecting. and cannot disconnect (require F5 to reload)
but if i select Betaflight STM then it connects.

sonarcloud · 2024-05-16T19:10:07Z

Quality Gate failed

Failed conditions
8.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

McGiverGim · 2024-05-16T19:11:32Z

@haslinghuis fixed.
@nerdCopter can you try again with this version?

nerdCopter · 2024-05-16T19:22:26Z

5c40efca works well 🚀 https://youtu.be/i2D0beUe5aY

McGiverGim added the Don't merge label May 13, 2024

haslinghuis assigned McGiverGim May 13, 2024

haslinghuis added this to the 11.0 milestone May 13, 2024

haslinghuis added Work in Progress RN: IMPROVEMENT labels May 13, 2024

chmelevskij reviewed May 13, 2024

View reviewed changes

src/js/port_handler.js Outdated Show resolved Hide resolved

chmelevskij requested changes May 13, 2024

View reviewed changes

chmelevskij reviewed May 13, 2024

View reviewed changes

haslinghuis mentioned this pull request May 13, 2024

PWA project #3960

Open

33 tasks

McGiverGim force-pushed the port_handler_pwa branch from c93c1b5 to 3f5c126 Compare May 15, 2024 13:49

nerdCopter approved these changes May 15, 2024

View reviewed changes

haslinghuis approved these changes May 15, 2024

View reviewed changes

McGiverGim force-pushed the port_handler_pwa branch 2 times, most recently from 3b0c55f to 5b7f7c8 Compare May 15, 2024 15:49

McGiverGim force-pushed the port_handler_pwa branch from 5b7f7c8 to f3a9541 Compare May 15, 2024 15:58

haslinghuis requested a review from chmelevskij May 15, 2024 16:00

McGiverGim force-pushed the port_handler_pwa branch from f3a9541 to 5e605c2 Compare May 15, 2024 16:01

McGiverGim removed Don't merge Work in Progress labels May 15, 2024

chmelevskij reviewed May 15, 2024

View reviewed changes

src/js/port_handler.js Show resolved Hide resolved

chmelevskij reviewed May 15, 2024

View reviewed changes

src/components/init.js Show resolved Hide resolved

chmelevskij reviewed May 15, 2024

View reviewed changes

src/components/port-picker/PortsInput.vue Outdated Show resolved Hide resolved

chmelevskij reviewed May 15, 2024

View reviewed changes

src/js/webSerial.js Outdated Show resolved Hide resolved

McGiverGim added 4 commits May 16, 2024 07:31

Make port_handler work with PWA

a35e630

Modify the port_handler more the Vue way

1d0527e

Fixes after review

db85a9a

Fix request permission option not being deselected

a9c862a

McGiverGim force-pushed the port_handler_pwa branch from d36f99b to a9c862a Compare May 16, 2024 05:31

Hide baud selection in port picker if virtual port

389c590

Added port override option for manual

eb736b2

Fix virtual port state when loading the page

4a15f84

Fix request permission adds the same device several times

eb8c0dd

Fix automatic selection of device under Linux

5c40efc

haslinghuis approved these changes May 16, 2024

View reviewed changes

nerdCopter approved these changes May 16, 2024

View reviewed changes

chmelevskij approved these changes May 17, 2024

View reviewed changes

haslinghuis merged commit fce0c83 into betaflight:master May 17, 2024
6 of 7 checks passed

haslinghuis added the PWA label May 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make port_handler work with PWA #3958

Make port_handler work with PWA #3958

McGiverGim commented May 13, 2024

netlify bot commented May 13, 2024 •

edited

chmelevskij left a comment

chmelevskij May 13, 2024

McGiverGim May 14, 2024

McGiverGim commented May 14, 2024

McGiverGim commented May 15, 2024

nerdCopter commented May 15, 2024 •

edited

nerdCopter left a comment

McGiverGim commented May 15, 2024

haslinghuis left a comment

McGiverGim commented May 15, 2024

McGiverGim commented May 15, 2024

McGiverGim commented May 16, 2024

McGiverGim commented May 16, 2024

McGiverGim commented May 16, 2024

McGiverGim commented May 16, 2024

haslinghuis commented May 16, 2024 •

edited

McGiverGim commented May 16, 2024

McGiverGim commented May 16, 2024

haslinghuis commented May 16, 2024

McGiverGim commented May 16, 2024

haslinghuis commented May 16, 2024 •

edited

McGiverGim commented May 16, 2024

nerdCopter commented May 16, 2024

sonarcloud bot commented May 16, 2024

McGiverGim commented May 16, 2024

nerdCopter commented May 16, 2024

	mounted() {
	EventBus.$on('config-storage:set', this.setShowVirtual);
	},
	destroyed() {
	EventBus.$off('config-storage:set', this.setShowVirtual);
	},

Make port_handler work with PWA #3958

Make port_handler work with PWA #3958

Conversation

McGiverGim commented May 13, 2024

netlify bot commented May 13, 2024 • edited

✅ Deploy Preview for origin-betaflight-app ready!

chmelevskij left a comment

Choose a reason for hiding this comment

chmelevskij May 13, 2024

Choose a reason for hiding this comment

McGiverGim May 14, 2024

Choose a reason for hiding this comment

McGiverGim commented May 14, 2024

McGiverGim commented May 15, 2024

nerdCopter commented May 15, 2024 • edited

nerdCopter left a comment

Choose a reason for hiding this comment

McGiverGim commented May 15, 2024

haslinghuis left a comment

Choose a reason for hiding this comment

McGiverGim commented May 15, 2024

McGiverGim commented May 15, 2024

McGiverGim commented May 16, 2024

McGiverGim commented May 16, 2024

McGiverGim commented May 16, 2024

McGiverGim commented May 16, 2024

haslinghuis commented May 16, 2024 • edited

McGiverGim commented May 16, 2024

McGiverGim commented May 16, 2024

haslinghuis commented May 16, 2024

McGiverGim commented May 16, 2024

haslinghuis commented May 16, 2024 • edited

McGiverGim commented May 16, 2024

nerdCopter commented May 16, 2024

sonarcloud bot commented May 16, 2024

Quality Gate failed

McGiverGim commented May 16, 2024

nerdCopter commented May 16, 2024

netlify bot commented May 13, 2024 •

edited

nerdCopter commented May 15, 2024 •

edited

haslinghuis commented May 16, 2024 •

edited

haslinghuis commented May 16, 2024 •

edited