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

port.write() sometimes hangs on Windows #1403

Closed
gniezen opened this issue Nov 22, 2017 · 12 comments
Closed

port.write() sometimes hangs on Windows #1403

gniezen opened this issue Nov 22, 2017 · 12 comments

Comments

@gniezen
Copy link

gniezen commented Nov 22, 2017

  • SerialPort Version: 6.0.4

  • NodeJS Version: 7.9.0 (Electron)

  • Operating System and Hardware Platform: Windows 10 32-bit

  • Have you checked the right version of the api docs?: Yes

  • Are you having trouble installing and you checked the Installation Special Cases docs? No

  • Are you using Electron and have you checked the Electron Docs?: Yes

Summary of Problem

After upgrading from v4.0.7 to v6.0.4, we are experiencing intermittent cases of port.write() not returning on Windows. macOS works fine. v5.0.0 works fine too, but v6.0.0 produces a Writing to COM port (WriteFileEx): The parameter is incorrect error. v6.0.4 continues past the stage where v6.0.0 errored out, but stalls later in the process, where it does not produce an error but also does not return, so the event loop hangs. Maybe this is still related to #1363?

Steps and Code to Reproduce the Issue

It seems to occur when there are multiple writes. My guess is that the first write is still pending when we do the next write, but shouldn't v6.0.4 return a "pending" error in this instance? And why is this only an issue on Windows?

I've tried port.drain() after every port.write(), but that doesn't resolve the issue either. If I put the port.drain in front of the port.write(), it hangs while draining.

@reconbot
Copy link
Member

Cc @dustmop

@dustmop
Copy link
Contributor

dustmop commented Nov 24, 2017

Could you provide some minimal code example that triggers the issue?

Windows is using a completely different code path for Read and Write, which was rewritten for 6.0.0, which is why it isn't a problem on mac.

@gniezen
Copy link
Author

gniezen commented Nov 27, 2017

@dustmop I wish I could, but I haven't been able to identify the exact sequence of events that triggers it. I can reliably reproduce the issue, but that wouldn't help you as even though our code is open-source, the devices I'm connecting to are insulin pumps, which are not easy to obtain.

I can point you at the code that we use to do serial read/write (https://github.com/tidepool-org/chrome-uploader/blob/15838b3d834cc1e422fb26bd1b399b2f345a2448/lib/serialDevice.js) in case we're using the API in an unexpected way?

@gniezen
Copy link
Author

gniezen commented Dec 13, 2017

@dustmop Is there any way in which I can help this along by providing debug logs etc.? I just discovered that there is a bugfix in v6 (#1279) that we need for Linux support, so would love to be able to upgrade to v6 if we can figure out what is going on here.

@reconbot
Copy link
Member

reconbot commented Feb 5, 2018

I'm going to close this issue due to it's age, but if you'd like to continue with it feel free to comment and we'll reopen.

@reconbot reconbot closed this as completed Feb 5, 2018
@gniezen
Copy link
Author

gniezen commented Feb 15, 2018

I just tested with v6.1.0 and the issue still exists.

@gniezen
Copy link
Author

gniezen commented Apr 20, 2018

@reconbot Is it possible that we can reopen this issue? I have just tested with v6.2.0 and the problem still exists. Looks like duplicate of this issue was reported as #1434.

@reconbot reconbot reopened this Apr 20, 2018
@reconbot
Copy link
Member

What hardware are you talking to? What's the driver for the serialport?

@gniezen
Copy link
Author

gniezen commented Apr 20, 2018

I'm talking to insulin pumps, and it's specifically Tandem and Animas pumps that are causing issues. The Tandem reports as a STMicroelectronics STM32F407, and the Animas pump is using a Prolific PL2303 chip (using the Prolific virtual serial port driver).

@gniezen
Copy link
Author

gniezen commented May 3, 2018

@reconbot I think I found the issue! When there is a comms timeout, we flush the buffer and retry. According to #1409 flush() is broken on Windows. If I comment out flush() it seems to resolve the issue.

I'm still running some more tests to see this resolves it for all devices on various Windows versions. If successful, I think we can close out this issue as it seems to be a duplicate of #1409.

@reconbot
Copy link
Member

reconbot commented May 3, 2018

oh this is great! I've been trying to reproduce / read through code paths that could cause this.

@gniezen
Copy link
Author

gniezen commented Jul 12, 2018

Closing this issue as it is indeed due to flush() being broken on Windows.

@gniezen gniezen closed this as completed Jul 12, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jan 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

3 participants