NordicSemiconductor / pc-ble-driver-js

Node.js interface to the C/C++ pc-ble-driver library. API docs: https://nordicsemiconductor.github.io/pc-ble-driver-js/
Other
79 stars 41 forks source link

Scan not working as expected on 32 bit windows #199

Closed pambardekar closed 5 years ago

pambardekar commented 6 years ago

Hello,

I am observing strange issue where after a few cycles of start scan and stop scan (different every time), I don't get deviceDiscovered callback anymore. Start and stop scans return successfully themselves.

I am using following stack. The application is built on Windows 7, 32 bit. I have tested it on 32 bit Windows 7 and Windows 10.

pc-ble-driver-js: 2.4.2 electron: 1.8.1 Windows 7 and 10: 32 bit NRF 52840 dongle with connectivity firmware installed by NRF Connect 2.4.0.

Same code works fine on 64 bit Windows 7 and 10.

The start and stop scan code looks something like:

function start_scan() {
    if (false === scan_in_progress) {
        scan_in_progress = true;
        scan_timer = window.setTimeout(function () {
            stop_scan();
        }, SCAN_INTERVAL); // let scan run for 3 seconds, then stop it
        try {
            NordicAdapter.startScan();
        } catch(err) {
            logger.error(COMM_BLE, "Failed to start scan");
        }
    } else {
        logger.info(COMM_BLE, "Scan already in progress");
    }
}

function stop_scan() {
    if (true === scan_in_progress) {
        scan_in_progress = false;
        scan_timer = window.setTimeout(function () {
            start_scan();
        }, SCAN_SILENCE); // wait for 3 seconds before starting scan again

        try {
            NordicAdapter.stopScan();
        } catch (err) {
            logger.error(COMM_BLE, "Failed to stop scan");
        }
    }
}

The functions inside NordicAdapter module are

startScan() {
    if(this.adapter.state.scanning) {
        return;
    }
    const scanParameters = {
        active: true,
        interval: 100,
        window: 50,
        timeout: 0,
    };

    this.adapter.startScan(scanParameters, function(err)  {
        if (err) {
            logger.error(COMM_BLE, "Failed to start scan");
        } else {
            logger.info(COMM_BLE, "Successfully started scan");
        }
    });
};

stopScan() {
    if(!this.adapter.state.scanning) {
        return;
    }
    this.adapter.stopScan(err => {
        if(err) {
            logger.error(COMM_BLE, "Failed to stop scan");
        } else {
            logger.info(COMM_BLE, "Successfully stopped scan");
        }
    })
}

Is there a problem in the way the APIs are used?

pambardekar commented 5 years ago

Hi,

Scan started working in a better way after we changed the scan parameters to following

    const scanParameters = {
        active: true,
        interval: 100,
        window: 20,
        timeout: 3
    };

It would be great to know if this is the recommended configuration,

bihanssen commented 5 years ago

Hi, I can't see anything wrong with the parameters you were using initially.

However, we recently discovered a bug in connectivity firmware that could lead to advertising reports stopping to appear. You may be hitting that same issue. Setting the window to 20 will cause fewer reports, which may in turn lead to the bug not being triggered.

A fix will be released with next version of pc-ble-driver-js (though that won't be until early 2019).

lilliesAndRoses commented 5 years ago

Thanks for your reply. If the bug lies in connectivity firmware, how would a fix in pc-ble-driver-js help? Is there a patch available for the fix in connectivity firmware?

bihanssen commented 5 years ago

@lilliesAndRoses, to be specific, the fix is released with next pc-ble-driver-js release, though the fix itself is in pc-ble-driver and brought to pc-ble-driver-js through an update of the git submodule reference. It will be possible to bring the fix in manually when pc-ble-driver v4.0 is released.

lilliesAndRoses commented 5 years ago

@bihanssen I got the impression that the fix is in the firmware (hex) file that gets flashed on the NRF dongle. Thanks for the clarification.

bihanssen commented 5 years ago

@lilliesAndRoses the fix is (or will be) in the connectivity firmware, that is correct. My point was that users of pc-ble-driver-js will not see the updated firmware unless we release a new version of pc-ble-driver-js that includes the new firmware. And before that we need to release the pc-ble-driver that pc-ble-drivers-js submodule should point to. Sorry if my wording is unclear.

lilliesAndRoses commented 5 years ago

Is this patch available on pc-ble-driver repo under any particular branch/commit?

bihanssen commented 5 years ago

There is work-in-progress in pc-ble-driver branch consolidate-sd-apis.