search

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.59k stars 6.48k forks source link

Bluetooth: Host: CONFIG_BT_BONDABLE inconsistent text and implementation #79035

Open Thalley opened 4 days ago

Thalley commented 4 days ago

Is your enhancement proposal related to a problem? Please describe. The Kconfig is at the time of writing this defined as 

config BT_BONDABLE
    bool "Bondable Mode"
    default y
    help
      This option enables support for Bondable Mode. In this mode,
      Bonding flag in AuthReq of SMP Pairing Request/Response will be set
      indicating the support for this mode.

However the use of the Kconfig option seems to be different that the text. The "main" use of the Kconfig option is 

static bool bondable = IS_ENABLED(CONFIG_BT_BONDABLE);

and 

#if defined(CONFIG_BT_BONDABLE)
#define BT_SMP_AUTH_BONDING_FLAGS BT_SMP_AUTH_BONDING
#else
#define BT_SMP_AUTH_BONDING_FLAGS 0
#endif /* CONFIG_BT_BONDABLE */

Today the Kconfig value only affect the default value of the bondable variable in smp.c. Thus it is possible to set CONFIG_BT_BONDABLE=n and then call bt_set_bondable(true).

Describe the solution you'd like The Kconfig option should either

  1. Keep the current description but actually enable/disable support for bondable, and not make it possible to runtime to enable it if CONFIG_BT_BONDABLE=n
  2. Remove the This option enables support for Bondable Mode. part of the description (as being done in https://github.com/zephyrproject-rtos/zephyr/pull/78833)
  3. Remove the Kconfig option. If we can set the bondable flag at runtime, then it's overkill to have a Kconfig option for the default value.

**Describe alternatives you've conside N/A

Additional context See discussions on this in https://github.com/zephyrproject-rtos/zephyr/pull/78833

jhedberg commented 4 days ago

I'd vote for option 3. We have no in-tree samples or tests that do CONFIG_BT_BONDABLE=n so the upstream removal is at least simple. There should at the very least be a note in the migration guide about this, however.

Thalley commented 4 days ago

I wonder if and how much optimization we can get for non-bondable devices with option 1 though. If there are significant RAM and ROM savings for be found for such simple devices, that may be the better approach.

If the choice ends up being between 2 and 3, then I'm strongly in favor of 3.