openbci-archive/OpenBCI_NodeJS

Graceful termination

Opened this issue · 11 comments

Hey AJ,

I'm working on making openbci reactive and noticed how the board is fully terminated on the example files:

https://github.com/OpenBCI/OpenBCI_NodeJS/blob/master/examples/debug/debug.js
https://github.com/OpenBCI/OpenBCI_NodeJS/blob/master/examples/getStreamingDaisy/getStreamingDaisy.js

Are there any use cases where the way to fully terminate is different?
Why not include these artifacts in the library itself? Seems like a lot of boilerplate.

Best,

Alex

Are you looking for how to not terminate connecting on disconnect? Just don't call disconnect? Bottom line: what ever you want to change, submit a proposal and I'll merge it so long as the test pass!

It should always terminate the process on disconnect, no? I can work on a PR.

Basically, this part taken from the example should be included in the library:

function exitHandler (options, err) {
  if (options.cleanup) {
    if (verbose) console.log('clean');
    ourBoard.removeAllListeners();
    /** Do additional clean up here */
  }
  if (err) console.log(err.stack);
  if (options.exit) {
    if (verbose) console.log('exit');
    ourBoard.disconnect().catch(console.log);
  }
}

if (process.platform === 'win32') {
  const rl = require('readline').createInterface({
    input: process.stdin,
    output: process.stdout
  });

  rl.on('SIGINT', function () {
    process.emit('SIGINT');
  });
}

// do something when app is closing
process.on('exit', exitHandler.bind(null, {
  cleanup: true
}));

// catches ctrl+c event
process.on('SIGINT', exitHandler.bind(null, {
  exit: true
}));

// catches uncaught exceptions
process.on('uncaughtException', exitHandler.bind(null, {
  exit: true
}));

Ahhhh ok I see what your saying, hmm yea would love to see a PR for this

is this still needed? should we make a different disconnect function? like disconnectGraceful?

I think the current disconnect should handle all of this. Can you think of one scenario where the user won't need to disconnect gracefully?

well the example you copied just removes the listeners. I more implement that in the examples to give people a place to clean up code. so you want a call to remove all listeners in disconnect?
Do you want the process.on()'s implemented? I'm struggling to figure out what I need to add to close this issue for you.

He's saying if the process.on stuff is needed, it should go into the library and be automatic, calling disconnect. User can then handle cleanup using disconnect event and using the library this way is simpler and quicker.

ah the only thing that is really needed is removing any attached event emitters. can you do that from the module?

shouldn't it call disconnect so the dongle stops streaming? you can do anything in a module you can do in a main file

I guess the question is what is it... Are you saying the module shouod be able to detect these three different ways to kill a program? These are used for like making sure if you hit control C, the board disconnects.

Like I make an electron application with this so I would not want to use the process.on because I have different hooks that get fired on app quit.