Node-serialport: No close event emitted when a device is physically removed for an open port.

Created on 4 Mar 2020  Â·  22Comments  Â·  Source: serialport/node-serialport

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

Most helpful comment

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 https://github.com/serialport/node-serialport/commit/50f967e788f362da57d782829712542c8f15f8c8 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:
https://github.com/serialport/node-serialport/blob/45f83ab666c81ccf0838bd83e09b9c23b2dc386a/packages/stream/lib/index.js#L387-L393
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?

All 22 comments

Same problem on Windows 10

I can confirm the issue on macOS v10.15.1

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.

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).

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.

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.

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

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

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

Maybe downgrade your node version? Why are you downloading?

On Wed, Jun 10, 2020 at 8:54 AM Sebastian notifications@github.com wrote:

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

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/serialport/node-serialport/issues/2043#issuecomment-641987198,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAGK3SSOUCY55KS6J6H5HDRV57BRANCNFSM4LBI4EOA
.

>


Francis Gulotta
[email protected]

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

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

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

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.

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 https://github.com/serialport/node-serialport/commit/50f967e788f362da57d782829712542c8f15f8c8 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:
https://github.com/serialport/node-serialport/blob/45f83ab666c81ccf0838bd83e09b9c23b2dc386a/packages/stream/lib/index.js#L387-L393
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?

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

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

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

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

Thanks @reconbot!

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

Working great, thanks again for fixing this!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jdshin123 picture jdshin123  Â·  3Comments

callen5914 picture callen5914  Â·  7Comments

henkbredell picture henkbredell  Â·  5Comments

byarchil picture byarchil  Â·  3Comments

hanfengcan picture hanfengcan  Â·  5Comments