* [Bluez PATCH v2] input/hog: support multiple variable length notification
@ 2021-04-07 8:37 Archie Pusaka
2021-04-07 9:04 ` [Bluez,v2] " bluez.test.bot
2021-04-08 17:08 ` [Bluez PATCH v2] " Luiz Augusto von Dentz
0 siblings, 2 replies; 3+ messages in thread
From: Archie Pusaka @ 2021-04-07 8:37 UTC (permalink / raw)
To: linux-bluetooth, Luiz Augusto von Dentz
Cc: CrosBT Upstreaming, Archie Pusaka, Sonny Sasaka
From: Archie Pusaka <apusaka@chromium.org>
Processing Multiple Variable Length Notification is mandatory if EATT
is enabled (Core Spec Vol 3 Part G Sec 4.2).
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
---
Changes in v2:
* fix compilation error
attrib/att.h | 1 +
profiles/input/hog-lib.c | 109 ++++++++++++++++++++++++++++-----------
src/attrib-server.c | 1 +
3 files changed, 81 insertions(+), 30 deletions(-)
diff --git a/attrib/att.h b/attrib/att.h
index 13a0c3a31f..0dbfb14b83 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -42,6 +42,7 @@
#define ATT_OP_HANDLE_NOTIFY 0x1B
#define ATT_OP_HANDLE_IND 0x1D
#define ATT_OP_HANDLE_CNF 0x1E
+#define ATT_OP_HANDLE_NOTIFY_MULTI 0x23
#define ATT_OP_SIGNED_WRITE_CMD 0xD2
/* Error codes for Error response PDU */
diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
index e5e3d3e7f5..aa6c64a3f2 100644
--- a/profiles/input/hog-lib.c
+++ b/profiles/input/hog-lib.c
@@ -65,7 +65,6 @@
#define HOG_REPORT_MAP_MAX_SIZE 512
#define HID_INFO_SIZE 4
-#define ATT_NOTIFICATION_HEADER_SIZE 3
struct bt_hog {
int ref_count;
@@ -112,7 +111,8 @@ struct report {
uint16_t value_handle;
uint8_t properties;
uint16_t ccc_handle;
- guint notifyid;
+ guint notify_id;
+ guint notify_multi_id;
uint16_t len;
uint8_t *value;
};
@@ -283,22 +283,14 @@ static void find_included(struct bt_hog *hog, GAttrib *attrib,
free(req);
}
-static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data)
+static void process_notification(struct report *report, guint16 len,
+ const uint8_t *data)
{
- struct report *report = user_data;
struct bt_hog *hog = report->hog;
struct uhid_event ev;
uint8_t *buf;
int err;
- if (len < ATT_NOTIFICATION_HEADER_SIZE) {
- error("Malformed ATT notification");
- return;
- }
-
- pdu += ATT_NOTIFICATION_HEADER_SIZE;
- len -= ATT_NOTIFICATION_HEADER_SIZE;
-
memset(&ev, 0, sizeof(ev));
ev.type = UHID_INPUT;
buf = ev.u.input.data;
@@ -306,19 +298,78 @@ static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data)
if (hog->has_report_id) {
buf[0] = report->id;
len = MIN(len, sizeof(ev.u.input.data) - 1);
- memcpy(buf + 1, pdu, len);
- ev.u.input.size = ++len;
+ memcpy(buf + 1, data, len);
+ ev.u.input.size = len + 1;
} else {
len = MIN(len, sizeof(ev.u.input.data));
- memcpy(buf, pdu, len);
+ memcpy(buf, data, len);
ev.u.input.size = len;
}
err = bt_uhid_send(hog->uhid, &ev);
- if (err < 0) {
+ if (err < 0)
error("bt_uhid_send: %s (%d)", strerror(-err), -err);
- return;
+}
+
+static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data)
+{
+ struct report *report = user_data;
+ uint8_t opcode = pdu[0];
+ guint16 report_len;
+ guint16 header_len;
+
+ /* Skip opcode field */
+ pdu += 1;
+ len -= 1;
+
+ if (opcode == ATT_OP_HANDLE_NOTIFY_MULTI)
+ header_len = 4;
+ else
+ header_len = 2;
+
+ if (len < header_len)
+ goto fail;
+
+ while (len >= header_len) {
+ /* Skip first 2 bytes (handle) */
+ pdu += 2;
+ len -= 2;
+
+ if (opcode == ATT_OP_HANDLE_NOTIFY_MULTI) {
+ report_len = get_le16(pdu);
+ pdu += 2;
+ len -= 2;
+
+ if (report_len > len)
+ goto fail;
+ } else {
+ report_len = len;
+ }
+
+ process_notification(report, report_len, pdu);
+
+ pdu += report_len;
+ len -= report_len;
}
+
+ if (len == 0)
+ return;
+
+fail:
+ error("Malformed ATT notification");
+}
+
+static void register_notify_handler(struct bt_hog *hog, struct report *report)
+{
+ report->notify_id = g_attrib_register(hog->attrib,
+ ATT_OP_HANDLE_NOTIFY,
+ report->value_handle,
+ report_value_cb, report, NULL);
+
+ report->notify_multi_id = g_attrib_register(hog->attrib,
+ ATT_OP_HANDLE_NOTIFY_MULTI,
+ report->value_handle,
+ report_value_cb, report, NULL);
}
static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
@@ -336,13 +387,10 @@ static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
return;
}
- if (report->notifyid)
+ if (report->notify_id)
return;
- report->notifyid = g_attrib_register(hog->attrib,
- ATT_OP_HANDLE_NOTIFY,
- report->value_handle,
- report_value_cb, report, NULL);
+ register_notify_handler(hog, report);
DBG("Report characteristic descriptor written: notifications enabled");
}
@@ -1711,13 +1759,10 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt)
for (l = hog->reports; l; l = l->next) {
struct report *r = l->data;
- if (r->notifyid)
+ if (r->notify_id)
continue;
- r->notifyid = g_attrib_register(hog->attrib,
- ATT_OP_HANDLE_NOTIFY,
- r->value_handle,
- report_value_cb, r, NULL);
+ register_notify_handler(hog, r);
}
return true;
@@ -1764,9 +1809,13 @@ void bt_hog_detach(struct bt_hog *hog)
for (l = hog->reports; l; l = l->next) {
struct report *r = l->data;
- if (r->notifyid > 0) {
- g_attrib_unregister(hog->attrib, r->notifyid);
- r->notifyid = 0;
+ if (r->notify_id > 0) {
+ g_attrib_unregister(hog->attrib, r->notify_id);
+ r->notify_id = 0;
+ }
+ if (r->notify_multi_id > 0) {
+ g_attrib_unregister(hog->attrib, r->notify_multi_id);
+ r->notify_multi_id = 0;
}
}
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 5a178f95ea..fb11d3db2d 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -1085,6 +1085,7 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
return;
case ATT_OP_HANDLE_IND:
case ATT_OP_HANDLE_NOTIFY:
+ case ATT_OP_HANDLE_NOTIFY_MULTI:
/* The attribute client is already handling these */
return;
case ATT_OP_READ_MULTI_REQ:
--
2.31.0.208.g409f899ff0-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread* RE: [Bluez,v2] input/hog: support multiple variable length notification
2021-04-07 8:37 [Bluez PATCH v2] input/hog: support multiple variable length notification Archie Pusaka
@ 2021-04-07 9:04 ` bluez.test.bot
2021-04-08 17:08 ` [Bluez PATCH v2] " Luiz Augusto von Dentz
1 sibling, 0 replies; 3+ messages in thread
From: bluez.test.bot @ 2021-04-07 9:04 UTC (permalink / raw)
To: linux-bluetooth, apusaka
[-- Attachment #1: Type: text/plain, Size: 758 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=462197
---Test result---
##############################
Test: CheckPatch - PASS
##############################
Test: CheckGitLint - PASS
##############################
Test: CheckBuild: Setup ELL - PASS
##############################
Test: CheckBuild: Setup - PASS
##############################
Test: CheckBuild - PASS
##############################
Test: MakeCheck - PASS
##############################
Test: CheckBuild w/external ell - PASS
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Bluez PATCH v2] input/hog: support multiple variable length notification
2021-04-07 8:37 [Bluez PATCH v2] input/hog: support multiple variable length notification Archie Pusaka
2021-04-07 9:04 ` [Bluez,v2] " bluez.test.bot
@ 2021-04-08 17:08 ` Luiz Augusto von Dentz
1 sibling, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2021-04-08 17:08 UTC (permalink / raw)
To: Archie Pusaka
Cc: linux-bluetooth, CrosBT Upstreaming, Archie Pusaka, Sonny Sasaka
Hi Archie,
On Wed, Apr 7, 2021 at 1:38 AM Archie Pusaka <apusaka@google.com> wrote:
>
> From: Archie Pusaka <apusaka@chromium.org>
>
> Processing Multiple Variable Length Notification is mandatory if EATT
> is enabled (Core Spec Vol 3 Part G Sec 4.2).
>
> Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
> ---
>
> Changes in v2:
> * fix compilation error
>
> attrib/att.h | 1 +
> profiles/input/hog-lib.c | 109 ++++++++++++++++++++++++++++-----------
> src/attrib-server.c | 1 +
> 3 files changed, 81 insertions(+), 30 deletions(-)
>
> diff --git a/attrib/att.h b/attrib/att.h
> index 13a0c3a31f..0dbfb14b83 100644
> --- a/attrib/att.h
> +++ b/attrib/att.h
> @@ -42,6 +42,7 @@
> #define ATT_OP_HANDLE_NOTIFY 0x1B
> #define ATT_OP_HANDLE_IND 0x1D
> #define ATT_OP_HANDLE_CNF 0x1E
> +#define ATT_OP_HANDLE_NOTIFY_MULTI 0x23
> #define ATT_OP_SIGNED_WRITE_CMD 0xD2
I rather not keep adding support for the attrib directory since we
want to get rid of it at some point and actually the main reason it
was not removed yet is because hog was not converted to use
bt_gatt_client so I'd say this would be a good time for bite the
bullet and make the conversion so we can proceed to remove it.
> /* Error codes for Error response PDU */
> diff --git a/profiles/input/hog-lib.c b/profiles/input/hog-lib.c
> index e5e3d3e7f5..aa6c64a3f2 100644
> --- a/profiles/input/hog-lib.c
> +++ b/profiles/input/hog-lib.c
> @@ -65,7 +65,6 @@
>
> #define HOG_REPORT_MAP_MAX_SIZE 512
> #define HID_INFO_SIZE 4
> -#define ATT_NOTIFICATION_HEADER_SIZE 3
>
> struct bt_hog {
> int ref_count;
> @@ -112,7 +111,8 @@ struct report {
> uint16_t value_handle;
> uint8_t properties;
> uint16_t ccc_handle;
> - guint notifyid;
> + guint notify_id;
> + guint notify_multi_id;
> uint16_t len;
> uint8_t *value;
> };
> @@ -283,22 +283,14 @@ static void find_included(struct bt_hog *hog, GAttrib *attrib,
> free(req);
> }
>
> -static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data)
> +static void process_notification(struct report *report, guint16 len,
> + const uint8_t *data)
> {
> - struct report *report = user_data;
> struct bt_hog *hog = report->hog;
> struct uhid_event ev;
> uint8_t *buf;
> int err;
>
> - if (len < ATT_NOTIFICATION_HEADER_SIZE) {
> - error("Malformed ATT notification");
> - return;
> - }
> -
> - pdu += ATT_NOTIFICATION_HEADER_SIZE;
> - len -= ATT_NOTIFICATION_HEADER_SIZE;
> -
> memset(&ev, 0, sizeof(ev));
> ev.type = UHID_INPUT;
> buf = ev.u.input.data;
> @@ -306,19 +298,78 @@ static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data)
> if (hog->has_report_id) {
> buf[0] = report->id;
> len = MIN(len, sizeof(ev.u.input.data) - 1);
> - memcpy(buf + 1, pdu, len);
> - ev.u.input.size = ++len;
> + memcpy(buf + 1, data, len);
> + ev.u.input.size = len + 1;
> } else {
> len = MIN(len, sizeof(ev.u.input.data));
> - memcpy(buf, pdu, len);
> + memcpy(buf, data, len);
> ev.u.input.size = len;
> }
>
> err = bt_uhid_send(hog->uhid, &ev);
> - if (err < 0) {
> + if (err < 0)
> error("bt_uhid_send: %s (%d)", strerror(-err), -err);
> - return;
> +}
> +
> +static void report_value_cb(const guint8 *pdu, guint16 len, gpointer user_data)
> +{
> + struct report *report = user_data;
> + uint8_t opcode = pdu[0];
> + guint16 report_len;
> + guint16 header_len;
> +
> + /* Skip opcode field */
> + pdu += 1;
> + len -= 1;
> +
> + if (opcode == ATT_OP_HANDLE_NOTIFY_MULTI)
> + header_len = 4;
> + else
> + header_len = 2;
> +
> + if (len < header_len)
> + goto fail;
> +
> + while (len >= header_len) {
> + /* Skip first 2 bytes (handle) */
> + pdu += 2;
> + len -= 2;
> +
> + if (opcode == ATT_OP_HANDLE_NOTIFY_MULTI) {
> + report_len = get_le16(pdu);
> + pdu += 2;
> + len -= 2;
> +
> + if (report_len > len)
> + goto fail;
> + } else {
> + report_len = len;
> + }
> +
> + process_notification(report, report_len, pdu);
> +
> + pdu += report_len;
> + len -= report_len;
> }
> +
> + if (len == 0)
> + return;
> +
> +fail:
> + error("Malformed ATT notification");
> +}
> +
> +static void register_notify_handler(struct bt_hog *hog, struct report *report)
> +{
> + report->notify_id = g_attrib_register(hog->attrib,
> + ATT_OP_HANDLE_NOTIFY,
> + report->value_handle,
> + report_value_cb, report, NULL);
> +
> + report->notify_multi_id = g_attrib_register(hog->attrib,
> + ATT_OP_HANDLE_NOTIFY_MULTI,
> + report->value_handle,
> + report_value_cb, report, NULL);
> }
>
> static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
> @@ -336,13 +387,10 @@ static void report_ccc_written_cb(guint8 status, const guint8 *pdu,
> return;
> }
>
> - if (report->notifyid)
> + if (report->notify_id)
> return;
>
> - report->notifyid = g_attrib_register(hog->attrib,
> - ATT_OP_HANDLE_NOTIFY,
> - report->value_handle,
> - report_value_cb, report, NULL);
> + register_notify_handler(hog, report);
>
> DBG("Report characteristic descriptor written: notifications enabled");
> }
> @@ -1711,13 +1759,10 @@ bool bt_hog_attach(struct bt_hog *hog, void *gatt)
> for (l = hog->reports; l; l = l->next) {
> struct report *r = l->data;
>
> - if (r->notifyid)
> + if (r->notify_id)
> continue;
>
> - r->notifyid = g_attrib_register(hog->attrib,
> - ATT_OP_HANDLE_NOTIFY,
> - r->value_handle,
> - report_value_cb, r, NULL);
> + register_notify_handler(hog, r);
> }
>
> return true;
> @@ -1764,9 +1809,13 @@ void bt_hog_detach(struct bt_hog *hog)
> for (l = hog->reports; l; l = l->next) {
> struct report *r = l->data;
>
> - if (r->notifyid > 0) {
> - g_attrib_unregister(hog->attrib, r->notifyid);
> - r->notifyid = 0;
> + if (r->notify_id > 0) {
> + g_attrib_unregister(hog->attrib, r->notify_id);
> + r->notify_id = 0;
> + }
> + if (r->notify_multi_id > 0) {
> + g_attrib_unregister(hog->attrib, r->notify_multi_id);
> + r->notify_multi_id = 0;
> }
> }
>
> diff --git a/src/attrib-server.c b/src/attrib-server.c
> index 5a178f95ea..fb11d3db2d 100644
> --- a/src/attrib-server.c
> +++ b/src/attrib-server.c
> @@ -1085,6 +1085,7 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
> return;
> case ATT_OP_HANDLE_IND:
> case ATT_OP_HANDLE_NOTIFY:
> + case ATT_OP_HANDLE_NOTIFY_MULTI:
> /* The attribute client is already handling these */
> return;
> case ATT_OP_READ_MULTI_REQ:
> --
> 2.31.0.208.g409f899ff0-goog
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-04-08 17:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-04-07 8:37 [Bluez PATCH v2] input/hog: support multiple variable length notification Archie Pusaka
2021-04-07 9:04 ` [Bluez,v2] " bluez.test.bot
2021-04-08 17:08 ` [Bluez PATCH v2] " Luiz Augusto von Dentz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.