Skip to content
Permalink
Browse files

Bluetooth: Simplify bt_gatt_notify_cb() API

This API had several issues:

 - The parameter types and order were inconsistent with e.g.
   bt_le_adv_start()
 - There were no real users of num_params, which just caused increased
   code size and memory consumption for no good reason.
 - The error handling policy was arbitrary: if one of the
   notifications would fail it would be impossible for the caller to
   know if some notifications succeeded, i.e. at what point the
   failure happened. Some callers might also want to make note of the
   failure but continue trying to notify for the remaining parameters.

The first issue is easily fixable, but because of the other two I
think it's best we don't have this code as part of the stack, rather
require whoever needs it to do the for loop themselves. It's just a
few lines of code, so the benefit of having this in the stack was
anyway quite minimal.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
  • Loading branch information...
jhedberg committed Jun 15, 2019
1 parent a84ded7 commit 4396dc9c71de721302515681ab4693a8c5c31610
Showing with 9 additions and 27 deletions.
  1. +3 −5 include/bluetooth/gatt.h
  2. +5 −21 subsys/bluetooth/host/gatt.c
  3. +1 −1 subsys/bluetooth/shell/gatt.c
@@ -788,8 +788,7 @@ struct bt_gatt_notify_params {
*
* This function works in the same way as @ref bt_gatt_notify.
* With the addition that after sending the notification the
* callback function will be called and can dispatch multiple
* notifications at once.
* callback function will be called.
*
* The callback is run from System Workqueue context.
*
@@ -798,12 +797,11 @@ struct bt_gatt_notify_params {
* start range when looking up for possible matches.
*
* @param conn Connection object.
* @param num_params Number of Notification parameters.
* @param params Notification parameters.
*
* @return 0 in case of success or negative value in case of error.
*/
int bt_gatt_notify_cb(struct bt_conn *conn, u16_t num_params,
int bt_gatt_notify_cb(struct bt_conn *conn,
struct bt_gatt_notify_params *params);

/** @brief Notify attribute value change.
@@ -837,7 +835,7 @@ static inline int bt_gatt_notify(struct bt_conn *conn,
params.data = data;
params.len = len;

return bt_gatt_notify_cb(conn, 1, &params);
return bt_gatt_notify_cb(conn, &params);
}

/** @typedef bt_gatt_indicate_func_t
@@ -1517,13 +1517,16 @@ static u8_t match_uuid(const struct bt_gatt_attr *attr, void *user_data)
return BT_GATT_ITER_STOP;
}

static int gatt_notify_params(struct bt_conn *conn,
struct bt_gatt_notify_params *params)
int bt_gatt_notify_cb(struct bt_conn *conn,
struct bt_gatt_notify_params *params)
{
struct notify_data data;
const struct bt_gatt_attr *attr;
u16_t handle;

__ASSERT(params, "invalid parameters\n");
__ASSERT(params->attr, "invalid parameters\n");

attr = params->attr;

handle = attr->handle ? : find_static_attr(attr);
@@ -1572,25 +1575,6 @@ static int gatt_notify_params(struct bt_conn *conn,
return data.err;
}

int bt_gatt_notify_cb(struct bt_conn *conn, u16_t num_params,
struct bt_gatt_notify_params *params)
{
int i, ret;

__ASSERT(params, "invalid parameters\n");
__ASSERT(num_params, "invalid parameters\n");
__ASSERT(params->attr, "invalid parameters\n");

for (i = 0; i < num_params; i++) {
ret = gatt_notify_params(conn, &params[i]);
if (ret < 0) {
return ret;
}
}

return 0;
}

int bt_gatt_indicate(struct bt_conn *conn,
struct bt_gatt_indicate_params *params)
{
@@ -790,7 +790,7 @@ static int cmd_notify(const struct shell *shell, size_t argc, char *argv[])
params.func = notify_cb;
params.user_data = (void *)shell;

bt_gatt_notify_cb(NULL, 1, &params);
bt_gatt_notify_cb(NULL, &params);

return 0;
}

0 comments on commit 4396dc9

Please sign in to comment.
You can’t perform that action at this time.