Skip to content

samples: Bluetooth: ISO: Update conn and adv intervals #81227

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
Nov 25, 2024
Merged

samples: Bluetooth: ISO: Update conn and adv intervals #81227

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
23 changes: 20 additions & 3 deletions samples/bluetooth/iso_broadcast/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
/*
* Copyright (c) 2021 Nordic Semiconductor ASA
* Copyright (c) 2021-2024 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: Apache-2.0
*/
#include <stddef.h>
#include <stdint.h>

#include <zephyr/bluetooth/bluetooth.h>
#include <zephyr/bluetooth/gap.h>
#include <zephyr/bluetooth/iso.h>
#include <zephyr/sys/byteorder.h>

Expand Down Expand Up @@ -88,6 +91,20 @@ static const struct bt_data ad[] = {

int main(void)
{
/* Some controllers work best while Extended Advertising interval to be a multiple
* of the ISO Interval minus 10 ms (max. advertising random delay). This is
* required to place the AUX_ADV_IND PDUs in a non-overlapping interval with the
* Broadcast ISO radio events.
* For 10ms SDU interval a extended advertising interval of 60 - 10 = 50 is suitable
*/
Copy link
Contributor

@rugeGerritsen rugeGerritsen Nov 19, 2024

Choose a reason for hiding this comment

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

Can we add a comment:

Suggested change
*/
* Other controllers are able to generate conflict-free scheduling patterns without setting the advertising interval
* in this way.
*/

Copy link
Contributor Author

@Thalley Thalley Nov 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Sure :) This one is very specific to ZLL, and not 100% sure if host samples should assume that ZLL is being used. For single-CPU builds we can check it, but for multi-CPU boards like the nRF5340 we cannot tell (unless we can read out some controller information at runtime).

Copy link
Contributor

@cvinayak cvinayak Nov 19, 2024

Choose a reason for hiding this comment

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

Having the documentation for Zephyr Controller specific (limitation) is correct, and not to comment on other controller that may or may not support. (if they support handling overlaps, the specific interval are not problem anyway).

Copy link
Contributor Author

@Thalley Thalley Nov 19, 2024

Choose a reason for hiding this comment

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

@cvinayak since we have samples that have host features not supported by the controller, there is already a general understanding/consensus that BT samples are not specific for any controller, not even the Zephyr controller. I'm unsure whether we should mention that specifically here. Coul also just be vague and say "some controllers"?

Copy link
Contributor

@rugeGerritsen rugeGerritsen Nov 20, 2024

Choose a reason for hiding this comment

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

+1 for mentioning that other controllers may not need this. We have a similar case for documenting scan window when CONFIG_BT_SCAN_AND_INITIATE_IN_PARALLEL is used.

Copy link
Contributor Author

@Thalley Thalley Nov 20, 2024

Choose a reason for hiding this comment

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

Replaced the specific mentioning of Zephyr with Some controllers work best. Is that OK?

Copy link
Contributor

@rugeGerritsen rugeGerritsen Nov 20, 2024

Choose a reason for hiding this comment

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

I was initially thinking it was fine to mention that the zephyr controller needs this but others don't. But I don't have a strong preference here

Copy link
Contributor Author

@Thalley Thalley Nov 20, 2024

Choose a reason for hiding this comment

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

Given that these samples are not ZLL specific, I'd rather keep it vague but still explain that it may be required.

const uint16_t adv_interval_ms = 60U;
const uint16_t ext_adv_interval_ms = adv_interval_ms - 10U;
const struct bt_le_adv_param *ext_adv_param = BT_LE_ADV_PARAM(
BT_LE_ADV_OPT_EXT_ADV, BT_GAP_MS_TO_ADV_INTERVAL(ext_adv_interval_ms),
BT_GAP_MS_TO_ADV_INTERVAL(ext_adv_interval_ms), NULL);
const struct bt_le_per_adv_param *per_adv_param = BT_LE_PER_ADV_PARAM(
BT_GAP_MS_TO_PER_ADV_INTERVAL(adv_interval_ms),
BT_GAP_MS_TO_PER_ADV_INTERVAL(adv_interval_ms), BT_LE_PER_ADV_OPT_NONE);
uint32_t timeout_counter = INITIAL_TIMEOUT_COUNTER;
struct bt_le_ext_adv *adv;
struct bt_iso_big *big;
Expand All @@ -106,7 +123,7 @@ int main(void)
}

/* Create a non-connectable advertising set */
err = bt_le_ext_adv_create(BT_LE_EXT_ADV_NCONN, NULL, &adv);
err = bt_le_ext_adv_create(ext_adv_param, NULL, &adv);
if (err) {
printk("Failed to create advertising set (err %d)\n", err);
return 0;
Expand All @@ -120,7 +137,7 @@ int main(void)
}

/* Set periodic advertising parameters */
err = bt_le_per_adv_set_param(adv, BT_LE_PER_ADV_DEFAULT);
err = bt_le_per_adv_set_param(adv, per_adv_param);
if (err) {
printk("Failed to set periodic advertising parameters"
" (err %d)\n", err);
Expand Down
58 changes: 55 additions & 3 deletions samples/bluetooth/iso_broadcast_benchmark/src/broadcaster.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
/*
* Copyright (c) 2021 Nordic Semiconductor ASA
* Copyright (c) 2021-2024 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <ctype.h>
#include <stdlib.h>
#include <stdint.h>

#include <zephyr/bluetooth/gap.h>
#include <zephyr/console/console.h>
#include <zephyr/bluetooth/bluetooth.h>
#include <zephyr/bluetooth/iso.h>
#include <zephyr/sys/byteorder.h>

#include <zephyr/logging/log.h>
#include <zephyr/sys/util.h>
#include <zephyr/sys_clock.h>
LOG_MODULE_REGISTER(iso_broadcast_broadcaster, LOG_LEVEL_DBG);

#define DEFAULT_BIS_RTN 2
Expand Down Expand Up @@ -613,13 +618,60 @@ static void iso_timer_timeout(struct k_work *work)
seq_num++;
}

static uint16_t calc_adv_interval(uint32_t sdu_interval_us)
{
/* Default to 6 times the SDU interval and then reduce until we reach a reasonable maximum
* advertising interval (BT_GAP_PER_ADV_SLOW_INT_MAX)
* sdu_interval_us can never be larger than 1048ms (BT_ISO_SDU_INTERVAL_MAX)
* so a multiple of it will always be possible to keep under BT_GAP_PER_ADV_SLOW_INT_MAX
* (1200ms)
*/

/* Convert from microseconds to advertising interval units (0.625ms)*/
const uint32_t interval = BT_GAP_US_TO_ADV_INTERVAL(sdu_interval_us);
uint32_t adv_interval = interval * 6U;

/* Ensure that the adv interval is between BT_GAP_PER_ADV_FAST_INT_MIN_1 and
* BT_GAP_PER_ADV_SLOW_INT_MAX for the sake of interopability
*/
while (adv_interval < BT_GAP_PER_ADV_FAST_INT_MIN_1) {
adv_interval += interval;
}

while (adv_interval > BT_GAP_PER_ADV_SLOW_INT_MAX) {
adv_interval -= interval;
}

/* If we cannot convert back then it's not a lossless conversion */
if (big_create_param.framing == BT_ISO_FRAMING_UNFRAMED &&
BT_GAP_ADV_INTERVAL_TO_US(interval) != sdu_interval_us) {
LOG_INF("Advertising interval of 0x04%x is not a multiple of the advertising "
"interval unit (0.625 ms) and may be subpar. Suggest to adjust SDU "
"interval %u to be a multiple of 0.625 ms",
adv_interval, sdu_interval_us);
}

return adv_interval;
}

static int create_big(struct bt_le_ext_adv **adv, struct bt_iso_big **big)
{
/* Some controllers work best when Extended Advertising interval is a multiple
* of the ISO Interval minus 10 ms (max. advertising random delay). This is
* required to place the AUX_ADV_IND PDUs in a non-overlapping interval with the
* Broadcast ISO radio events.
*/
const uint16_t adv_interval = calc_adv_interval(big_create_param.interval);
const uint16_t ext_adv_interval = adv_interval - BT_GAP_MS_TO_ADV_INTERVAL(10U);
const struct bt_le_adv_param *ext_adv_param =
BT_LE_ADV_PARAM(BT_LE_ADV_OPT_EXT_ADV, ext_adv_interval, ext_adv_interval, NULL);
const struct bt_le_per_adv_param *per_adv_param =
BT_LE_PER_ADV_PARAM(adv_interval, adv_interval, BT_LE_PER_ADV_OPT_NONE);
int err;

/* Create a non-connectable advertising set */
LOG_INF("Creating Extended Advertising set");
err = bt_le_ext_adv_create(BT_LE_EXT_ADV_NCONN, NULL, adv);
err = bt_le_ext_adv_create(ext_adv_param, NULL, adv);
if (err != 0) {
LOG_ERR("Failed to create advertising set (err %d)", err);
return err;
Expand All @@ -634,7 +686,7 @@ static int create_big(struct bt_le_ext_adv **adv, struct bt_iso_big **big)

LOG_INF("Setting Periodic Advertising parameters");
/* Set periodic advertising parameters */
err = bt_le_per_adv_set_param(*adv, BT_LE_PER_ADV_DEFAULT);
err = bt_le_per_adv_set_param(*adv, per_adv_param);
if (err != 0) {
LOG_ERR("Failed to set periodic advertising parameters (err %d)",
err);
Expand Down
9 changes: 6 additions & 3 deletions samples/bluetooth/iso_central/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
/*
* Copyright (c) 2021 2021 Nordic Semiconductor ASA
* Copyright (c) 2021-2024 Nordic Semiconductor ASA
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <zephyr/bluetooth/gap.h>
#include <zephyr/types.h>
#include <stddef.h>
#include <errno.h>
Expand Down Expand Up @@ -90,6 +91,9 @@ static void iso_timer_timeout(struct k_work *work)
static void device_found(const bt_addr_le_t *addr, int8_t rssi, uint8_t type,
struct net_buf_simple *ad)
{
const struct bt_le_conn_param *conn_param =
BT_LE_CONN_PARAM(BT_GAP_MS_TO_CONN_INTERVAL(60), BT_GAP_MS_TO_CONN_INTERVAL(60), 0,
BT_GAP_MS_TO_CONN_TIMEOUT(4000));
char addr_str[BT_ADDR_LE_STR_LEN];
int err;

Expand All @@ -115,8 +119,7 @@ static void device_found(const bt_addr_le_t *addr, int8_t rssi, uint8_t type,
return;
}

err = bt_conn_le_create(addr, BT_CONN_LE_CREATE_CONN, BT_LE_CONN_PARAM_DEFAULT,
&default_conn);
err = bt_conn_le_create(addr, BT_CONN_LE_CREATE_CONN, conn_param, &default_conn);
if (err) {
printk("Create conn to %s failed (%u)\n", addr_str, err);
start_scan();
Expand Down
40 changes: 35 additions & 5 deletions samples/bluetooth/iso_connected_benchmark/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,14 @@
*/

#include <ctype.h>
#include <zephyr/bluetooth/conn.h>
#include <zephyr/bluetooth/gap.h>
#include <zephyr/kernel.h>
#include <string.h>
#include <stdint.h>
#include <stdlib.h>
#include <zephyr/sys/util.h>
#include <zephyr/sys_clock.h>
#include <zephyr/types.h>


Expand Down Expand Up @@ -1046,6 +1051,12 @@ static int change_central_settings(void)

static int central_create_connection(void)
{
/* Give the controller a large range of intervals to pick from. In this benchmark sample we
* want to prioritize ISO over ACL, but will leave the actual choice up to the controller.
*/
const struct bt_le_conn_param *conn_param =
BT_LE_CONN_PARAM(BT_GAP_INIT_CONN_INT_MIN, BT_GAP_MS_TO_CONN_INTERVAL(500U), 0,
BT_GAP_MS_TO_CONN_TIMEOUT(4000));
int err;

advertiser_found = false;
Expand All @@ -1071,8 +1082,7 @@ static int central_create_connection(void)
}

LOG_INF("Connecting");
err = bt_conn_le_create(&adv_addr, BT_CONN_LE_CREATE_CONN,
BT_LE_CONN_PARAM_DEFAULT, &default_conn);
err = bt_conn_le_create(&adv_addr, BT_CONN_LE_CREATE_CONN, conn_param, &default_conn);
if (err != 0) {
LOG_ERR("Create connection failed: %d", err);
return err;
Expand All @@ -1089,7 +1099,6 @@ static int central_create_connection(void)

static int central_create_cig(void)
{
struct bt_iso_connect_param connect_param[CONFIG_BT_ISO_MAX_CHAN];
int err;

iso_conn_start_time = 0;
Expand All @@ -1102,6 +1111,16 @@ static int central_create_cig(void)
return err;
}

return 0;
}

static int central_connect_cis(void)
{
struct bt_iso_connect_param connect_param[CONFIG_BT_ISO_MAX_CHAN];
int err;

iso_conn_start_time = 0;

LOG_INF("Connecting ISO channels");

for (int i = 0; i < cig_create_param.num_cis; i++) {
Expand Down Expand Up @@ -1205,15 +1224,26 @@ static int run_central(void)
}
}

/* Creating the CIG before connecting verified that it's possible before establishing a
* connection, while also providing the controller information about our use case before
* creating the connection, which should provide additional information to the controller
* about which connection interval to use
*/
err = central_create_cig();
if (err != 0) {
LOG_ERR("Failed to create CIG: %d", err);
return err;
}

err = central_create_connection();
if (err != 0) {
LOG_ERR("Failed to create connection: %d", err);
return err;
}

err = central_create_cig();
err = central_connect_cis();
if (err != 0) {
LOG_ERR("Failed to create CIG or connect CISes: %d", err);
LOG_ERR("Failed to connect CISes: %d", err);
return err;
}

Expand Down
Loading