sjanc commented on code in PR #1537:
URL: https://github.com/apache/mynewt-nimble/pull/1537#discussion_r1328309874
##########
nimble/host/include/host/ble_gap.h:
##########
@@ -1558,10 +1588,15 @@ int ble_gap_periodic_adv_configure(uint8_t instance,
* Start periodic advertising for specified advertising instance.
*
* @param instance Instance ID
+ * @param params Additional arguments specifying the particulars
+ * of periodic advertising.
*
* @return 0 on success, error code on failure.
*/
-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);
Review Comment:
nitpick: since function is called adv_start I'd name params structure same
(is ble_gap_periodic_adv_start_params)
##########
nimble/host/include/host/ble_gap.h:
##########
@@ -1524,6 +1524,30 @@ struct ble_gap_periodic_adv_params {
uint16_t itvl_max;
};
+/** @brief Periodic advertising enable parameters */
+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;
+#endif
+};
+
+/** @brief Periodic advertising sync reporting parameters */
+struct ble_gap_periodic_adv_sync_report_params {
+#if MYNEWT_VAL(BLE_PERIODIC_ADV_ENH)
+ /** If filter duplicates */
+ unsigned int filter_duplicates:1;
Review Comment:
ditto
##########
nimble/host/src/ble_gap.c:
##########
@@ -4215,6 +4281,44 @@ periodic_adv_transfer_enable(uint16_t conn_handle,
cmd.conn_handle = htole16(conn_handle);
cmd.sync_cte_type = 0x00;
cmd.mode = params->reports_disabled ? 0x01 : 0x02;
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV_ENH)
+ if (params->filter_duplicates)
+ cmd.mode = 0x03;
+#endif
+
+ cmd.skip = htole16(params->skip);
+ cmd.sync_timeout = htole16(params->sync_timeout);
+
+ rc = ble_hs_hci_cmd_tx(opcode, &cmd, sizeof(cmd), &rsp, sizeof(rsp));
+ if (!rc) {
+ BLE_HS_DBG_ASSERT(le16toh(rsp.conn_handle) == conn_handle);
+ }
+
+ return rc;
+}
+
+/* BLE_HCI_OCF_LE_SET_DEFAULT_SYNC_TRANSFER_PARAMS command api */
+int
+periodic_adv_set_default_sync_params(uint16_t conn_handle,
Review Comment:
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...
##########
nimble/host/include/host/ble_gap.h:
##########
@@ -1625,10 +1664,14 @@ int ble_gap_periodic_adv_sync_terminate(uint16_t
sync_handle);
*
* @param sync_handle Handle identifying synchronization.
* @param enable If reports should be enabled.
+ * @param params Additional arguments specifying the particulars
+ * of periodic reports.
*
* @return 0 on success; nonzero on failure.
*/
-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);
Review Comment:
same, lest keep params names consistent with function name ie.
ble_gap_periodic_adv_sync_reporting_params
##########
nimble/host/src/ble_gap.c:
##########
@@ -4215,6 +4281,44 @@ periodic_adv_transfer_enable(uint16_t conn_handle,
cmd.conn_handle = htole16(conn_handle);
cmd.sync_cte_type = 0x00;
cmd.mode = params->reports_disabled ? 0x01 : 0x02;
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV_ENH)
+ if (params->filter_duplicates)
+ cmd.mode = 0x03;
+#endif
+
+ cmd.skip = htole16(params->skip);
+ cmd.sync_timeout = htole16(params->sync_timeout);
+
+ rc = ble_hs_hci_cmd_tx(opcode, &cmd, sizeof(cmd), &rsp, sizeof(rsp));
+ if (!rc) {
+ BLE_HS_DBG_ASSERT(le16toh(rsp.conn_handle) == conn_handle);
+ }
+
+ return rc;
+}
+
+/* BLE_HCI_OCF_LE_SET_DEFAULT_SYNC_TRANSFER_PARAMS command api */
+int
+periodic_adv_set_default_sync_params(uint16_t conn_handle,
+ const struct ble_gap_periodic_sync_params
*params)
+{
+ struct ble_hci_le_periodic_adv_sync_transfer_params_cp cmd;
+ struct ble_hci_le_periodic_adv_sync_transfer_params_rp rsp;
+ uint16_t opcode;
+ int rc;
+
+ opcode = BLE_HCI_OP(BLE_HCI_OGF_LE,
BLE_HCI_OCF_LE_SET_DEFAULT_SYNC_TRANSFER_PARAMS);
+
+ cmd.conn_handle = htole16(conn_handle);
+ cmd.sync_cte_type = 0x00;
+ cmd.mode = params->reports_disabled ? 0x01 : 0x02;
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV_ENH)
+ if (params->filter_duplicates)
Review Comment:
this should probably be set only if params->reports_disabled is not set (ie
when reports are not disabled)
##########
nimble/host/src/ble_gap.c:
##########
@@ -41,6 +41,10 @@
#define bssnz_t
#endif
+#if MYNEWT_VAL(BLE_PERIODIC_ADV_ENH)
+#define SET_BIT(t, n) (t |= 1UL << (n))
Review Comment:
no need to put macros under #ifdefs
##########
nimble/host/include/host/ble_gap.h:
##########
@@ -1524,6 +1524,30 @@ struct ble_gap_periodic_adv_params {
uint16_t itvl_max;
};
+/** @brief Periodic advertising enable parameters */
+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;
+#endif
+};
+
+/** @brief Periodic advertising sync reporting parameters */
+struct ble_gap_periodic_adv_sync_report_params {
+#if MYNEWT_VAL(BLE_PERIODIC_ADV_ENH)
+ /** If filter duplicates */
+ unsigned int filter_duplicates:1;
+#endif
+};
+
+/** @brief Periodic adv set data parameters */
+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;
Review Comment:
ditto
##########
nimble/host/include/host/ble_gap.h:
##########
@@ -1578,10 +1613,14 @@ int ble_gap_periodic_adv_stop(uint8_t instance);
*
* @param instance Instance ID
* @param data Chain containing the periodic advertising data.
+ * @param params Additional arguments specifying the particulars
+ of periodic advertising data.
*
* @return 0 on success or error code on failure.
*/
-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,
Review Comment:
indentation is not correct here?
##########
nimble/host/include/host/ble_gap.h:
##########
@@ -1524,6 +1524,30 @@ struct ble_gap_periodic_adv_params {
uint16_t itvl_max;
};
+/** @brief Periodic advertising enable parameters */
+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;
Review Comment:
there should be spaces around ":" ie. include_adi : 1;
(to keep this consistent with codebase)
##########
nimble/host/src/ble_gap.c:
##########
@@ -3703,6 +3713,25 @@ ble_gap_periodic_adv_start(uint8_t instance)
return 0;
}
+#if MYNEWT_VAL(BLE_PERIODIC_ADV_ENH)
+static int
+ble_gap_periodic_adv_update_did(uint8_t instance)
+{
+ 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));
Review Comment:
empty line between variables declarations and code
##########
nimble/host/src/ble_gap.c:
##########
@@ -4215,6 +4281,44 @@ periodic_adv_transfer_enable(uint16_t conn_handle,
cmd.conn_handle = htole16(conn_handle);
cmd.sync_cte_type = 0x00;
cmd.mode = params->reports_disabled ? 0x01 : 0x02;
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV_ENH)
+ if (params->filter_duplicates)
+ cmd.mode = 0x03;
+#endif
+
+ cmd.skip = htole16(params->skip);
+ cmd.sync_timeout = htole16(params->sync_timeout);
+
+ rc = ble_hs_hci_cmd_tx(opcode, &cmd, sizeof(cmd), &rsp, sizeof(rsp));
+ if (!rc) {
+ BLE_HS_DBG_ASSERT(le16toh(rsp.conn_handle) == conn_handle);
+ }
+
+ return rc;
+}
+
+/* BLE_HCI_OCF_LE_SET_DEFAULT_SYNC_TRANSFER_PARAMS command api */
+int
+periodic_adv_set_default_sync_params(uint16_t conn_handle,
+ const struct ble_gap_periodic_sync_params
*params)
+{
+ struct ble_hci_le_periodic_adv_sync_transfer_params_cp cmd;
+ struct ble_hci_le_periodic_adv_sync_transfer_params_rp rsp;
+ uint16_t opcode;
+ int rc;
+
+ opcode = BLE_HCI_OP(BLE_HCI_OGF_LE,
BLE_HCI_OCF_LE_SET_DEFAULT_SYNC_TRANSFER_PARAMS);
+
+ cmd.conn_handle = htole16(conn_handle);
+ cmd.sync_cte_type = 0x00;
+ cmd.mode = params->reports_disabled ? 0x01 : 0x02;
Review Comment:
and how to disable this? maybe null params could be used for this?
##########
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) {
Review Comment:
"params->update_did" (no spaces around ->)
--
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]
