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

No close event emitted when a device is physically removed for an open port. #2043

Closed
t0mpa opened this issue Mar 4, 2020 · 24 comments · Fixed by #2120
Closed

No close event emitted when a device is physically removed for an open port. #2043

t0mpa opened this issue Mar 4, 2020 · 24 comments · Fixed by #2120

Comments

@t0mpa
Copy link

t0mpa commented Mar 4, 2020

Summary of Problem

(Please answer all 3)

  • What are you trying to do?
    Detect when a device is physically disconnected from the usb-port.

  • What happens?
    When a physical device is unplugged there's no close event emitted with [email protected] (tested with all versions between 8.0.1 to 8.0.7)
    Doing the same thing with [email protected] it works fine.

Output from the provided code below:

Test with node v13.9.0 and node [email protected]

% node app.js

Port opened
Port writable: true
Port open: true
Port writable: true
Port open: true

--- Unplugged the device here! The closed event is NOT being emitted!!! ---

Port writable: true
Port open: true
Port writable: true

--- Sending a write command to the device after it is unplugged, will emit an error (of course) and then the port close event is emitted! It shall not be necessary to write to the port to detect that a physical device has been removed from the system. --- 

Error:  ENXIO: no such device or address, write
Port closed!
Port writable: true
Port open: false

Same code and a test with node v12.6.0 and node [email protected]

% node app.js

Port opened
Port writable: true
Port open: true
Port writable: true
Port open: true

--- Unplugged the device here! The closed event is emitted instantly as it should. ---

Port closed!
Port writable: true
Port open: false
Port writable: true
Port open: false
  • What should have happened?
    The close event shall be emitted when the physical device is disconnected, without delay and without the need to write to the port. It's no longer visible in the system, verified with ls -l /dev/tty.*

It shall work like it did in node [email protected].

Code to Reproduce the Issue

const SerialPort = require("serialport");

const port = new SerialPort("/dev/tty.usbmodems2731______1", {
  BaudRate: 115200,
  autoOpen: false
});

port.on("readable", function() {
  var data = port.read();
  console.log("Data:", data.toString());
});

port.on("close", function() {
  console.log("Port closed!");
});

port.on("open", function() {
  console.log("Port opened");
  setInterval(() => {
    console.log("Port writable:", port.writable);
    console.log("Port open:", port.isOpen);
  }, 1000);
});

port.on("error", function(error) {
  if (error) {
    return console.log("Error: ", error.message);
  }
});

port.open(function(error) {
  if (error) {
    return console.log("Error opening port: ", error.message);
  }
});

Versions, Operating System and Hardware

[email protected]
Node.js v13.9.0
Mac OS X 10.15.3
MacBook Pro 13

@NicholasBertazzon
Copy link

Same problem on Windows 10

@gcarlsson
Copy link

I can confirm the issue on macOS v10.15.1

@kevingtzz
Copy link

I have the same issue.

Platform: MacOS v10.15.2
Hardware: Arduino Mega, Xbee explorer
Node version: 13.8.0
serialport version: 8.0.7

Close event is not being triggered.

@EarthyOrange
Copy link

We are periodically checking the status of the device with get as a work around for this issue.

But this API behaves differently on different platforms:

  • Windows: the CB is called with updated modem status on disconnection or connection.
  • Mac: the CB is called with an error on disconnect, but on a good connection the modem status never gets updated (All the flags are set to false).

@ijager
Copy link

ijager commented Apr 16, 2020

For me the close event still fires on 8.0.4 but not on >8.0.5.

However it was very tricky to downgrade because npm kept installing 8.0.7, ignoring the lower version. In the end I pasted the package-lock.json entries for [email protected] from an older project and installed them with npm ci.

ijager added a commit to JitterCompany/SerialWatch that referenced this issue Apr 17, 2020
See serialport/node-serialport#2043

No close event is emitted when physically removing the serial device.
@thetroy
Copy link

thetroy commented Apr 27, 2020

It seems like responsibilities shifted around between versions and I don't fully follow that, so I can't propose a fix. Perhaps what I did find will help with fixing.

when you unplug the cable, this code (in stream):

      debug('binding.read', `error`, err)
      if (!err.canceled) {
        this._disconnected(err)
      }

https://github.com/serialport/node-serialport/blob/master/packages/stream/lib/index.js#L388-L391
...no longer calls _disconnected like it did in 6.2.2. This suggests that canceled should not have been set.

canceled is set in the catch in unixRead:

    const disconnectError =
      err.code === 'EBADF' || // Bad file number means we got closed
      err.code === 'ENXIO' || // No such device or address probably usb disconnect
      err.code === 'UNKNOWN' ||
      err.errno === -1 // generic error

    if (disconnectError) {
      err.canceled = true
      logger('disconnecting', err)
    }

https://github.com/serialport/node-serialport/blob/master/packages/bindings/lib/unix-read.js#L42-L51

Note: it is also set in 2 other places above that, likely each needs to be double-checked.

I also found this comment that relates (in binding-abstract)

The in progress reads must error when the port is closed with an error object that has the property canceled equal to true. Any other error will cause a disconnection.

...there is a similar comment for write. Perhaps that comment could be clarified once someone wraps their head around this.

@SeBassTian23
Copy link

The issue persists in the latest version for me:

Platform: MacOS v10.15.4
Hardware: Teensy 3.2
Node version: 12.13.0
Serialport version: 9.0.0

Close event is not being triggered after physical disconnect

@SeBassTian23
Copy link

For me the close event still fires on 8.0.4 but not on >8.0.5.

However it was very tricky to downgrade because npm kept installing 8.0.7, ignoring the lower version. In the end I pasted the package-lock.json entries for [email protected] from an older project and installed them with npm ci.

I tried the version 8.0.4 but unfortunately it is not working for me:
Platform: MacOS v10.15.4
Hardware: Teensy 3.2
Node version: 12.13.0

@SeBassTian23
Copy link

I'm trying to downgrade to version 7.1.5 which worked for me in the past, but unfortunately I run into this error now:

> prebuild-install --tag-prefix @serialport/bindings@ || node-gyp rebuild
    
      CXX(target) Release/obj.target/bindings/src/serialport.o
    
                    errorOut=prebuild-install WARN install No prebuilt binaries found (target=8.2.4 runtime=electron arch=x64 libc= platform=darwin)
    ../src/serialport.cpp:329:14: error: no matching member function for call to 'Set'
        results->Set(Nan::New<v8::String>("cts").ToLocalChecked(), Nan::New<v8::Boolean>(data->cts));
        ~~~~~~~~~^~~
    /Users/kuhlgert/.electron-gyp/8.2.4/include/node/v8.h:3611:37: note: candidate function not viable: requires 3 arguments, but 2 were provided
      V8_WARN_UNUSED_RESULT Maybe<bool> Set(Local<Context> context,
                                        ^
    /Users/kuhlgert/.electron-gyp/8.2.4/include/node/v8.h:3614:37: note: candidate function not viable: requires 3 arguments, but 2 were provided
      V8_WARN_UNUSED_RESULT Maybe<bool> Set(Local<Context> context, uint32_t index,
                                        ^
    ../src/serialport.cpp:330:14: error: no matching member function for call to 'Set'
        results->Set(Nan::New<v8::String>("dsr").ToLocalChecked(), Nan::New<v8::Boolean>(data->dsr));
        ~~~~~~~~~^~~
    /Users/kuhlgert/.electron-gyp/8.2.4/include/node/v8.h:3611:37: note: candidate function not viable: requires 3 arguments, but 2 were provided
      V8_WARN_UNUSED_RESULT Maybe<bool> Set(Local<Context> context,
                                        ^
    /Users/kuhlgert/.electron-gyp/8.2.4/include/node/v8.h:3614:37: note: candidate function not viable: requires 3 arguments, but 2 were provided
      V8_WARN_UNUSED_RESULT Maybe<bool> Set(Local<Context> context, uint32_t index,
                                        ^
    ../src/serialport.cpp:331:14: error: no matching member function for call to 'Set'
        results->Set(Nan::New<v8::String>("dcd").ToLocalChecked(), Nan::New<v8::Boolean>(data->dcd));
        ~~~~~~~~~^~~
    /Users/[username]/.electron-gyp/8.2.4/include/node/v8.h:3611:37: note: candidate function not viable: requires 3 arguments, but 2 were provided
      V8_WARN_UNUSED_RESULT Maybe<bool> Set(Local<Context> context,
                                        ^
    /Users/[username]/.electron-gyp/8.2.4/include/node/v8.h:3614:37: note: candidate function not viable: requires 3 arguments, but 2 were provided
      V8_WARN_UNUSED_RESULT Maybe<bool> Set(Local<Context> context, uint32_t index,
                                        ^
    ../src/serialport.cpp:378:14: error: no matching member function for call to 'Set'
        results->Set(Nan::New<v8::String>("baudRate").ToLocalChecked(), Nan::New<v8::Integer>(data->baudRate));
        ~~~~~~~~~^~~
    /Users/[username]/.electron-gyp/8.2.4/include/node/v8.h:3611:37: note: candidate function not viable: requires 3 arguments, but 2 were provided
      V8_WARN_UNUSED_RESULT Maybe<bool> Set(Local<Context> context,
                                        ^
    /Users/[username]/.electron-gyp/8.2.4/include/node/v8.h:3614:37: note: candidate function not viable: requires 3 arguments, but 2 were provided
      V8_WARN_UNUSED_RESULT Maybe<bool> Set(Local<Context> context, uint32_t index,
                                        ^
    4 errors generated.

Any ideas on how to fix this?

Platform: MacOS v10.15.4
Electron: 8.2.4
Node version: 12.13.0

@reconbot
Copy link
Member

reconbot commented Jun 11, 2020 via email

@dunkmann00
Copy link

I can also confirm that this occurs on the new version 9.0.0 (running on MacOS 10.15.2).

@SeBassTian23
Copy link

Maybe downgrade your node version? Why are you downloading?
-- --- Francis Gulotta [email protected]

I will try to downgrade node and see if that works. I can't download the prebuild libraries, so I would have to rebuild them and it seems that process is failing. Proper install only works for releases of serialport version 8.x and up

@SeBassTian23
Copy link

I finally find some time to get working on the issue. Any particular node version you would recommend using @reconbot ?

@dunkmann00
Copy link

I’ve looked into this a bit more. On Windows the problem does not occur. I was trying to look into the code a bit and do some testing, I’m not totally sure but it seems like the Poller binding is not receiving any disconnect event.

@dunkmann00
Copy link

dunkmann00 commented Jun 22, 2020

I think I have tracked down the problem. First, it seems that physically disconnecting something triggers a read. It doesn't trigger a disconnect at the low level code in binding/src and it never did. Secondly, it seems that in commit 50f967e unix-read.js was changed:

unix-read.js L#48-51

     if (disconnectError) {
-      err.disconnect = true
+      err.canceled = true
       logger('disconnecting', err)
     }

With this, the disconnect function won't get called in stream/lib/index.js:

err => {
debug('binding.read', `error`, err)
if (!err.canceled) {
this._disconnected(err)
}
this._read(bytesToRead) // prime to read more once we're reconnected
}

And from what I can tell, it just gets stuck in an (asynchronous) loop and does nothing until you try to write, at which point the disconnect function is called and the closed event is emitted.

I tried just changing unix-read.js back to what it was, but then it fails one of the unit tests.

In binding-abstract/lib/index.js the description for the read function states:

The in progress reads must error when the port is closed with an error object that has the property `canceled` equal to `true`. Any other error will cause a disconnection.

So this clearly was added on purpose, but since physical disconnects trigger in progress reads, I'm not sure what should be changed to maintain the current desired behavior (actual in progress reads erring with canceled equal to true), while adding the disconnect event for disconnects.

@reconbot, thoughts on this?

@antmarot
Copy link

Same issue with 9.0.0 — node v10.17.0 and macOS 10.15.5

@isopterix
Copy link

Same here with version 9.0.0 and node v13.12.0 and macOS 10.13.6.

@dunkmann00
Copy link

@reconbot Have you had a chance to look into this. If you have any suggestions I'd be happy to make a pull request.

@reconbot
Copy link
Member

reconbot commented Aug 7, 2020

This was an oversight on the refactor. I've changed the code back per @dunkmann00. The test was also wrong.

@reconbot reconbot reopened this Aug 7, 2020
@dunkmann00
Copy link

Thanks @reconbot!

@reconbot
Copy link
Member

reconbot commented Aug 8, 2020

I published [email protected] which has this fix. Let me know how it works.

@reconbot reconbot closed this as completed Aug 8, 2020
@dunkmann00
Copy link

Working great, thanks again for fixing this!

@nvbach91
Copy link

nvbach91 commented Apr 30, 2023

Issue is still present on v10.4.0 and v11.0.0. You have to manually call the port.read() method (in a loop) to actually trigger the close event with the Disconnect Error. Not sure if repeatedly calling read() will cause any problems in production.

here's my log

2023-04-30T12:53:47.202Z connected at COM3, pnpId USB\VID_067B&PID_2303\11111111
2023-04-30T12:53:47.981Z port.isOpen === true Manually calling port.read(): null
2023-04-30T12:53:48.994Z port.isOpen === true Manually calling port.read(): null
2023-04-30T12:53:49.998Z port.isOpen === true Manually calling port.read(): null
2023-04-30T12:53:51.012Z port.isOpen === true Manually calling port.read(): null
2023-04-30T12:53:51.233Z        Port COM3 closed DisconnectedError: Setting COM timeout (SetCommTimeouts): Unknown error code 22
    at SerialPort._disconnected (C:\...\testserialport\node_modules\@serialport\stream\dist\index.js:216:31)
    at C:\...\testserialport\node_modules\@serialport\stream\dist\index.js:205:22 {
  disconnected: true
}
2023-04-30T12:53:52.015Z port.isOpen === false
2023-04-30T12:53:53.018Z port.isOpen === false
2023-04-30T12:53:54.025Z port.isOpen === false
2023-04-30T12:53:55.039Z port.isOpen === false
2023-04-30T12:53:56.041Z port.isOpen === false

and the code (tested on Node.js 16.18.0 and 18.12.0, Windows 10, Windows 11)

const { SerialPort } = require('serialport');
const config = {
    pnpId: 'USB\\VID_067B&PID_2303\\11111111',
    baudRate: 9600,
    dataBits: 8,
    stopBits: 1,
    parity: 'none',
};
const path = 'COM3';

const port = new SerialPort({ ...config, path });
const log = (...msgs) => {
    console.log(new Date().toISOString(), ...msgs);
};
port.on('open', () => {
    log(`connected at ${path}, pnpId ${config.pnpId}`);
});
port.on('close', (disconnectError) => {
    log(`\tPort ${path} closed`, disconnectError);
});

setInterval(() => {
    if (port.isOpen) {
        log('port.isOpen ===', port.isOpen, 'Manually calling port.read():', port.read());
    } else {
        log('port.isOpen ===', port.isOpen);
    }
}, 1000);

my setup

COM               name COM3
Manufacturer      Prolific
PnP ID            USB\VID_067B&PID_2303\11111111
Friendly name     Prolific USB-to-Serial Comm Port (COM3)

Hope this helps somebody.

@BCsabaEngine
Copy link

BCsabaEngine commented Nov 19, 2023

Same issue with Linux and node20 and SerialPort v12. Close never occured when disconnected physically. I need to call a write operation to affect closed.
port.write('\n')

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

Successfully merging a pull request may close this issue.