Skip to content

Bluetooth: Host: Add advertising state to bt_le_ext_adv_info #90185

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 17, 2025
Merged

Bluetooth: Host: Add advertising state to bt_le_ext_adv_info #90185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 30 additions & 2 deletions include/zephyr/bluetooth/bluetooth.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1714,6 +1714,27 @@ int bt_le_ext_adv_delete(struct bt_le_ext_adv *adv);
*/
uint8_t bt_le_ext_adv_get_index(struct bt_le_ext_adv *adv);

/** Advertising states. */
enum bt_le_ext_adv_state {
/** The advertising set has been created but not enabled. */
BT_LE_EXT_ADV_STATE_DISABLED,

/** The advertising set is enabled. */
BT_LE_EXT_ADV_STATE_ENABLED,
};
Comment on lines +1718 to +1724
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why no BT_LE_EXT_ADV_STATE_NONE ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

#90185 (comment)

Copy link
Collaborator

@alwa-nordic alwa-nordic Jun 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I assume you are asking why there is a none state in one but not the other enum.

The none state in the periodic advertiser case means that no configuration has been set for periodic advertising for this set. We could remove the need for the none state in both of the enums if the Host sets a default configuration for periodic advertising. But this is extra code in the Host, so we should weight up the benefits.

Copy link
Collaborator

Choose a reason for hiding this comment

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

#90185 (comment)

An enabled boolean member in the info structure instead is better for application to check.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The enum is far more extendable if we ever want to add back a state like PAUSED if that becomes a necessity, and it's also more consistent with the per_adv_state which cannot be a boolean

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed. No action required from my side.


/** Periodic Advertising states. */
enum bt_le_per_adv_state {
/** Not configured for periodic advertising. */
BT_LE_PER_ADV_STATE_NONE,

/** The advertising set has been configured for periodic advertising, but is not enabled. */
BT_LE_PER_ADV_STATE_DISABLED,

/** Periodic advertising is enabled. */
BT_LE_PER_ADV_STATE_ENABLED,
};

/** @brief Advertising set info structure. */
struct bt_le_ext_adv_info {
/** Local identity handle. */
Expand All @@ -1724,15 +1745,22 @@ struct bt_le_ext_adv_info {

/** Current local advertising address used. */
const bt_addr_le_t *addr;

/** Extended advertising state. */
enum bt_le_ext_adv_state ext_adv_state;

/** Periodic advertising state. */
enum bt_le_per_adv_state per_adv_state;
Comment on lines +1752 to +1753
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be guarded by the periodic advertising feature support? according to implementation later in this PR, the referencing to conditionally guarded to always return BT_LE_PER_ADV_STATE_NONE, instead this member is not required and application code shall fail to compile when periodic advertising is not supported.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did consider this, and was also discussed when I added something similar w.r.t. ISO in #86092, but it was, IIRC, decided that a small increase in the struct for something that is short-lived, would be OK.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar to the other info structs, it's only stack allocated so it wouldn't be taking up much memory, but might run into stack overflows in some cases I guess.

Ok, as the info struct in typical use is a auto variable, this is acceptable.

};

/**
* @brief Get advertising set info
*
* @param adv Advertising set object
* @param info Advertising set info object
* @param info Advertising set info object. The values in this object are only valid on success.
*
* @return Zero on success or (negative) error code on failure.
* @retval 0 Success.
* @retval -EINVAL @p adv is not valid advertising set or @p info is NULL.
*/
int bt_le_ext_adv_get_info(const struct bt_le_ext_adv *adv,
struct bt_le_ext_adv_info *info);
Expand Down
33 changes: 33 additions & 0 deletions subsys/bluetooth/host/adv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1592,10 +1592,43 @@ void bt_le_adv_resume(void)
int bt_le_ext_adv_get_info(const struct bt_le_ext_adv *adv,
struct bt_le_ext_adv_info *info)
{
if (!IS_ARRAY_ELEMENT(adv_pool, adv)) {
LOG_DBG("adv %p is a valid pointer from bt_le_ext_adv_create", adv);
return -EINVAL;
}

if (!atomic_test_bit(adv->flags, BT_ADV_CREATED)) {
LOG_DBG("Advertising set %p is not created", adv);
return -EINVAL;
}

if (info == NULL) {
LOG_DBG("info is NULL");
return -EINVAL;
}

info->id = adv->id;
info->tx_power = adv->tx_power;
info->addr = &adv->random_addr;

if (atomic_test_bit(adv->flags, BT_ADV_ENABLED)) {
info->ext_adv_state = BT_LE_EXT_ADV_STATE_ENABLED;
} else {
info->ext_adv_state = BT_LE_EXT_ADV_STATE_DISABLED;
}

if (IS_ENABLED(CONFIG_BT_PER_ADV)) {
if (atomic_test_bit(adv->flags, BT_PER_ADV_ENABLED)) {
info->per_adv_state = BT_LE_PER_ADV_STATE_ENABLED;
} else if (atomic_test_bit(adv->flags, BT_PER_ADV_PARAMS_SET)) {
info->per_adv_state = BT_LE_PER_ADV_STATE_DISABLED;
} else {
info->per_adv_state = BT_LE_PER_ADV_STATE_NONE;
}
} else {
info->per_adv_state = BT_LE_PER_ADV_STATE_NONE;
}

return 0;
}

Expand Down
7 changes: 6 additions & 1 deletion subsys/bluetooth/host/shell/bt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2571,14 +2571,19 @@ static int cmd_adv_info(const struct shell *sh, size_t argc, char *argv[])

err = bt_le_ext_adv_get_info(adv, &info);
if (err) {
shell_error(sh, "OOB data failed");
shell_error(sh, "Failed to get advertising set info: %d", err);
return err;
}

shell_print(sh, "Advertiser[%d] %p", selected_adv, adv);
shell_print(sh, "Id: %d, TX power: %d dBm", info.id, info.tx_power);
shell_print(sh, "Adv state: %d", info.ext_adv_state);
print_le_addr("Address", info.addr);

if (IS_ENABLED(CONFIG_BT_PER_ADV)) {
shell_print(sh, "Per Adv state: %d", info.per_adv_state);
}

return 0;
}

Expand Down