SumeetSingh19 commented on code in PR #1426:
URL: https://github.com/apache/mynewt-nimble/pull/1426#discussion_r1414940171
##########
nimble/host/src/ble_gattc.c:
##########
@@ -4212,6 +4223,131 @@ 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,
+ struct ble_gatt_hv * tuples, uint16_t num_tuples)
+{
+#if !MYNEWT_VAL(BLE_GATT_MULTI_NOTIFY)
+ return BLE_HS_ENOTSUP;
+#endif
+
+ int i;
+ int rc;
+ uint16_t mtu;
+ uint16_t pdu_size;
+ int split_at;
+ struct os_mbuf *om = NULL;
+
+ STATS_INC(ble_gattc_stats, multi_notify);
+ ble_gattc_log_multi_notify(tuples, num_tuples);
+
+ for (i = 0; i < num_tuples; i++) {
+ if (tuples[i].value == NULL) {
+ /* No custom attribute data; read the value from the specified
+ * attribute
+ */
+ tuples[i].value = ble_hs_mbuf_att_pkt();
+ if (tuples[i].value == NULL) {
+ rc = BLE_HS_ENOMEM;
+ goto done;
+ }
+ rc = ble_att_svr_read_handle(BLE_HS_CONN_HANDLE_NONE,
+ tuples[i].handle, 0, tuples[i].value,
NULL);
+ if (rc != 0) {
+ /* Fatal error; application disallowed attribute read. */
+ rc = BLE_HS_EAPP;
+ goto done;
+ }
+ }
+ }
+
+ mtu = ble_att_mtu(conn_handle);
+ pdu_size = sizeof(uint8_t); /* Opcode */
+ split_at = 0;
+
+ for (i = 0; i < num_tuples; i++) {
+ pdu_size += (2 * sizeof(uint16_t)) + OS_MBUF_PKTLEN(tuples[i].value);
+ if (pdu_size > mtu) {
+ /* The notification will need to be split */
+ if (i == split_at) {
+ /* Single notification too large for server,
+ * cannot send notify without truncating.
+ */
+ rc = BLE_HS_ENOMEM;
+ goto done;
+ }
+ split_at = i;
+ /* Reinitialize loop */
+ i--;
+ pdu_size = sizeof(uint8_t);
+ }
+ }
Review Comment:
Spec says we cannot truncate multi-notification, and if it is greater than
mtu, it needs to be split into smaller multi-notifications.
The second loop sends a multi-notification in each iteration that is less
than the mtu.
If the multi-notification contains a notification that itself is too large
for the mtu, it cannot be split and we cannot send the entire pdu and the
operation needs to be aborted.
this "too-large" notification can be anywhere inside the multi-notification.
Hence, we cannot risk splitting and sending a few multi-notifications only to
find that the next one has a "too-large" notification and the operation needs
to be aborted, cause at this point the client has received the notification
that have been sent.
That is why the first loop is needed to check if there isn't any "too-large"
single notifications.
I agree this is a very corner case and the loops look redundant, I will see
what I can do about the two loops.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
