Cannot REMOVE "error" listener from serialport #285

Closed
Analogreality opened this Issue Dec 27, 2013 · 13 comments

Comments

Projects
None yet
4 participants

I have some code similar to the following:

var serialport = require("serialport");
var SerialPort = serialport.SerialPort;

var testPort = new SerialPort(port.comName, {
}, true);

serialport.on("error", function (err) {
...
});

That's good and everything however there are two problems:

  1. The 'error' event is subscribed to ALL instances I create of new SerialPort.. a problem when I want each port to have its own error logic.
  2. I can't remove the event from the serialport object. Not even serialport.removeAllListeners() seems to actually work.

Any ideas?

Collaborator

reconbot commented Dec 27, 2013

Can you be a little more specific about which errors are called on all instances of serialport? Open errors in specific are emitted on the module's object. Write read and close errors should happen only on the single instance. I wrote some narly tests for read erorrs and it's working alright. Let me know.

I get the same error, called X times.

I am trying to go through all available ports, connect at a given speed, then send a message. If I get an OK back, I know that the target device understands me at that speed. This is for auto-negotiation of baudrate.

So, I have methods that are on a timer that go through a port, trying a connection, waiting for a timeout (or handling an error such as access denied, which means another app is using that port).

This method executes every X seconds.. to detect the device I open the port and get either an error message, a timeout of the OK message, or the OK message itself.

Since I open the port multiple times, I want it to remove ALL listeners I set up previously for that port.. so before I leave the method I try to testPort.removeListeners("error") and testPort.removeAllListeners().. however it doesn't seem to work. The instanciated testTest port I created with 'new SerialPort' does not listen to an error event.. so removing it does no good. Additionally, there seems to be a more global .on("error") event but that fires on all ports I try to connect on, if there is an error. It too is ignoring the .removeAllListeners() method call..

Collaborator

reconbot commented Dec 27, 2013

Can you share some code?
On Dec 27, 2013 1:43 PM, "Analogreality" notifications@github.com wrote:

I get the same error, called X times.

I am trying to go through all available ports, connect at a given speed,
then send a message. If I get an OK back, I know that the target device
understands me at that speed. This is for auto-negotiation of baudrate.

So, I have methods that are on a timer that go through a port, trying a
connection, waiting for a timeout (or handling an error such as access
denied, which means another app is using that port).

This method executes every X seconds.. to detect the device I open the
port and get either an error message, a timeout of the OK message, or the
OK message itself.

Since I open the port multiple times, I want it to remove ALL listeners I
set up previously for that port.. so before I leave the method I try to
testPort.removeListeners("error") and testPort.removeAllListeners()..
however it doesn't seem to work. The instanciated testTest port I created
with 'new SerialPort' does not listen to an error event.. so removing it
does no good. Additionally, there seems to be a more global .on("error")
event but that fires on all ports I try to connect on, if there is an
error. It too is ignoring the .removeAllListeners() method call..


Reply to this email directly or view it on GitHubhttps://github.com/voodootikigod/node-serialport/issues/285#issuecomment-31274621
.

Sure, one minute.

    var serialport = require("serialport");
    var SerialPort = serialport.SerialPort;

    // DO IT TO IT
    var coms = new SerialPort(parent.targetHardwareDevice, {
        "baudrate": parent.targetHardwareBaudrate,
        "parser": serialport.parsers.readline("\n"),
        "disconnectedCallback": function () {
            console.log("DEVICE: Disconnected.");
        }
    });

    coms.once("error", function (err) {
        if (!err) {
            return;
        }

        console.log("DEVICE: ERROR: " + err);   

        coms.removeAllListeners("error");
    });

    coms.open(function (err) {
        if (err) {
            console.log("DEVICE: CONNECT ERROR: " + err);   
        }

        ...
    });

I intentionally open up the COM port with Putty, blocking nodejs from acquiring the port.

When the previous code is run, I get:

events.js:2817: Uncaught Error: Opening .\COM6: Access denied

Collaborator

reconbot commented Dec 28, 2013

A few things.

You're immediately opening the serial port so you don't need the second open call. It's also why you're not getting an error passed to your callback when you call open. You can either pass the open handling function to the constructor or pass false as the last argument so it doesn't open immediately.

Since you're not passing a callback to the constructor and it is immediately opening the port, the error has no place to go so the module emits it. If you wanted to handle it on the module you could add.

serialport.on('error', function(){});

You also don't need to remove the listener as you're using once.

Hope that helps!

-Francis

  1. Ah, the implicit default value of reconnectImmediately is True, .. that makes sense.. it errors before the error handler connects.
  2. Ah, I see the .once() part was a mistake.. basically after this method I do NOT want any listeners bound to that object.. or even that object to exist any more.. I will change it to be .on() and then do a .removeAllListeners() call after it, based on a timer.

I will report my findings soon.

Collaborator

JayBeavers commented Jan 17, 2014

Let me rewrite your code a little to give some clarity on what's going on:

var SerialPortFactory = require("serialport");
var SerialPort = SerialPortFactory.SerialPort;

var serialPortInstance = new SerialPort(port.comName, {
}, true);

SerialPortFactory.on("error", function (err) {
...
});

I made that change so you can see that you're hooking the error handler on the 'parent object' from which SerialPorts are made (e.g. the Factory). Basically the error design is 'if you hook the factory error handler, it will tell you all the unhandled errors'. If you hook the object like:

serialPortInstance.on("error"...)

then the error will get handled there and not forwarded on to SerialPortFactory.on("error")

So in summary,

  • hook the serialPortInstance.on('error') if you want per instance error handling
  • hook the SerialPortFactory.on('error') if you want global all instances error handling

Awesome.. I will be able to look at this soon.. I will let you know my
results.

Seems to me I was just using it incorrectly.

I'll close this issue when I am done looking at it.

Thanks for your help.
On Jan 17, 2014 11:36 AM, "Jay Beavers" notifications@github.com wrote:

Let me rewrite your code a little to give some clarity on what's going on:

var SerialPortFactory = require("serialport");
var SerialPort = SerialPortFactory.SerialPort;

var testPort = new SerialPort(port.comName, {
}, true);

SerialPortFactory.on("error", function (err) {
...
});

I made that change so you can see that you're hooking the error handler on
the 'parent object' from which SerialPorts are made (e.g. the Factory).
Basically the error design is 'if you hook the factory error handler, it
will tell you all the unhandled errors'. If you look the object factory
like:

testPort.on("error"...)

then the error will get handled there and not forwarded on to
SerialPortFactory.

So in summary,

  • hook the serialPortInstance.on('error') if you want per instance

    error handling

    hook the serialPortFactory.on('error') if you want global all

    instances error handling

    jcb


Reply to this email directly or view it on GitHubhttps://github.com/voodootikigod/node-serialport/issues/285#issuecomment-32621213
.

Collaborator

JayBeavers commented Jan 17, 2014

Take a look, leave issue open -- at a minimum docs should be updated with this information.

Collaborator

voodootikigod commented Dec 31, 2014

@JayBeavers Can we close this at this point?

Collaborator

reconbot commented Mar 25, 2016

Closing due to age, I hope it worked out though! Feel free to file a new one.

reconbot closed this Mar 25, 2016

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