-
Notifications
You must be signed in to change notification settings - Fork 301
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
Chip-id in port selection (VSC-1359) #1195
Conversation
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.
LGTM
Hi @AndriiFilippov Please verify this |
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.
Few validations I think would be good but otherwise LGTM
src/espIdf/serial/serialPort.ts
Outdated
resolve(choices); | ||
} else { | ||
reject(new Error("No serial ports found")); | ||
serialPort.chipType = chipIdString2[1].trim(); |
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.
Could we add validation that match exists and it has at least 2 elements here ?
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.
good idea!
@kolipakakondal @brianignacio5 hi ! Tested under: Do see chip info while multiple boards are connected. LGTM 👍 |
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.
LGTM
[## Description
Switcxhed serial port listing to node serialport and providing additional information about connected board chip ( if the board is ESP).
Type of change
Steps to test this pull request
Provide a list of steps to test changes in this PR and required output
Click on "Select port to use"
Wait for ~2 seconds
Observe results.
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Dependent components impacted by this PR:
Checklist