[GitHub] [mynewt-nimble] andrzej-kaczmarek commented on a diff in pull request #1507: Add BIGInfo reports to controller and host

["andrzej-kaczmarek (via GitHub)" <gi...@apache.org>]

andrzej-kaczmarek commented on code in PR #1507:
URL: https://github.com/apache/mynewt-nimble/pull/1507#discussion_r1206382324


##########
nimble/controller/src/ble_ll_sync.c:
##########
@@ -1009,6 +1134,21 @@ ble_ll_sync_check_acad(struct ble_ll_sync_sm *sm,
             sm->chan_map_new_instant = le16toh(chmu->instant);
             sm->flags |= BLE_LL_SYNC_SM_FLAG_NEW_CHANMAP;
             break;
+
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_BIGINFO_REPORTS)
+        case BLE_LL_ACAD_BIGINFO:
+            if (ad_len - 1 == BLE_LL_SYNC_BIGINFO_LEN) {
+                ble_ll_sync_parse_biginfo(&acad[2], 0);
+                sm->flags |= BLE_LE_SYNC_SM_FLAG_BIGINFO;
+            } else if (ad_len - 1 == BLE_LL_SYNC_BIGINFO_LEN_ENC) {
+                ble_ll_sync_parse_biginfo(&acad[2], 1);
+                sm->flags |= BLE_LE_SYNC_SM_FLAG_BIGINFO;
+            } else {
+                return false;
+            }

Review Comment:
   you can just pass pointer and length to function, no need to check for encryption here



##########
nimble/controller/src/ble_ll_sync.c:
##########
@@ -117,6 +122,10 @@ struct ble_ll_sync_sm {
     uint16_t event_cntr_last_received;
     uint8_t adv_addr_rpa[6];
 #endif
+
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_BIGINFO_REPORTS)
+    struct ble_ll_acad_biginfo biginfo;
+#endif

Review Comment:
   there's no need to keep this huge structure in sm all the time. all you need is likely to check if biginfo is present in acad, store the offset and then parse it directly to hci event.
   
   for now I think it will be enough to return that offset from `ble_ll_sync_check_acad` and use it in `ble_ll_sync_rx_pkt_in` to check whether to send biginfo report



##########
nimble/controller/src/ble_ll_sync.c:
##########
@@ -661,6 +670,44 @@ ble_ll_sync_send_truncated_per_adv_rpt(struct ble_ll_sync_sm *sm, uint8_t *evbuf
     ble_ll_hci_event_send(hci_ev);
 }
 
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_BIGINFO_REPORTS)
+static void
+ble_ll_sync_send_biginfo_adv_rpt(struct ble_ll_sync_sm *sm)
+{
+    struct ble_hci_ev_le_subev_biginfo_adv_report *ev;
+    struct ble_hci_ev *hci_ev;
+
+    if (!ble_ll_hci_is_le_event_enabled(BLE_HCI_LE_SUBEV_BIGINFO_ADV_REPORT)) {
+        return;
+    }
+
+    hci_ev = ble_transport_alloc_evt(1);
+    if (!hci_ev) {
+        assert(0);

Review Comment:
   no need to assert here, we'll just skip that event



##########
nimble/controller/src/ble_ll_sync.c:
##########
@@ -1117,6 +1257,14 @@ ble_ll_sync_rx_pkt_in(struct os_mbuf *rxpdu, struct ble_mbuf_hdr *hdr)
         /* send reports from this PDU */
         ble_ll_sync_send_per_adv_rpt(sm, rxpdu, hdr->rxinfo.rssi, tx_power,
                                      datalen, aux, aux_scheduled);
+
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_BIGINFO_REPORTS)
+        /* send biginfo only if it is present and at least one per_adv_rpt event was sent */
+        if ((sm->flags & BLE_LE_SYNC_SM_FLAG_BIGINFO) &&
+            !(sm->flags & BLE_LL_SYNC_SM_FLAG_HCI_NO_DATA_SENT)) {
+            ble_ll_sync_send_biginfo_adv_rpt(sm);
+        }

Review Comment:
   you just send biginfo report if it's present in current PDU, no need to use additional flags



-- 
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-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org