nkolban / esp32-snippets

Sample ESP32 snippets and code fragments
https://leanpub.com/kolban-ESP32
Apache License 2.0
2.35k stars 710 forks source link

cpp_utils BLERemoteCharacteristic ESP_GATTC_REG_FOR_NOTIFY_EVT handling #289

Open mecowhy opened 6 years ago

mecowhy commented 6 years ago

Hello, first of all, thanks @nkolban for your work, I just started playing with esp32, and your code makes it so much easier.

I'm trying to get some data out of my ble digital weight scale, and after some sniffing and wireshark pcaps analyze (phone<->scale) I found how to get data out of it. the scale exposes characteristic with notify property but when I subscribe to this characteristics notification nothing, it gets not triggered. after some googling I found following issue https://github.com/espressif/esp-idf/issues/568 , where espressif stuff claims that it's required to call: esp_ble_gattc_write_char_descr after ESP_GATTC_REG_FOR_NOTIFY_EVT arrives, but I cant see it in the code.

the question is is there someone who tested this part of code with success, or is there something indeed missing.

thanks in advance Mikołaj.

chegewara commented 6 years ago

To receive notifications we need to write to descriptor 0x2902 in characteristic we want to receive notifications. Please study this example code, in there is described issue you are having:

https://github.com/nkolban/esp32-snippets/issues/269#issuecomment-352101385

mecowhy commented 6 years ago

@chegewara thanks for response, now I get notifications, my initial idea/question/thought was also: "is there reason not to do this in ESP_GATTC_REG_FOR_NOTIFY_EVT handler and leave for client code ?"

chegewara commented 6 years ago

ESP_GATTC_REG_FOR_NOTIFY_EVT needs to be triggered by something, by user action. You want to receive notifications you need to register for notify, in this case you need to writeValue to descriptor. When you write 0x0100 to descriptor then ESP_GATTC_REG_FOR_NOTIFY_EVT will be triggered in response from peripheral that your registration for notify was succesful.

mecowhy commented 6 years ago

yes ESP_GATTC_REG_FOR_NOTIFY_EVT needs to be triggered and is but by esp_ble_gattc_register_for_notify , and after that you need to write the descriptor as you pointed out. my question was, couldn't it be done already in ESP_GATTC_REG_FOR_NOTIFY_EVT handler.

nkolban commented 6 years ago

I'm going to talk out loud here to see if we agree with the story and see if anything needs changed. As I see it there is the concept of a BLE server that "owns" the value in a characteristic. When the value of the characteristic owned by the server changes ... presumably because something in the server made a change (eg. a new temperature value) ... then the BLE server can choose to send a notification to its connected clients (can a server have more than 1 connected client at a time?). However, BLE is all about not wasting energy ... so when the value of a characteristic changes, before the BLE server sends the notification it asks the question "Should I send a notification?"... It answers that question by looking in the BLE2902 descriptor for the characteristic. If the flag is on, the notification is sent ... if the flag is off, the notification is not sent. Then there is the client side ... when the server does perform a notification, something on the client has to be informed of the notification. This means that we interrupt the client from what it is currently doing. What if the client doesn't care about the notification?

Putting this together ... I see 3 notions:

  1. The server detecting that a characteristic value has changed
  2. Sending a notification IF and ONLY IF allowed by the BLE2902 descriptor
  3. The client determining whether or not it is interested in this notification

... and I'm think the these 3 notions are NOT directly related to each other. For example, when a client performs a "registration for a notification" ... it is asking for (3) above ... meaning it is saying "IF a notification arrives THEN I am interested in seeing it"

Obviously a notification at the client will never arrive if the BLE2902 flag on the server doesn't say it should be sent ... but I don't think I believe that a request by the client which says "IF a notification is received at the client THEN deliver it to me" should be the same as "Tell the BLE Server to start sending notifications AND If a notification is received then deliver it to me".

I'm going to guess that the choice of the value of the BLE2902 flag can also be set (or cleared) by logic in the server. Imagine a button on the server that says "Stop publishing new values" ... in this case, the client shouldn't be responsible for forcing the server to publish new values. All the client can do is ask "If they are sent, then I am interested".

Again ... I'm fuzzy here ... so if others have alternative views, lets chat.

mecowhy commented 6 years ago

ok so my view is following.

On the client side, if I'm subscribing for notifications, the effect I'm expecting is that if some value on server changes (defined by bleAddress,ServiceUUID,CharacteristicUUID) I'll get notified. What means are necessary to achieve this result is irrelevant from my (clientAppCode) point of view, I just need the data.

On the other side, if I'm serverSideCode, that uses BLE to output my data, if there is whatever reason I dont't want to publish the data changes to the clients( like the button you mentioned), I just don't pass it to the BLEServer. It's just messenger if I pass he delivers (if someone is interested). I don't think BLEServer shouldn't be aware of existence of some button that can stop the notifications. In that case if I decide to change the interface, for example for serial only the interface code changes not the internal logic of the device that says: "if red button is pressed don't notify clients about value changes".

So from my point of view if there is some other reason not to set BLE2902 flag (I'm new to BLE so I may be unaware of such), inside ESP_GATTC_REG_FOR_NOTIFY_EVT handler, there would be convenient to have some method like BLERemoteCharacterstic::enableNotify() that does set this flag, so that if one is writing code using BLERemoteCharacteristic can see: "oh there is registerForNotify and enableNotify methods, so I have to call both to get my data", and not "I registered for notify, where is my data ?.... oh wait I have to write the magic descriptor 0x2902, to enable it" ;). so that the only thing I have to know is where is my data and how to make use of it.

That is how I see it.

nkolban commented 6 years ago

What you say make good sense. The purpose of these libraries is to delight (and at a minimum satisfy) programmers who want to make use of BLE on the ESP32. What we want to avoid is having to make good folks like yourself have to fish through specifications and other arcane knowledge to get the task achieved. Leave it with me a while longer. What I want to now do is examine BLE APIs available on a variety of other platforms and see if I can get a sense of what they do and follow suit. While the "story" on Descriptor 2902 may (or may not be) technically accurate, the goal of the C++ classes is to make life easier ... and I am getting the feeling that the knowledge that one has to set the BLE 2902 flag and the actual necessity of currently having to set that is less than desirous. Let's see what we can find about other non ESP32 APIs and how they might go about it and fall in-line.

mecowhy commented 6 years ago

Definitely good idea to check how it looks like in other libs, as I said I'm new to BLE so I may be wrong. I quickly checked how does it look like in Android API and it looks like there also these two steps are required: https://developer.android.com/guide/topics/connectivity/bluetooth-le.html#notification

sremberg commented 6 years ago

Good morning, I like this discussion and just wanted to add some experiences I just made. First of all let me say I really like this BLE lib. I started yesterday with my first ESP32 and BLE is working already (after finally finding the 2902 Descriptor). I'm usually getting in touch with new libs by having a look at the examples. Since BLE comes with good ones, that was my point of start. And https://github.com/nkolban/ESP32_BLE_Arduino/blob/master/examples/BLE_client/BLE_client.ino does exactly what I needed, register for a notification and.... wait until I receive it. Unfortunately there is no 2902 Descriptor written or mentioned so I just thought, everything would be handled in the registerForNotify function (I'm pretty new to BLE and didn't know 2902 at all...). I agree with @mecowhy that a function for enabling/disabling would be very helpful (checking the API is always the first thing I do when I need help). But I think a first good step would be adding the Descriptor to the example and set it to 1 to enable the notification. Just my thoughts, Simon

inphotalk commented 6 years ago

I was facing similar problem (ESP32 in Arduino IDE reading heart rate from MIband 1s). After calling registerForNotify notifications for heart rate notify characteristic didn't come. After writing 0x0100 to the 0x2902 descriptor of heart rate notify characteristic everything started to work (notifications come from the Heart Rate from MIband 1s).

So I'm rather sure that the registerForNotify just sets the address of callback function without modifying the remote descriptor 0x2902.

Writting the descriptor returns warning form GATT because in the library function for writing the descriptor had fixed ESP_GATT_WRITE_TYPE_NO_RSP - Mi band requires write with response: ESP_GATT_WRITE_TYPE_RSP. Reading the descriptor causes the library hangup because the BLERemoteDescriptor.cpp was lacking the gattClientEventHandler. I've made modifications of BLERemoteDescriptor.cpp BLERemoteDescriptor.h and BLERemoteCharacteristic.cpp, and uploaded to github.