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

Upgrade serialport to latest stable #390

Closed
beriberikix opened this issue Nov 1, 2017 · 7 comments
Closed

Upgrade serialport to latest stable #390

beriberikix opened this issue Nov 1, 2017 · 7 comments
Assignees
Labels

Comments

@beriberikix
Copy link

Looking at the package.json we're running v4.x.x of node-serialport, while the latest stable version is v6.x.x. I propose we evaluate upgrading to the latest stable build. Part of the motivation to upgrade would be to take advantage of the Windows' build's performance and reliability improvements over the past 2 major versions. I've read about and have experienced the load time difference between macOS and win and speculate that upgrading will help.

P.S. I believe this is a non-trivial amount of work and will likely break things 😄
P.P.S As of v5.x.x, support for node v0.10 was dropped. We're currently targeting v0.10.40, which was released in 2015 and has no LTS. v8.9.0 is the most recent LTS release of node.

@monkbroc
Copy link
Member

monkbroc commented Nov 1, 2017

I wasn't aware of the engines field in package.json. I doubt the CLI works on Node version below 4 at this point.

@reconbot
Copy link

There are so many bugs fixed in SerialPort between 4 and 6 it's unbelievable. Please please please update. I'm more than happy to help debug issues and talk through design.

@monkbroc
Copy link
Member

monkbroc commented Nov 28, 2017

Thanks for the offer @reconbot.

The biggest issues I've hit is on Windows.

Flashing a file through the YModem protocol on the serial port now fail with Reading from COM port (ReadIOCompletion): Operation aborted. Our YModem implementation uses the port.on('data', handler) to receive chunks of data.

Here's what the log of the interactions look like:

  serialport:binding:auto-detect loading WindowsBinding +0ms
  serialport:main .list +0ms
  serialport:main opening path: COM16 +40ms
  serialport:bindings open +0ms
  serialport:main opened path: COM16 +14ms
  serialport:main _write 1 bytes of data +2ms
  serialport:bindings write 1 bytes +16ms
  serialport:main _read reading +5ms
  serialport:bindings read +5ms
  serialport:main binding.write write finished +13ms
  serialport:main binding.read finished +86ms
  serialport:main _read reading +4ms
  sending file: H:\Programming\Particle\blink\photon_firmware_1481929407037.bin
serialport:bindings read +102ms
send file header
write 0 <Buffer 01 00 ff> 133
  serialport:main #flush +72ms
  serialport:bindings flush +72ms
  serialport:main binding.flush finished +4ms
  serialport:main _write 133 bytes of data +2ms
  serialport:bindings write 133 bytes +5ms
  serialport:main binding.read error Error: Reading from COM port (ReadIOCompletion): Operation aborted +3ms
  serialport:main disconnected Error: Reading from COM port (ReadIOCompletion): Operation aborted +1ms
  serialport:main #close +1ms
  serialport:bindings close +4ms
  serialport:main _read queueing _read for after open +1ms
  serialport:main binding.write write finished +0ms
  serialport:main binding.close finished +1s

Doing the serial setup now fails with Error: Writing to COM port (GetOverlappedResult): Operation aborted. The code is here.

Do you have a hint of what these errors are and why we'd start seeing them with serialport 6?

All these commands still work on Linux (I haven't tried on Mac yet).

@monkbroc
Copy link
Member

I can get rid of the ReadIOCompletion Operation aborted errors that were closing the serial port by removing the serialPort.flush calls. For example here we had flush followed by write and drain. Is it a bug that flush disconnects the port on Windows?

@reconbot
Copy link

I mean... absolutely yes

@monkbroc
Copy link
Member

I'll make a tiny program to reproduce the behavior I saw and open an issue on the serialport repo. Thanks for chiming in @reconbot!

@reconbot
Copy link

reconbot commented Nov 28, 2017 via email

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

No branches or pull requests

3 participants