node-ble Extend listener removal in BusHelper

trafficstars

Hello,

if the intended use of Device.disconnect also is to clean up the remaining event listeners this PR extends this to include the leftover BusHelper._propsProxy event listeners adding a new function BusHelper.removeListeners for this purpose.

This seems to do it for my current issue in #35.

May 27 '22 11:05 tuxedoxt

Bump :)

Any feedback?

Jul 07 '22 11:07 tuxedoxt

This approach looks better, but I have to try it. I will next weekend, promised. In the mean time, can you add some unit test?

Jul 07 '22 19:07 chrvadala

This seems to do it for my current issue in #35.

@tuxedoxt How did you get the error to stop? I tried using your fork and calling await device.disconnect(), but I still got error.

Sep 03 '22 21:09 ChocolateLoverRaj

Related issue, after calling Adapter.devices in a loop once every 5 seconds I start to get (node:4278) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 {"path":"/org/bluez/hci0/dev_6C_EC_26_F4_E4_5C","interface":"org.freedesktop.DBus.Properties","member":"PropertiesChanged"} listeners added to [EventEmitter]. Use emitter.setMaxListeners() to increase limit

This is the code in question, called from a setInterval loop every 5 seconds.

    const devices = await adapter.devices();
    for (let i = 0; i < devices.length; i++) {
        try {

            const device = await adapter.getDevice(devices[i]);
            const name = await device.getName();
            const rssi = await device.getRSSI();
            await device.disconnect();
            ...

I've tried the changes in this PR but they didn't affect the error.

Oct 12 '22 22:10 mxc42

@tuxedoxt How did you get the error to stop? I tried using your fork and calling await device.disconnect(), but I still got error.

I ended up using this change combined with https://github.com/dbusjs/node-dbus-next/pull/110

Since none got merged still using the forked versions explicitly binding the github urls in package.json where needed.

Example, temporary forked node-ble which has this change and a reference to forked dbus-next change:

"node-ble": "git+https://github.com/tuxedoxt/node-ble.git#match-and-event-leak-fixes",

Oct 14 '22 13:10 tuxedoxt

Coverage: 95.783% (-0.05%) from 95.833% when pulling 1855c3b4a59b785a6b1f89e69b0b54a11bca2ef6 on tuxedoxt:device-listener-cleanup into eb20e9c32d431df65ee5718c6c0e968b09524085 on chrvadala:main.