Skip to content

[2.0.0] Add BLE characteristic callbacks overloads #4832

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 15, 2021

Conversation

mixa3607
Copy link
Contributor

Add BLE characteristic callbacks overloads with esp_ble_gatts_cb_param_t* param.
Example:

class BleCharactCallback : public BLECharacteristicCallbacks
{
    void onRead(BLECharacteristic *pCharacteristic, esp_ble_gatts_cb_param_t *param)
    {
        auto addr = param->read.bda;
        ESP_LOGV(TAG, "Device " ESP_BD_ADDR_STR " request data", ESP_BD_ADDR_HEX(addr));
    }
    void onWrite(BLECharacteristic *pCharacteristic, esp_ble_gatts_cb_param_t *param)
    {
        auto addr = param->write.bda;
        ESP_LOGV(TAG, "Device " ESP_BD_ADDR_STR " transmit data", ESP_BD_ADDR_HEX(addr));
    }
};

@me-no-dev
Copy link
Member

Hi @mixa3607 :) Thanks for the PR!
I have a few things that do bother me a bit though, like calling both functions one after another.
Here is what I propose:

  1. Mark the callbacks without param as deprecated
  2. Call the callbacks without param from the default handlers with param
  3. Remove logging from the default default handlers with param

What do you think?

@me-no-dev me-no-dev changed the title Add BLE characteristic callbacks overloads [2.0.0] Add BLE characteristic callbacks overloads Feb 22, 2021
@mixa3607
Copy link
Contributor Author

Hi @me-no-dev!
Marked overloads with single arg as "DEPRECATED" only in doc comments, and didn't add __attribute__ ((deprecated)) to the definition.
Moved call onXXX(pCharact) to onXXX(pCharact, param).
And also fix logging messages like log_d("BLECharacteristicCallbacks", "<< onStatus"); by change to log_d("<< onStatus");

@VojtechBartoska VojtechBartoska added this to the 2.0.0 milestone Mar 2, 2021
@me-no-dev me-no-dev merged commit 89e7893 into espressif:master Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants