sjanc commented on code in PR #1594:
URL: https://github.com/apache/mynewt-nimble/pull/1594#discussion_r1376098995
##########
nimble/host/include/host/ble_gatt.h:
##########
@@ -634,6 +644,33 @@ int ble_gattc_write_reliable(uint16_t conn_handle,
int ble_gatts_notify_custom(uint16_t conn_handle, uint16_t att_handle,
struct os_mbuf *om);
+/**
+ * Sends a "free-form" multiple handle variable length characteristic
+ * notification. This function consumes supplied mbufs regardless of the
+ * outcome. Notifications are sent in order of supplied entries.
+ * Function tries to send minimum amount of PDUs. If PDU can't contain all
+ * of the characteristic values, multiple notifications are sent. If only one
+ * handle-value pair fits into PDU, or only one characteristic remains in the
+ * list, regular characteristic notification is sent.
+ *
+ * If GATT client doesn't support receiving multiple handle notifications,
+ * this will use GATT notification for each characteristic, separately.
+ *
+ * If value of characteristic is not specified it will be read from local
+ * GATT database.
+ *
+ * @param conn_handle The connection over which to execute the
+ * procedure.
+ * @param chr_count Number of characteristics to notify about.
+ * @param tuples Handle-value pairs in form of `ble_gatt_notif`
+ * structures.
+ *
+ * @return 0 on success; nonzero on failure.
+ */
+int ble_gatts_notify_multiple_custom(uint16_t conn_handle,
+ uint16_t chr_count,
Review Comment:
I'd make chr_count unsigned int (or site_t)
##########
nimble/host/include/host/ble_gatt.h:
##########
@@ -659,6 +696,32 @@ int ble_gatts_notify(uint16_t conn_handle, uint16_t
chr_val_handle);
*/
int ble_gattc_notify(uint16_t conn_handle, uint16_t chr_val_handle);
+/**
+ * Sends a multiple handle variable length characteristic notification. The
+ * content of the message is read from the specified characteristics.
+ * Notifications are sent in order of supplied handles. Function tries to
+ * send minimum amount of PDUs. If PDU can't contain all of the
+ * characteristic values, multiple notifications are sent. If only one
+ * handle-value pair fits into PDU, or only one characteristic remains in the
+ * list, regular characteristic notification is sent.
+ *
+ * If GATT client doesn't support receiving multiple handle notifications,
+ * this will use GATT notification for each characteristic, separately.
+ *
+ * @param conn_handle The connection over which to execute the
+ * procedure.
+ * @param num_handles The number of entries in the "chr_val_handles"
+ * array.
+ * @param chr_val_handles Array of attribute handles of the
+ * characteristics to include in the outgoing
+ * notification.
+ *
+ * @return 0 on success; nonzero on failure.
+ */
+int ble_gatts_notify_multiple(uint16_t conn_handle,
+ uint8_t num_handles,
Review Comment:
ditto
##########
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)
Review Comment:
this whole feature should be available only for BLE >= 5.2
##########
nimble/host/src/ble_gattc.c:
##########
@@ -4413,6 +4413,120 @@ ble_gatts_notify(uint16_t conn_handle, uint16_t
chr_val_handle)
return rc;
}
+int
+ble_gatts_notify_multiple_custom(uint16_t conn_handle,
+ uint16_t chr_count,
+ struct ble_gatt_notif *tuples)
+{
+#if !MYNEWT_VAL(BLE_GATT_NOTIFY_MULTIPLE)
+ return BLE_HS_ENOTSUP;
+#endif
+
+ int rc;
+ int i = 0;
+ 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();
+ struct ble_hs_conn *conn;
+
+ conn = ble_hs_conn_find(conn_handle);
+ if (conn == NULL) {
+ return ENOTCONN;
+ }
+
+ if ((conn->bhc_gatt_svr.peer_cl_sup_feat[0] & 0x04) == 0) {
+ BLE_HS_LOG_DEBUG("not sup\n");
Review Comment:
this contradicts with description:
" * If GATT client doesn't support receiving multiple handle notifications,
* this will use GATT notification for each characteristic, separately."
##########
nimble/host/src/ble_gattc.c:
##########
@@ -4413,6 +4413,120 @@ ble_gatts_notify(uint16_t conn_handle, uint16_t
chr_val_handle)
return rc;
}
+int
+ble_gatts_notify_multiple_custom(uint16_t conn_handle,
+ uint16_t chr_count,
+ struct ble_gatt_notif *tuples)
+{
+#if !MYNEWT_VAL(BLE_GATT_NOTIFY_MULTIPLE)
+ return BLE_HS_ENOTSUP;
+#endif
+
+ int rc;
+ int i = 0;
+ 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();
+ struct ble_hs_conn *conn;
+
+ conn = ble_hs_conn_find(conn_handle);
+ if (conn == NULL) {
+ return ENOTCONN;
+ }
+
+ if ((conn->bhc_gatt_svr.peer_cl_sup_feat[0] & 0x04) == 0) {
+ BLE_HS_LOG_DEBUG("not sup\n");
+ return ENOTSUP;
+ }
+
+ /* Read missing values */
+ for (i = 0; i < chr_count; i++) {
+ if (tuples->handle == 0) {
+ rc = BLE_HS_EINVAL;
+ goto done;
+ }
+ if (tuples[i].value == NULL) {
+ rc = ble_att_svr_read_local(tuples[i].handle, &tuples[i].value);
+ if (rc != 0) {
+ goto done;
+ }
+ }
+ }
+
+ for (i = 0; i < chr_count; i++) {
+ if (txom->om_len + tuples[i].value->om_len > mtu && cur_chr_cnt < 2) {
+ rc = ble_att_clt_tx_notify(conn_handle, tuples[i].handle,
+ tuples[i].value);
+ if (rc != 0) {
+ goto done;
+ }
+ continue;
+ } else if (txom->om_len + tuples[i].value->om_len > mtu) {
+ rc = ble_att_clt_tx_notify_mult(conn_handle, txom);
+ if (rc != 0) {
+ goto done;
+ }
+ cur_chr_cnt = 0;
+ /* buffer was consumed, allocate new one */
+ txom = ble_hs_mbuf_att_pkt();
+ }
+
+ os_mbuf_append(txom, &tuples[i].handle, sizeof(uint16_t));
+ os_mbuf_append(txom, &tuples[i].value->om_len,
+ sizeof(uint16_t));
+ os_mbuf_concat(txom, tuples[i].value);
+ cur_chr_cnt++;
+ }
+
+ if (cur_chr_cnt == 1) {
+ rc = ble_att_clt_tx_notify(conn_handle, tuples[chr_count].handle,
+ tuples[chr_count].value);
+ } else {
+ rc = ble_att_clt_tx_notify_mult(conn_handle, txom);
+ }
+
+done:
+ return rc;
+}
+
+int
+ble_gatts_notify_multiple(uint16_t conn_handle,
+ uint8_t num_handles,
+ const uint16_t *chr_val_handles)
+{
+#if !MYNEWT_VAL(BLE_GATT_NOTIFY_MULTIPLE)
+ return BLE_HS_ENOTSUP;
+#endif
+ int rc, i;
+ struct ble_gatt_notif tuples[num_handles];
+ struct ble_hs_conn *conn;
+
+ BLE_HS_LOG_DEBUG("conn_handle %d\n", conn_handle);
+ conn = ble_hs_conn_find(conn_handle);
+ if (conn == NULL) {
+ return ENOTCONN;
+ }
+
+ /** Skip sending to client that doesn't support this feature */
Review Comment:
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."
##########
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 {
Review Comment:
this doesn't seem to be used anywhere
##########
nimble/host/src/ble_gattc.c:
##########
@@ -4413,6 +4413,120 @@ ble_gatts_notify(uint16_t conn_handle, uint16_t
chr_val_handle)
return rc;
}
+int
+ble_gatts_notify_multiple_custom(uint16_t conn_handle,
+ uint16_t chr_count,
+ struct ble_gatt_notif *tuples)
+{
+#if !MYNEWT_VAL(BLE_GATT_NOTIFY_MULTIPLE)
+ return BLE_HS_ENOTSUP;
+#endif
+
+ int rc;
+ int i = 0;
+ 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();
Review Comment:
null check
##########
nimble/host/src/ble_gattc.c:
##########
@@ -4413,6 +4413,120 @@ ble_gatts_notify(uint16_t conn_handle, uint16_t
chr_val_handle)
return rc;
}
+int
+ble_gatts_notify_multiple_custom(uint16_t conn_handle,
+ uint16_t chr_count,
+ struct ble_gatt_notif *tuples)
+{
+#if !MYNEWT_VAL(BLE_GATT_NOTIFY_MULTIPLE)
+ return BLE_HS_ENOTSUP;
+#endif
+
+ int rc;
+ int i = 0;
+ 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();
+ struct ble_hs_conn *conn;
+
+ conn = ble_hs_conn_find(conn_handle);
+ if (conn == NULL) {
+ return ENOTCONN;
+ }
+
+ if ((conn->bhc_gatt_svr.peer_cl_sup_feat[0] & 0x04) == 0) {
+ BLE_HS_LOG_DEBUG("not sup\n");
+ return ENOTSUP;
+ }
+
+ /* Read missing values */
+ for (i = 0; i < chr_count; i++) {
+ if (tuples->handle == 0) {
+ rc = BLE_HS_EINVAL;
+ goto done;
+ }
+ if (tuples[i].value == NULL) {
+ rc = ble_att_svr_read_local(tuples[i].handle, &tuples[i].value);
+ if (rc != 0) {
+ goto done;
+ }
+ }
+ }
+
+ for (i = 0; i < chr_count; i++) {
+ if (txom->om_len + tuples[i].value->om_len > mtu && cur_chr_cnt < 2) {
+ rc = ble_att_clt_tx_notify(conn_handle, tuples[i].handle,
+ tuples[i].value);
+ if (rc != 0) {
+ goto done;
+ }
+ continue;
+ } else if (txom->om_len + tuples[i].value->om_len > mtu) {
+ rc = ble_att_clt_tx_notify_mult(conn_handle, txom);
+ if (rc != 0) {
+ goto done;
+ }
+ cur_chr_cnt = 0;
+ /* buffer was consumed, allocate new one */
+ txom = ble_hs_mbuf_att_pkt();
Review Comment:
other code seems to always check if ble_hs_mbuf_att_pkt() returned valid
pointer
##########
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
Review Comment:
hmm where this requirement comes from?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
