Refactor port handler #3984
Refactor port handler #3984
Conversation
✅ Deploy Preview for origin-betaflight-app ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
McGiverGim
force-pushed
the
refactor_port_handler
branch
from
c699b1d to
e07947a
Compare
McGiverGim
force-pushed
the
refactor_port_handler
branch
4 times, most recently
from
1a33f83 to
9fe595c
Compare
McGiverGim
force-pushed
the
refactor_port_handler
branch
from
9fe595c to
d457473
Compare
|
I think this is finally ready to be reviewed... |
|
Please also check
Putting a ? on line 222 and 231 would solve it
|
|
Fixed as per review, and tested the flashing with the fix suggested. Thanks! |
|
|
The auto connect problem is a problem with switchery, the library that we use to show the checboxes. It does not refresh when vue changes the state of it. I don't know what will be a good fix for that, but it's out of this PR. |
|
Pushed a change to not auto-connect for virtual/manual ports.
@nerdCopter I think there must be some other variable in game. I can't reproduce it. In my case it's populated, and selected, and, if I have auto-connect enabled, it connects with it. I will continue testing but until now, no luck... |
|
Added a new commit, that fixes the auto-connect state... finally it was not a switchery problem, it was a simple bug... |
The checkbox is "hidden" under the switchary library, so move to the parent to be able to show it.
|
@nerdCopter I think I've reproduced it. It happens when the ports has permissions (it appears in the combo) but you select I can't find my device and selects it. I will look into it. |
|
@nerdCopter I think I fixed it in the latest commit. Can you test it again? |
| } | ||
| return null; | ||
| return newPermissionPort; |
There was a problem hiding this comment.
newPermissionPort remains null ?
There was a problem hiding this comment.
When the user does not select any port in the request permission yes. It's a way to know if it has selected one or not.
There was a problem hiding this comment.
Cannot find where newPermissionPort is set (only on line 97) ?
There was a problem hiding this comment.
True, at some change I lost it... good catch, I will fix it asap
There was a problem hiding this comment.
@haslinghuis fixed!!!
|
|
|
This is an intent to simplify the
port_handler.js. The way it is now, is a very complicated structure produced from patch after patch, and using callback handlers to do the things, but now we have events.This let's the file in a clean way to add the next things missing. All the callback system can be removed by simply sending events and listen in the other place (for example, when detected a DFU port send an event and in listen to it in the firmare flasher to start to flash).
The changes are big, and GitHub does not help in the merge view, it shows a lot more changes than what really are, because I moved the code from one place to another.