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

Bluetooth: controller: Properly guards calls to ull_adv_sync #30007

Merged
merged 1 commit into from
Nov 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions subsys/bluetooth/controller/ll_sw/ull_adv_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ ull_adv_aux_hdr_len_fill(struct pdu_adv_com_ext_adv *com_hdr, uint8_t len)
/* helper function to fill the aux ptr structure in common ext adv payload */
void ull_adv_aux_ptr_fill(uint8_t **dptr, uint8_t phy_s);

#if defined(CONFIG_BT_CTLR_ADV_PERIODIC)
int ull_adv_sync_init(void);
int ull_adv_sync_reset(void);

Expand All @@ -129,5 +128,4 @@ uint32_t ull_adv_sync_start(struct ll_adv_sync_set *sync,

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

/* helper function to schedule a mayfly to get sync offset */
void ull_adv_sync_offset_get(struct ll_adv_set *adv);
#endif /* CONFIG_BT_CTLR_ADV_PERIODIC */
#endif /* CONFIG_BT_CTLR_ADV_EXT */
2 changes: 0 additions & 2 deletions subsys/bluetooth/controller/ll_sw/ull_adv_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ struct ll_adv_aux_set {
uint8_t is_started:1;
};

#if defined(CONFIG_BT_CTLR_ADV_PERIODIC)
struct ll_adv_sync_set {
struct evt_hdr evt;
struct ull_hdr ull;
Expand All @@ -60,5 +59,4 @@ struct ll_adv_sync_set {
uint8_t is_enabled:1;
uint8_t is_started:1;
};
#endif /* CONFIG_BT_CTLR_ADV_PERIODIC */
#endif /* CONFIG_BT_CTLR_ADV_EXT */