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

subsys: bluetooth: User data in HID service callback #19692

Merged
merged 1 commit into from
Jan 8, 2025
Merged

subsys: bluetooth: User data in HID service callback #19692

Show file tree
Hide file tree
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
29 changes: 26 additions & 3 deletions include/bluetooth/services/hids.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,26 @@ int bt_hids_connected(struct bt_hids *hids_obj, struct bt_conn *conn);
*/
int bt_hids_disconnected(struct bt_hids *hids_obj, struct bt_conn *conn);

/** @brief Send Input Report, operation complete callback has user data as argument.
*
* @note The function is not thread safe.
* It cannot be called from multiple threads at the same time.
*
* @param hids_obj Pointer to HIDS instance.
* @param conn Pointer to Connection Object.
* @param rep_index Index of report descriptor.
* @param rep Pointer to the report data.
* @param len Length of report data.
* @param cb Notification complete callback (can be NULL).
* @param userdata Argument passed to notificaion complete callback.
*
* @return 0 If the operation was successful. Otherwise, a (negative) error
* code is returned.
*/
int bt_hids_inp_rep_send_userdata(struct bt_hids *hids_obj, struct bt_conn *conn,
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good, but I would consider deprecating/removing the old approach to keep API simpler.
We may use a Kconfig to toggle between APIs (the Kconfig could be deprecated and old API would be removed at some point). We could also consider simply breaking the API for NCS 3.0 if it's acceptable.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should also add a release note (API change)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would leave the old api as is to avoid generating noise. Note that if you deprecate the old function people will need to update their app and so waste time for no good reason.

I don't mind mentioning new function in release note but do we really intend to add a release note for any new api function if any random lib/service/module? Any opinion @b-gent ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would not insist here. On the other hand, keeping API smaller/simpler e.g. makes it easier for new users to get started.

uint8_t rep_index, uint8_t const *rep, uint8_t len,
bt_gatt_complete_func_t cb, void *userdata);

/** @brief Send Input Report.
*
* @note The function is not thread safe.
Expand All @@ -547,9 +567,12 @@ int bt_hids_disconnected(struct bt_hids *hids_obj, struct bt_conn *conn);
* @return 0 If the operation was successful. Otherwise, a (negative) error
* code is returned.
*/
int bt_hids_inp_rep_send(struct bt_hids *hids_obj, struct bt_conn *conn,
uint8_t rep_index, uint8_t const *rep, uint8_t len,
bt_gatt_complete_func_t cb);
static inline int bt_hids_inp_rep_send(struct bt_hids *hids_obj, struct bt_conn *conn,
uint8_t rep_index, uint8_t const *rep, uint8_t len,
bt_gatt_complete_func_t cb)
{
return bt_hids_inp_rep_send_userdata(hids_obj, conn, rep_index, rep, len, cb, NULL);
}

/** @brief Send Boot Mouse Input Report.
*
Expand Down
9 changes: 5 additions & 4 deletions subsys/bluetooth/services/hids.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1106,10 +1106,10 @@ static int inp_rep_notify_all(struct bt_hids *hids_obj,
}
}

int bt_hids_inp_rep_send(struct bt_hids *hids_obj,
struct bt_conn *conn, uint8_t rep_index,
uint8_t const *rep, uint8_t len,
bt_gatt_complete_func_t cb)
int bt_hids_inp_rep_send_userdata(struct bt_hids *hids_obj,
struct bt_conn *conn, uint8_t rep_index,
uint8_t const *rep, uint8_t len,
bt_gatt_complete_func_t cb, void *userdata)
{
struct bt_hids_inp_rep *hids_inp_rep =
&hids_obj->inp_rep_group.reports[rep_index];
Expand Down Expand Up @@ -1147,6 +1147,7 @@ int bt_hids_inp_rep_send(struct bt_hids *hids_obj,
params.data = rep;
params.len = hids_inp_rep->size;
params.func = cb;
params.user_data = userdata;

int err = bt_gatt_notify_cb(conn, &params);

Expand Down
Loading