Bluetooth: controller: Properly guards calls to ull_adv_sync

[Thalley]

Two ull_adv_sync functions calls, ull_adv_sync_reset and
ull_adv_sync_init was not properly guarded by a #if.

Signed-off-by: Emil Gydesen [email protected]

[Thalley]

See https://buildkite.com/zephyr/zephyr/builds/13410#a0617064-8efe-4ebf-8003-c6e86fe5b897 for an example of where this fails

[wopu-ot]

Fixes #30027

The usual way of doing this in Zephyr is to not have guards around the prototypes in the header files and leave the guards in the C files as they were.

[cvinayak]

The usual way of doing this in Zephyr is to not have guards around the prototypes in the header files and leave the guards in the C files as they were.

I tried to do so in while working on #29987, but needs changes to all c files that include ull_adv_internal.h to include additional LLL includes.
Maybe we move out the definitions from ull_adv_internal.h into a new ull_sync_internal.h ?

[Thalley]

@wopu-ot I also think that's the best way of handling this.

@cvinayak Is there a reason to guard the declarations and structs in the first place?

[cvinayak]

structs in the first place?

@Emil-Gydesen-Bose Structures get included in parent structures, and hence need to be conditionally excluded if features are disabled. By guarding them in header files, there is less chances of redundant RAM usage when features get disabled.

[Thalley]

structs in the first place?

@Emil-Gydesen-Bose Structures get included in parent structures, and hence need to be conditionally excluded if features are disabled. By guarding them in header files, there is less chances of redundant RAM usage when features get disabled.

I understand that guarding fields in structs makes sense, but guarding struct definitions, or function declarations, does not really save any RAM.

[wopu-ot]

The definition of struct ll_adv_sync_set in ull_adv_types.h is guarded with CONFIG_BT_CTLR_ADV_PERIODIC. Don't we need a forward declaration for a compilation without warnings?

[Thalley]

Yup, you are right.

I guess this could be handled in different ways:

1. Guard the ull_adv_sync_start with the CONFIG_BT_CTLR_ADV_PERIODIC again
2. Remove the CONFIG_BT_CTLR_ADV_PERIODIC guard from ll_adv_sync_set
3. Forward declaration

I opted for 2) (removing a guard from a struct definition shouldn't have any side effects).

@cvinayak Please let me know if you disagree with this approach

[wopu-ot]

LGTM

[carlescufi]

@cvinayak please take a look