You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by st...@apache.org on 2016/08/09 23:08:08 UTC

[34/42] incubator-mynewt-core git commit: BLE Host - Send exec-write when prep-writes cmplt.

BLE Host - Send exec-write when prep-writes cmplt.

This fixes a bug where the GATT client would never send the execute
write command when performing a long-write procedure.  Instead, it would
repeatedly send 0-length prepare write requests.

Also, the GATT client is now more strict about when it accepts prepare
write responses and execute write responses.  Some unit tests were
passing, despite the bug described in the above paragraph, because an
incoming execute write response was accepted when it should have been.


Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/aadba628
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/aadba628
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/aadba628

Branch: refs/heads/sterly_refactor
Commit: aadba6283aabeacd9a4dfc7c7c3c0ddfbe8b04c1
Parents: fbdb1ba
Author: Christopher Collins <cc...@apache.org>
Authored: Sat Aug 6 16:06:06 2016 -0700
Committer: Sterling Hughes <st...@apache.org>
Committed: Tue Aug 9 16:05:21 2016 -0700

----------------------------------------------------------------------
 net/nimble/host/src/ble_gattc.c                | 44 ++++++-----
 net/nimble/host/src/test/ble_gatt_write_test.c | 82 +++++++++++++++++----
 net/nimble/host/src/test/ble_hs_test_util.c    | 23 ++++++
 net/nimble/host/src/test/ble_hs_test_util.h    |  3 +
 4 files changed, 120 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/aadba628/net/nimble/host/src/ble_gattc.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/ble_gattc.c b/net/nimble/host/src/ble_gattc.c
index 67c13d6..43d2134 100644
--- a/net/nimble/host/src/ble_gattc.c
+++ b/net/nimble/host/src/ble_gattc.c
@@ -177,7 +177,6 @@ struct ble_gattc_proc {
         struct {
             struct ble_gatt_attr attr;
             uint16_t length;
-            uint8_t exec_sent:1;
             ble_gatt_attr_fn *cb;
             void *cb_arg;
         } write_long;
@@ -187,7 +186,6 @@ struct ble_gattc_proc {
             uint8_t num_attrs;
             uint8_t cur_attr;
             uint16_t length;
-            uint8_t exec_sent:1;
             ble_gatt_reliable_attr_fn *cb;
             void *cb_arg;
         } write_reliable;
@@ -3370,6 +3368,7 @@ ble_gattc_write_long_go(struct ble_gattc_proc *proc, int cb_on_err)
     struct ble_att_prep_write_cmd prep_req;
     struct ble_att_exec_write_req exec_req;
     struct os_mbuf *om;
+    int write_len;
     int max_sz;
     int rc;
 
@@ -3377,14 +3376,6 @@ ble_gattc_write_long_go(struct ble_gattc_proc *proc, int cb_on_err)
 
     om = NULL;
 
-    if (proc->write_long.attr.offset +
-        OS_MBUF_PKTLEN(proc->write_long.attr.om) <= 0) {
-
-        exec_req.baeq_flags = BLE_ATT_EXEC_WRITE_F_CONFIRM;
-        rc = ble_att_clt_tx_exec_write(proc->conn_handle, &exec_req);
-        goto done;
-    }
-
     max_sz = ble_att_mtu(proc->conn_handle) -
              BLE_ATT_PREP_WRITE_CMD_BASE_SZ;
     if (max_sz <= 0) {
@@ -3393,11 +3384,17 @@ ble_gattc_write_long_go(struct ble_gattc_proc *proc, int cb_on_err)
         goto done;
     }
 
-    proc->write_long.length =
-        min(max_sz,
-            OS_MBUF_PKTLEN(proc->write_long.attr.om) -
-                proc->write_long.attr.offset);
+    write_len = min(max_sz,
+                    OS_MBUF_PKTLEN(proc->write_long.attr.om) -
+                        proc->write_long.attr.offset);
 
+    if (write_len <= 0) {
+        exec_req.baeq_flags = BLE_ATT_EXEC_WRITE_F_CONFIRM;
+        rc = ble_att_clt_tx_exec_write(proc->conn_handle, &exec_req);
+        goto done;
+    }
+
+    proc->write_long.length = write_len;
     om = ble_hs_mbuf_att_pkt();
     if (om == NULL) {
         rc = BLE_HS_ENOMEM;
@@ -3481,6 +3478,15 @@ ble_gattc_write_long_rx_prep(struct ble_gattc_proc *proc,
     }
 
     /* Verify the response. */
+    if (proc->write_long.attr.offset >=
+        OS_MBUF_PKTLEN(proc->write_long.attr.om)) {
+
+        /* Expecting a prepare write response, not an execute write
+         * response.
+         */
+        rc = BLE_HS_EBADDATA;
+        goto err;
+    }
     if (rsp->bapc_handle != proc->write_long.attr.handle) {
         rc = BLE_HS_EBADDATA;
         goto err;
@@ -3531,9 +3537,13 @@ ble_gattc_write_long_rx_exec(struct ble_gattc_proc *proc, int status)
 {
     ble_gattc_dbg_assert_proc_not_inserted(proc);
 
-    /* Ignore the response if we haven't sent the corresponding request yet. */
-    if (!proc->write_long.exec_sent) {
-        return 0;
+    if (proc->write_long.attr.offset <
+        OS_MBUF_PKTLEN(proc->write_long.attr.om)) {
+
+        /* Expecting an execute write response, not a prepare write
+         * response.
+         */
+        return BLE_HS_EBADDATA;
     }
 
     ble_gattc_write_long_cb(proc, status, 0);

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/aadba628/net/nimble/host/src/test/ble_gatt_write_test.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/test/ble_gatt_write_test.c b/net/nimble/host/src/test/ble_gatt_write_test.c
index f145ea8..f548198 100644
--- a/net/nimble/host/src/test/ble_gatt_write_test.c
+++ b/net/nimble/host/src/test/ble_gatt_write_test.c
@@ -123,6 +123,7 @@ ble_gatt_write_test_rx_exec_rsp(uint16_t conn_handle)
 static void
 ble_gatt_write_test_misc_long_good(int attr_len)
 {
+    uint16_t mtu;
     int off;
     int len;
     int rc;
@@ -132,6 +133,8 @@ ble_gatt_write_test_misc_long_good(int attr_len)
     ble_hs_test_util_create_conn(2, ((uint8_t[]){2,3,4,5,6,7,8,9}),
                                  NULL, NULL);
 
+    mtu = ble_att_mtu(2);
+
     rc = ble_hs_test_util_gatt_write_long_flat(
         2, 100, ble_gatt_write_test_attr_value, attr_len,
         ble_gatt_write_test_cb_good, &attr_len);
@@ -139,17 +142,18 @@ ble_gatt_write_test_misc_long_good(int attr_len)
 
     off = 0;
     while (off < attr_len) {
-        /* Send the pending ATT Prep Write Command. */
-        ble_hs_test_util_tx_all();
-
-        /* Receive Prep Write response. */
-        len = BLE_ATT_MTU_DFLT - BLE_ATT_PREP_WRITE_CMD_BASE_SZ;
+        len = mtu - BLE_ATT_PREP_WRITE_CMD_BASE_SZ;
         if (off + len > attr_len) {
             len = attr_len - off;
         }
-        ble_gatt_write_test_rx_prep_rsp(2, 100, off,
-                                        ble_gatt_write_test_attr_value + off,
-                                        len);
+
+        /* Send the pending ATT Prep Write Command. */
+        ble_hs_test_util_verify_tx_prep_write(
+            100, off, ble_gatt_write_test_attr_value + off, len);
+
+        /* Receive Prep Write response. */
+        ble_gatt_write_test_rx_prep_rsp(
+            2, 100, off, ble_gatt_write_test_attr_value + off, len);
 
         /* Verify callback hasn't gotten called. */
         TEST_ASSERT(!ble_gatt_write_test_cb_called);
@@ -157,6 +161,9 @@ ble_gatt_write_test_misc_long_good(int attr_len)
         off += len;
     }
 
+    /* Verify execute write request sent. */
+    ble_hs_test_util_verify_tx_exec_write(BLE_ATT_EXEC_WRITE_F_CONFIRM);
+
     /* Receive Exec Write response. */
     ble_hs_test_util_tx_all();
     ble_gatt_write_test_rx_exec_rsp(2);
@@ -172,6 +179,7 @@ static void
 ble_gatt_write_test_misc_long_bad(int attr_len,
                                   ble_gatt_write_test_long_fail_fn *cb)
 {
+    uint16_t mtu;
     int fail_now;
     int off;
     int len;
@@ -181,6 +189,7 @@ ble_gatt_write_test_misc_long_bad(int attr_len,
 
     ble_hs_test_util_create_conn(2, ((uint8_t[]){2,3,4,5,6,7,8,9}),
                                  NULL, NULL);
+    mtu = ble_att_mtu(2);
 
     rc = ble_hs_test_util_gatt_write_long_flat(
         2, 100, ble_gatt_write_test_attr_value, attr_len,
@@ -190,9 +199,14 @@ ble_gatt_write_test_misc_long_bad(int attr_len,
     fail_now = 0;
     off = 0;
     while (off < attr_len) {
+        len = mtu - BLE_ATT_PREP_WRITE_CMD_BASE_SZ;
+        if (off + len > attr_len) {
+            len = attr_len - off;
+        }
+
         /* Send the pending ATT Prep Write Command. */
-        ble_hs_test_util_tx_all();
-        TEST_ASSERT(ble_hs_test_util_prev_tx_dequeue() != NULL);
+        ble_hs_test_util_verify_tx_prep_write(
+            100, off, ble_gatt_write_test_attr_value + off, len);
 
         /* Receive Prep Write response. */
         len = BLE_ATT_MTU_DFLT - BLE_ATT_PREP_WRITE_CMD_BASE_SZ;
@@ -283,9 +297,13 @@ static void
 ble_gatt_write_test_misc_reliable_good(
     struct ble_hs_test_util_flat_attr *flat_attrs)
 {
+    const struct ble_hs_test_util_flat_attr *attr;
     struct ble_gatt_attr attrs[16];
+    uint16_t mtu;
     int num_attrs;
     int attr_idx;
+    int len;
+    int off;
     int rc;
     int i;
 
@@ -299,24 +317,43 @@ ble_gatt_write_test_misc_reliable_good(
 
     ble_hs_test_util_create_conn(2, ((uint8_t[]){2,3,4,5,6,7,8,9}),
                                  NULL, NULL);
+    mtu = ble_att_mtu(2);
 
     rc = ble_gattc_write_reliable(2, attrs, num_attrs,
                                   ble_gatt_write_test_reliable_cb_good, NULL);
     TEST_ASSERT(rc == 0);
 
-    for (attr_idx = 0; attr_idx < num_attrs; attr_idx++) {
+    attr_idx = 0;
+    off = 0;
+    while (attr_idx < num_attrs) {
+        attr = flat_attrs + attr_idx;
+
+        len = mtu - BLE_ATT_PREP_WRITE_CMD_BASE_SZ;
+        if (off + len > attr->value_len) {
+            len = attr->value_len - off;
+        }
+
         /* Send the pending ATT Prep Write Command. */
-        ble_hs_test_util_tx_all();
+        ble_hs_test_util_verify_tx_prep_write(attr->handle, off,
+                                              attr->value + off, len);
 
         /* Receive Prep Write response. */
-        ble_gatt_write_test_rx_prep_rsp(2, flat_attrs[attr_idx].handle, 0,
-                                        flat_attrs[attr_idx].value,
-                                        flat_attrs[attr_idx].value_len);
+        ble_gatt_write_test_rx_prep_rsp(2, attr->handle, off,
+                                        attr->value + off, len);
 
         /* Verify callback hasn't gotten called. */
         TEST_ASSERT(!ble_gatt_write_test_cb_called);
+
+        off += len;
+        if (off >= attr->value_len) {
+            attr_idx++;
+            off = 0;
+        }
     }
 
+    /* Verify execute write request sent. */
+    ble_hs_test_util_verify_tx_exec_write(BLE_ATT_EXEC_WRITE_F_CONFIRM);
+
     /* Receive Exec Write response. */
     ble_hs_test_util_tx_all();
     ble_gatt_write_test_rx_exec_rsp(2);
@@ -509,6 +546,21 @@ TEST_CASE(ble_gatt_write_test_reliable_good)
         }, {
             0
         } }));
+
+    /*** Long attributes. */
+    ble_gatt_write_test_misc_reliable_good(
+        ((struct ble_hs_test_util_flat_attr[]) { {
+            .handle = 100,
+            .value_len = 20,
+            .value = { 1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20 },
+        }, {
+            .handle = 144,
+            .value_len = 20,
+            .value = { 11,12,13,14,15,16,17,18,19,110,
+                       111,112,113,114,115,116,117,118,119,120 },
+        }, {
+            0
+        } }));
 }
 
 TEST_CASE(ble_gatt_write_test_long_queue_full)

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/aadba628/net/nimble/host/src/test/ble_hs_test_util.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/test/ble_hs_test_util.c b/net/nimble/host/src/test/ble_hs_test_util.c
index 4f1c706..54a3527 100644
--- a/net/nimble/host/src/test/ble_hs_test_util.c
+++ b/net/nimble/host/src/test/ble_hs_test_util.c
@@ -1051,6 +1051,29 @@ ble_hs_test_util_tx_all(void)
 }
 
 void
+ble_hs_test_util_verify_tx_prep_write(uint16_t attr_handle, uint16_t offset,
+                                      const void *data, int data_len)
+{
+    struct ble_att_prep_write_cmd req;
+    struct os_mbuf *om;
+
+    ble_hs_test_util_tx_all();
+    om = ble_hs_test_util_prev_tx_dequeue();
+    TEST_ASSERT_FATAL(om != NULL);
+    TEST_ASSERT(OS_MBUF_PKTLEN(om) ==
+                BLE_ATT_PREP_WRITE_CMD_BASE_SZ + data_len);
+
+    om = os_mbuf_pullup(om, BLE_ATT_PREP_WRITE_CMD_BASE_SZ);
+    TEST_ASSERT_FATAL(om != NULL);
+
+    ble_att_prep_write_req_parse(om->om_data, om->om_len, &req);
+    TEST_ASSERT(req.bapc_handle == attr_handle);
+    TEST_ASSERT(req.bapc_offset == offset);
+    TEST_ASSERT(os_mbuf_cmpf(om, BLE_ATT_PREP_WRITE_CMD_BASE_SZ,
+                             data, data_len) == 0);
+}
+
+void
 ble_hs_test_util_verify_tx_exec_write(uint8_t expected_flags)
 {
     struct ble_att_exec_write_req req;

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/aadba628/net/nimble/host/src/test/ble_hs_test_util.h
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/test/ble_hs_test_util.h b/net/nimble/host/src/test/ble_hs_test_util.h
index 00b090f..0ae6cf5 100644
--- a/net/nimble/host/src/test/ble_hs_test_util.h
+++ b/net/nimble/host/src/test/ble_hs_test_util.h
@@ -129,6 +129,9 @@ void ble_hs_test_util_rx_disconn_complete_event(
 uint8_t *ble_hs_test_util_verify_tx_hci(uint8_t ogf, uint16_t ocf,
                                         uint8_t *out_param_len);
 void ble_hs_test_util_tx_all(void);
+void ble_hs_test_util_verify_tx_prep_write(uint16_t attr_handle,
+                                           uint16_t offset,
+                                           const void *data, int data_len);
 void ble_hs_test_util_verify_tx_exec_write(uint8_t expected_flags);
 void ble_hs_test_util_verify_tx_read_rsp(uint8_t *attr_data, int attr_len);
 void ble_hs_test_util_verify_tx_read_blob_rsp(uint8_t *attr_data,