`esp_gatt_value_t` unnecessarily holds 512 byte value (IDFGH-12362)

[nebkat]

Answers checklist.

• [X] I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
• [X] I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
• [X] I have searched the issue tracker for a similar issue and not found a similar issue.

General issue report

esp_gatt_value_t has a ESP_GATT_MAX_ATTR_LEN sized array to hold any potential response values.

https://github.com/espressif/esp-idf/blob/4f900edb06b29c1489ea9ae5318ce3ee261b7939/components/bt/host/bluedroid/api/include/api/esp_gatt_defs.h#L374-L380

This is unnecessary as in btc_gatts_arg_deep_copy this value gets deep copied anyway:

https://github.com/espressif/esp-idf/blob/4f900edb06b29c1489ea9ae5318ce3ee261b7939/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gatts.c#L85-L94

For esp_ble_gatts_send_response a pointer + length should be accepted rather than making a second copy.

[nebkat]

Also believe this was wrongly closed: https://github.com/espressif/esp-idf/issues/10206 - maximum value has always been 512. The 600 was copied over from the original Android implementation which later dropped it to the correct value as per Bluetooth spec.

[esp-zhp]

Hello, thank you for your feedback. I will make the necessary corrections. ps: BLUETOOTH CORE SPECIFICATION Version 5.4 | Vol 3, Part F page 1408

[Indastri]

Closing the issue as the user did not reply. If you will encounter any problems, feel free to reopen this one or create a new issues.

Cheers!

[nebkat]

@Indastri I presume this was an automated message - no reply pending here.

The ESP_GATT_MAX_ATTR_LEN issue was fixed in https://github.com/espressif/esp-idf/commit/20a780da8cccfe4b5af14c55e223a12af0432cd7, however the large stack value problem remains -esp_ble_gatts_send_response should ideally accept a pointer + size.

[Indastri]

Hi @nebkat, I will point my colleague towards this issue. @zhp0406

P.S. - the message was not automated. :) Cheers!

[esp-zhp]

@nebkat hi, Can you specify what issues still exist? What do you think the correct behavior should be?thanks.

[nebkat]

@esp-zhp

esp_ble_gatts_send_response accepts an esp_gatt_rsp_t *:

https://github.com/espressif/esp-idf/blob/5ca9f2a49aaabbfaf726da1cc3597c0edb3c4d37/components/bt/host/bluedroid/api/include/api/esp_gatt_defs.h#L602-L605

Which contains an esp_gatt_value_t:

https://github.com/espressif/esp-idf/blob/5ca9f2a49aaabbfaf726da1cc3597c0edb3c4d37/components/bt/host/bluedroid/api/include/api/esp_gatt_defs.h#L591-L597

Which contains uint8_t value[ESP_GATT_MAX_ATTR_LEN];

This means that in order to call esp_ble_gatts_send_response, the user must allocate >512 bytes even if they are only sending a few bytes, and then copy the contents of their response into this struct.

However, as soon as esp_ble_gatts_send_response is called, the arguments get passed to btc_gatts_arg_deep_copy which does a complete memcpy of the contents of esp_gatt_value_t anyway.

https://github.com/espressif/esp-idf/blob/5ca9f2a49aaabbfaf726da1cc3597c0edb3c4d37/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gatts.c#L85-L94

Thus there is no benefit to the initial allocation by the user - it is just an unnecessary copy. Sending even a single byte results in >1024 bytes having to be allocated to be passed to the BT api and then deallocated.

---

Other APIs such as esp_ble_gatts_send_indicate and esp_ble_gatts_set_attr_value accept uint16_t length, const uint8_t *value, so the user can just pass a pointer + length to the data they would like to use. Not only does this remove the need for the user allocation, but in btc_gatts_arg_deep_copy these will only allocate as many bytes as needed to hold the value:

https://github.com/espressif/esp-idf/blob/5ca9f2a49aaabbfaf726da1cc3597c0edb3c4d37/components/bt/host/bluedroid/btc/profile/std/gatt/btc_gatts.c#L69-L83

---

The correct behavior would be that esp_gatt_value_t contains uint16_t length, const uint8_t *value, instead of uint8_t value[ESP_GATT_MAX_ATTR_LEN];, and that btc_gatts_arg_deep_copy only allocates/copies length bytes. This would then match all the other APIs, and would be much more efficient.