search

noble / noble-device

A Node.js lib to abstract BLE (Bluetooth Low Energy) peripherals, uses noble
MIT License
97 stars 66 forks source link

Add disconnect listener in connect #22

Closed danielbh closed 8 years ago

danielbh commented 8 years ago

Prior version has a memory leak when the same device is discovered multiple times without connecting.

Changes:

Moves adding a 'disconnect' listener from NobleDevice to the connect function. Also adds tests. Testing uses real bluetooth devices so testing success depends on what is around you and able to connect. Ideally this should be mocked.

sandeepmistry commented 8 years ago

@danielbh thanks!

Ideally this should be mocked.

Yes, agreed. Do you have time to work on something? 2/4 of the new tests fail if i don't have a device present.

danielbh commented 8 years ago

Do you have any ideas on the best way to do this?

np. Thanks for accepting.

sandeepmistry commented 8 years ago

A few options:

1) we can make noble-device more testable by making it not require noble internally, so a mock can be passed in. @jacobrosenthal has some examples in https://github.com/sandeepmistry/noble/pull/431

2) Use something like mockery to mock the require. There's an example here: https://github.com/sandeepmistry/node-blink1/blob/master/unit-tests.js

1) is nicer if possible, please let me or @jacobrosenthal know if you have any questions or need to bounce some idea's off us about this.