h2zero / esp-nimble-cpp

C++ library for the esp32 NimBLE stack based on and mostly compatible with @nkolban cpp_utils BLE library.
https://h2zero.github.io/esp-nimble-cpp/
Apache License 2.0
181 stars 62 forks source link

[BREAKING] Refactor attributes #192

Closed h2zero closed 2 months ago

h2zero commented 3 months ago

Refactor attributes to reduce code duplication and improve maintainability.

finger563 commented 1 month ago

@h2zero I'm not sure why, but after attempting to upgrade to use this code, I'm finding that my GFPS implementation is no longer working. Compilation works just fine (removing the const from the auth and passkey callbacks), but for whatever reason the onConfirmPIN callback is never being called - and I've found that the event is not even being triggered (the underlying esp-nimble-cpp NimBLEServer::handleGapEvent` is not being called when it should be). I've just started debugging it, but I figured I'd let you know and see if you possibly have some idea of what change might be causing this? The only thing I can think of right now (given that it's the only real change that seems to be coming from this PR) is the change to how attributes are notified/indicated, but I'm not sure...

h2zero commented 1 month ago

Thanks for the report, not sure what would have caused that.

finger563 commented 1 month ago

Did some further testing - it's not just GFPS that has broken - the HID service i have is also broken - it seems to be something about how the setValue/indicate/notify change occurred means that the notifications that i send (calling notify) are not being sent to the phone, which manifests as HID inputs not being received by the phone and the GFPS pairing information not being received by the phone either.

finger563 commented 1 month ago

Confirmed, even just connecting to the example app, and trying to read the Device Infomation service's characteristics no longer works (testing using the BLE Scanner app on Android).

Edit: these don't always break, they simply break after certain characteristics are read...

Investigation continues...

Edit 2: If i try to read one of the characteristic descriptors, then whatever nimble is doing now cause the BLE Scanner app to crash (oddly no errors on the esp32 side)

h2zero commented 1 month ago

Thanks, did a quick check myself just now. Looks like the call to send the notification returns with ENOT_CONNECTED, I guess the connection handle is wrong somehow.

finger563 commented 1 month ago

Looking through the code, I'm not sure where the m_handle for the NimBLECharacteristic is getting set, since it's initialized in the constructor to 0.

Grepping in the code, I'm not finding any places in the code where it's set.