ARMmbed / ble-x-nucleo-idb0xa1

port of BLE_API to ST BLE shield
14 stars 5 forks source link

wrong delay calculation results in a long wait when stopScan() is called #31

Closed pmancele closed 6 years ago

pmancele commented 6 years ago

Hello, I'm using the ST ble driver for a while a i may have found an issue during the scan procedure. Indeed when the stopScan() function is called, the function Discover_CB is then called with the reason = DISCOVERY_COMPLETE. So we enter this section of the switch case :

  case DISCOVERY_COMPLETE:
    // The discovery is complete. If this is due to a stop scanning (i.e., the device
    // we are interested in has been found) and a connection has been requested
    // then we start the device connection.
    PRINTF("DISCOVERY_COMPLETE\n\r");
    _scanning = false;

    // Since the DISCOVERY_COMPLETE event can be received during the scanning interval,
    // we need to delay the starting of connection
    uint16_t delay = 2*(_scanningParams.getInterval());

#ifdef AST_FOR_MBED_OS
    if(_connecting) {
      minar::Scheduler::postCallback(makeConnection).delay(minar::milliseconds(delay));
    }
#else
    Clock_Wait(delay);
    if(_connecting) {
      makeConnection();
    }

I may be wrong but the result of the delay calculation, in the line bellow, is way too long ! https://github.com/ARMmbed/ble-x-nucleo-idb0xa1/blob/1616127fd90a7bbf7dafc939e108a21693da1c41/source/BlueNRGGap.cpp#L1081 Indeed getInterval() returns a time in unit of 0.625 ms so we should write getInterval()*0.625 to get a ms delay. Also, is it necessary to multiply the interval by 2 ?

By default the scan interval is 10.5 seconds, so when you call stopScan(), you then have to wait for 2*10.5/0.625 = 33.6 seconds !! I tried to replace it with

    uint16_t delay = (_scanningParams.getInterval()*0.625);

Is it ok ?

Thank you ! Pierre-Marie

apalmieriGH commented 6 years ago

Hi Perre-Marie, thanks for detecting the possible issue. Indeed, factor 2 was introduced for safety due to an implementation choice of the underlying stack. Your setting seems ok, it will be integrated it in the next commit.

Andrea

pmancele commented 6 years ago

@apalmieriGH Thank you ! I will wait for the commit then to close the issue