Skip to content

bluetooth: add macros for BT_BAP_PAST_SERVICE_DATA #95487

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+71 −10
Open

bluetooth: add macros for BT_BAP_PAST_SERVICE_DATA #95487

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
29 changes: 29 additions & 0 deletions include/zephyr/bluetooth/audio/bap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,35 @@ extern "C" {
((_bis_bitfield) == 0U || (_bis_bitfield) == BT_BAP_BIS_SYNC_NO_PREF || \
BT_ISO_VALID_BIS_BITFIELD(_bis_bitfield))


/** @brief flags for Periodic Advertising Sync Transfer (PAST) */
enum bt_bap_past_flag {
/** No flag set for PAST Address matching source or ADV_EXT_IND */
BT_BAP_PAST_FLAG_NONE = 0,

/** Advertising Address does not match ADV_EXT_IND */
BT_BAP_PAST_FLAG_NO_MATCH_ADV_EXT_IND = BIT(0),

/** Advertising Address does not match the source */
BT_BAP_PAST_FLAG_NO_MATCH_SRC_ADDR = BIT(1),
};

/**
* @brief Helper to pack addr_type and src_id
*
* @param _past_flags past_addr_types bits0..7
* @param _src_id past_addr_types bits8..15
*/
#define BT_BAP_PAST_SERVICE_DATA(_past_flags, _src_id) \
((uint16_t)(_src_id) & 0xFFU) << 8U \
| ((uint16_t)(_past_flags) & 0xFFU)
Comment on lines +186 to +187
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a 100-line maximum in Zephy, so we could do

Suggested change
((uint16_t)(_src_id) & 0xFFU) << 8U \
| ((uint16_t)(_past_flags) & 0xFFU)
((uint16_t)(_src_id) & 0xFFU) << 8U | ((uint16_t)(_past_flags) & 0xFFU)

else I would put the | on the first line

Suggested change
((uint16_t)(_src_id) & 0xFFU) << 8U \
| ((uint16_t)(_past_flags) & 0xFFU)
((uint16_t)(_src_id) & 0xFFU) << 8U | \
((uint16_t)(_past_flags) & 0xFFU)


/** Extract BAP addr_type from 16-bit Service Data */
#define BT_BAP_PAST_GET_SRC_ID(_data) (uint8_t)(((_data) & 0xFFU) >> 8U)

/** Extract the BAP flags from 16-bit Service Data */
#define BT_BAP_PAST_GET_FLAGS(_data) (uint8_t)(((_data) & 0xFFU))

/**
* @brief Helper to declare elements of bt_bap_qos_cfg
*
Expand Down
18 changes: 14 additions & 4 deletions subsys/bluetooth/audio/shell/bap_broadcast_assistant.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ static void bap_broadcast_assistant_recv_state_cb(
char le_addr[BT_ADDR_LE_STR_LEN];
char bad_code[33];
bool is_bad_code;
struct bt_le_ext_adv_info info;

if (err != 0) {
bt_shell_error("BASS recv state read failed (%d)", err);
Expand Down Expand Up @@ -176,11 +177,14 @@ static void bap_broadcast_assistant_recv_state_cb(
}

if (per_adv_sync && IS_ENABLED(CONFIG_BT_PER_ADV_SYNC_TRANSFER_SENDER)) {
uint8_t past_flags = BT_BAP_PAST_FLAG_NO_MATCH_ADV_EXT_IND;
uint8_t src_id = state->src_id;

bt_shell_print("Sending PAST");

err = bt_le_per_adv_sync_transfer(per_adv_sync,
conn,
BT_UUID_BASS_VAL);
conn,
BT_BAP_PAST_SERVICE_DATA(past_flags, src_id));

if (err != 0) {
bt_shell_error("Could not transfer periodic adv sync: %d", err);
Expand Down Expand Up @@ -213,10 +217,16 @@ static void bap_broadcast_assistant_recv_state_cb(

if (ext_adv != NULL && IS_ENABLED(CONFIG_BT_PER_ADV) &&
IS_ENABLED(CONFIG_BT_PER_ADV_SYNC_TRANSFER_SENDER)) {
uint8_t past_flags = BT_BAP_PAST_FLAG_NO_MATCH_ADV_EXT_IND;

bt_le_ext_adv_get_info(ext_adv, &info);
Copy link
Contributor

Choose a reason for hiding this comment

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

Our coding guidelines require that all functions that may return an error, shall have the return value checked, so this must be

Suggested change
bt_le_ext_adv_get_info(ext_adv, &info);
err = bt_le_ext_adv_get_info(ext_adv, &info);

In cases like this where it should never fail unless something terribly has happened, I'd test this with an ASSERT

Suggested change
bt_le_ext_adv_get_info(ext_adv, &info);
err = bt_le_ext_adv_get_info(ext_adv, &info);
__ASSERT(err == 0, "Failed to get adv info: %d", err);

This also applies to the other cases in this PR

uint8_t src_id = info.sid;

bt_shell_print("Sending local PAST");

err = bt_le_per_adv_set_info_transfer(ext_adv, conn,
BT_UUID_BASS_VAL);
err = bt_le_per_adv_set_info_transfer(ext_adv,
conn,
BT_BAP_PAST_SERVICE_DATA(past_flags, src_id));

if (err != 0) {
bt_shell_error("Could not transfer per adv set info: %d", err);
Expand Down
9 changes: 6 additions & 3 deletions tests/bluetooth/tester/src/audio/btp_bap_broadcast.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1794,10 +1794,11 @@ uint8_t btp_bap_broadcast_assistant_send_past(const void *cmd, uint16_t cmd_len,
uint16_t *rsp_len)
{
int err;
uint16_t service_data;
struct bt_conn *conn;
struct bt_le_per_adv_sync *pa_sync;
const struct btp_bap_send_past_cmd *cp = cmd;
uint8_t past_flags = BT_BAP_PAST_FLAG_NO_MATCH_ADV_EXT_IND;
uint8_t src_id = cp->src_id;

LOG_DBG("");

Expand All @@ -1818,9 +1819,11 @@ uint8_t btp_bap_broadcast_assistant_send_past(const void *cmd, uint16_t cmd_len,
/* If octet 0 is set to 0, it means AdvA in PAST matches AdvA in ADV_EXT_IND.
* Octet 1 shall be set to Source_ID.
*/
service_data = cp->src_id << 8;

err = bt_le_per_adv_sync_transfer(pa_sync, conn, service_data);
err = bt_le_per_adv_sync_transfer(pa_sync,
conn,
BT_BAP_PAST_SERVICE_DATA(past_flags, src_id));

if (err != 0) {
LOG_DBG("Could not transfer periodic adv sync: %d", err);

Expand Down
13 changes: 11 additions & 2 deletions tests/bsim/bluetooth/audio/src/bap_broadcast_assistant_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ static void bap_broadcast_assistant_recv_state_cb(
{
char le_addr[BT_ADDR_LE_STR_LEN];
char bad_code[BT_ISO_BROADCAST_CODE_SIZE * 2 + 1];
struct bt_le_ext_adv_info info;

if (err != 0) {
FAIL("BASS recv state read failed (%d)\n", err);
Expand Down Expand Up @@ -179,8 +180,16 @@ static void bap_broadcast_assistant_recv_state_cb(

#if defined(CONFIG_BT_PER_ADV_SYNC_TRANSFER_SENDER)
if (state->pa_sync_state == BT_BAP_PA_STATE_INFO_REQ) {
err = bt_le_per_adv_sync_transfer(g_pa_sync, conn,
BT_UUID_BASS_VAL);
uint8_t past_flags = BT_BAP_PAST_FLAG_NO_MATCH_ADV_EXT_IND;

bt_le_per_adv_sync_get_info(g_pa_sync, &info);
uint8_t src_id = info.sid;


err = bt_le_per_adv_sync_transfer(g_pa_sync,
conn,
BT_BAP_PAST_SERVICE_DATA(past_flags, src_id));
Comment on lines +186 to +191
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
uint8_t src_id = info.sid;
err = bt_le_per_adv_sync_transfer(g_pa_sync,
conn,
BT_BAP_PAST_SERVICE_DATA(past_flags, src_id));
err = bt_le_per_adv_sync_transfer(g_pa_sync,
conn,
BT_BAP_PAST_SERVICE_DATA(past_flags, info.src_id));


if (err != 0) {
FAIL("Could not transfer periodic adv sync: %d\n", err);
return;
Expand Down
12 changes: 11 additions & 1 deletion tests/bsim/bluetooth/audio/src/cap_commander_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,8 @@ bap_broadcast_assistant_recv_state_cb(struct bt_conn *conn, int err,
char le_addr[BT_ADDR_LE_STR_LEN];
char bad_code[BT_ISO_BROADCAST_CODE_SIZE * 2 + 1];
size_t acceptor_count = get_dev_cnt() - 2;
struct bt_le_ext_adv_info info;


if (err != 0) {
FAIL("BASS recv state read failed (%d)\n", err);
Expand Down Expand Up @@ -534,7 +536,15 @@ bap_broadcast_assistant_recv_state_cb(struct bt_conn *conn, int err,

#if defined(CONFIG_BT_PER_ADV_SYNC_TRANSFER_SENDER)
if (state->pa_sync_state == BT_BAP_PA_STATE_INFO_REQ) {
err = bt_le_per_adv_sync_transfer(g_pa_sync, conn, BT_UUID_BASS_VAL);
uint8_t past_flags = BT_BAP_PAST_FLAG_NO_MATCH_ADV_EXT_IND;

bt_le_per_adv_sync_get_info(g_pa_sync, &info);
uint8_t src_id = info.sid;

err = bt_le_per_adv_sync_transfer(g_pa_sync,
conn,
BT_BAP_PAST_SERVICE_DATA(past_flags, src_id));
Comment on lines +541 to +546
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bt_le_per_adv_sync_get_info(g_pa_sync, &info);
uint8_t src_id = info.sid;
err = bt_le_per_adv_sync_transfer(g_pa_sync,
conn,
BT_BAP_PAST_SERVICE_DATA(past_flags, src_id));
bt_le_per_adv_sync_get_info(g_pa_sync, &info);
err = bt_le_per_adv_sync_transfer(g_pa_sync,
conn,
BT_BAP_PAST_SERVICE_DATA(past_flags, info.src_id));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for feedback.
but I am getting this error:
error: ‘struct bt_le_per_adv_sync_info’ has no member named ‘src_id’

The defintion looks like this:

/** @brief Periodic advertising set info structure. */
struct bt_le_per_adv_sync_info {
	/** Periodic Advertiser Address */
	bt_addr_le_t addr;

	/** Advertising Set Identifier, valid range @ref BT_GAP_SID_MIN to @ref BT_GAP_SID_MAX. */
	uint8_t sid;

	/** Periodic advertising interval (N * 1.25 ms) */
	uint16_t interval;

	/** Advertiser PHY (see @ref bt_gap_le_phy). */
	uint8_t phy;
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, this is on me. It's not the sid that we need, but the src_id of the receive state. bt_le_per_adv_sync_get_info and bt_le_ext_adv_get_info shouldn't be used for this purpose. Sorry for a bad recommendation when I suggested that :D

We need to store the src_id of the receive state somewhere and then use that use. I confused myself with sid, which is not the same as src_id


if (err != 0) {
FAIL("Could not transfer periodic adv sync: %d\n", err);
return;
Expand Down
Loading