Skip to content
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

Jump to bottom

Bluetooth: GATT: factor out notify callback #75401

Merged
merged 1 commit into from
Jul 5, 2024
Merged

Bluetooth: GATT: factor out notify callback #75401

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
51 changes: 24 additions & 27 deletions subsys/bluetooth/host/gatt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3606,37 +3606,46 @@ static bool check_subscribe_security_level(struct bt_conn *conn,
return true;
}

void bt_gatt_notification(struct bt_conn *conn, uint16_t handle,
const void *data, uint16_t length)
static void call_notify_cb_and_maybe_unsubscribe(struct bt_conn *conn, struct gatt_sub *sub,
uint16_t handle, const void *data, uint16_t length)
{
struct bt_gatt_subscribe_params *params, *tmp;
struct gatt_sub *sub;

LOG_DBG("handle 0x%04x length %u", handle, length);

sub = gatt_sub_find(conn);
if (!sub) {
return;
}
int err;

SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&sub->list, params, tmp, node) {
if (handle != params->value_handle) {
continue;
}

if (check_subscribe_security_level(conn, params)) {
if (params->notify(conn, params, data, length) ==
BT_GATT_ITER_STOP) {
bt_gatt_unsubscribe(conn, params);
if (params->notify(conn, params, data, length) == BT_GATT_ITER_STOP) {
err = bt_gatt_unsubscribe(conn, params);
Comment on lines +3621 to +3622
Copy link
Member

Choose a reason for hiding this comment

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

Not related to this PR, but I have to say that this looks a bit awkward as an API, since it seems like it'd be trivial (and more clear & explicit) for the callback itself to do this unsubscribe() call rather than implying it through the return value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree. I spelled it out in the function name to surface that awkwardness :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also agree - Maybe something we can avoid if we get a new GATT API for EATT

if (err != 0) {
LOG_WRN("Failed to unsubscribe (err %d)", err);
}
}
}
}
}

void bt_gatt_notification(struct bt_conn *conn, uint16_t handle,
const void *data, uint16_t length)
{
struct gatt_sub *sub;

LOG_DBG("handle 0x%04x length %u", handle, length);

sub = gatt_sub_find(conn);
if (!sub) {
return;
}

call_notify_cb_and_maybe_unsubscribe(conn, sub, handle, data, length);
}

void bt_gatt_mult_notification(struct bt_conn *conn, const void *data,
uint16_t length)
{
struct bt_gatt_subscribe_params *params, *tmp;
const struct bt_att_notify_mult *nfy;
struct net_buf_simple buf;
struct gatt_sub *sub;
Expand Down Expand Up @@ -3666,19 +3675,7 @@ void bt_gatt_mult_notification(struct bt_conn *conn, const void *data,
return;
}

SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&sub->list, params, tmp,
node) {
if (handle != params->value_handle) {
continue;
}

if (check_subscribe_security_level(conn, params)) {
if (params->notify(conn, params, nfy->value, len) ==
BT_GATT_ITER_STOP) {
bt_gatt_unsubscribe(conn, params);
}
}
}
call_notify_cb_and_maybe_unsubscribe(conn, sub, handle, nfy->value, len);

net_buf_simple_pull_mem(&buf, len);
}
Expand Down
Loading