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

Add API to "stop" noble #299

Open
AKST opened this Issue Jan 9, 2016 · 18 comments

Comments

Projects
None yet
7 participants
@AKST

AKST commented Jan 9, 2016

Is there any reason why nobel would cause a program to hang, even after closing noble.stopScanning() has been called and no peripherals are connected?

@sandeepmistry

This comment has been minimized.

Show comment
Hide comment
@sandeepmistry

sandeepmistry Jan 9, 2016

Member

@AKST noble continuously monitoring for adapter state change events: https://github.com/sandeepmistry/noble#adapter-state-change

Member

sandeepmistry commented Jan 9, 2016

@AKST noble continuously monitoring for adapter state change events: https://github.com/sandeepmistry/noble#adapter-state-change

@AKST

This comment has been minimized.

Show comment
Hide comment
@AKST

AKST Jan 13, 2016

is there a means to stop this once I no longer wish to connect any BLE devices?

AKST commented Jan 13, 2016

is there a means to stop this once I no longer wish to connect any BLE devices?

@sandeepmistry

This comment has been minimized.

Show comment
Hide comment
@sandeepmistry

sandeepmistry Jan 17, 2016

Member

@AKST not currently. What is the goal for stopping this?

Is using process.exit(0) to exit you app an option?

Member

sandeepmistry commented Jan 17, 2016

@AKST not currently. What is the goal for stopping this?

Is using process.exit(0) to exit you app an option?

@AKST

This comment has been minimized.

Show comment
Hide comment
@AKST

AKST Jan 18, 2016

Well I’m using this library as part of another library, and I don’t think it’s fair to expect the users of my own library to have to call process.exit(0) to close there applications which is why I’m seeking a means to stop this once the library is no longer being used.

On 18 Jan 2016, at 1:31 am, Sandeep Mistry notifications@github.com wrote:

@AKST https://github.com/AKST not currently. What is the goal for stopping this?

Is using process.exit(0) to exit you app an option?


Reply to this email directly or view it on GitHub #299 (comment).

AKST commented Jan 18, 2016

Well I’m using this library as part of another library, and I don’t think it’s fair to expect the users of my own library to have to call process.exit(0) to close there applications which is why I’m seeking a means to stop this once the library is no longer being used.

On 18 Jan 2016, at 1:31 am, Sandeep Mistry notifications@github.com wrote:

@AKST https://github.com/AKST not currently. What is the goal for stopping this?

Is using process.exit(0) to exit you app an option?


Reply to this email directly or view it on GitHub #299 (comment).

@sandeepmistry sandeepmistry changed the title from process hanging? to Add API to "stop" noble Jan 23, 2016

@sandeepmistry sandeepmistry added enhancement and removed question labels Jan 23, 2016

@sandeepmistry

This comment has been minimized.

Show comment
Hide comment
@sandeepmistry

sandeepmistry Jan 23, 2016

Member

@AKST I've renamed this issue, and labelled it as an enhancement request.

Member

sandeepmistry commented Jan 23, 2016

@AKST I've renamed this issue, and labelled it as an enhancement request.

@AKST

This comment has been minimized.

Show comment
Hide comment
@AKST

AKST Jan 23, 2016

Thanks @sandeepmistry, that's fair enough.

I would be happy to help out where I can to progress this issue, so if you feel like delegating any work just let me know, as I went poking through quite a bit of the source to try and find a solution myself before posting this.

Otherwise I'll be more than happy to just test any changes.

AKST commented Jan 23, 2016

Thanks @sandeepmistry, that's fair enough.

I would be happy to help out where I can to progress this issue, so if you feel like delegating any work just let me know, as I went poking through quite a bit of the source to try and find a solution myself before posting this.

Otherwise I'll be more than happy to just test any changes.

@sandeepmistry

This comment has been minimized.

Show comment
Hide comment
@sandeepmistry

sandeepmistry Jan 24, 2016

Member

If you could help out that would be great!

I think we'll have to add equivalent API's in noble's dependencies, as they continuously listen:

Member

sandeepmistry commented Jan 24, 2016

If you could help out that would be great!

I think we'll have to add equivalent API's in noble's dependencies, as they continuously listen:

@AKST

This comment has been minimized.

Show comment
Hide comment
@AKST

AKST Jan 27, 2016

Awesome, I start having a look around the weekend!

— Angus

On 25 Jan 2016, at 1:28 AM, Sandeep Mistry notifications@github.com wrote:

If you could help out that would be great!

I think we'll have to add equivalent API's in noble's dependencies, as they continuously listen:

https://github.com/sandeepmistry/node-xpc-connection https://github.com/sandeepmistry/node-xpc-connection - for OS X
https://github.com/sandeepmistry/node-bluetooth-hci-socket https://github.com/sandeepmistry/node-bluetooth-hci-socket - for non-OS X

Reply to this email directly or view it on GitHub #299 (comment).

AKST commented Jan 27, 2016

Awesome, I start having a look around the weekend!

— Angus

On 25 Jan 2016, at 1:28 AM, Sandeep Mistry notifications@github.com wrote:

If you could help out that would be great!

I think we'll have to add equivalent API's in noble's dependencies, as they continuously listen:

https://github.com/sandeepmistry/node-xpc-connection https://github.com/sandeepmistry/node-xpc-connection - for OS X
https://github.com/sandeepmistry/node-bluetooth-hci-socket https://github.com/sandeepmistry/node-bluetooth-hci-socket - for non-OS X

Reply to this email directly or view it on GitHub #299 (comment).

@recursify

This comment has been minimized.

Show comment
Hide comment
@recursify

recursify Jan 27, 2016

@sandeepmistry Can we also add an API to start noble?

For example, this simple script will hang:

require('noble')

IMO - requiring a module should not hang. So something like:

var noble = require('noble');
noble.start();
// Do stuff ..
noble.stop();

recursify commented Jan 27, 2016

@sandeepmistry Can we also add an API to start noble?

For example, this simple script will hang:

require('noble')

IMO - requiring a module should not hang. So something like:

var noble = require('noble');
noble.start();
// Do stuff ..
noble.stop();
@sandeepmistry

This comment has been minimized.

Show comment
Hide comment
@sandeepmistry

sandeepmistry Feb 7, 2016

Member

@recursify this would be a breaking change, something to start discussing in #325.

Member

sandeepmistry commented Feb 7, 2016

@recursify this would be a breaking change, something to start discussing in #325.

@sandeepmistry

This comment has been minimized.

Show comment
Hide comment
@sandeepmistry

sandeepmistry Jun 6, 2016

Member

@AKST any progress on this?

Member

sandeepmistry commented Jun 6, 2016

@AKST any progress on this?

@AKST

This comment has been minimized.

Show comment
Hide comment
@AKST

AKST Jun 6, 2016

Eh unfortunately not, I probably shouldn't have put my name forward to eagerly. Feel free to delegate it someone else. But yeah just started running short on time, and forgot to follow up, apologies about that.

AKST commented Jun 6, 2016

Eh unfortunately not, I probably shouldn't have put my name forward to eagerly. Feel free to delegate it someone else. But yeah just started running short on time, and forgot to follow up, apologies about that.

@jayalfredprufrock

This comment has been minimized.

Show comment
Hide comment
@jayalfredprufrock

jayalfredprufrock Feb 21, 2017

aww, too bad this never made it into a release. I'm in the same boat as OP. Another argument for this feature is that its nice to be able to fully shut down modules when using test libraries that hang until all processes exit.

I think it does make sense to also add a corresponding start() method. To avoid a breaking change, we could call start() on initialization by default and have an option to disable that behavior.

I'll look into picking this up in a month or two if I can find some extra time but I'd be forever grateful if somebody already familiar with the internals could knock this out!

jayalfredprufrock commented Feb 21, 2017

aww, too bad this never made it into a release. I'm in the same boat as OP. Another argument for this feature is that its nice to be able to fully shut down modules when using test libraries that hang until all processes exit.

I think it does make sense to also add a corresponding start() method. To avoid a breaking change, we could call start() on initialization by default and have an option to disable that behavior.

I'll look into picking this up in a month or two if I can find some extra time but I'd be forever grateful if somebody already familiar with the internals could knock this out!

@oyooyo

This comment has been minimized.

Show comment
Hide comment
@oyooyo

oyooyo Mar 10, 2017

I would highly welcome introducing a stop() method as well. As I understand, process.exit isn't the recommended way of stopping a node program.

I'm in favor of adding a start() method as well. When it comes to the problem of this being a breaking change:
Wouldn't it be possible to auto-start noble if a function is called that requires noble to be started, but it hasn't been started yet? For example by adding a little check in noble.on('stateChange') or noble.startScanning()?

oyooyo commented Mar 10, 2017

I would highly welcome introducing a stop() method as well. As I understand, process.exit isn't the recommended way of stopping a node program.

I'm in favor of adding a start() method as well. When it comes to the problem of this being a breaking change:
Wouldn't it be possible to auto-start noble if a function is called that requires noble to be started, but it hasn't been started yet? For example by adding a little check in noble.on('stateChange') or noble.startScanning()?

@sandeepmistry

This comment has been minimized.

Show comment
Hide comment
@sandeepmistry

sandeepmistry Mar 13, 2017

Member

I would highly welcome introducing a stop() method as well. As I understand, process.exit isn't the recommended way of stopping a node program.

@oyooyo please open a pull request to get this going

Member

sandeepmistry commented Mar 13, 2017

I would highly welcome introducing a stop() method as well. As I understand, process.exit isn't the recommended way of stopping a node program.

@oyooyo please open a pull request to get this going

@oyooyo

This comment has been minimized.

Show comment
Hide comment
@oyooyo

oyooyo Mar 20, 2017

@sandeepmistry I opened a pull request as you requested: #577
The "solution" I provided probably doesn't really work properly, but might eventually serve as a kind of starting point for a proper stop() function.

oyooyo commented Mar 20, 2017

@sandeepmistry I opened a pull request as you requested: #577
The "solution" I provided probably doesn't really work properly, but might eventually serve as a kind of starting point for a proper stop() function.

@someone-noone

This comment has been minimized.

Show comment
Hide comment
@someone-noone

someone-noone commented Oct 15, 2017

any updates?

@readeral

This comment has been minimized.

Show comment
Hide comment
@readeral

readeral Aug 26, 2018

Are these pull requests going to be considered?

readeral commented Aug 26, 2018

Are these pull requests going to be considered?

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