From 93bfe7a2c93779e1dfa210edbb85fe015ec370fc Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Tue, 31 Aug 2021 09:51:20 +0200 Subject: [PATCH] Bluetooth: ISO: Update BIS index to start from 0x01 The HCI spec defines the BIS index range as starting from index 0x01. We had previously implemented it such that it starts from 0x00, and then simply adding 1 to the index when sending over HCI. However, this may cause issue with other HCI, or other SIG defined specification, commands and events, and thus it is probably simpler if we just use the HCI defined range. This commit disallows BIT(0) (representing the BIS index 0x00) to be set, and removes the addition of 1 when sending over HCI. Signed-off-by: Emil Gydesen --- include/bluetooth/iso.h | 7 ++++++- samples/bluetooth/iso_broadcast_benchmark/src/receiver.c | 3 ++- samples/bluetooth/iso_receive/src/main.c | 2 +- subsys/bluetooth/host/iso.c | 6 +++--- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/include/bluetooth/iso.h b/include/bluetooth/iso.h index 84fc7015ad7..705e99fb66b 100644 --- a/include/bluetooth/iso.h +++ b/include/bluetooth/iso.h @@ -313,7 +313,12 @@ struct bt_iso_big_sync_param { */ uint8_t num_bis; - /** Bitfield of the BISes to sync to */ + /** Bitfield of the BISes to sync to + * + * The BIS indexes start from 0x01, so the lowest allowed bit is + * BIT(1) that represents index 0x01. To synchronize to e.g. BIS + * indexes 0x01 and 0x02, the bitfield value should be BIT(1) | BIT(2). + */ uint32_t bis_bitfield; /** @brief Maximum subevents diff --git a/samples/bluetooth/iso_broadcast_benchmark/src/receiver.c b/samples/bluetooth/iso_broadcast_benchmark/src/receiver.c index 0638f0de0a2..998d688017a 100644 --- a/samples/bluetooth/iso_broadcast_benchmark/src/receiver.c +++ b/samples/bluetooth/iso_broadcast_benchmark/src/receiver.c @@ -363,7 +363,8 @@ static int create_big_sync(struct bt_iso_big **big, struct bt_le_per_adv_sync *s sync_timeout_ms = iso_interval_ms * ISO_RETRY_COUNT; big_sync_param.sync_timeout = CLAMP(sync_timeout_ms / 10, 0x000A, 0x4000); /* 10 ms units */ big_sync_param.num_bis = bis_count; - for (int i = 0; i < big_sync_param.num_bis; i++) { + /* BIS indexes start from 0x01, so add one to `i` */ + for (int i = 1; i <= big_sync_param.num_bis; i++) { big_sync_param.bis_bitfield |= BIT(i); } diff --git a/samples/bluetooth/iso_receive/src/main.c b/samples/bluetooth/iso_receive/src/main.c index a65cb6848d5..e5f8211131d 100644 --- a/samples/bluetooth/iso_receive/src/main.c +++ b/samples/bluetooth/iso_receive/src/main.c @@ -263,7 +263,7 @@ static struct bt_iso_chan *bis[BIS_ISO_CHAN_COUNT] = { &bis_iso_chan }; static struct bt_iso_big_sync_param big_sync_param = { .bis_channels = bis, .num_bis = BIS_ISO_CHAN_COUNT, - .bis_bitfield = (BIT(BIS_ISO_CHAN_COUNT) - 1), + .bis_bitfield = (BIT_MASK(BIS_ISO_CHAN_COUNT) << 1), .mse = 1, .sync_timeout = 100, /* in 10 ms units */ }; diff --git a/subsys/bluetooth/host/iso.c b/subsys/bluetooth/host/iso.c index 67ee0dd18db..8db970123c8 100644 --- a/subsys/bluetooth/host/iso.c +++ b/subsys/bluetooth/host/iso.c @@ -1850,13 +1850,13 @@ static int hci_le_big_create_sync(const struct bt_le_per_adv_sync *sync, struct req->sync_timeout = sys_cpu_to_le16(param->sync_timeout); req->num_bis = big->num_bis; /* Transform from bitfield to array */ - for (int i = 0; i < BT_ISO_MAX_GROUP_ISO_COUNT; i++) { + for (int i = 1; i <= BT_ISO_MAX_GROUP_ISO_COUNT; i++) { if (param->bis_bitfield & BIT(i)) { if (bit_idx == big->num_bis) { BT_DBG("BIG cannot contain %u BISes", bit_idx + 1); return -EINVAL; } - req->bis[bit_idx++] = i + 1; /* indices start from 1 */ + req->bis[bit_idx++] = i; } } @@ -1894,7 +1894,7 @@ int bt_iso_big_sync(struct bt_le_per_adv_sync *sync, struct bt_iso_big_sync_para return -EINVAL; } - CHECKIF(!param->bis_bitfield) { + CHECKIF(param->bis_bitfield <= BIT(0)) { BT_DBG("Invalid BIS bitfield 0x%08x", param->bis_bitfield); return -EINVAL; }