Skip to content

Bluetooth: Host: Bitwise-or CCC values #44136

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

Closed
wants to merge 3 commits into from
Closed

Bluetooth: Host: Bitwise-or CCC values #44136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2ac347f
Bluetooth: Host: Ensure only connected peers affect `_bt_gatt_ccc.value`
alwa-nordic Mar 23, 2022
677fea4
Bluetooth: Host: Bitwise-or CCC values in `_bt_gatt_ccc.value`
alwa-nordic Mar 23, 2022
8fa7c03
Bluetooth: Host: Document `_bt_gatt_ccc.cfg_changed`
alwa-nordic Mar 23, 2022
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
2 changes: 2 additions & 0 deletions doc/releases/release-notes-3.1.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
========================
Expand Down
18 changes: 14 additions & 4 deletions include/bluetooth/gatt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,13 +703,23 @@ 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
/** @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.
Comment on lines +714 to +716
Copy link
Contributor Author

@alwa-nordic alwa-nordic Mar 23, 2022

Choose a reason for hiding this comment

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

I would like to change this to trigger on any change to the cfg field. RFC

Copy link
Contributor Author

@alwa-nordic alwa-nordic Mar 23, 2022

Choose a reason for hiding this comment

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

On the other hand, it would be awkward because only value is passed as a parameter here.

Copy link
Contributor

@Thalley Thalley Mar 24, 2022

Choose a reason for hiding this comment

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

I see your point.
Technically a device keeps a CCCD for each client which is of course abstracted away by the API, so it appears as just a single value.

It does sound a bit odd to call the callback every time there is a change, since the value may not actually change here, and the application would thus not really be able to use that information for anything.

I think in the case where an application needs/wants more specific knowledge or handling of CCCD writes, it would just just the BT_GATT_CCC_MANAGED to do that.

*
* 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);

Expand Down
12 changes: 10 additions & 2 deletions subsys/bluetooth/host/gatt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1890,8 +1890,16 @@ 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) {
value |= ccc->cfg[i].value;

bt_conn_unref(conn);
conn = NULL;
Copy link
Member

@jhedberg jhedberg Mar 29, 2022

Choose a reason for hiding this comment

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

This setting to NULL doesn't really give you anything, as the function returns immediately thereafter and the variable goes out of scope

}
}

Expand Down