sjanc commented on code in PR #1426:
URL: https://github.com/apache/mynewt-nimble/pull/1426#discussion_r1095553574
##########
nimble/include/nimble/nimble_opt_auto.h:
##########
@@ -105,6 +105,10 @@ extern "C" {
#define NIMBLE_BLE_ATT_CLT_INDICATE \
(MYNEWT_VAL(BLE_GATT_INDICATE))
+#undef NIMBLE_BLE_ATT_CLT_MULTI_NOTIFY
+#define NIMBLE_BLE_ATT_CLT_MULTI_NOTIFY \
+ (MYNEWT_VAL(BLE_GATT_MULTI_NOTIFY))
Review Comment:
MYNEWT_VAL(BLE_GATT_MULTI_NOTIFY) is never defined, is syscfg.yml change
missing?
##########
nimble/host/include/host/ble_gatt.h:
##########
@@ -473,6 +473,23 @@ 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 multiple characteristic notifications on the specified
+ * attribute handles. This function consumes the mbuf of the
+ * notification value after sending notification.
+ *
+ * @param conn_handle The connection over which to execute the
+ * procedure.
+ * @param att_handles The list of attribute handles to which
+ * notifications have to be sent.
+ * @param notif_values The list of notification value mbufs.
+ * @param num_handles The number of handles to notify.
+ *
+ * @return 0 on success; nonzero on failure.
+ */
+int ble_gatts_multi_notify_custom(uint16_t conn_handle, uint16_t * att_handles,
Review Comment:
So i'd prefer if att_handle and notif_value be combined eg.
struct ble_gatt_notif {
uint16_t handle;
struct os_mbuf * value;
}
int ble_gatts_multi_notify_custom(uint16_t conn_handle, struct
ble_gatt_notif * notif, unsigned int count);
##########
porting/examples/linux/include/syscfg/syscfg.h:
##########
@@ -531,6 +531,10 @@
#define MYNEWT_VAL_BLE_ATT_SVR_NOTIFY (1)
#endif
+#ifndef MYNEWT_VAL_BLE_ATT_SVR_MULTI_NOTIFY
+#define MYNEWT_VAL_BLE_ATT_SVR_MULTI_NOTIFY (1)
Review Comment:
hint: ports configuration should be updated with
porting/update_generated_files.sh script, not manually
##########
nimble/host/src/ble_gattc.c:
##########
@@ -4212,6 +4223,61 @@ ble_gatts_notify_custom(uint16_t conn_handle, uint16_t
chr_val_handle,
return rc;
}
+int
+ble_gatts_multi_notify_custom(uint16_t conn_handle, uint16_t * att_handles,
+ struct os_mbuf ** notif_values, uint16_t
num_handles)
+{
+#if !MYNEWT_VAL(BLE_GATT_MULTI_NOTIFY)
+ return BLE_HS_ENOTSUP;
+#endif
+
+ int i;
+ int rc;
+
+ STATS_INC(ble_gattc_stats, multi_notify);
+ ble_gattc_log_multi_notify(att_handles, num_handles);
+
+ for (i = 0; i < num_handles; i++) {
+ if (notif_values[i] == NULL) {
+ /* No custom attribute data; read the value from the specified
+ * attribute
+ */
+ notif_values[i] = ble_hs_mbuf_att_pkt();
+ if (notif_values[i] == NULL) {
+ rc = BLE_HS_ENOMEM;
+ goto done;
+ }
+ rc = ble_att_svr_read_handle(BLE_HS_CONN_HANDLE_NONE,
+ att_handles[i], 0, notif_values[i],
NULL);
+ if (rc != 0) {
+ /* Fatal error; application disallowed attribute read. */
+ rc = BLE_HS_EAPP;
+ goto done;
+ }
+ }
+ }
+
+ rc = ble_att_clt_tx_multi_notify(conn_handle, att_handles,
+ notif_values, num_handles);
+ if (rc != 0) {
Review Comment:
this check is not needed, done is right below
##########
nimble/host/src/ble_gattc.c:
##########
@@ -4212,6 +4223,61 @@ ble_gatts_notify_custom(uint16_t conn_handle, uint16_t
chr_val_handle,
return rc;
}
+int
+ble_gatts_multi_notify_custom(uint16_t conn_handle, uint16_t * att_handles,
+ struct os_mbuf ** notif_values, uint16_t
num_handles)
+{
+#if !MYNEWT_VAL(BLE_GATT_MULTI_NOTIFY)
+ return BLE_HS_ENOTSUP;
+#endif
+
+ int i;
+ int rc;
+
+ STATS_INC(ble_gattc_stats, multi_notify);
+ ble_gattc_log_multi_notify(att_handles, num_handles);
+
+ for (i = 0; i < num_handles; i++) {
+ if (notif_values[i] == NULL) {
+ /* No custom attribute data; read the value from the specified
Review Comment:
this behavior should be documented (as there is on _custom variant for this
procedure)
##########
nimble/host/src/ble_att_clt.c:
##########
@@ -907,6 +907,59 @@ ble_att_clt_tx_notify(uint16_t conn_handle, uint16_t
handle,
return rc;
}
+/*****************************************************************************
+* $multi handle value notification *
+*****************************************************************************/
+
+int
+ble_att_clt_tx_multi_notify(uint16_t conn_handle, uint16_t * att_handles,
+ struct os_mbuf ** notif_values, uint16_t
num_handles)
+{
+#if !NIMBLE_BLE_ATT_CLT_MULTI_NOTIFY
+ return BLE_HS_ENOTSUP;
+#endif
+
+ struct os_mbuf * txom;
+ int rc;
+ int i;
+
+ if (ble_att_cmd_get(BLE_ATT_OP_MULTI_NOTIFY_REQ, 0, &txom) == NULL) {
+ rc = BLE_HS_ENOMEM;
+ goto err;
+ }
+
+ for (i = 0; i < num_handles; i++) {
+ /* Handle */
+ rc = os_mbuf_copyinto(txom, OS_MBUF_PKTLEN(txom),
+ &att_handles[i], sizeof(att_handles[i]));
+ if (rc != 0) {
+ rc = BLE_HS_ENOMEM;
+ goto err;
+ }
+ /* Length */
+ rc = os_mbuf_copyinto(txom, OS_MBUF_PKTLEN(txom),
+ &(OS_MBUF_PKTLEN(notif_values[i])),
+ sizeof(OS_MBUF_PKTLEN(notif_values[i])));
+ if (rc != 0) {
+ rc = BLE_HS_ENOMEM;
+ goto err;
+ }
+ /* Value */
+ rc = os_mbuf_copyinto(txom, OS_MBUF_PKTLEN(txom),
Review Comment:
this may not work if source is chained, maybe os_mbuf_appendfrom should be
used instead?
##########
nimble/host/src/ble_att_clt.c:
##########
@@ -907,6 +907,59 @@ ble_att_clt_tx_notify(uint16_t conn_handle, uint16_t
handle,
return rc;
}
+/*****************************************************************************
+* $multi handle value notification *
+*****************************************************************************/
+
+int
+ble_att_clt_tx_multi_notify(uint16_t conn_handle, uint16_t * att_handles,
+ struct os_mbuf ** notif_values, uint16_t
num_handles)
+{
+#if !NIMBLE_BLE_ATT_CLT_MULTI_NOTIFY
+ return BLE_HS_ENOTSUP;
+#endif
+
+ struct os_mbuf * txom;
+ int rc;
+ int i;
+
+ if (ble_att_cmd_get(BLE_ATT_OP_MULTI_NOTIFY_REQ, 0, &txom) == NULL) {
+ rc = BLE_HS_ENOMEM;
+ goto err;
+ }
+
+ for (i = 0; i < num_handles; i++) {
+ /* Handle */
+ rc = os_mbuf_copyinto(txom, OS_MBUF_PKTLEN(txom),
+ &att_handles[i], sizeof(att_handles[i]));
+ if (rc != 0) {
+ rc = BLE_HS_ENOMEM;
+ goto err;
+ }
+ /* Length */
+ rc = os_mbuf_copyinto(txom, OS_MBUF_PKTLEN(txom),
+ &(OS_MBUF_PKTLEN(notif_values[i])),
+ sizeof(OS_MBUF_PKTLEN(notif_values[i])));
+ if (rc != 0) {
+ rc = BLE_HS_ENOMEM;
+ goto err;
+ }
+ /* Value */
+ rc = os_mbuf_copyinto(txom, OS_MBUF_PKTLEN(txom),
+ OS_MBUF_DATA(notif_values[i], void *),
+ OS_MBUF_PKTLEN(notif_values[i]));
+ if (rc != 0) {
+ rc = BLE_HS_ENOMEM;
+ goto err;
+ }
+ }
+
+ return ble_att_tx(conn_handle, txom);
Review Comment:
this function truncate data to ATT MTU size but according to spec "The
server shall not truncate a Handle Length Value Tuple".
I'd lean towards splitting such case into multiple multi-notification...
(probably in ble_gatts_multi_notify_custom)
--
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
