host/gatt: Add support for sending multiple handle notifications #1594

KKopyscinski · 2023-08-16T05:57:15Z

No description provided.

KKopyscinski · 2023-08-18T06:44:51Z

This relies on #1586 so will not build at this moment

rymanluk · 2023-10-16T08:30:40Z

nimble/host/src/ble_att_clt.c

+
+    cid = ble_eatt_get_available_chan_cid(conn_handle, BLE_GATT_OP_DUMMY);
+    rc = ble_att_tx(conn_handle, cid, txom2);
+    ble_eatt_release_chan(conn_handle, BLE_GATT_OP_DUMMY);


if (cid != BLE_L2CAP_CID_ATT) {
ble_eatt_release_chan()
}

rymanluk · 2023-10-16T08:34:02Z

nimble/host/src/ble_gattc.c

@@ -4413,6 +4413,109 @@ ble_gatts_notify(uint16_t conn_handle, uint16_t chr_val_handle)
    return rc;
 }

+int
+ble_gatts_notify_multiple_custom(uint16_t conn_handle,


if this is exposed to the user, I guess this function should also make sure that remote supports Multiple Notifications,

rymanluk · 2023-10-16T08:35:50Z

apps/btshell/src/cmd.c

+#endif
+    },
+    {
+        .sc_cmd = "gatt-send-pending-notif",


nit: "gatt-send-queued-notif"

rymanluk · 2023-10-16T08:36:07Z

apps/btshell/src/cmd.c

+#endif
+    },
+    {
+        .sc_cmd = "gatt-clear-pending-notif",


nit: gatt-clear-queued-notif

sjanc · 2023-10-30T12:11:14Z

nimble/host/include/host/ble_gatt.h

+ * @return                      0 on success; nonzero on failure.
+ */
+int ble_gatts_notify_multiple_custom(uint16_t conn_handle,
+                                     uint16_t chr_count,


I'd make chr_count unsigned int (or site_t)

changed to size_t

sjanc · 2023-10-30T12:12:13Z

nimble/host/include/host/ble_gatt.h

+ * @return                      0 on success; nonzero on failure.
+ */
+int ble_gatts_notify_multiple(uint16_t conn_handle,
+                              uint8_t num_handles,


sjanc · 2023-10-30T12:13:41Z

nimble/host/src/ble_att_cmd_priv.h

@@ -316,6 +316,9 @@ struct ble_att_exec_write_req {
 * | Attribute Handle Length Value Tuple List      | 8 to (ATT_MTU-1)  |
 */
 #define BLE_ATT_NOTIFY_MULTI_REQ_BASE_SZ      9
+struct ble_att_notify_mult_req {


this doesn't seem to be used anywhere

sjanc · 2023-10-30T12:19:02Z

nimble/host/src/ble_gattc.c

+            }
+            cur_chr_cnt = 0;
+            /* buffer was consumed, allocate new one */
+            txom = ble_hs_mbuf_att_pkt();


other code seems to always check if ble_hs_mbuf_att_pkt() returned valid pointer

sjanc · 2023-10-30T12:20:09Z

nimble/host/src/ble_gattc.c

+        return ENOTCONN;
+    }
+
+    /** Skip sending to client that doesn't support this feature */


this contradicts with description in header:
" * If GATT client doesn't support receiving multiple handle notifications,

this will use GATT notification for each characteristic, separately."

sjanc · 2023-10-30T12:20:34Z

nimble/host/src/ble_gattc.c

+    }
+
+    if ((conn->bhc_gatt_svr.peer_cl_sup_feat[0] & 0x04) == 0) {
+        BLE_HS_LOG_DEBUG("not sup\n");


this contradicts with description:
" * If GATT client doesn't support receiving multiple handle notifications,

this will use GATT notification for each characteristic, separately."

sjanc · 2023-10-30T12:21:11Z

nimble/host/src/ble_gattc.c

+    uint16_t cur_chr_cnt = 0;
+    /* mtu = MTU - 1 octet (OP code) */
+    uint16_t mtu = ble_att_mtu(conn_handle) - 1;
+    struct os_mbuf *txom = ble_hs_mbuf_att_pkt();


sjanc · 2023-10-30T12:30:24Z

nimble/host/syscfg.yml

@@ -277,6 +277,10 @@ syscfg.defs:
        description: >
            Enables sending and receiving of GATT indications. (0/1)
        value: 1
+    BLE_GATT_NOTIFY_MULTIPLE:
+        description: >
+            Enables sending and receiving of GATT multiple handle notifications. (0/1)


this whole feature should be available only for BLE >= 5.2

sjanc · 2023-10-30T12:31:26Z

nimble/host/syscfg.yml

@@ -304,6 +308,7 @@ syscfg.defs:
        description: >
            Number of CoC channels allocated to EATT
        value: 0
+        restrictions: BLE_GATT_NOTIFY_MULTIPLE


hmm where this requirement comes from?

This setting eanbles EATT, and this restriction comes from ICS 1.4 GATT Features, Multiple Variable Length Notifications, status C.10:

Excluded IF SUM ICS 31/17 “Core v4.2” OR SUM ICS 31/18 “Core v4.2+HS” OR SUM ICS 31/19 “Core v5.0” OR SUM ICS 31/20 “Core v5.1”, otherwise Mandatory IF GATT 2/3a “Enhanced ATT bearer over LE” OR GATT 2/3b “Enhanced ATT bearer over BR/EDR”, otherwise Optional.

so Mandatory IF GATT 2/3a “Enhanced ATT bearer over LE applies. So I think it's correct: to enable EATT restriction of enabled Multiple Notifications muist be fulfilled. Multiple Notifications can be also enabled without EATT, as optional

It is mandatory to support sending them if receiving is supported. Implemented functionality and added public API.

Sending Multiple Handle notifications is triggered by writing multiple values, at once, to GATT database. New values are set and notification is sent for each connection.

Added commands for queueing notifications to be sent using Multiple Handle Notification, sending them and clearing pending queue.

KKopyscinski · 2024-02-13T07:48:42Z

@sjanc can this be merged?

KKopyscinski force-pushed the multiple_notif branch 2 times, most recently from 0337f5b to 38a85f0 Compare August 18, 2023 05:50

KKopyscinski force-pushed the multiple_notif branch from 38a85f0 to f94160e Compare August 25, 2023 05:46

KKopyscinski marked this pull request as ready for review August 25, 2023 05:46

KKopyscinski force-pushed the multiple_notif branch 2 times, most recently from a75c2b4 to 12ec60e Compare September 26, 2023 09:13

rymanluk reviewed Oct 16, 2023

View reviewed changes

KKopyscinski force-pushed the multiple_notif branch from 12ec60e to 5fef664 Compare October 16, 2023 10:11

sjanc requested changes Oct 30, 2023

View reviewed changes

KKopyscinski force-pushed the multiple_notif branch from 5fef664 to e770607 Compare November 6, 2023 05:46

KKopyscinski force-pushed the multiple_notif branch 2 times, most recently from a9b7ea7 to 21b630e Compare November 17, 2023 11:01

KKopyscinski added 3 commits December 5, 2023 13:29

host/gatt_srv: Implement sending Multiple Handle Notifications

31e4933

It is mandatory to support sending them if receiving is supported. Implemented functionality and added public API.

Apps/bttester: add support for sending Multiple Handle Notifications

b54d1b0

Sending Multiple Handle notifications is triggered by writing multiple values, at once, to GATT database. New values are set and notification is sent for each connection.

apps/btshell: add support for sending Multiple handle Notifications

9ec3375

Added commands for queueing notifications to be sent using Multiple Handle Notification, sending them and clearing pending queue.

KKopyscinski force-pushed the multiple_notif branch from 21b630e to 9ec3375 Compare December 5, 2023 12:30

sjanc approved these changes Feb 13, 2024

View reviewed changes

KKopyscinski merged commit 1f559ab into apache:master Feb 13, 2024
15 of 16 checks passed

KKopyscinski deleted the multiple_notif branch February 14, 2024 06:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

host/gatt: Add support for sending multiple handle notifications #1594

host/gatt: Add support for sending multiple handle notifications #1594

KKopyscinski commented Aug 16, 2023

KKopyscinski commented Aug 18, 2023

rymanluk Oct 16, 2023 •

edited

Loading

rymanluk Oct 16, 2023

KKopyscinski Oct 16, 2023

rymanluk Oct 16, 2023

KKopyscinski Oct 16, 2023

rymanluk Oct 16, 2023

KKopyscinski Oct 16, 2023

sjanc Oct 30, 2023

KKopyscinski Nov 17, 2023

sjanc Oct 30, 2023

KKopyscinski Nov 17, 2023

sjanc Oct 30, 2023

sjanc Oct 30, 2023

sjanc Oct 30, 2023

sjanc Oct 30, 2023

sjanc Oct 30, 2023

sjanc Oct 30, 2023

sjanc Oct 30, 2023

KKopyscinski Nov 17, 2023

KKopyscinski commented Feb 13, 2024

host/gatt: Add support for sending multiple handle notifications #1594

host/gatt: Add support for sending multiple handle notifications #1594

Conversation

KKopyscinski commented Aug 16, 2023

KKopyscinski commented Aug 18, 2023

rymanluk Oct 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KKopyscinski commented Feb 13, 2024

rymanluk Oct 16, 2023 •

edited

Loading