Added periodic adv feature updates in ble 5.3 #1537

RoshanESP · 2023-06-28T09:53:12Z

Added periodic adv enhancements(adi support) in Bluetooth spec 5.3
Current APIs don't have a way to add the parameters for periodic adv commands. So, modified the APIs wherever necessary.

RoshanESP · 2023-07-03T05:00:22Z

@sjanc can you PTAL?

sjanc

maybe instead of new MYNEWT_VAL(BLE_PERIODIC_ADV_ENH) simply depend some code on selected BLE version?

sjanc · 2023-09-18T06:59:09Z

nimble/host/include/host/ble_gap.h

-int ble_gap_periodic_adv_start(uint8_t instance);
+int
+ble_gap_periodic_adv_start(uint8_t instance,
+                           const struct ble_gap_periodic_adv_enable_params *params);


nitpick: since function is called adv_start I'd name params structure same (is ble_gap_periodic_adv_start_params)

sjanc · 2023-09-18T07:01:17Z

nimble/host/include/host/ble_gap.h

+struct ble_gap_periodic_adv_enable_params {
+#if MYNEWT_VAL(BLE_PERIODIC_ADV_ENH)
+    /** If include adi in aux_sync_ind PDU */
+    unsigned int include_adi:1;


there should be spaces around ":" ie. include_adi : 1;
(to keep this consistent with codebase)

sjanc · 2023-09-18T07:01:33Z

nimble/host/include/host/ble_gap.h

+struct ble_gap_periodic_adv_sync_report_params {
+#if MYNEWT_VAL(BLE_PERIODIC_ADV_ENH)
+    /** If filter duplicates */
+    unsigned int filter_duplicates:1;


sjanc · 2023-09-18T07:01:40Z

nimble/host/include/host/ble_gap.h

+struct ble_gap_periodic_adv_set_data_params {
+#if MYNEWT_VAL(BLE_PERIODIC_ADV_ENH)
+    /** If include adi in aux_sync_ind PDU */
+    unsigned int update_did:1;


sjanc · 2023-09-18T07:02:45Z

nimble/host/include/host/ble_gap.h

-int ble_gap_periodic_adv_sync_reporting(uint16_t sync_handle, bool enable);
+int ble_gap_periodic_adv_sync_reporting(uint16_t sync_handle,
+                                        bool enable,
+                                        const struct ble_gap_periodic_adv_sync_report_params *params);


same, lest keep params names consistent with function name ie. ble_gap_periodic_adv_sync_reporting_params

sjanc · 2023-09-18T07:03:50Z

nimble/host/src/ble_gap.c

+    static uint8_t buf[sizeof(struct ble_hci_le_set_periodic_adv_data_cp)];
+    struct ble_hci_le_set_periodic_adv_data_cp *cmd = (void *) buf;
+    uint16_t opcode;
+    memset(buf, 0, sizeof(buf));


empty line between variables declarations and code

sjanc · 2023-09-18T07:04:36Z

nimble/host/src/ble_gap.c

@@ -3838,7 +3877,16 @@ ble_gap_periodic_adv_set_data(uint8_t instance, struct os_mbuf *data)
        goto done;
    }

+#if MYNEWT_VAL(BLE_PERIODIC_ADV_ENH)
+    if (params && params -> update_did) {


"params->update_did" (no spaces around ->)

sjanc · 2023-09-18T07:11:40Z

nimble/host/src/ble_gap.c

+
+/* BLE_HCI_OCF_LE_SET_DEFAULT_SYNC_TRANSFER_PARAMS command api */
+int
+periodic_adv_set_default_sync_params(uint16_t conn_handle,


this doesn't look correct, default sync params shouldn't need connection handle, right?

also ble_hci_le_periodic_adv_sync_transfer_params_cp is not matching BLE_HCI_OCF_LE_SET_DEFAULT_SYNC_TRANSFER_PARAMS opcode...

sjanc · 2023-09-18T07:14:21Z

nimble/host/src/ble_gap.c

+
+    cmd.conn_handle = htole16(conn_handle);
+    cmd.sync_cte_type = 0x00;
+    cmd.mode = params->reports_disabled ? 0x01 : 0x02;


and how to disable this? maybe null params could be used for this?

sjanc · 2023-09-18T07:15:09Z

nimble/host/src/ble_gap.c

+    cmd.mode = params->reports_disabled ? 0x01 : 0x02;
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV_ENH)
+    if (params->filter_duplicates)


this should probably be set only if params->reports_disabled is not set (ie when reports are not disabled)

sjanc · 2023-09-18T07:22:17Z

oh, and while I'm fine with breaking public API here (unfortunate but...) with additions of parameter structure, all samples in repo needs to be updated in same commit to avoid CI breaking

RoshanESP · 2023-09-20T03:54:16Z

Hi @sjanc , Made the suggested changes. PTAL

sjanc

bttester application also requires update (it was missing in CI check due to misconfiguration of CI target, seehttps://github.com//pull/1629/ )

and some params can be const, otherwise looks OK

sjanc · 2023-09-29T09:21:54Z

nimble/host/include/host/ble_gap.h

-int ble_gap_periodic_adv_set_data(uint8_t instance, struct os_mbuf *data);
+int ble_gap_periodic_adv_set_data(uint8_t instance,
+                                  struct os_mbuf *data,
+                                  struct ble_gap_periodic_adv_set_data_params *params);


could be const

sjanc · 2023-09-29T09:22:55Z

nimble/host/src/ble_gap.c

-ble_gap_periodic_adv_set_data(uint8_t instance, struct os_mbuf *data)
+ble_gap_periodic_adv_set_data(uint8_t instance,
+                              struct os_mbuf *data,
+                              struct ble_gap_periodic_adv_set_data_params *params)


sjanc reviewed Sep 18, 2023

View reviewed changes

RoshanESP force-pushed the feature/periodic_adv_enhancement branch 3 times, most recently from a621764 to ba41c24 Compare September 20, 2023 03:27

RoshanESP force-pushed the feature/periodic_adv_enhancement branch 2 times, most recently from 2b90aa7 to 561838e Compare September 26, 2023 05:36

sjanc reviewed Sep 29, 2023

View reviewed changes

RoshanESP force-pushed the feature/periodic_adv_enhancement branch from 561838e to 4c5cc4e Compare October 18, 2023 10:28

added periodic adv feature updates in ble 5.3

a5b0c09

RoshanESP force-pushed the feature/periodic_adv_enhancement branch from 4c5cc4e to a5b0c09 Compare October 21, 2023 16:27

sjanc approved these changes Oct 23, 2023

View reviewed changes

sjanc merged commit fb3ccfd into apache:master Oct 24, 2023
14 checks passed

KKopyscinski mentioned this pull request Apr 23, 2024

host: GAP: ble_gap_periodic_adv_sync_create should use reports_disabled field #1512

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added periodic adv feature updates in ble 5.3 #1537

Added periodic adv feature updates in ble 5.3 #1537

RoshanESP commented Jun 28, 2023

RoshanESP commented Jul 3, 2023

sjanc left a comment

sjanc Sep 18, 2023

RoshanESP Sep 28, 2023

sjanc Sep 18, 2023

RoshanESP Sep 28, 2023

sjanc Sep 18, 2023

RoshanESP Sep 28, 2023

sjanc Sep 18, 2023

RoshanESP Sep 28, 2023

sjanc Sep 18, 2023

RoshanESP Sep 28, 2023

sjanc Sep 18, 2023

RoshanESP Sep 28, 2023

sjanc Sep 18, 2023

sjanc Sep 18, 2023

sjanc Sep 18, 2023

sjanc Sep 18, 2023

sjanc commented Sep 18, 2023

RoshanESP commented Sep 20, 2023

sjanc left a comment

sjanc Sep 29, 2023

RoshanESP Oct 18, 2023

sjanc Sep 29, 2023

RoshanESP Oct 18, 2023

Added periodic adv feature updates in ble 5.3 #1537

Added periodic adv feature updates in ble 5.3 #1537

Conversation

RoshanESP commented Jun 28, 2023

RoshanESP commented Jul 3, 2023

sjanc left a 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

sjanc commented Sep 18, 2023

RoshanESP commented Sep 20, 2023

sjanc left a 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