You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by ja...@apache.org on 2019/06/08 14:15:16 UTC

[mynewt-nimble] 01/02: nimble/host: Refactor periodic sync support

This is an automated email from the ASF dual-hosted git repository.

janc pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-nimble.git

commit fafb11514f0e6192db05eecdfe81b3f900ba5a2e
Author: Szymon Janc <sz...@codecoup.pl>
AuthorDate: Tue Jun 4 12:08:33 2019 +0200

    nimble/host: Refactor periodic sync support
    
    This includes:
     - use of ble_addr_t where possible
     - use of struct parameters in public API for future proof
     - use only MYNEWT_VAL(BLE_PERIODIC_ADV) - deps are handled by newt
     - simplify some naming conventions
     - allow per sync event handler
     - simplify syncs list handling
     - generation of sync lost event on sync terminate (to align behavior
       with connection API)
     - some file rename for consistency
     - bugfixes
---
 nimble/host/include/host/ble_gap.h                 |  78 ++---
 nimble/host/src/ble_gap.c                          | 350 ++++++++++++++-------
 nimble/host/src/ble_hs_hci_cmd.c                   |  12 +-
 nimble/host/src/ble_hs_hci_evt.c                   |  16 +-
 ...e_hs_periodic_disc.c => ble_hs_periodic_sync.c} | 114 +------
 ...dic_disc_priv.h => ble_hs_periodic_sync_priv.h} |  30 +-
 nimble/host/src/ble_hs_priv.h                      |   2 +-
 7 files changed, 320 insertions(+), 282 deletions(-)

diff --git a/nimble/host/include/host/ble_gap.h b/nimble/host/include/host/ble_gap.h
index 7a805f0..8cbdc13 100644
--- a/nimble/host/include/host/ble_gap.h
+++ b/nimble/host/include/host/ble_gap.h
@@ -124,9 +124,9 @@ struct hci_conn_update;
 #define BLE_GAP_EVENT_REPEAT_PAIRING        17
 #define BLE_GAP_EVENT_PHY_UPDATE_COMPLETE   18
 #define BLE_GAP_EVENT_EXT_DISC              19
-#define BLE_GAP_EVENT_PERIODIC_ADV_SYNC_ESTAB    20
-#define BLE_GAP_EVENT_PERIODIC_DISC              21
-#define BLE_GAP_EVENT_PERIODIC_ADV_SYNC_LOST     22
+#define BLE_GAP_EVENT_PERIODIC_SYNC         20
+#define BLE_GAP_EVENT_PERIODIC_REPORT       21
+#define BLE_GAP_EVENT_PERIODIC_SYNC_LOST    22
 
 /*** Reason codes for the subscribe GAP event. */
 
@@ -284,6 +284,7 @@ struct ble_gap_ext_disc_desc {
     uint8_t sid;
     uint8_t prim_phy;
     uint8_t sec_phy;
+    uint16_t periodic_adv_itvl;
     uint8_t length_data;
     uint8_t *data;
     /***
@@ -711,45 +712,47 @@ struct ble_gap_event {
             uint8_t tx_phy;
             uint8_t rx_phy;
         } phy_updated;
-#if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
         /**
-         * Represents an periodic advertising sync established during discovery
-         * procedure.  Valid for the following event types:
-         *     o BLE_GAP_EVENT_PERIODIC_ADV_SYNC_ESTAB
+         * Represents a periodic advertising sync established during discovery
+         * procedure. Valid for the following event types:
+         *     o BLE_GAP_EVENT_PERIODIC_SYNC
          */
         struct {
             uint8_t status;
             uint16_t sync_handle;
             uint8_t sid;
-            uint8_t adv_addr_type;
-            uint8_t adv_addr[6];
+            ble_addr_t adv_addr;
             uint8_t adv_phy;
             uint16_t per_adv_ival;
             uint8_t adv_clk_accuracy;
-        } periodic_adv_sync_estab;
+        } periodic_sync;
 
         /**
-         * Represents a Periodic advertising report received during discovery
-         * procedure.  Valid for the following event types:
-         *     o BLE_GAP_EVENT_PERIODIC_DISC
+         * Represents a periodic advertising report received on established
+         * sync. Valid for the following event types:
+         *     o BLE_GAP_EVENT_PERIODIC_REPORT
          */
         struct {
             uint16_t sync_handle;
-            uint8_t tx_power;
+            int8_t tx_power;
             int8_t rssi;
             uint8_t data_status;
             uint8_t data_length;
             uint8_t *data;
-        } periodic_disc;
+        } periodic_report;
 
         /**
-         * Represents an periodic advertising sync Lost during discovery
-         * procedure.  Valid for the following event types:
-         *     o BLE_GAP_EVENT_PERIODIC_ADV_SYNC_LOST
+         * Represents a periodic advertising sync lost of established sync.
+         * Sync lost reason can be BLE_HS_ETIMEOUT (sync timeout) or
+         * BLE_HS_EDONE (sync terminated locally).
+         * Valid for the following event types:
+         *     o BLE_GAP_EVENT_PERIODIC_SYNC_LOST
          */
         struct {
             uint16_t sync_handle;
-        } periodic_adv_sync_lost;
+            int reason;
+        } periodic_sync_lost;
 #endif
     };
 };
@@ -946,15 +949,6 @@ struct ble_gap_ext_adv_params {
     uint8_t sid;
 };
 
-#if MYNEWT_VAL(BLE_PERIODIC_ADV)
-struct ble_gap_periodic_adv_params {
-    unsigned int include_tx_power:1;
-
-    uint16_t itvl_min;
-    uint16_t itvl_max;
-};
-#endif
-
 int ble_gap_ext_adv_configure(uint8_t instance,
                               const struct ble_gap_ext_adv_params *params,
                               int8_t *selected_tx_power,
@@ -972,19 +966,34 @@ int ble_gap_ext_adv_remove(uint8_t instance);
  *                      other error code on failure.
  */
 int ble_gap_ext_adv_clear(void);
+#endif
 
-#if MYNEWT_VAL(BLE_PERIODIC_ADV)
 /* Periodic Advertising */
-int ble_gap_periodic_adv_configure(uint8_t instance, const struct ble_gap_periodic_adv_params *params);
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+struct ble_gap_periodic_adv_params {
+    unsigned int include_tx_power:1;
+
+    uint16_t itvl_min;
+    uint16_t itvl_max;
+};
+
+struct ble_gap_periodic_sync_params {
+    uint16_t skip;
+    uint16_t sync_timeout;
+};
+
+int ble_gap_periodic_adv_configure(uint8_t instance,
+                                   const struct ble_gap_periodic_adv_params *params);
 int ble_gap_periodic_adv_start(uint8_t instance);
 int ble_gap_periodic_adv_stop(uint8_t instance);
 int ble_gap_periodic_adv_set_data(uint8_t instance, struct os_mbuf *data);
-int ble_gap_periodic_adv_create_sync(const ble_addr_t *peer_addr, uint8_t adv_sid,
-                                     uint8_t filter_policy, uint16_t skip,
-                                     uint16_t sync_timeout);
+
+int ble_gap_periodic_adv_create_sync(const ble_addr_t *addr, uint8_t adv_sid,
+                                     const struct ble_gap_periodic_sync_params *params,
+                                     ble_gap_event_fn *cb, void *cb_arg);
 int ble_gap_periodic_adv_create_sync_cancel(void);
 int ble_gap_periodic_adv_terminate_sync(uint16_t sync_handle);
-int ble_gap_add_dev_to_periodic_adv_list(uint8_t adv_add_type, const uint8_t *adv_add,
+int ble_gap_add_dev_to_periodic_adv_list(const ble_addr_t *peer_addr,
                                          uint8_t adv_sid);
 int ble_gap_rem_dev_from_periodic_adv_list(const ble_addr_t *peer_addr,
                                            uint8_t adv_sid);
@@ -992,7 +1001,6 @@ int ble_gap_clear_periodic_adv_list(void);
 int ble_gap_read_periodic_adv_list_size(uint8_t *per_adv_list_size);
 #endif
 
-#endif
 
 /**
  * Performs the Limited or General Discovery Procedures.
diff --git a/nimble/host/src/ble_gap.c b/nimble/host/src/ble_gap.c
index 128a17f..e54aff1 100644
--- a/nimble/host/src/ble_gap.c
+++ b/nimble/host/src/ble_gap.c
@@ -79,6 +79,7 @@
 #define BLE_GAP_OP_M_CONN           2
 #define BLE_GAP_OP_S_ADV            1
 #define BLE_GAP_OP_S_PERIODIC_ADV   2
+#define BLE_GAP_OP_SYNC             1
 
 /**
  * If an attempt to cancel an active procedure fails, the attempt is retried
@@ -129,15 +130,24 @@ struct ble_gap_master_state {
             uint8_t limited:1;
         } disc;
     };
+};
+static bssnz_t struct ble_gap_master_state ble_gap_master;
 
 #if MYNEWT_VAL(BLE_PERIODIC_ADV)
-    /* Tells if create_sync was called without receiving sync
-     * established event
-     */
-    uint8_t        pending_create_sync;
-#endif
+/**
+ * The state of the in-progress sync creation. If no sync creation connection is
+ * currently in progress, then the op field is set to BLE_GAP_OP_NULL.
+ */
+struct ble_gap_sync_state {
+    uint8_t op;
+    struct ble_hs_periodic_sync *psync;
+
+    ble_gap_event_fn *cb;
+    void *cb_arg;
 };
-static bssnz_t struct ble_gap_master_state ble_gap_master;
+
+static bssnz_t struct ble_gap_sync_state ble_gap_sync;
+#endif
 
 /**
  * The state of the in-progress slave connection.  If no slave connection is
@@ -158,7 +168,7 @@ struct ble_gap_slave_state {
     unsigned int rnd_addr_set:1;
 #if MYNEWT_VAL(BLE_PERIODIC_ADV)
     unsigned int periodic_configured:1;
-    uint8_t       periodic_op;
+    uint8_t      periodic_op;
 #endif
     uint8_t rnd_addr[6];
 #else
@@ -1295,106 +1305,132 @@ ble_gap_rx_adv_set_terminated(struct hci_le_adv_set_terminated *evt)
 #endif
 
 /* Periodic adv events */
-#if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+
 void
-ble_gap_rx_peroidic_adv_sync_estab(
-        struct hci_le_subev_periodic_adv_sync_estab *evt)
+ble_gap_rx_peroidic_adv_sync_estab(struct hci_le_subev_periodic_adv_sync_estab *evt)
 {
     struct ble_gap_event event;
-    struct ble_gap_master_state state;
-    struct ble_hs_periodic_sync * psync;
+    ble_gap_event_fn *cb;
+    void *cb_arg;
 
     memset(&event, 0, sizeof event);
 
-    /* There must be memory for psync as the check was done when creating
-     * sync
-     */
-    psync = ble_hs_periodic_sync_alloc();
-    BLE_HS_DBG_ASSERT(psync != NULL);
+    event.type = BLE_GAP_EVENT_PERIODIC_SYNC;
+    event.periodic_sync.status = evt->status;
 
-    psync->sync_handle               = evt->sync_handle;
-    psync->adv_sid                   = evt->sid;
-    psync->advertiser_addr_type      = evt->adv_addr_type;
+    ble_hs_lock();
 
-    memcpy(psync->advertiser_addr.val, evt->adv_addr, 6);
+    BLE_HS_DBG_ASSERT(ble_gap_sync.psync);
 
-    psync->advertiser_phy            = evt->adv_phy;
-    psync->periodic_adv_itvl         = evt->per_adv_ival;
-    psync->advertiser_clock_accuracy = evt->adv_clk_accuracy;
+    if (!evt->status) {
+        ble_gap_sync.psync->sync_handle  = evt->sync_handle;
+        ble_gap_sync.psync->adv_sid = evt->sid;
+        memcpy(ble_gap_sync.psync->advertiser_addr.val, evt->adv_addr, 6);
+        ble_gap_sync.psync->advertiser_addr.type = evt->adv_addr_type;
 
-    ble_hs_lock();
-    ble_hs_periodic_sync_insert(psync);
-    ble_hs_unlock();
+        ble_gap_sync.psync->cb = ble_gap_sync.cb;
+        ble_gap_sync.psync->cb_arg = ble_gap_sync.cb_arg;
 
-    event.type = BLE_GAP_EVENT_PERIODIC_ADV_SYNC_ESTAB;
-    event.periodic_adv_sync_estab.status             = evt->status;
-    event.periodic_adv_sync_estab.sync_handle        = evt->sync_handle;
-    event.periodic_adv_sync_estab.sid                = evt->sid;
-    event.periodic_adv_sync_estab.adv_addr_type      = evt->adv_addr_type;
+        event.periodic_sync.sync_handle = evt->sync_handle;
+        event.periodic_sync.sid = evt->sid;
+        event.periodic_sync.adv_addr = ble_gap_sync.psync->advertiser_addr;
+        event.periodic_sync.adv_phy = evt->adv_phy;
+        event.periodic_sync.per_adv_ival = evt->per_adv_ival;
+        event.periodic_sync.adv_clk_accuracy = evt->adv_clk_accuracy;
 
-    memcpy(event.periodic_adv_sync_estab.adv_addr, evt->adv_addr, 6);
+        ble_hs_periodic_sync_insert(ble_gap_sync.psync);
+    } else {
+        ble_hs_periodic_sync_free(ble_gap_sync.psync);
+    }
 
-    event.periodic_adv_sync_estab.adv_phy            = evt->adv_phy;
-    event.periodic_adv_sync_estab.per_adv_ival       = evt->per_adv_ival;
-    event.periodic_adv_sync_estab.adv_clk_accuracy   = evt->adv_clk_accuracy;
+    cb = ble_gap_sync.cb;
+    cb_arg = ble_gap_sync.cb_arg;
 
-    ble_gap_master.pending_create_sync = 0;
+    ble_gap_sync.op = BLE_GAP_OP_NULL;
+    ble_gap_sync.cb_arg = NULL;
+    ble_gap_sync.cb_arg = NULL;
+    ble_gap_sync.psync = NULL;
 
-    ble_gap_master_extract_state(&state, 0);
-    if (ble_gap_has_client(&state)) {
-        state.cb(&event, state.cb_arg);
+    ble_hs_unlock();
+
+    ble_gap_event_listener_call(&event);
+    if (cb) {
+        cb(&event, cb_arg);
     }
 }
 
 void
 ble_gap_rx_periodic_adv_rpt(struct hci_le_subev_periodic_adv_rpt *evt)
 {
+    struct ble_hs_periodic_sync *psync;
     struct ble_gap_event event;
-    struct ble_gap_master_state state;
+    ble_gap_event_fn *cb;
+    void *cb_arg;
+
+    ble_hs_lock();
+    psync = ble_hs_periodic_sync_find_by_handle(evt->sync_handle);
+    if (psync) {
+        cb = psync->cb;
+        cb_arg = psync->cb_arg;
+    }
+    ble_hs_unlock();
+
+    if (!psync || !cb) {
+        return;
+    }
 
     memset(&event, 0, sizeof event);
 
-    event.type = BLE_GAP_EVENT_PERIODIC_DISC;
-    event.periodic_disc.sync_handle = evt->sync_handle;
-    event.periodic_disc.tx_power = evt->tx_power;
-    event.periodic_disc.rssi = evt->rssi;
-    event.periodic_disc.data_status = evt->data_status;
-    event.periodic_disc.data_length = evt->data_length;
-    event.periodic_disc.data = evt->data;
+    event.type = BLE_GAP_EVENT_PERIODIC_REPORT;
+    event.periodic_report.sync_handle = evt->sync_handle;
+    event.periodic_report.tx_power = evt->tx_power;
+    event.periodic_report.rssi = evt->rssi;
+    event.periodic_report.data_status = evt->data_status;
+    event.periodic_report.data_length = evt->data_length;
+    event.periodic_report.data = evt->data;
 
-    ble_gap_master_extract_state(&state, 0);
-    if (ble_gap_has_client(&state)) {
-        state.cb(&event, state.cb_arg);
-    }
+    /* TODO should we allow for listener too? this can be spammy and is more
+     * like ACL data, not general event
+     */
+     cb(&event, cb_arg);
 }
 
 void
-ble_gap_rx_periodic_adv_sync_lost(
-        struct hci_le_subev_periodic_adv_sync_lost *evt)
+ble_gap_rx_periodic_adv_sync_lost(struct hci_le_subev_periodic_adv_sync_lost *evt)
 {
-    struct ble_gap_event event;
-    struct ble_gap_master_state state;
     struct ble_hs_periodic_sync *psync;
+    struct ble_gap_event event;
+    ble_gap_event_fn *cb;
+    void *cb_arg;
 
     memset(&event, 0, sizeof event);
 
-    event.type = BLE_GAP_EVENT_PERIODIC_ADV_SYNC_LOST;
-    event.periodic_adv_sync_lost.sync_handle = evt->sync_handle;
+    event.type = BLE_GAP_EVENT_PERIODIC_SYNC_LOST;
+    event.periodic_sync_lost.sync_handle = evt->sync_handle;
+    event.periodic_sync_lost.reason = BLE_HS_ETIMEOUT;
 
     ble_hs_lock();
     /* The handle must be in the list */
-    psync = ble_hs_periodic_sync_find_assert(evt->sync_handle);
+    psync = ble_hs_periodic_sync_find_by_handle(evt->sync_handle);
+    BLE_HS_DBG_ASSERT(psync);
+
+    cb = psync->cb;
+    cb_arg = psync->cb_arg;
 
     /* Remove the handle from the list */
     ble_hs_periodic_sync_remove(psync);
+    ble_hs_unlock();
+
+    /* remove any sync_lost event from queue */
+    ble_npl_eventq_remove(ble_hs_evq_get(), &psync->lost_ev);
 
     /* Free the memory occupied by psync as it is no longer needed */
     ble_hs_periodic_sync_free(psync);
-    ble_hs_unlock();
 
-    ble_gap_master_extract_state(&state, 0);
-    if (ble_gap_has_client(&state)) {
-        state.cb(&event, state.cb_arg);
+    ble_gap_event_listener_call(&event);
+    if (cb) {
+        cb(&event, cb_arg);
     }
 }
 #endif
@@ -3025,8 +3061,7 @@ ble_gap_ext_adv_clear(void)
         }
 
 #if MYNEWT_VAL(BLE_PERIODIC_ADV)
-        if ((ble_gap_slave[instance].periodic_op ==
-                        BLE_GAP_OP_S_PERIODIC_ADV)) {
+        if (ble_gap_slave[instance].periodic_op == BLE_GAP_OP_S_PERIODIC_ADV) {
             ble_hs_unlock();
             return BLE_HS_EBUSY;
         }
@@ -3194,7 +3229,7 @@ ble_gap_periodic_adv_start(uint8_t instance)
 
 static int
 ble_gap_periodic_adv_set(uint8_t instance, uint16_t opcode,
-        struct os_mbuf **data)
+                         struct os_mbuf **data)
 {
     /* In that case we always fit all data in single HCI command */
 #if MYNEWT_VAL(BLE_EXT_ADV_MAX_SIZE) <= BLE_HCI_MAX_PERIODIC_ADV_DATA_LEN
@@ -3295,7 +3330,7 @@ ble_gap_periodic_adv_set(uint8_t instance, uint16_t opcode,
 
 static int
 ble_gap_periodic_adv_set_data_validate(uint8_t instance,
-        struct os_mbuf *data)
+                                       struct os_mbuf *data)
 {
     /* The corresponding extended advertising instance should be configured */
     if (!ble_gap_slave[instance].configured) {
@@ -3332,10 +3367,8 @@ ble_gap_periodic_adv_set_data(uint8_t instance, struct os_mbuf *data)
         goto done;
     }
 
-    rc = ble_gap_periodic_adv_set(
-            instance,
-            BLE_HCI_OCF_LE_SET_PERIODIC_ADV_DATA,
-            &data);
+    rc = ble_gap_periodic_adv_set(instance,
+                                  BLE_HCI_OCF_LE_SET_PERIODIC_ADV_DATA, &data);
 
     ble_hs_unlock();
 
@@ -3384,43 +3417,100 @@ ble_gap_periodic_adv_stop(uint8_t instance)
     return rc;
 }
 
+static void
+ble_gap_npl_sync_lost(struct ble_npl_event *ev)
+{
+    struct ble_hs_periodic_sync *psync;
+    struct ble_gap_event event;
+    ble_gap_event_fn *cb;
+    void *cb_arg;
+
+    /* this psync is no longer on list so no lock needed */
+    psync = ble_npl_event_get_arg(ev);
+    cb = psync->cb;
+    cb_arg = psync->cb_arg;
+
+    memset(&event, 0, sizeof event);
+
+    event.type = BLE_GAP_EVENT_PERIODIC_SYNC_LOST;
+    event.periodic_sync_lost.sync_handle = psync->sync_handle;
+    event.periodic_sync_lost.reason = BLE_HS_EDONE;
+
+    /* Free the memory occupied by psync as it is no longer needed */
+    ble_hs_periodic_sync_free(psync);
+
+    ble_gap_event_listener_call(&event);
+    if (cb) {
+        cb(&event, cb_arg);
+    }
+}
+
 int
-ble_gap_periodic_adv_create_sync(const ble_addr_t *peer_addr, uint8_t adv_sid,
-                                 uint8_t filter_policy, uint16_t skip,
-                                 uint16_t sync_timeout)
+ble_gap_periodic_adv_create_sync(const ble_addr_t *addr, uint8_t adv_sid,
+                                 const struct ble_gap_periodic_sync_params *params,
+                                 ble_gap_event_fn *cb, void *cb_arg)
 {
     uint8_t buf[BLE_HCI_LE_PERIODIC_ADV_CREATE_SYNC_LEN];
+    struct ble_hs_periodic_sync *psync;
     uint16_t opcode;
     int rc = 0;
 
-    if (!ble_hs_periodic_sync_can_alloc()) {
-        return BLE_HS_ENOMEM;
-    }
-    opcode = BLE_HCI_OP(BLE_HCI_OGF_LE,
-                                    BLE_HCI_OCF_LE_PERIODIC_ADV_CREATE_SYNC);
+    ble_hs_lock();
 
     /* No sync can be created if another sync is still pending */
-    if (ble_gap_master.pending_create_sync) {
+    if (ble_gap_sync.op == BLE_GAP_OP_SYNC) {
+        ble_hs_unlock();
         return BLE_HS_EBUSY;
     }
 
-    rc = ble_hs_hci_cmd_build_le_periodic_adv_create_sync(filter_policy,
-                                                          adv_sid,
-                                                          peer_addr->type,
-                                                          peer_addr->val,
-                                                          skip, sync_timeout,
-                                                          buf, sizeof(buf));
+    /* cannot create another sync if already synchronized */
+    if (ble_hs_periodic_sync_find(addr, adv_sid)) {
+        ble_hs_unlock();
+        return BLE_HS_EALREADY;
+    }
+
+    /* preallocate sync element */
+    psync = ble_hs_periodic_sync_alloc();
+    if (!psync) {
+        ble_hs_unlock();
+        return BLE_HS_ENOMEM;
+    }
+
+    ble_npl_event_init(&psync->lost_ev, ble_gap_npl_sync_lost, psync);
+
+    if (addr) {
+        rc = ble_hs_hci_cmd_build_le_periodic_adv_create_sync(0x00, adv_sid,
+                                        addr->type, addr->val, params->skip,
+                                        params->sync_timeout, buf, sizeof(buf));
+    } else {
+        rc = ble_hs_hci_cmd_build_le_periodic_adv_create_sync(0x01, adv_sid,
+                            BLE_ADDR_ANY->type, BLE_ADDR_ANY->val, params->skip,
+                            params->sync_timeout, buf, sizeof(buf));
+    }
+
     if (rc != 0) {
+        ble_hs_periodic_sync_free(psync);
+        ble_hs_unlock();
         return rc;
     }
 
-    /* This shall be reset upon receiving sync_established event,
-     * or if the sync is cancelled before receiving that event.
-     */
-    ble_gap_master.pending_create_sync = 1;
-
+    opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, BLE_HCI_OCF_LE_PERIODIC_ADV_CREATE_SYNC);
     rc = ble_hs_hci_cmd_tx_empty_ack(opcode, buf, sizeof(buf));
 
+    if (!rc) {
+        /* This shall be reset upon receiving sync_established event,
+         * or if the sync is cancelled before receiving that event.
+         */
+        ble_gap_sync.op = BLE_GAP_OP_SYNC;
+        ble_gap_sync.cb = cb;
+        ble_gap_sync.cb_arg = cb_arg;
+        ble_gap_sync.psync = psync;
+    } else {
+        ble_hs_periodic_sync_free(psync);
+    }
+
+    ble_hs_unlock();
+
     return rc;
 }
 
@@ -3430,18 +3520,28 @@ ble_gap_periodic_adv_create_sync_cancel(void)
     uint16_t opcode;
     int rc = 0;
 
-    if (!ble_gap_master.pending_create_sync) {
+    ble_hs_lock();
+
+    if (ble_gap_sync.op != BLE_GAP_OP_SYNC) {
+        ble_hs_unlock();
         return BLE_HS_EBUSY;
     }
 
     opcode = BLE_HCI_OP(BLE_HCI_OGF_LE,
-            BLE_HCI_OCF_LE_PERIODIC_ADV_CREATE_SYNC_CANCEL);
+                        BLE_HCI_OCF_LE_PERIODIC_ADV_CREATE_SYNC_CANCEL);
 
     rc = ble_hs_hci_cmd_tx_empty_ack(opcode, NULL, 0);
 
     if (!rc) {
-        ble_gap_master.pending_create_sync = 0;
+        ble_gap_sync.op = BLE_GAP_OP_NULL;
+        ble_gap_sync.cb = NULL;
+        ble_gap_sync.cb_arg = NULL;
+        ble_hs_periodic_sync_free(ble_gap_sync.psync);
+        ble_gap_sync.psync = NULL;
     }
+
+    ble_hs_unlock();
+
     return rc;
 }
 
@@ -3453,52 +3553,65 @@ ble_gap_periodic_adv_terminate_sync(uint16_t sync_handle)
     struct ble_hs_periodic_sync *psync;
     int rc = 0;
 
-    if (ble_gap_master.pending_create_sync) {
+    ble_hs_lock();
+
+    if (ble_gap_sync.op == BLE_GAP_OP_SYNC) {
+        ble_hs_unlock();
         return BLE_HS_EBUSY;
     }
 
-    ble_hs_lock();
     /* The handle must be in the list. If it doesn't exist, it means
      * that the sync may have been lost at the same moment in which
-     * the app wants to terminate that sync handle */
-    psync = ble_hs_periodic_sync_find(sync_handle);
+     * the app wants to terminate that sync handle
+     */
+    psync = ble_hs_periodic_sync_find_by_handle(sync_handle);
     if (!psync) {
         /* Sync already terminated.*/
+        ble_hs_unlock();
         return BLE_HS_ENOTCONN;
     }
 
-    /* Remove the handle from the list */
-    ble_hs_periodic_sync_remove(psync);
-
-    /* Free the memory occupied by psync as it is no longer needed */
-    ble_hs_periodic_sync_free(psync);
-    ble_hs_unlock();
-
     opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, BLE_HCI_OCF_LE_PERIODIC_ADV_TERM_SYNC);
 
     rc = ble_hs_hci_cmd_build_le_periodic_adv_terminate_sync(sync_handle,
-                                                            buf, sizeof(buf));
+                                                             buf, sizeof(buf));
     if (rc != 0) {
+        ble_hs_unlock();
         return rc;
     }
 
     rc = ble_hs_hci_cmd_tx_empty_ack(opcode, buf, sizeof(buf));
 
+    if (rc == 0) {
+        /* Remove the handle from the list */
+        ble_hs_periodic_sync_remove(psync);
+
+        /* send sync_lost event, this is to mimic connection behavior and thus
+         * simplify application error handling
+         */
+        ble_npl_eventq_put(ble_hs_evq_get(), &psync->lost_ev);
+    }
+
+    ble_hs_unlock();
+
     return rc;
 }
 
 int
-ble_gap_add_dev_to_periodic_adv_list(uint8_t adv_addr_type,
-        const uint8_t *adv_addr, uint8_t adv_sid)
+ble_gap_add_dev_to_periodic_adv_list(const ble_addr_t *peer_addr,
+                                     uint8_t adv_sid)
 {
     uint8_t buf[BLE_HCI_LE_ADD_DEV_TO_PERIODIC_ADV_LIST_LEN];
     uint16_t opcode;
     int rc = 0;
 
-    opcode = BLE_HCI_OP(BLE_HCI_OGF_LE, BLE_HCI_OCF_LE_ADD_DEV_TO_PERIODIC_ADV_LIST);
+    opcode = BLE_HCI_OP(BLE_HCI_OGF_LE,
+                        BLE_HCI_OCF_LE_ADD_DEV_TO_PERIODIC_ADV_LIST);
 
-    rc = ble_hs_hci_cmd_build_le_add_dev_to_periodic_adv_list(adv_addr_type,
-            adv_addr, adv_sid, buf, sizeof(buf));
+    rc = ble_hs_hci_cmd_build_le_add_dev_to_periodic_adv_list(peer_addr->type,
+                                                              peer_addr->val,
+                                                              adv_sid, buf,
+                                                              sizeof(buf));
     if (rc != 0) {
         return rc;
     }
@@ -3516,10 +3629,12 @@ ble_gap_rem_dev_from_periodic_adv_list(const ble_addr_t *peer_addr, uint8_t adv_
     int rc = 0;
 
     opcode = BLE_HCI_OP(BLE_HCI_OGF_LE,
-            BLE_HCI_OCF_LE_REM_DEV_FROM_PERIODIC_ADV_LIST);
+                        BLE_HCI_OCF_LE_REM_DEV_FROM_PERIODIC_ADV_LIST);
 
     rc = ble_hs_hci_cmd_build_le_rem_dev_from_periodic_adv_list(peer_addr->type,
-            peer_addr->val, adv_sid, buf, sizeof(buf));
+                                                                peer_addr->val,
+                                                                adv_sid, buf,
+                                                                sizeof(buf));
     if (rc != 0) {
         return rc;
     }
@@ -3565,7 +3680,6 @@ ble_gap_read_periodic_adv_list_size(uint8_t *per_adv_list_size)
 
     return 0;
 }
-
 #endif
 
 /*****************************************************************************
@@ -5501,8 +5615,12 @@ ble_gap_init(void)
 {
     int rc;
 
-    memset(&ble_gap_master, 0, sizeof ble_gap_master);
-    memset(ble_gap_slave, 0, sizeof ble_gap_slave);
+    memset(&ble_gap_master, 0, sizeof(ble_gap_master));
+    memset(ble_gap_slave, 0, sizeof(ble_gap_slave));
+
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
+    memset(&ble_gap_sync, 0, sizeof(ble_gap_sync));
+#endif
 
     ble_npl_mutex_init(&preempt_done_mutex);
 
diff --git a/nimble/host/src/ble_hs_hci_cmd.c b/nimble/host/src/ble_hs_hci_cmd.c
index 036a4af..a73a5f7 100644
--- a/nimble/host/src/ble_hs_hci_cmd.c
+++ b/nimble/host/src/ble_hs_hci_cmd.c
@@ -1679,12 +1679,12 @@ ble_hs_hci_cmd_build_le_periodic_adv_data(uint8_t handle,
 
 int
 ble_hs_hci_cmd_build_le_periodic_adv_create_sync(uint8_t filter_policy,
-                                        uint8_t adv_sid,
-                                        uint8_t adv_add_type,
-                                        const uint8_t *adv_addr,
-                                        uint16_t skip,
-                                        uint16_t sync_timeout,
-                                        uint8_t *cmd, int cmd_len)
+                                                 uint8_t adv_sid,
+                                                 uint8_t adv_add_type,
+                                                 const uint8_t *adv_addr,
+                                                 uint16_t skip,
+                                                 uint16_t sync_timeout,
+                                                 uint8_t *cmd, int cmd_len)
 {
     BLE_HS_DBG_ASSERT(cmd_len >= BLE_HCI_LE_PERIODIC_ADV_CREATE_SYNC_LEN);
 
diff --git a/nimble/host/src/ble_hs_hci_evt.c b/nimble/host/src/ble_hs_hci_evt.c
index c394df9..2ce0e03 100644
--- a/nimble/host/src/ble_hs_hci_evt.c
+++ b/nimble/host/src/ble_hs_hci_evt.c
@@ -616,6 +616,7 @@ ble_hs_hci_evt_le_ext_adv_rpt(uint8_t subevent, uint8_t *data, int len)
         desc.sid = params->sid;
         desc.prim_phy = params->prim_phy;
         desc.sec_phy = params->sec_phy;
+        desc.periodic_adv_itvl = params->per_adv_itvl;
         ble_gap_rx_ext_adv_report(&desc);
         params += 1;
     }
@@ -625,9 +626,9 @@ ble_hs_hci_evt_le_ext_adv_rpt(uint8_t subevent, uint8_t *data, int len)
 
 static int
 ble_hs_hci_evt_le_periodic_adv_sync_estab(uint8_t subevent, uint8_t *data,
-                                              int len)
+                                          int len)
 {
-#if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
     struct hci_le_subev_periodic_adv_sync_estab evt;
 
     if (len < BLE_HCI_LE_PERIODIC_ADV_SYNC_ESTAB_LEN) {
@@ -650,17 +651,16 @@ ble_hs_hci_evt_le_periodic_adv_sync_estab(uint8_t subevent, uint8_t *data,
 }
 
 static int
-ble_hs_hci_evt_le_periodic_adv_rpt(uint8_t subevent, uint8_t *data,
-                                      int len)
+ble_hs_hci_evt_le_periodic_adv_rpt(uint8_t subevent, uint8_t *data, int len)
 {
-#if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
     struct hci_le_subev_periodic_adv_rpt* evt;
 
     if (len < BLE_HCI_LE_PERIODIC_ADV_RPT_LEN) {
         return BLE_HS_EBADDATA;
     }
 
-    evt = (struct hci_le_subev_periodic_adv_rpt *)data;
+    evt = (struct hci_le_subev_periodic_adv_rpt *)(data + 1);
     ble_gap_rx_periodic_adv_rpt(evt);
 #endif
 
@@ -669,9 +669,9 @@ return 0;
 
 static int
 ble_hs_hci_evt_le_periodic_adv_sync_lost(uint8_t subevent, uint8_t *data,
-                                             int len)
+                                         int len)
 {
-#if MYNEWT_VAL(BLE_EXT_ADV) && MYNEWT_VAL(BLE_PERIODIC_ADV)
+#if MYNEWT_VAL(BLE_PERIODIC_ADV)
     struct hci_le_subev_periodic_adv_sync_lost evt;
 
     if (len < BLE_HCI_LE_PERIODIC_ADV_SYNC_LOST_LEN) {
diff --git a/nimble/host/src/ble_hs_periodic_disc.c b/nimble/host/src/ble_hs_periodic_sync.c
similarity index 54%
rename from nimble/host/src/ble_hs_periodic_disc.c
rename to nimble/host/src/ble_hs_periodic_sync.c
index f04831d..5d2d6bd 100644
--- a/nimble/host/src/ble_hs_periodic_disc.c
+++ b/nimble/host/src/ble_hs_periodic_sync.c
@@ -24,7 +24,7 @@
 #include "host/ble_hs_id.h"
 #include "ble_hs_priv.h"
 
-static SLIST_HEAD(, ble_hs_periodic_sync) ble_hs_periodic_sync_handles;
+static SLIST_HEAD(, ble_hs_periodic_sync) g_ble_hs_periodic_sync_handles;
 static struct os_mempool ble_hs_periodic_sync_pool;
 
 static os_membuf_t ble_hs_psync_elem_mem[
@@ -32,46 +32,24 @@ static os_membuf_t ble_hs_psync_elem_mem[
                     sizeof (struct ble_hs_periodic_sync))
 ];
 
-int
-ble_hs_periodic_sync_can_alloc(void)
-{
-#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
-    return 0;
-#endif
-
-    return (ble_hs_periodic_sync_pool.mp_num_free >= 1);
-}
-
 struct ble_hs_periodic_sync *
 ble_hs_periodic_sync_alloc(void)
 {
-#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
-    return NULL;
-#endif
-
     struct ble_hs_periodic_sync *psync;
 
     psync = os_memblock_get(&ble_hs_periodic_sync_pool);
-    if (psync == NULL) {
-        goto err;
+    if (psync) {
+        memset(psync, 0, sizeof(*psync));
     }
-    memset(psync, 0, sizeof *psync);
 
     return psync;
-
-err:
-    ble_hs_periodic_sync_free(psync);
-    return NULL;
 }
 
 void
 ble_hs_periodic_sync_free(struct ble_hs_periodic_sync *psync)
 {
-#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
-    return;
-#endif
-
     int rc;
+
     if (psync == NULL) {
         return;
     }
@@ -86,45 +64,31 @@ ble_hs_periodic_sync_free(struct ble_hs_periodic_sync *psync)
 void
 ble_hs_periodic_sync_insert(struct ble_hs_periodic_sync *psync)
 {
-#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
-    return;
-#endif
-
     BLE_HS_DBG_ASSERT(ble_hs_locked_by_cur_task());
 
     BLE_HS_DBG_ASSERT_EVAL(
-                       ble_hs_periodic_sync_find(psync->sync_handle) == NULL);
+                       ble_hs_periodic_sync_find_by_handle(psync->sync_handle) == NULL);
 
-    SLIST_INSERT_HEAD(&ble_hs_periodic_sync_handles, psync, bhc_next);
+    SLIST_INSERT_HEAD(&g_ble_hs_periodic_sync_handles, psync, next);
 }
 
 void
 ble_hs_periodic_sync_remove(struct ble_hs_periodic_sync *psync)
 {
-#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
-    return;
-#endif
-
     BLE_HS_DBG_ASSERT(ble_hs_locked_by_cur_task());
 
-    SLIST_REMOVE(&ble_hs_periodic_sync_handles,
-                 psync,
-                 ble_hs_periodic_sync,
-                 bhc_next);
+    SLIST_REMOVE(&g_ble_hs_periodic_sync_handles, psync, ble_hs_periodic_sync,
+                 next);
 }
 
 struct ble_hs_periodic_sync *
-ble_hs_periodic_sync_find(uint16_t sync_handle)
+ble_hs_periodic_sync_find_by_handle(uint16_t sync_handle)
 {
-#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
-    return NULL;
-#endif
-
     struct ble_hs_periodic_sync *psync;
 
     BLE_HS_DBG_ASSERT(ble_hs_locked_by_cur_task());
 
-    SLIST_FOREACH(psync, &ble_hs_periodic_sync_handles, bhc_next) {
+    SLIST_FOREACH(psync, &g_ble_hs_periodic_sync_handles, next) {
         if (psync->sync_handle == sync_handle) {
             return psync;
         }
@@ -134,23 +98,8 @@ ble_hs_periodic_sync_find(uint16_t sync_handle)
 }
 
 struct ble_hs_periodic_sync *
-ble_hs_periodic_sync_find_assert(uint16_t sync_handle)
-{
-    struct ble_hs_periodic_sync *psync;
-
-    psync = ble_hs_periodic_sync_find(sync_handle);
-    BLE_HS_DBG_ASSERT(psync != NULL);
-
-    return psync;
-}
-
-struct ble_hs_periodic_sync *
-ble_hs_periodic_sync_find_by_adv_addr(const ble_addr_t *addr)
+ble_hs_periodic_sync_find(const ble_addr_t *addr, uint8_t sid)
 {
-#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
-    return NULL;
-#endif
-
     struct ble_hs_periodic_sync *psync;
 
     BLE_HS_DBG_ASSERT(ble_hs_locked_by_cur_task());
@@ -159,8 +108,9 @@ ble_hs_periodic_sync_find_by_adv_addr(const ble_addr_t *addr)
         return NULL;
     }
 
-    SLIST_FOREACH(psync, &ble_hs_periodic_sync_handles, bhc_next) {
-        if (ble_addr_cmp(&psync->advertiser_addr, addr) == 0) {
+    SLIST_FOREACH(psync, &g_ble_hs_periodic_sync_handles, next) {
+        if ((ble_addr_cmp(&psync->advertiser_addr, addr) == 0) &&
+                (psync->adv_sid == sid)) {
             return psync;
         }
     }
@@ -168,48 +118,16 @@ ble_hs_periodic_sync_find_by_adv_addr(const ble_addr_t *addr)
     return NULL;
 }
 
-struct ble_hs_periodic_sync *
-ble_hs_periodic_sync_find_by_adv_sid(uint16_t sid)
-{
-#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
-    return 0;
-#endif
-
-    struct ble_hs_periodic_sync *psync;
-
-    BLE_HS_DBG_ASSERT(ble_hs_locked_by_cur_task());
-
-    SLIST_FOREACH(psync, &ble_hs_periodic_sync_handles, bhc_next) {
-        if (psync->adv_sid == sid) {
-            return psync;
-        }
-    }
-
-    return NULL;
-}
-
-int
-ble_hs_periodic_sync_exists(uint16_t sync_handle)
-{
-#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
-    return 0;
-#endif
-    return ble_hs_periodic_sync_find(sync_handle) != NULL;
-}
-
 /**
  * Retrieves the first periodic discovery handle in the list.
  */
 struct ble_hs_periodic_sync *
 ble_hs_periodic_sync_first(void)
 {
-#if !MYNEWT_VAL(BLE_PERIODIC_ADV)
-    return NULL;
-#endif
     struct ble_hs_periodic_sync *psync;
 
     ble_hs_lock();
-    psync = SLIST_FIRST(&ble_hs_periodic_sync_handles);
+    psync = SLIST_FIRST(&g_ble_hs_periodic_sync_handles);
     ble_hs_unlock();
 
     return psync;
@@ -228,7 +146,7 @@ ble_hs_periodic_sync_init(void)
         return BLE_HS_EOS;
     }
 
-    SLIST_INIT(&ble_hs_periodic_sync_handles);
+    SLIST_INIT(&g_ble_hs_periodic_sync_handles);
 
     return 0;
 }
diff --git a/nimble/host/src/ble_hs_periodic_disc_priv.h b/nimble/host/src/ble_hs_periodic_sync_priv.h
similarity index 58%
rename from nimble/host/src/ble_hs_periodic_disc_priv.h
rename to nimble/host/src/ble_hs_periodic_sync_priv.h
index 5cf2bfa..2471188 100644
--- a/nimble/host/src/ble_hs_periodic_disc_priv.h
+++ b/nimble/host/src/ble_hs_periodic_sync_priv.h
@@ -17,8 +17,8 @@
  * under the License.
  */
 
-#ifndef H_BLE_HS_PERIODIC_DISC_
-#define H_BLE_HS_PERIODIC_DISC_
+#ifndef H_BLE_HS_PERIODIC_SYNC_
+#define H_BLE_HS_PERIODIC_SYNC_
 
 #include <inttypes.h>
 #include "os/queue.h"
@@ -27,30 +27,24 @@ extern "C" {
 #endif
 
 struct ble_hs_periodic_sync {
-    SLIST_ENTRY(ble_hs_periodic_sync) bhc_next;
+    SLIST_ENTRY(ble_hs_periodic_sync) next;
     uint16_t   sync_handle;
-    uint8_t    adv_sid;
-    uint8_t    advertiser_addr_type;
     ble_addr_t advertiser_addr;
-    uint8_t    advertiser_phy;
-    uint16_t   periodic_adv_itvl;
-    uint8_t    advertiser_clock_accuracy;
+    uint8_t    adv_sid;
+
+    ble_gap_event_fn *cb;
+    void *cb_arg;
+
+    struct ble_npl_event lost_ev;
 };
 
-int ble_hs_periodic_sync_can_alloc(void);
 struct ble_hs_periodic_sync *ble_hs_periodic_sync_alloc(void);
 void ble_hs_periodic_sync_free(struct ble_hs_periodic_sync *psync);
 void ble_hs_periodic_sync_insert(struct ble_hs_periodic_sync *psync);
 void ble_hs_periodic_sync_remove(struct ble_hs_periodic_sync *psync);
-struct ble_hs_periodic_sync *ble_hs_periodic_sync_find(
-                                                         uint16_t sync_handle);
-struct ble_hs_periodic_sync *ble_hs_periodic_sync_find_assert(
-                                                         uint16_t sync_handle);
-struct ble_hs_periodic_sync *ble_hs_periodic_sync_find_by_adv_addr(
-                                                      const ble_addr_t *addr);
-struct ble_hs_periodic_sync *ble_hs_periodic_sync_find_by_adv_sid(
-                                                                 uint16_t sid);
-int ble_hs_periodic_sync_exists(uint16_t sync_handle);
+struct ble_hs_periodic_sync *ble_hs_periodic_sync_find_by_handle(uint16_t sync_handle);
+struct ble_hs_periodic_sync *ble_hs_periodic_sync_find(const ble_addr_t *addr,
+                                                       uint8_t sid);
 struct ble_hs_periodic_sync *ble_hs_periodic_sync_first(void);
 int ble_hs_periodic_sync_init(void);
 
diff --git a/nimble/host/src/ble_hs_priv.h b/nimble/host/src/ble_hs_priv.h
index dec12ff..27bf904 100644
--- a/nimble/host/src/ble_hs_priv.h
+++ b/nimble/host/src/ble_hs_priv.h
@@ -30,7 +30,6 @@
 #include "ble_hs_hci_priv.h"
 #include "ble_hs_atomic_priv.h"
 #include "ble_hs_conn_priv.h"
-#include "ble_hs_periodic_disc_priv.h"
 #include "ble_hs_atomic_priv.h"
 #include "ble_hs_mbuf_priv.h"
 #include "ble_hs_startup_priv.h"
@@ -42,6 +41,7 @@
 #include "ble_hs_flow_priv.h"
 #include "ble_hs_pvcy_priv.h"
 #include "ble_hs_id_priv.h"
+#include "ble_hs_periodic_sync_priv.h"
 #include "ble_uuid_priv.h"
 #include "host/ble_hs.h"
 #include "host/ble_monitor.h"