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

Add WSL serial support #224

Merged
merged 2 commits into from
Dec 25, 2020

Conversation

brianignacio5
Copy link
Collaborator

Add code to support get serial ports, flash and monitor on WSL.

Tested on Windows 10 19041 with WSL 2 with Microsoft Ubuntu 20.04 distribution for WSL.

@brianignacio5 brianignacio5 added the enhancement New feature or request label Dec 1, 2020
@brianignacio5 brianignacio5 requested a review from pwmb December 1, 2020 09:05
@brianignacio5 brianignacio5 self-assigned this Dec 1, 2020
src/espIdf/serial/serialPort.ts Outdated Show resolved Hide resolved
let result = "";
try {
result = await execChildProcess(
`powershell.exe -Command "$(cat wsl_get_serial_list.ps)"`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this compatible with both old wsl and wsl2 ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also source for this file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add the source in the file ? I though the code is small and general enough.

The .NET Framework 2.0 defines the System.IO.Ports.SerialPort and should be independent of WSL version.

Will test it soon.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its better to add source, in case in future we don't remember what code we wrote to generate this bin.

Also it will be much easier for git to track

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the source in wsl_get_serial_list.ps

@pwmb pwmb added this to the next milestone Dec 4, 2020
@pwmb pwmb self-requested a review December 18, 2020 06:44
Copy link
Contributor

@pwmb pwmb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, tested on win 10

@zebulon501
Copy link

I understand this changes ease the access to some people, but it has a few limitations IMHO:

  • Native Windows processes are launched without any warning;
  • Python must be installed on Windows, along with pySerial (missing dependency by the way);
  • Specifying an RFC2217 serial port doesn't prevent using this mechanism;
  • WSL2 will soon offer USBIP (see https://devblogs.microsoft.com/commandline/connecting-usb-devices-to-wsl/), thus bringing native WSL USB ports, like /dev/ttyUSB0, which is unusable due to this mechanism;
  • Using devcontainer is also broken: this configuration is detected as WSL but has no access to Windows binaries.

This behavior (using powershell.exe and path substitution) shall only be used when the user specifies idf.port as COMx and running under Linux. Detecting WSL using the kernel information is not working (docker use case), better use the environment variable WSL_DISTRO_NAME.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants