BLE Host - Corner case: chr updated before ind ack
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/58dde847 Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/58dde847 Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/58dde847 Branch: refs/heads/develop Commit: 58dde847493c483902626fcaea92c5e131a47291 Parents: 18a91e8 Author: Christopher Collins <[email protected]> Authored: Sat May 21 13:38:13 2016 -0700 Committer: Christopher Collins <[email protected]> Committed: Sat May 21 13:38:13 2016 -0700 ---------------------------------------------------------------------- net/nimble/host/include/host/ble_store.h | 8 +-- net/nimble/host/src/ble_att_clt.c | 19 +----- net/nimble/host/src/ble_gap.c | 4 ++ net/nimble/host/src/ble_gatt_priv.h | 3 + net/nimble/host/src/ble_gattc.c | 27 ++++---- net/nimble/host/src/ble_gatts.c | 90 ++++++++++++++++++++++++++- net/nimble/host/src/ble_hs_conn_priv.h | 1 - 7 files changed, 116 insertions(+), 36 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/58dde847/net/nimble/host/include/host/ble_store.h ---------------------------------------------------------------------- diff --git a/net/nimble/host/include/host/ble_store.h b/net/nimble/host/include/host/ble_store.h index e7c0171..9b8f37b 100644 --- a/net/nimble/host/include/host/ble_store.h +++ b/net/nimble/host/include/host/ble_store.h @@ -42,15 +42,15 @@ struct ble_store_value_ltk { struct ble_store_key_cccd { /** - * Key by peer address; peer_addr_type=BLE_STORE_PEER_ADDR_TYPE_NONE means - * don't key off peer. + * Key by peer identity address; + * peer_addr_type=BLE_STORE_PEER_ADDR_TYPE_NONE means don't key off peer. */ uint8_t peer_addr[6]; uint8_t peer_addr_type; /** - * Key by characteristic definition handle; chr_def_handle=0 means don't - * key off characteristic handle. + * Key by characteristic definition handle; + * chr_def_handle=0 means don't key off characteristic handle. */ uint16_t chr_def_handle; http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/58dde847/net/nimble/host/src/ble_att_clt.c ---------------------------------------------------------------------- diff --git a/net/nimble/host/src/ble_att_clt.c b/net/nimble/host/src/ble_att_clt.c index a049f33..548e665 100644 --- a/net/nimble/host/src/ble_att_clt.c +++ b/net/nimble/host/src/ble_att_clt.c @@ -100,8 +100,7 @@ ble_att_clt_copy_attr_to_flatbuf(struct os_mbuf *om, void **out_attr_val, } static int -ble_att_clt_tx_req_flags(uint16_t conn_handle, struct os_mbuf *txom, - ble_hs_conn_flags_t flags_on_success) +ble_att_clt_tx_req(uint16_t conn_handle, struct os_mbuf *txom) { struct ble_l2cap_chan *chan; struct ble_hs_conn *conn; @@ -129,10 +128,6 @@ ble_att_clt_tx_req_flags(uint16_t conn_handle, struct os_mbuf *txom, rc = ble_l2cap_tx(conn, chan, txom); txom = NULL; - - if (rc == 0) { - conn->bhc_flags |= flags_on_success; - } } ble_hs_unlock(); @@ -141,15 +136,6 @@ ble_att_clt_tx_req_flags(uint16_t conn_handle, struct os_mbuf *txom, return rc; } -static int -ble_att_clt_tx_req(uint16_t conn_handle, struct os_mbuf *txom) -{ - int rc; - - rc = ble_att_clt_tx_req_flags(conn_handle, txom, 0); - return rc; -} - /***************************************************************************** * $error response * *****************************************************************************/ @@ -1454,8 +1440,7 @@ ble_att_clt_tx_indicate(uint16_t conn_handle, return rc; } - rc = ble_att_clt_tx_req_flags(conn_handle, txom, - BLE_HS_CONN_F_INDICATE_TXED); + rc = ble_att_clt_tx_req(conn_handle, txom); if (rc != 0) { return rc; } http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/58dde847/net/nimble/host/src/ble_gap.c ---------------------------------------------------------------------- diff --git a/net/nimble/host/src/ble_gap.c b/net/nimble/host/src/ble_gap.c index f6949cb..32656b0 100644 --- a/net/nimble/host/src/ble_gap.c +++ b/net/nimble/host/src/ble_gap.c @@ -559,6 +559,10 @@ ble_gap_conn_broken(struct ble_gap_snapshot *snap, int reason) { struct ble_gap_conn_ctxt ctxt; + /* XXX: Consider removing the connection from the list and handing it to + * each fo the "connection_broken" functions below. + */ + ble_l2cap_sm_connection_broken(snap->desc.conn_handle); ble_gattc_connection_broken(snap->desc.conn_handle); http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/58dde847/net/nimble/host/src/ble_gatt_priv.h ---------------------------------------------------------------------- diff --git a/net/nimble/host/src/ble_gatt_priv.h b/net/nimble/host/src/ble_gatt_priv.h index 5653057..d19a911 100644 --- a/net/nimble/host/src/ble_gatt_priv.h +++ b/net/nimble/host/src/ble_gatt_priv.h @@ -91,6 +91,8 @@ typedef uint8_t ble_gatts_conn_flags; struct ble_gatts_conn { struct ble_gatts_clt_cfg *clt_cfgs; int num_clt_cfgs; + + uint16_t indicate_val_handle; }; /*** @client. */ @@ -142,6 +144,7 @@ int ble_gattc_init(void); #define BLE_GATTS_INC_SVC_LEN_NO_UUID 4 #define BLE_GATTS_INC_SVC_LEN_UUID 6 +int ble_gatts_rx_indicate_ack(uint16_t conn_handle, uint16_t chr_val_handle); int ble_gatts_send_next_indicate(uint16_t conn_handle); /*** @misc. */ http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/58dde847/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 338010d..de828a9 100644 --- a/net/nimble/host/src/ble_gattc.c +++ b/net/nimble/host/src/ble_gattc.c @@ -3649,23 +3649,18 @@ ble_gattc_indicate_err(struct ble_gattc_proc *proc, int status, static void ble_gattc_indicate_rx_rsp(struct ble_gattc_proc *proc) { - struct ble_hs_conn *conn; + int rc; ble_gattc_dbg_assert_proc_not_inserted(proc); - ble_gattc_indicate_cb(proc, 0, 0); - - ble_hs_lock(); - conn = ble_hs_conn_find(proc->conn_handle); - if (conn != NULL) { - conn->bhc_flags &= ~BLE_HS_CONN_F_INDICATE_TXED; + rc = ble_gatts_rx_indicate_ack(proc->conn_handle, + proc->indicate.chr_val_handle); + if (rc != BLE_HS_ENOTCONN && rc != BLE_HS_ENOENT) { + ble_gattc_indicate_cb(proc, rc, 0); } - ble_hs_unlock(); /* Send the next indication if one is pending. */ - if (conn != NULL) { - ble_gatts_send_next_indicate(proc->conn_handle); - } + ble_gatts_send_next_indicate(proc->conn_handle); } /** @@ -3682,6 +3677,7 @@ ble_gattc_indicate(uint16_t conn_handle, uint16_t chr_val_handle, struct ble_att_svr_access_ctxt ctxt; struct ble_att_indicate_req req; struct ble_gattc_proc *proc; + struct ble_hs_conn *conn; int rc; STATS_INC(ble_gattc_stats, indicate); @@ -3717,6 +3713,15 @@ ble_gattc_indicate(uint16_t conn_handle, uint16_t chr_val_handle, goto done; } + ble_hs_lock(); + conn = ble_hs_conn_find(conn_handle); + if (conn != NULL) { + BLE_HS_DBG_ASSERT(conn->bhc_gatt_svr.indicate_val_handle == 0); + conn->bhc_gatt_svr.indicate_val_handle = chr_val_handle; + } + ble_hs_unlock(); + + done: if (rc != 0) { STATS_INC(ble_gattc_stats, indicate_fail); http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/58dde847/net/nimble/host/src/ble_gatts.c ---------------------------------------------------------------------- diff --git a/net/nimble/host/src/ble_gatts.c b/net/nimble/host/src/ble_gatts.c index 15d772f..9a1be0d 100644 --- a/net/nimble/host/src/ble_gatts.c +++ b/net/nimble/host/src/ble_gatts.c @@ -17,6 +17,8 @@ * under the License. */ +/* XXX: Standardize on chr_val_handle; never use definition handle. */ + #include <stddef.h> #include <string.h> #include "console/console.h" @@ -1039,9 +1041,8 @@ ble_gatts_send_next_indicate(uint16_t conn_handle) BLE_HS_DBG_ASSERT(clt_cfg->flags & BLE_GATTS_CLT_CFG_F_INDICATE); - /* Clear updated flag in anticipation of intication tx. */ + /* Clear updated flag in anticipation of indication tx. */ clt_cfg->flags &= ~BLE_GATTS_CLT_CFG_F_UPDATED; - chr_val_handle = clt_cfg->chr_def_handle + 1; break; } @@ -1066,6 +1067,83 @@ ble_gatts_send_next_indicate(uint16_t conn_handle) return 0; } +int +ble_gatts_rx_indicate_ack(uint16_t conn_handle, uint16_t chr_val_handle) +{ + struct ble_store_value_cccd cccd_value; + struct ble_gatts_clt_cfg *clt_cfg; + struct ble_hs_conn *conn; + uint16_t chr_def_handle; + int clt_cfg_idx; + int persist; + int rc; + + chr_def_handle = chr_val_handle - 1; + + clt_cfg_idx = ble_gatts_clt_cfg_find_idx(ble_gatts_clt_cfgs, + chr_def_handle); + if (clt_cfg_idx == -1) { + /* This characteristic does not have a CCCD. */ + return BLE_HS_ENOENT; + } + + clt_cfg = ble_gatts_clt_cfgs + clt_cfg_idx; + if (!(clt_cfg->allowed & BLE_GATTS_CLT_CFG_F_INDICATE)) { + /* This characteristic does not allow indications. */ + return BLE_HS_ENOENT; + } + + ble_hs_lock(); + + conn = ble_hs_conn_find(conn_handle); + BLE_HS_DBG_ASSERT(conn != NULL); + if (conn->bhc_gatt_svr.indicate_val_handle == chr_val_handle) { + /* This acknowledgement is expected. */ + rc = 0; + + /* Mark that there is no longer an outstanding txed indicate. */ + conn->bhc_gatt_svr.indicate_val_handle = 0; + + /* Determine if we need to persist that there is no pending indication + * for this peer-characteristic pair. If the characteristic has not + * been modified since we sent the indication, there is no indication + * pending. + */ + BLE_HS_DBG_ASSERT(conn->bhc_gatt_svr.num_clt_cfgs > clt_cfg_idx); + clt_cfg = conn->bhc_gatt_svr.clt_cfgs + clt_cfg_idx; + BLE_HS_DBG_ASSERT(clt_cfg->chr_def_handle == chr_def_handle); + + persist = conn->bhc_sec_state.bonded && + !(clt_cfg->flags & BLE_GATTS_CLT_CFG_F_UPDATED); + if (persist) { + cccd_value.peer_addr_type = conn->bhc_addr_type; + memcpy(cccd_value.peer_addr, conn->bhc_addr, 6); + cccd_value.flags = clt_cfg->flags; + cccd_value.value_changed = 0; + } + } else { + /* This acknowledgement doesn't correspnod to the outstanding + * indication; ignore it. + */ + rc = BLE_HS_ENOENT; + } + + ble_hs_unlock(); + + if (rc != 0) { + return rc; + } + + if (persist) { + rc = ble_store_write_cccd(&cccd_value); + if (rc != 0) { + return rc; + } + } + + return 0; +} + void ble_gatts_chr_updated(uint16_t chr_def_handle) { @@ -1107,7 +1185,13 @@ ble_gatts_chr_updated(uint16_t chr_def_handle) if (clt_cfg->flags & BLE_GATTS_CLT_CFG_F_NOTIFY) { clt_cfg_flags = clt_cfg->flags; } else if (clt_cfg->flags & BLE_GATTS_CLT_CFG_F_INDICATE) { - if (conn->bhc_flags & BLE_HS_CONN_F_INDICATE_TXED) { + /* Only one outstanding indication per peer is allowed. If we + * are still awaiting an ack, mark this CCCD as updated so that + * an indication will get sent after the current one is + * acknowledged. If there isn't an outstanding indication, + * just send this one now. + */ + if (conn->bhc_gatt_svr.indicate_val_handle != 0) { clt_cfg->flags |= BLE_GATTS_CLT_CFG_F_UPDATED; clt_cfg_flags = 0; } else { http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/58dde847/net/nimble/host/src/ble_hs_conn_priv.h ---------------------------------------------------------------------- diff --git a/net/nimble/host/src/ble_hs_conn_priv.h b/net/nimble/host/src/ble_hs_conn_priv.h index 36f6747..a77428e 100644 --- a/net/nimble/host/src/ble_hs_conn_priv.h +++ b/net/nimble/host/src/ble_hs_conn_priv.h @@ -32,7 +32,6 @@ typedef uint8_t ble_hs_conn_flags_t; #define BLE_HS_CONN_F_MASTER 0x01 #define BLE_HS_CONN_F_UPDATE 0x02 -#define BLE_HS_CONN_F_INDICATE_TXED 0x04 struct ble_hs_conn { SLIST_ENTRY(ble_hs_conn) bhc_next;
