Add BIGInfo reports to controller and host #1507

m-gorecki · 2023-05-25T13:45:37Z

No description provided.

andrzej-kaczmarek · 2023-05-26T08:03:41Z

nimble/controller/src/ble_ll_sync.c

+                sm->flags |= BLE_LE_SYNC_SM_FLAG_BIGINFO;
+            } else {
+                return false;
+            }


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

andrzej-kaczmarek · 2023-05-26T08:04:54Z

nimble/controller/src/ble_ll_sync.c

+        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);
+        }


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

andrzej-kaczmarek · 2023-05-26T08:07:34Z

nimble/controller/src/ble_ll_sync.c

+
+    hci_ev = ble_transport_alloc_evt(1);
+    if (!hci_ev) {
+        assert(0);


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

andrzej-kaczmarek · 2023-05-26T08:10:22Z

nimble/controller/src/ble_ll_sync.c

+
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_BIGINFO_REPORTS)
+    struct ble_ll_acad_biginfo biginfo;
+#endif


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

no need to keep offset in sm, ble_ll_sync_check_acad is called from ble_ll_sync_rx_pkt_in so it can just return pointer to biginfo and biginfo length

andrzej-kaczmarek · 2023-05-26T10:42:21Z

nimble/host/include/host/ble_gap.h

+            uint16_t max_sdu;
+
+            /** Advertising PHY */
+            uint8_t phy;


this is big phy, not advertising phy

andrzej-kaczmarek · 2023-05-26T10:42:38Z

nimble/host/include/host/ble_gap.h

+            uint16_t max_pdu;
+
+            /** Service Data Unit Interval */
+            uint8_t sdu_interval[3];


uint32_t will be easier to handle by app
also move below max_sdu so we don't waste space on alignment unnecessarily

andrzej-kaczmarek · 2023-05-26T10:50:07Z

nimble/host/include/host/ble_gap.h

+            uint8_t framing;
+
+            /** Encryption */
+            uint8_t encryption;


use bitfield for both

andrzej-kaczmarek · 2023-05-26T10:51:10Z

nimble/host/src/ble_hs_startup.c

+    if (version >= BLE_HCI_VER_BCS_5_2) {
+        /**
+         * Enable the following LE events:
+         * 0x0000000200000000 LE Periodic Advertising Sync Transfer Received event


this is not the proper event (at least according to description)

andrzej-kaczmarek · 2023-05-26T10:52:02Z

nimble/host/src/ble_hs_hci_evt.c

@@ -128,6 +131,9 @@ static ble_hs_hci_evt_le_fn * const ble_hs_hci_evt_le_dispatch[] = {
    [BLE_HCI_LE_SUBEV_ADV_SET_TERMINATED] = ble_hs_hci_evt_le_adv_set_terminated,
    [BLE_HCI_LE_SUBEV_SCAN_REQ_RCVD] = ble_hs_hci_evt_le_scan_req_rcvd,
    [BLE_HCI_LE_SUBEV_PERIODIC_ADV_SYNC_TRANSFER] = ble_hs_hci_evt_le_periodic_adv_sync_transfer,
+#if MYNEWT_VAL(BLE_BIGINFO_REPORTS)
+    [BLE_HCI_LE_SUBEV_BIGINFO_ADV_REPORT] = ble_hs_hci_evt_le_biginfo_adv_rpt,


...adv_report

andrzej-kaczmarek · 2023-05-26T10:53:40Z

nimble/host/src/ble_gap.c

+    }
+    ble_hs_unlock();
+
+    if (!psync || !cb) {


it does not make sense to return if !cb since the same condition is checked below

andrzej-kaczmarek · 2023-05-29T14:29:42Z

nimble/controller/src/ble_ll_sync.c

+#define BLE_LL_SYNC_SM_FLAG_NEW_CHANMAP         0x0200
+#define BLE_LL_SYNC_SM_FLAG_CHAIN               0x0400
+
+#define BLE_LL_SYNC_ITVL_USECS                  1250


do not reformat code unnecesarilly

andrzej-kaczmarek · 2023-05-29T14:30:44Z

nimble/controller/src/ble_ll_sync.c

@@ -661,6 +669,72 @@ 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_parse_biginfo_adv_data_to_ev(struct ble_hci_ev_le_subev_biginfo_adv_report *ev, const uint8_t *acad, uint8_t biginfo_acad_offset)


ble_ll_sync_parse_biginfo_to_ev

also parameters should be biginfo pointer and length, it does not make sense to pass acad and offset and calculate biginfo pointer over and over again

andrzej-kaczmarek · 2023-05-30T08:25:25Z

nimble/host/src/ble_gap.c

+    event.biginfo_report.pto = ev->pto;
+    event.biginfo_report.irc = ev->irc;
+    event.biginfo_report.max_pdu = ev->max_pdu;
+    event.biginfo_report.sdu_interval = get_le32(&ev->sdu_interval[0]) & 0x00FFFFFF;


andrzej-kaczmarek · 2023-05-30T08:26:11Z

nimble/controller/include/controller/ble_ll.h

+    uint8_t framing;
+    uint8_t giv[8];
+    uint8_t gskd[16];
+};


this is not used anywhere

andrzej-kaczmarek · 2023-05-30T08:27:02Z

nimble/controller/src/ble_ll_sync.c

+
+#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_BIGINFO_REPORTS)
+    struct ble_ll_acad_biginfo biginfo;
+#endif


no need to keep offset in sm, ble_ll_sync_check_acad is called from ble_ll_sync_rx_pkt_in so it can just return pointer to biginfo and biginfo length

andrzej-kaczmarek · 2023-05-30T08:31:11Z

nimble/controller/src/ble_ll_sync.c

+}
+
+static void
+ble_ll_sync_send_biginfo_adv_rpt(struct ble_ll_sync_sm *sm, const uint8_t *acad)


biginfo and biginfo_len instead of acad

andrzej-kaczmarek · 2023-05-30T08:37:17Z

nimble/controller/syscfg.yml

@@ -323,6 +323,12 @@ syscfg.defs:
            Advertising Sync Transfer Feature.
        value: MYNEWT_VAL(BLE_PERIODIC_ADV_SYNC_TRANSFER)

+    BLE_LL_CFG_FEAT_LL_BIGINFO_REPORTS:


BLE_LL_PERIODIC_ADV_SYNC_BIGINFO_REPORTS

This allows to receive BIGInfo reports in periodic advertisement sync callback function

andrzej-kaczmarek requested changes May 26, 2023

View reviewed changes

m-gorecki force-pushed the biginfo branch from f43c544 to 0b87dac Compare May 29, 2023 10:48

andrzej-kaczmarek requested changes May 30, 2023

View reviewed changes

m-gorecki force-pushed the biginfo branch 2 times, most recently from ac0cecd to ae70df0 Compare May 31, 2023 07:10

andrzej-kaczmarek approved these changes May 31, 2023

View reviewed changes

m-gorecki added 2 commits May 31, 2023 10:35

nimble/ll: Add HCI events for BIGInfo reports

bf1e482

nimble/host: Add BIGInfo report event

f41d3b7

This allows to receive BIGInfo reports in periodic advertisement sync callback function

m-gorecki force-pushed the biginfo branch from ae70df0 to f41d3b7 Compare May 31, 2023 08:35

andrzej-kaczmarek merged commit 8d6cc49 into apache:master May 31, 2023
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add BIGInfo reports to controller and host #1507

Add BIGInfo reports to controller and host #1507

m-gorecki commented May 25, 2023

andrzej-kaczmarek May 26, 2023

andrzej-kaczmarek May 26, 2023

andrzej-kaczmarek May 26, 2023

andrzej-kaczmarek May 26, 2023

andrzej-kaczmarek May 30, 2023

andrzej-kaczmarek May 26, 2023

andrzej-kaczmarek May 26, 2023

andrzej-kaczmarek May 26, 2023

andrzej-kaczmarek May 26, 2023

andrzej-kaczmarek May 26, 2023

andrzej-kaczmarek May 26, 2023

andrzej-kaczmarek May 29, 2023

andrzej-kaczmarek May 29, 2023

andrzej-kaczmarek May 30, 2023

andrzej-kaczmarek May 30, 2023

andrzej-kaczmarek May 30, 2023

andrzej-kaczmarek May 30, 2023

andrzej-kaczmarek May 30, 2023

Add BIGInfo reports to controller and host #1507

Add BIGInfo reports to controller and host #1507

Conversation

m-gorecki commented May 25, 2023

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