sjanc closed pull request #41: Connection complete fix
URL: https://github.com/apache/mynewt-nimble/pull/41
 
 
   

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/host/src/ble_gap.c b/nimble/host/src/ble_gap.c
index 83a3a7e4..6c79842a 100644
--- a/nimble/host/src/ble_gap.c
+++ b/nimble/host/src/ble_gap.c
@@ -449,7 +449,7 @@ ble_gap_extract_conn_cb(uint16_t conn_handle,
 {
     const struct ble_hs_conn *conn;
 
-    BLE_HS_DBG_ASSERT(conn_handle != 0);
+    BLE_HS_DBG_ASSERT(conn_handle <= BLE_HCI_LE_CONN_HANDLE_MAX);
 
     ble_hs_lock();
 
diff --git a/nimble/host/src/ble_hs_hci_evt.c b/nimble/host/src/ble_hs_hci_evt.c
index 7007023e..b5174e05 100644
--- a/nimble/host/src/ble_hs_hci_evt.c
+++ b/nimble/host/src/ble_hs_hci_evt.c
@@ -323,39 +323,41 @@ ble_hs_hci_evt_le_conn_complete(uint8_t subevent, uint8_t 
*data, int len)
         return BLE_HS_ECONTROLLER;
     }
 
+    memset(&evt, 0, sizeof(evt));
+
     evt.subevent_code = data[0];
     evt.status = data[1];
-    evt.connection_handle = get_le16(data + 2);
-    evt.role = data[4];
-    evt.peer_addr_type = data[5];
-    memcpy(evt.peer_addr, data + 6, BLE_DEV_ADDR_LEN);
-
-    /* enhanced connection event has the same information with these
-     * extra fields stuffed into the middle */
-    if (subevent == BLE_HCI_LE_SUBEV_ENH_CONN_COMPLETE) {
-        memcpy(evt.local_rpa, data + 12, BLE_DEV_ADDR_LEN);
-        memcpy(evt.peer_rpa, data + 18, BLE_DEV_ADDR_LEN);
-        extended_offset = 12;
-    } else {
-        memset(evt.local_rpa, 0, BLE_DEV_ADDR_LEN);
-        memset(evt.peer_rpa, 0, BLE_DEV_ADDR_LEN);
-    }
-
-    evt.conn_itvl = get_le16(data + 12 + extended_offset);
-    evt.conn_latency = get_le16(data + 14 + extended_offset);
-    evt.supervision_timeout = get_le16(data + 16 + extended_offset);
-    evt.master_clk_acc = data[18 + extended_offset];
-
-    if (evt.status == 0) {
-        if (evt.role != BLE_HCI_LE_CONN_COMPLETE_ROLE_MASTER &&
-            evt.role != BLE_HCI_LE_CONN_COMPLETE_ROLE_SLAVE) {
 
-            return BLE_HS_EBADDATA;
+    if (evt.status == BLE_ERR_SUCCESS) {
+        evt.connection_handle = get_le16(data + 2);
+        evt.role = data[4];
+        evt.peer_addr_type = data[5];
+        memcpy(evt.peer_addr, data + 6, BLE_DEV_ADDR_LEN);
+
+        /* enhanced connection event has the same information with these
+         * extra fields stuffed into the middle */
+        if (subevent == BLE_HCI_LE_SUBEV_ENH_CONN_COMPLETE) {
+            memcpy(evt.local_rpa, data + 12, BLE_DEV_ADDR_LEN);
+            memcpy(evt.peer_rpa, data + 18, BLE_DEV_ADDR_LEN);
+            extended_offset = 12;
+        } else {
+            memset(evt.local_rpa, 0, BLE_DEV_ADDR_LEN);
+            memset(evt.peer_rpa, 0, BLE_DEV_ADDR_LEN);
         }
+
+        evt.conn_itvl = get_le16(data + 12 + extended_offset);
+        evt.conn_latency = get_le16(data + 14 + extended_offset);
+        evt.supervision_timeout = get_le16(data + 16 + extended_offset);
+        evt.master_clk_acc = data[18 + extended_offset];
+    } else {
+#if MYNEWT_VAL(BLE_HS_DEBUG)
+        evt.connection_handle = BLE_HS_CONN_HANDLE_NONE;
+#endif
     }
 
 #if MYNEWT_VAL(BLE_EXT_ADV)
-    if (evt.role == BLE_HCI_LE_CONN_COMPLETE_ROLE_SLAVE) {
+    if (evt.status == BLE_ERR_DIR_ADV_TMO ||
+                            evt.role == BLE_HCI_LE_CONN_COMPLETE_ROLE_SLAVE) {
     /* store this until we get set terminated event with adv handle */
         memcpy(&pend_conn_complete, &evt, sizeof(evt));
         return 0;
diff --git a/nimble/host/test/src/ble_gap_test.c 
b/nimble/host/test/src/ble_gap_test.c
index 00cb3457..76fab7c0 100644
--- a/nimble/host/test/src/ble_gap_test.c
+++ b/nimble/host/test/src/ble_gap_test.c
@@ -993,6 +993,7 @@ TEST_CASE(ble_gap_test_case_conn_gen_fail_evt)
 {
     static const ble_addr_t peer_addr = {BLE_ADDR_PUBLIC, {1, 2, 3, 4, 5, 6}};
     struct hci_le_conn_complete evt;
+    struct hci_disconn_complete disc_evt;
     int rc;
 
     ble_gap_test_util_init();
@@ -1005,8 +1006,11 @@ TEST_CASE(ble_gap_test_case_conn_gen_fail_evt)
     /* Controller indicates failure via connect complete event. */
     memset(&evt, 0, sizeof evt);
     evt.subevent_code = BLE_HCI_LE_SUBEV_CONN_COMPLETE;
-    evt.status = BLE_ERR_CONN_ACCEPT_TMO;
+    evt.status = BLE_ERR_SUCCESS;
+    evt.connection_handle = 6;
     evt.role = BLE_HCI_LE_CONN_COMPLETE_ROLE_MASTER;
+    evt.peer_addr_type = BLE_ADDR_PUBLIC;
+    memcpy(evt.peer_addr, peer_addr.val, 6);
 
     rc = ble_gap_rx_conn_complete(&evt, 0);
     TEST_ASSERT_FATAL(rc == 0);
@@ -1014,7 +1018,19 @@ TEST_CASE(ble_gap_test_case_conn_gen_fail_evt)
     /* Ensure failed connect was reported to application. */
     TEST_ASSERT(ble_gap_test_event.type == BLE_GAP_EVENT_CONNECT);
     TEST_ASSERT(ble_gap_test_event.connect.status ==
-                BLE_HS_HCI_ERR(BLE_ERR_CONN_ACCEPT_TMO));
+                BLE_HS_HCI_ERR(BLE_ERR_SUCCESS));
+
+    memset(&disc_evt, 0, sizeof disc_evt);
+    disc_evt.connection_handle = 6;
+    disc_evt.status = BLE_ERR_SUCCESS;
+    disc_evt.reason = BLE_ERR_CONN_ESTABLISHMENT;
+
+    ble_gap_rx_disconn_complete(&disc_evt);
+
+    /* Ensure failed connect was reported to application. */
+    TEST_ASSERT(ble_gap_test_event.type == BLE_GAP_EVENT_DISCONNECT);
+    TEST_ASSERT(ble_gap_test_event.disconnect.reason ==
+                BLE_HS_HCI_ERR(BLE_ERR_CONN_ESTABLISHMENT));
 }
 
 TEST_SUITE(ble_gap_test_suite_conn_gen)
@@ -1055,6 +1071,9 @@ ble_gap_test_util_conn_cancel(uint8_t hci_status)
     memset(&evt, 0, sizeof evt);
     evt.subevent_code = BLE_HCI_LE_SUBEV_CONN_COMPLETE;
     evt.status = BLE_ERR_UNK_CONN_ID;
+    /* test if host correctly ignores other fields if status is error */
+    evt.connection_handle = 0x0fff;
+
     rc = ble_gap_rx_conn_complete(&evt, 0);
     TEST_ASSERT(rc == 0);
     TEST_ASSERT(!ble_gap_master_in_progress());
@@ -1489,9 +1508,18 @@ ble_gap_test_util_adv(uint8_t own_addr_type,
             memset(&evt, 0, sizeof evt);
             evt.subevent_code = BLE_HCI_LE_SUBEV_CONN_COMPLETE;
             evt.status = connect_status;
-            evt.connection_handle = 2;
-            evt.role = BLE_HCI_LE_CONN_COMPLETE_ROLE_SLAVE;
-            memcpy(evt.peer_addr, peer_addr->val, 6);
+
+            if (connect_status == BLE_ERR_SUCCESS) {
+                evt.connection_handle = 2;
+                evt.role = BLE_HCI_LE_CONN_COMPLETE_ROLE_SLAVE;
+                memcpy(evt.peer_addr, peer_addr->val, 6);
+            } else {
+                /* test if host correctly ignores other fields if status is
+                 * error
+                 */
+                evt.connection_handle = 0x0fff;
+            }
+
             rc = ble_gap_rx_conn_complete(&evt, 0);
             TEST_ASSERT(rc == 0);
 
@@ -2773,6 +2801,9 @@ ble_gap_test_util_conn_timeout(int32_t duration_ms)
     memset(&evt, 0, sizeof evt);
     evt.subevent_code = BLE_HCI_LE_SUBEV_CONN_COMPLETE;
     evt.status = BLE_ERR_UNK_CONN_ID;
+    /* test if host correctly ignores other fields if status is error */
+    evt.connection_handle = 0x0fff;
+
     rc = ble_gap_rx_conn_complete(&evt, 0);
     TEST_ASSERT_FATAL(rc == 0);
 
diff --git a/nimble/host/test/src/ble_hs_test_util_hci.c 
b/nimble/host/test/src/ble_hs_test_util_hci.c
index af32d5e1..f485133d 100644
--- a/nimble/host/test/src/ble_hs_test_util_hci.c
+++ b/nimble/host/test/src/ble_hs_test_util_hci.c
@@ -577,7 +577,8 @@ ble_hs_test_util_hci_rx_conn_cancel_evt(void)
     memset(&evt, 0, sizeof evt);
     evt.subevent_code = BLE_HCI_LE_SUBEV_CONN_COMPLETE;
     evt.status = BLE_ERR_UNK_CONN_ID;
-    evt.role = BLE_HCI_LE_CONN_COMPLETE_ROLE_MASTER;
+    /* test if host correctly ignores other fields if status is error */
+    evt.connection_handle = 0x0fff;
 
     rc = ble_gap_rx_conn_complete(&evt, 0);
     TEST_ASSERT_FATAL(rc == 0);
diff --git a/nimble/include/nimble/hci_common.h 
b/nimble/include/nimble/hci_common.h
index 443c563e..e887e21c 100644
--- a/nimble/include/nimble/hci_common.h
+++ b/nimble/include/nimble/hci_common.h
@@ -745,6 +745,9 @@ extern "C" {
 #define BLE_HCI_LE_CONN_COMPLETE_ROLE_MASTER    (0x00)
 #define BLE_HCI_LE_CONN_COMPLETE_ROLE_SLAVE     (0x01)
 
+/* Maximum valid connection handle value */
+#define BLE_HCI_LE_CONN_HANDLE_MAX              (0x0eff)
+
 /* LE advertising report event. (sub event 0x02) */
 #define BLE_HCI_LE_ADV_RPT_MIN_LEN          (12)
 #define BLE_HCI_LE_ADV_DIRECT_RPT_LEN       (18)


 

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to