h2zero / NimBLE-Arduino

A fork of the NimBLE library structured for compilation with Arduino, for use with ESP32, nRF5x.
https://h2zero.github.io/NimBLE-Arduino/
Apache License 2.0
734 stars 153 forks source link

Characterstic notification subscribtion unstable #362

Closed aenniw closed 2 years ago

aenniw commented 2 years ago

Subscribing to multiple characteristics (tested with 24) that have READ_ENC or READ_AUTH enabled fails. Tested against Android against nRF Connect that failed with either timetout error or error 6 (0x6) gatt req not supported and against react-native-ble-plx. Testing the same sketch with vanilla BLE works without any issue. If the characteristics have only READ/NOTIF everything seems to work fine also... Reducing the number total available characteristics seems to affect the number failed subscriptions...

Tested with arduino-esp32 @ 2.0.2 and platformio with espressif32 @ 3.5.0 both using h2zero/NimBLE-Arduino @ 1.3.7

#include <Arduino.h>
#include <NimBLEDevice.h>
//#include <BLEDevice.h>

#ifndef MAIN_NIMBLEDEVICE_H_
#define NimBLEUUID uint32_t
#endif

const char *name = "Nimble", *manufacturer = "0242ac130003";
const uint32_t secret = 223344;

BLEAdvertisementData advertData;
BLESecurity security;

class ServerCbs : public BLEServerCallbacks {
    void onConnect(BLEServer *s) override {
        BLEDevice::startAdvertising();
    }
};

NimBLEUUID fullUUID(uint32_t uuid) {
#ifndef MAIN_NIMBLEDEVICE_H_
    return uuid;
#else
    return {uuid, 0x0000, 0x1000u, (uint64_t(0x8000u) << 48u) + 0x00805f9b34fbu};
#endif
}

std::vector<NimBLEUUID> SERVICE_UUIDS = {
        fullUUID(0xab5fa770u), fullUUID(0xab5ff770u), fullUUID(0xab5ff771u), fullUUID(0xab5ff772u),
};

const uint32_t BLE_PROPERTY =
#ifndef MAIN_NIMBLEDEVICE_H_
        BLECharacteristic::PROPERTY_READ | BLECharacteristic::PROPERTY_NOTIFY;
#else
        NIMBLE_PROPERTY::READ | NIMBLE_PROPERTY::READ_ENC | NIMBLE_PROPERTY::NOTIFY;
#endif

const std::vector<NimBLEUUID> UNIQ_CHARACTERISTICS = {
        fullUUID(0xb534fb4eu), fullUUID(0x02f3714eu), fullUUID(0x02f3724eu),
        fullUUID(0x03f3704eu), fullUUID(0x02f3704eu), fullUUID(0x04f3704eu)
};
const std::vector<NimBLEUUID> INSTANCED_CHARACTERISTICS = {
        fullUUID(0x05f3704eu), fullUUID(0x604d979du), fullUUID(0xa7601c29u),
        fullUUID(0xb532fb4eu), fullUUID(0xb533fb4eu), fullUUID(0xb535fb4eu),
};

void setup() {
    BLEDevice::init(name);

#ifndef MAIN_NIMBLEDEVICE_H_
    BLEDevice::setEncryptionLevel(ESP_BLE_SEC_ENCRYPT);
    security.setStaticPIN(secret);
    security.setAuthenticationMode(ESP_LE_AUTH_REQ_SC_BOND);
#else
    BLEDevice::setSecurityAuth(true, true, true);
    BLEDevice::setSecurityPasskey(secret);
    BLEDevice::setSecurityIOCap(BLE_HS_IO_DISPLAY_ONLY);
#endif
    auto server = BLEDevice::createServer();
    server->setCallbacks(new ServerCbs());

    for (auto SUUID: SERVICE_UUIDS) {
        auto service = server->createService(SUUID, 30);
        auto CH_UUIDS = SERVICE_UUIDS.at(0) == SUUID ?
                        UNIQ_CHARACTERISTICS : INSTANCED_CHARACTERISTICS;
        for (auto CHUUID: CH_UUIDS) {
            auto characteristic = service->createCharacteristic(
                    CHUUID, BLE_PROPERTY
            );
#ifndef MAIN_NIMBLEDEVICE_H_
            characteristic->setAccessPermissions(ESP_GATT_PERM_READ_ENCRYPTED);
#endif
            characteristic->setValue("value");
        }
        service->start();
    }

    advertData.setName(name);
    advertData.setManufacturerData(manufacturer);

    auto advertising = BLEDevice::getAdvertising();
    advertising->setScanResponse(true);
    advertising->setMinPreferred(0x0);
    advertising->setAdvertisementData(advertData);
    advertising->start();
}

void loop() {
}

nimble.log

h2zero commented 2 years ago

Hard for me to say what is happening in this situation with any certainty but I can speculate a couple things.

Security in NimBLE and Bluedroid work differently, where Bluedroid will request a secure connection immediately upon a client connection, NimBLE will only request it when a characteristic is read that has encryption requirements. This may have something to do with the errors. To workaround this a call to NimBLEDevice::startSecurity() can be used in the NimBLEServerCallbacks::onConnect() handler to mimic the Bluedroid action.

This line: advertising->setMinPreferred(0x0); may be causing connection timing issues in NimBLE, depending on how the client uses it, I suggest removing it for both code bases and testing the effect.

aenniw commented 2 years ago

removing advertising->setMinPreferred(0x0) and adding

void onConnect(NimBLEServer *s, ble_gap_conn_desc *desc) override {
        NimBLEDevice::startAdvertising();
        NimBLEDevice::startSecurity(desc->conn_handle);
}

sadly didn't resolve the notification issue, any other idea where to look for?

h2zero commented 2 years ago

Could you clarify the issue, is this occurring when sending notifications or when the client is subscribing? The logs don't show the disconnect so I'm not able to see what is happening when.

I can say though that the timeout error occurs because no communication between the devices occurred within the supervision timeout, this is set by the client device. The server (peripheral) can request different parameters after the client connects. Requesting that change with a longer timeout setting may resolve that error.

If you could provide more logs, preferably with the nimble core logs enabled I may be able to diagnose this further.

aenniw commented 2 years ago

re-tested with:

    -DCONFIG_BT_NIMBLE_DEBUG
    -DCONFIG_NIMBLE_CPP_DEBUG_LEVEL=5
    -DCONFIG_NIMBLE_CPP_ENABLE_RETURN_CODE_TEXT
    -DCONFIG_NIMBLE_CPP_ENABLE_GAP_EVENT_CODE_TEXT
    -DCONFIG_NIMBLE_CPP_ENABLE_ADVERTISMENT_TYPE_TEXT

and it seems that after these entries the notification seems to misbehave

19:07:09.126 > error persisting cccd; too many entries (8)
19:07:09.126 > looking up our sec;
19:07:09.126 > looking up our sec;

increasing the CONFIG_BT_NIMBLE_MAX_CCCDS solves the issue :laughing: thanks!

h2zero commented 2 years ago

Oh, right! Thanks for the update, I completely forgot about that setting.