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
Fix PortHandler #2932
Fix PortHandler #2932
Conversation
46e88c2
to
85bb267
Compare
This comment has been minimized.
This comment has been minimized.
1458fe7
to
f6440e3
Compare
This comment has been minimized.
This comment has been minimized.
AUTOMERGE: (FAIL)
|
b648c3e
to
2cc77a4
Compare
This comment has been minimized.
This comment has been minimized.
2cc77a4
to
e21e750
Compare
This comment has been minimized.
This comment has been minimized.
e21e750
to
cbe6907
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's ok to me, only one thing I don't like, but approved.
@@ -267,7 +267,7 @@ PortHandler.selectPort = function(ports) { | |||
const pathSelect = ports[i].path; | |||
const isWindows = (OS === 'Windows'); | |||
const isTty = pathSelect.includes('tty'); | |||
const deviceRecognized = portName.includes('STM') || portName.includes('CP210'); | |||
const deviceRecognized = portName.includes('STM') || portName.includes('CP210') || portName.startsWith('SPR'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like too much to have exclusions for one vendor. I think SPR must include STM in the name. I suppose is a customized variant of STM right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we should have a policy on naming conventions. STM32H750 betaflight (legacy) target is shown as /dev/tty/ACM0 - SPRacingH7EXTREME
src/js/port_handler.js
Outdated
ConfigStorage.get('showVirtualMode', res => self.showVirtualMode = res.showVirtualMode); | ||
ConfigStorage.get('showAllSerialDevices', res => self.showAllSerialDevices = res.showAllSerialDevices); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't we move to sync api now?
ConfigStorage.get('showVirtualMode', res => self.showVirtualMode = res.showVirtualMode); | |
ConfigStorage.get('showAllSerialDevices', res => self.showAllSerialDevices = res.showAllSerialDevices); | |
self.showVirtualMode = ConfigStorage.get('showVirtualMode').showVirtualMode); | |
self.showAllSerialDevices = ConfigStorage.get('showAllSerialDevices').showAllSerialDevices; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes here: #2636 (will do afterwards when rebasing).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet, yeah that one been dangling for a while now 🙈 I always hope it's been merged in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥳 Finally got it approved. Rebased now.
8a26a1f
to
0b8f3d8
Compare
This comment has been minimized.
This comment has been minimized.
0b8f3d8
to
dbe33ae
Compare
This comment has been minimized.
This comment has been minimized.
753e311
to
912231f
Compare
This comment has been minimized.
This comment has been minimized.
912231f
to
503e129
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Do you want to test this code? Here you have an automated build: |
@McGiverGim found an issue with some GUI function continuous being called.
Fixes and improves following issues:
Due to changes it was needed to rethink how to enable the auto-detect button.