You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2018/04/11 12:27:30 UTC
[GitHub] sjanc closed pull request #39: Fixes for connection creation events
handling
sjanc closed pull request #39: Fixes for connection creation events handling
URL: https://github.com/apache/mynewt-nimble/pull/39
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/nimble/controller/src/ble_ll_adv.c b/nimble/controller/src/ble_ll_adv.c
index fc709119..9bb52de2 100644
--- a/nimble/controller/src/ble_ll_adv.c
+++ b/nimble/controller/src/ble_ll_adv.c
@@ -3270,16 +3270,16 @@ ble_ll_adv_send_conn_comp_ev(struct ble_ll_conn_sm *connsm,
ble_ll_conn_comp_event_send(connsm, BLE_ERR_SUCCESS, evbuf, advsm);
+#if (MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_CSA2) == 1)
+ ble_ll_hci_ev_le_csa(connsm);
+#endif
+
#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV)
if (advsm->flags & BLE_LL_ADV_SM_FLAG_ADV_TERMINATE_EVT) {
ble_ll_hci_ev_send_adv_set_terminated(0, advsm->adv_instance,
connsm->conn_handle, advsm->events);
}
#endif
-
-#if (MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_CSA2) == 1)
- ble_ll_hci_ev_le_csa(connsm);
-#endif
}
/**
diff --git a/nimble/controller/src/ble_ll_conn.c b/nimble/controller/src/ble_ll_conn.c
index e5e5bfbb..2ed695f1 100644
--- a/nimble/controller/src/ble_ll_conn.c
+++ b/nimble/controller/src/ble_ll_conn.c
@@ -2070,7 +2070,6 @@ ble_ll_conn_datalen_update(struct ble_ll_conn_sm *connsm,
void
ble_ll_conn_end(struct ble_ll_conn_sm *connsm, uint8_t ble_err)
{
- uint8_t *evbuf;
struct os_mbuf *m;
struct os_mbuf_pkthdr *pkthdr;
#if MYNEWT_VAL(BLE_LL_STRICT_CONN_SCHEDULING)
@@ -2136,21 +2135,16 @@ ble_ll_conn_end(struct ble_ll_conn_sm *connsm, uint8_t ble_err)
}
/*
- * We need to send a disconnection complete event or a connection complete
- * event when the connection ends. We send a connection complete event
- * only when we were told to cancel the connection creation. If the
- * ble error is "success" it means that the reset command was received
- * and we should not send an event
+ * We need to send a disconnection complete event. Connection Complete for
+ * canceling connection creation is sent from LE Create Connection Cancel
+ * Command handler.
+ *
+ * If the ble error is "success" it means that the reset command was
+ * received and we should not send an event.
*/
- if (ble_err) {
-
- if ((connsm->csmflags.cfbit.terminate_ind_rxd == 0) &&
- (ble_err == BLE_ERR_UNK_CONN_ID)) {
- evbuf = ble_ll_init_get_conn_comp_ev();
- ble_ll_conn_comp_event_send(connsm, ble_err, evbuf, NULL);
- } else {
- ble_ll_disconn_comp_event_send(connsm, ble_err);
- }
+ if (ble_err && (ble_err != BLE_ERR_UNK_CONN_ID ||
+ connsm->csmflags.cfbit.terminate_ind_rxd)) {
+ ble_ll_disconn_comp_event_send(connsm, ble_err);
}
/*
diff --git a/nimble/controller/src/ble_ll_conn_hci.c b/nimble/controller/src/ble_ll_conn_hci.c
index 859b7c3a..bcbfae41 100644
--- a/nimble/controller/src/ble_ll_conn_hci.c
+++ b/nimble/controller/src/ble_ll_conn_hci.c
@@ -246,6 +246,11 @@ ble_ll_conn_comp_event_send(struct ble_ll_conn_sm *connsm, uint8_t status,
put_le16(evdata + 2, connsm->slave_latency);
put_le16(evdata + 4, connsm->supervision_tmo);
evdata[6] = connsm->master_sca;
+ } else {
+ /* zero remaining bytes of event (2 bytes used for subevent opcode
+ * and status)
+ **/
+ memset(&evbuf[4], 0, evbuf[1] - 2);
}
ble_ll_hci_event_send(evbuf);
}
@@ -1000,6 +1005,19 @@ ble_ll_conn_hci_param_reply(uint8_t *cmdbuf, int positive_reply,
return rc;
}
+/* this is called from same context after cmd complete is send so it is
+ * safe to use g_ble_ll_conn_comp_ev
+ */
+static void
+ble_ll_conn_hci_cancel_conn_complete_event(void)
+{
+ assert(g_ble_ll_conn_comp_ev);
+
+ ble_ll_conn_comp_event_send(NULL, BLE_ERR_UNK_CONN_ID,
+ g_ble_ll_conn_comp_ev, NULL);
+ g_ble_ll_conn_comp_ev = NULL;
+}
+
/**
* Called when HCI command to cancel a create connection command has been
* received.
@@ -1009,12 +1027,11 @@ ble_ll_conn_hci_param_reply(uint8_t *cmdbuf, int positive_reply,
* @return int
*/
int
-ble_ll_conn_create_cancel(void)
+ble_ll_conn_create_cancel(ble_ll_hci_post_cmd_complete_cb *post_cmd_cb)
{
int rc;
struct ble_ll_conn_sm *connsm;
- /* XXX: BUG! I send the event before the command complete. Not good. */
/*
* If we receive this command and we have not got a connection
* create command, we have to return disallowed. The spec does not say
@@ -1027,6 +1044,9 @@ ble_ll_conn_create_cancel(void)
g_ble_ll_conn_create_sm = NULL;
ble_ll_scan_sm_stop(1);
ble_ll_conn_end(connsm, BLE_ERR_UNK_CONN_ID);
+
+ *post_cmd_cb = ble_ll_conn_hci_cancel_conn_complete_event;
+
rc = BLE_ERR_SUCCESS;
} else {
/* If we are not attempting to create a connection*/
diff --git a/nimble/controller/src/ble_ll_conn_priv.h b/nimble/controller/src/ble_ll_conn_priv.h
index a015a532..5f299b9d 100644
--- a/nimble/controller/src/ble_ll_conn_priv.h
+++ b/nimble/controller/src/ble_ll_conn_priv.h
@@ -59,6 +59,9 @@ extern "C" {
#define BLE_LL_CONN_AUTH_PYLD_OS_TMO(x) \
((((uint32_t)(x)) * 10 * OS_TICKS_PER_SEC) / 1000)
+
+typedef void (*ble_ll_hci_post_cmd_complete_cb)(void);
+
/* Global Link Layer connection parameters */
struct ble_ll_conn_global_params
{
@@ -148,7 +151,7 @@ int ble_ll_conn_hci_update(uint8_t *cmdbuf);
int ble_ll_conn_hci_set_chan_class(uint8_t *cmdbuf);
int ble_ll_conn_hci_param_reply(uint8_t *cmdbuf, int negative_reply,
uint8_t *rspbuf, uint8_t *rsplen);
-int ble_ll_conn_create_cancel(void);
+int ble_ll_conn_create_cancel(ble_ll_hci_post_cmd_complete_cb *post_cmd_cb);
void ble_ll_conn_num_comp_pkts_event_send(struct ble_ll_conn_sm *connsm);
void ble_ll_conn_comp_event_send(struct ble_ll_conn_sm *connsm, uint8_t status,
uint8_t *evbuf, struct ble_ll_adv_sm *advsm);
diff --git a/nimble/controller/src/ble_ll_hci.c b/nimble/controller/src/ble_ll_hci.c
index f437c0ab..3d082655 100644
--- a/nimble/controller/src/ble_ll_hci.c
+++ b/nimble/controller/src/ble_ll_hci.c
@@ -692,7 +692,8 @@ ble_ll_is_valid_adv_mode(uint8_t ocf)
* 256 gets added to the return value.
*/
static int
-ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen)
+ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen,
+ ble_ll_hci_post_cmd_complete_cb *cb)
{
int rc;
uint8_t cmdlen;
@@ -772,7 +773,7 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen)
rc = ble_ll_conn_create(cmdbuf);
break;
case BLE_HCI_OCF_LE_CREATE_CONN_CANCEL:
- rc = ble_ll_conn_create_cancel();
+ rc = ble_ll_conn_create_cancel(cb);
break;
case BLE_HCI_OCF_LE_CLEAR_WHITE_LIST:
rc = ble_ll_whitelist_clear();
@@ -1166,6 +1167,7 @@ ble_ll_hci_cmd_proc(struct os_event *ev)
uint8_t *cmdbuf;
uint16_t opcode;
uint16_t ocf;
+ ble_ll_hci_post_cmd_complete_cb post_cb = NULL;
/* The command buffer is the event argument */
cmdbuf = (uint8_t *)ev->ev_arg;
@@ -1193,7 +1195,7 @@ ble_ll_hci_cmd_proc(struct os_event *ev)
rc = ble_ll_hci_status_params_cmd_proc(cmdbuf, ocf, &rsplen);
break;
case BLE_HCI_OGF_LE:
- rc = ble_ll_hci_le_cmd_proc(cmdbuf, ocf, &rsplen);
+ rc = ble_ll_hci_le_cmd_proc(cmdbuf, ocf, &rsplen, &post_cb);
break;
default:
/* XXX: Need to support other OGF. For now, return unsupported */
@@ -1229,6 +1231,11 @@ ble_ll_hci_cmd_proc(struct os_event *ev)
/* Send the event (events cannot be masked) */
ble_ll_hci_event_send(cmdbuf);
+
+ /* Call post callback if set by command handler */
+ if (post_cb) {
+ post_cb();
+ }
}
/**
diff --git a/nimble/host/src/ble_gap.c b/nimble/host/src/ble_gap.c
index f721fe70..83a3a7e4 100644
--- a/nimble/host/src/ble_gap.c
+++ b/nimble/host/src/ble_gap.c
@@ -1326,61 +1326,51 @@ ble_gap_rx_conn_complete(struct hci_le_conn_complete *evt, uint8_t instance)
STATS_INC(ble_gap_stats, rx_conn_complete);
- /* Apply the event to the existing connection if it exists. */
- if (evt->status != BLE_ERR_UNK_CONN_ID &&
- ble_hs_atomic_conn_flags(evt->connection_handle, NULL) == 0) {
-
- /* XXX: Does this ever happen? */
-
- if (evt->status != 0) {
- ble_gap_conn_broken(evt->connection_handle,
- BLE_HS_HCI_ERR(evt->status));
- }
- return 0;
- }
-
- /* This event refers to a new connection. */
-
+ /* in that case *only* status field is valid so we determine role
+ * based on error code
+ */
if (evt->status != BLE_ERR_SUCCESS) {
- /* Determine the role from the status code. */
- if (evt->status == BLE_ERR_DIR_ADV_TMO) {
- evt->role = BLE_HCI_LE_CONN_COMPLETE_ROLE_SLAVE;
- }
-
- switch (evt->role) {
- case BLE_HCI_LE_CONN_COMPLETE_ROLE_MASTER:
- if (ble_gap_master_in_progress()) {
- if (evt->status == BLE_ERR_UNK_CONN_ID) {
- /* Connect procedure successfully cancelled. */
- if (ble_gap_master.preempted_op == BLE_GAP_OP_M_CONN) {
- ble_gap_master_failed(BLE_HS_EPREEMPTED);
- } else {
- ble_gap_master_connect_cancelled();
- }
- } else {
- ble_gap_master_failed(BLE_HS_HCI_ERR(evt->status));
- }
- }
- break;
-
- case BLE_HCI_LE_CONN_COMPLETE_ROLE_SLAVE:
-/* with ext advertising this is send from set terminated event */
+ switch (evt->status) {
+ case BLE_ERR_DIR_ADV_TMO:
+ /* slave role (HD directed advertising)
+ *
+ * with ext advertising this is send from set terminated event
+ */
#if !MYNEWT_VAL(BLE_EXT_ADV)
if (ble_gap_adv_active()) {
ble_gap_adv_finished(0, 0, 0);
}
#endif
break;
-
+ case BLE_ERR_UNK_CONN_ID:
+ /* master role */
+ if (ble_gap_master_in_progress()) {
+ /* Connect procedure successfully cancelled. */
+ if (ble_gap_master.preempted_op == BLE_GAP_OP_M_CONN) {
+ ble_gap_master_failed(BLE_HS_EPREEMPTED);
+ } else {
+ ble_gap_master_connect_cancelled();
+ }
+ }
+ break;
default:
- BLE_HS_LOG(INFO, "controller reported invalid role in connection "
- " complete event: %d", evt->role);
+ /* this should never happen, unless controller is broken */
+ BLE_HS_LOG(INFO, "controller reported invalid error code in conn"
+ "complete event: %u", evt->status);
+ assert(0);
break;
}
+ return 0;
+ }
+ /* Apply the event to the existing connection if it exists. */
+ if (ble_hs_atomic_conn_flags(evt->connection_handle, NULL) == 0) {
+ /* XXX: Does this ever happen? */
return 0;
}
+ /* This event refers to a new connection. */
+
switch (evt->role) {
case BLE_HCI_LE_CONN_COMPLETE_ROLE_MASTER:
rc = ble_gap_accept_master_conn();
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services