Skip to content

Updated BLERemoteCharacteristic to exposre esp_gatt_auth_req_t parame… #3531

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 3 commits into from
Jan 20, 2020

Conversation

cipherz
Copy link
Contributor

@cipherz cipherz commented Nov 29, 2019

For a project, I needed to be able to specify the esp_gatt_auth_req_t parameter for read and write value. I thought this might be valuable to others and I am therefore submitting a pull request for consideration.

Copy link
Contributor

@chegewara chegewara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better way to do that would be to add function to setup auth value in class, because auth level wont change over connection time, with default value set to NONE. This way code will be kept more clean and you dont have to pass additional argument over and over each time you read, write characteristic.

Beside that good improvement, thanks for contribution.

@cipherz
Copy link
Contributor Author

cipherz commented Nov 30, 2019

Makes sense, which class should be setup with it? BLEDevice, BLERemoteService or BLERemoteCharacteristic? I guess the question here is would the auth likely be the same for connections with client, all services or just for a single characteristic.

Thank you - and I'd be more than happy to update the PR based on your feedback.

@chegewara
Copy link
Contributor

BLERemoteCharacteristic and probably BLERemoteDescriptor too to make it consistent.

This of course will require to call function to set required auth for each characteristic, but according to bluetooth specs some characteristics cant have auth set to other than NONE.

@cipherz
Copy link
Contributor Author

cipherz commented Nov 30, 2019

Thank you for the quick response and guidance. I will update the PR and resubmit for reconsideration later today.

… to allow tweaking the authentication request type for that remotecharacteristic/descriptor without the need to add auth on each read/write.
@cipherz
Copy link
Contributor Author

cipherz commented Nov 30, 2019

I updated the PR to not have auth on each read/write as suggested but rather as a private variable for BLERemoteCharacteristic/Descriptor, which is defaulted to none when created and can be modified via setAuth.

@me-no-dev me-no-dev merged commit c2b37d9 into espressif:master Jan 20, 2020
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