zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.08k stars 6.19k forks source link

Bluetooth: gatt: Allow to use CONFIG_BT_GATT_DYNAMIC_DB without CONFIG_BT_GATT_SERVICE_CHANGED #54175

Open MarekPieta opened 1 year ago

MarekPieta commented 1 year ago

Describe the solution you'd like If GATT Services are registered/unregistered only before Bluetooth is enabled, the GATT database would not change (from remote device's perspective). We could allow to use the dynamic database GATT API when Service Changed support is disabled in that case (register/unregister functions could return an error when GATT Service is registered after Bluetooth was enabled).

Additional context Disabling support for Service Changed could allow developers to save quite a lot of memory (even~3500B of FLASH + ~ 500B of RAM when I checked locally; needed to remove implementation of sc_indicate to build successfully). The feature could be useful for memory-limited devices.

piotrParf commented 1 year ago

Hi, Have you investigated how to apply this feature(removal of Service Changed)? Have you prepared some sort of patch? I just came to this topic and found your question.

MarekPieta commented 1 year ago

To quickly test the feature, you could remove dependency on CONFIG_BT_GATT_SERVICE_CHANGED from CONFIG_BT_GATT_DYNAMIC_DB and comment out the mentioned sc_indicate implementation in bluetooth/host/gatt.c (to prevent build failures). Then you can disable the CONFIG_BT_GATT_SERVICE_CHANGED in your configuration. Make sure that you do not register any GATT Services after Bluetooth is enabled in that case too.

Proper solution may require applying additional changes to the code (to ensure that there are no hidden assumptions related to the removed dependency). Apart from that. I think that it would be good to add checks ensuring that no GATT Service is registered/unregistered after Bluetooth is enabled (we could either return an error or at least assert that).

I do not have patch for this one, but applying fixes mentioned in my first comment locally should be straightforward.

zephyrbot commented 5 months ago

Hi @jori-nordic, @jhedberg,

This issue, marked as an Enhancement, was opened a while ago and did not get any traction. It was just assigned to you based on the labels. If you don't consider yourself the right person to address this issue, please re-assing it to the right person.

Please take a moment to review if the issue is still relevant to the project. If it is, please provide feedback and direction on how to move forward. If it is not, has already been addressed, is a duplicate, or is no longer relevant, please close it with a short comment explaining the reason.

@MarekPieta you are also encouraged to help moving this issue forward by providing additional information and confirming this request/issue is still relevant to you.

Thanks!