From 2ac347ffcabb339391d32d4e7c26c2bc35d67480 Mon Sep 17 00:00:00 2001 From: Aleksander Wasaznik Date: Wed, 23 Mar 2022 16:54:36 +0100 Subject: [PATCH 1/3] Bluetooth: Host: Ensure only connected peers affect `_bt_gatt_ccc.value` The doc on `_bt_gatt_ccc.value` specifies that only connected peers contribute to that value. But before this change, it was all computed from all entries in `_bt_gatt_ccc.cfg`, which include bonded but not connected peers when `CONFIG_BT_SETTINGS_CCC_LAZY_LOADING` is set. Signed-off-by: Aleksander Wasaznik --- subsys/bluetooth/host/gatt.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/subsys/bluetooth/host/gatt.c b/subsys/bluetooth/host/gatt.c index 51616c9df41ea..6b14205e5c412 100644 --- a/subsys/bluetooth/host/gatt.c +++ b/subsys/bluetooth/host/gatt.c @@ -1890,8 +1890,18 @@ static void gatt_ccc_changed(const struct bt_gatt_attr *attr, uint16_t value = 0x0000; for (i = 0; i < ARRAY_SIZE(ccc->cfg); i++) { - if (ccc->cfg[i].value > value) { - value = ccc->cfg[i].value; + /* `ccc->value` shall be a summary of connected peers' CCC values, but + * `ccc->cfg` can contain entries for bonded but not connected peers. + */ + struct bt_conn *conn = bt_conn_lookup_addr_le(ccc->cfg[i].id, &ccc->cfg[i].peer); + + if (conn) { + if (ccc->cfg[i].value > value) { + value = ccc->cfg[i].value; + } + + bt_conn_unref(conn); + conn = NULL; } } From 677fea4d90ecc2cf1d604aede6f18f3a5e37ed17 Mon Sep 17 00:00:00 2001 From: Aleksander Wasaznik Date: Wed, 23 Mar 2022 16:54:36 +0100 Subject: [PATCH 2/3] Bluetooth: Host: Bitwise-or CCC values in `_bt_gatt_ccc.value` Summarize the value of the CCC using bitwise-or instead of max. This is more natural and affords distinguishing between when only indications are enabled and when both notifications and indications are enabled. It is not expected that this change will affect any applications. It will not affect attributes that allow only one of notifications and indications. It will also not affect applications that only compare this value to zero. But it simplifies the interface of `_bt_gatt_ccc.value` and `_bt_gatt_ccc.cfg_changed`. Signed-off-by: Aleksander Wasaznik --- doc/releases/release-notes-3.1.rst | 2 ++ include/bluetooth/gatt.h | 2 +- subsys/bluetooth/host/gatt.c | 4 +--- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/releases/release-notes-3.1.rst b/doc/releases/release-notes-3.1.rst index 68049305d4beb..1db152dd18f84 100644 --- a/doc/releases/release-notes-3.1.rst +++ b/doc/releases/release-notes-3.1.rst @@ -52,6 +52,8 @@ Bluetooth * The enum bt_l2cap_chan_state values BT_L2CAP_CONNECT and BT_L2CAP_DISCONNECT has been renamed to BT_L2CAP_CONNECTING and BT_L2CAP_DISCONNECTING. + * :c:member:`_bt_gatt_ccc.value` is now a bitwise-or reduction of the + individual CCC values of connected clients instead of the maximum. New APIs in this release ======================== diff --git a/include/bluetooth/gatt.h b/include/bluetooth/gatt.h index 373536d0278a6..799420fa15ed6 100644 --- a/include/bluetooth/gatt.h +++ b/include/bluetooth/gatt.h @@ -703,7 +703,7 @@ struct _bt_gatt_ccc { /** Configuration for each connection */ struct bt_gatt_ccc_cfg cfg[BT_GATT_CCC_MAX]; - /** Highest value of all connected peer's subscriptions */ + /** Bitwise-or of all connected peer's CCC values */ uint16_t value; /** @brief CCC attribute changed callback diff --git a/subsys/bluetooth/host/gatt.c b/subsys/bluetooth/host/gatt.c index 6b14205e5c412..e80e38fcf5ea1 100644 --- a/subsys/bluetooth/host/gatt.c +++ b/subsys/bluetooth/host/gatt.c @@ -1896,9 +1896,7 @@ static void gatt_ccc_changed(const struct bt_gatt_attr *attr, struct bt_conn *conn = bt_conn_lookup_addr_le(ccc->cfg[i].id, &ccc->cfg[i].peer); if (conn) { - if (ccc->cfg[i].value > value) { - value = ccc->cfg[i].value; - } + value |= ccc->cfg[i].value; bt_conn_unref(conn); conn = NULL; From 8fa7c0305c37d7b4afa9f4dcda6ac275fa20422b Mon Sep 17 00:00:00 2001 From: Aleksander Wasaznik Date: Wed, 23 Mar 2022 16:54:36 +0100 Subject: [PATCH 3/3] Bluetooth: Host: Document `_bt_gatt_ccc.cfg_changed` This documents the behavior and a suggested use case. Signed-off-by: Aleksander Wasaznik --- include/bluetooth/gatt.h | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/include/bluetooth/gatt.h b/include/bluetooth/gatt.h index 799420fa15ed6..4fa1c18faa903 100644 --- a/include/bluetooth/gatt.h +++ b/include/bluetooth/gatt.h @@ -706,10 +706,20 @@ struct _bt_gatt_ccc { /** Bitwise-or of all connected peer's CCC values */ uint16_t value; - /** @brief CCC attribute changed callback + /** @brief React to changes to the @c value field of this struct * - * @param attr The attribute that's changed value - * @param value New value + * Use this event to optimize when to broadcast using bt_gatt_indicate() + * and bt_gatt_notify(). + * + * This event will not trigger if only the @c cfg field changes but the + * @c value field remains the same. Combine the use of this event and + * @c cfg_write to catch all changes to the @c cfg field. + * + * Note: This event may come before the connection event when CCC values + * are restored from bond information. + * + * @param attr The attribute this CCC refers to + * @param value New value of the @c value field */ void (*cfg_changed)(const struct bt_gatt_attr *attr, uint16_t value);