Skip to content
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

Add setMTU function to BLEClient.cpp/.h #4999

Merged
merged 4 commits into from
Apr 15, 2021
Merged

Conversation

ericalbers
Copy link
Contributor

The current implementation has a getMTU function which returns the mtu sent in a message.

This function allows you to set the MTU value on the connected device, it first sets the MTU locally by calling esp_ble_gatt_set_local_mtu. It then calls esp_ble_gattc_send_mtu_req to have the connected device also change its MTU size.

@h2zero
Copy link
Contributor

h2zero commented Apr 1, 2021

Instead of duplicating all the code, why not just call BLEDevice::setMTU() ?

@ericalbers
Copy link
Contributor Author

ericalbers commented Apr 1, 2021

Instead of duplicating all the code, why not just call BLEDevice::setMTU() ?

the setMTU function in BLEDevice specifically says it only sets the local copy. I didn't think modifying it to also set the remote copy would be a good idea as there may be code out there dependent upon it only setting the local copy.
I'm all for setting the remote copy in the BLEDevice if others feel it won't break anything.

@h2zero
Copy link
Contributor

h2zero commented Apr 1, 2021

The MTU is determined based on the lowest value between the connecting devices. In this library all you need to do is set the local MTU once before connecting to a peer device and the MTU gets negotiated after the connection is established, this happens automatically (you can see the code that does this in the client on connected event handler).

After that there is nothing to be done. The local preferred MTU is a global setting for all connections so you could simple set it once to the max (517) and the actual used MTU will be determined at connection.
I'm not sure if this PR is going to accomplish what you're looking for beyond what is already available.

@chegewara
Copy link
Contributor

You can set local MTU from BLEDevice, then after connecting to peripheral just call esp_ble_gattc_send_mtu_req.
As it is event driven process you will have to add event that will get response from peer device, because the value can be smaller than you are trying to set.

@ericalbers
Copy link
Contributor Author

ericalbers commented Apr 1, 2021

I tried setting the MTU value before connection and it didn't set it on the device, the device refused to respond with data...once I added the call to esp_ble_gattc_send_mtu_req the device would respond with packets.
Not sure if the negotiation was failing somehow or if perhaps the device I was talking too just didn't negotiate correctly. I assumed it was only setting it locally and not on the remote, so added a function to set it explicitly.
This is the device for reference: https://stalker.sport/pro-ii-series/

@h2zero
Copy link
Contributor

h2zero commented Apr 1, 2021

I must apologize, I was not looking at this repos code and instead basing it on cpp_utils that this came from.

In this code base the call to exchange MTU is not automatic on connection so I withdraw my previous comment.

Considering this fact I would think it better to change the code in this PR to just call the BLEDevice method then call the MTU exchange function save a few bytes instead.

Alternatively, pulling the code from cpp_utils would be more seamless.
Good change either way.

@me-no-dev me-no-dev merged commit 7a4e706 into espressif:master Apr 15, 2021
poozy101 added a commit to poozy101/arduino-esp32 that referenced this pull request May 6, 2021
Pull request espressif#4999 added setMTU function to BLEClient.cpp/.h, this line provides implementation of this added functionality to the BLE client example to resolve cases in which data from notifyCallback exceeds 20 characters (3 bytes for command type and attribute ID, 20 bytes for attribute data (char*)pData).
@poozy101 poozy101 mentioned this pull request May 6, 2021
me-no-dev pushed a commit that referenced this pull request May 18, 2021
Pull request #4999 added setMTU function to BLEClient.cpp/.h, this line provides implementation of this added functionality to the BLE client example to resolve cases in which data from notifyCallback exceeds 20 characters (3 bytes for command type and attribute ID, 20 bytes for attribute data (char*)pData).
bjun626 pushed a commit to bjun626/arduino-esp32 that referenced this pull request Mar 9, 2023
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.

4 participants