Skip to content

nimble/host: Fix ATT Read By Type Response handling #2123

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

nimble/host: Fix ATT Read By Type Response handling #2123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion nimble/host/src/ble_att_clt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,13 +431,20 @@ ble_att_clt_rx_read_type(uint16_t conn_handle, uint16_t cid, struct os_mbuf **rx
adata.value_len = data_len - sizeof(*data);
adata.value = data->value;

ble_gattc_rx_read_type_adata(conn_handle, cid, &adata);
rc = ble_gattc_rx_read_type_adata(conn_handle, cid, &adata);
if (rc != 0) {
/* We should not call complete callback if this returned an error
* since proc is not added to the proc list.
*/
return rc;
}
os_mbuf_adj(*rxom, data_len);
}

done:
/* Notify GATT that the response is done being parsed. */
ble_gattc_rx_read_type_complete(conn_handle, cid, rc);

return rc;

}
Expand Down
4 changes: 2 additions & 2 deletions nimble/host/src/ble_gatt_priv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ void ble_gatts_indicate_fail_notconn(uint16_t conn_handle);

void ble_gattc_rx_err(uint16_t conn_handle, uint16_t cid, uint16_t handle, uint16_t status);
void ble_gattc_rx_mtu(uint16_t conn_handle, uint16_t cid, int status, uint16_t chan_mtu);
void ble_gattc_rx_read_type_adata(uint16_t conn_handle, uint16_t cid,
struct ble_att_read_type_adata *adata);
int ble_gattc_rx_read_type_adata(uint16_t conn_handle, uint16_t cid,
struct ble_att_read_type_adata *adata);
void ble_gattc_rx_read_type_complete(uint16_t conn_handle, uint16_t cid, int status);
void ble_gattc_rx_read_rsp(uint16_t conn_handle, uint16_t cid, int status,
struct os_mbuf **rxom);
Expand Down
85 changes: 48 additions & 37 deletions nimble/host/src/ble_gattc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -754,12 +754,16 @@ ble_gattc_proc_free(struct ble_gattc_proc *proc)
}

static void
ble_gattc_proc_insert(struct ble_gattc_proc *proc)
ble_gattc_proc_insert(struct ble_gattc_proc *proc, bool insert_head)
{
ble_gattc_dbg_assert_proc_not_inserted(proc);

ble_hs_lock();
STAILQ_INSERT_TAIL(&ble_gattc_procs, proc, next);
if (insert_head) {
STAILQ_INSERT_HEAD(&ble_gattc_procs, proc, next);
} else {
STAILQ_INSERT_TAIL(&ble_gattc_procs, proc, next);
}
ble_hs_unlock();
}

Expand Down Expand Up @@ -790,15 +794,15 @@ ble_gattc_proc_set_resume_timer(struct ble_gattc_proc *proc)
}

static void
ble_gattc_process_status(struct ble_gattc_proc *proc, int status)
ble_gattc_process_status(struct ble_gattc_proc *proc, int status, bool insert_head)
{
switch (status) {
case 0:
if (!(proc->flags & BLE_GATTC_PROC_F_STALLED)) {
ble_gattc_proc_set_exp_timer(proc);
}

ble_gattc_proc_insert(proc);
ble_gattc_proc_insert(proc, insert_head);
ble_hs_timer_resched();
break;

Expand Down Expand Up @@ -1187,7 +1191,7 @@ ble_gattc_resume_procs(void)

proc->flags &= ~BLE_GATTC_PROC_F_STALLED;
rc = resume_cb(proc);
ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
}
}

Expand Down Expand Up @@ -1398,7 +1402,7 @@ ble_gattc_exchange_mtu(uint16_t conn_handle, ble_gatt_mtu_fn *cb, void *cb_arg)
STATS_INC(ble_gattc_stats, mtu_fail);
}

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
return rc;
}

Expand Down Expand Up @@ -1623,7 +1627,7 @@ ble_gattc_disc_all_svcs(uint16_t conn_handle, ble_gatt_disc_svc_fn *cb,
STATS_INC(ble_gattc_stats, disc_all_svcs_fail);
}

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
return rc;
}

Expand Down Expand Up @@ -1837,7 +1841,7 @@ ble_gattc_disc_svc_by_uuid(uint16_t conn_handle, const ble_uuid_t *uuid,
STATS_INC(ble_gattc_stats, disc_svc_uuid_fail);
}

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
return rc;
}

Expand Down Expand Up @@ -2153,7 +2157,7 @@ ble_gattc_find_inc_svcs(uint16_t conn_handle, uint16_t start_handle,
STATS_INC(ble_gattc_stats, find_inc_svcs_fail);
}

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
return rc;
}

Expand Down Expand Up @@ -2386,7 +2390,7 @@ ble_gattc_disc_all_chrs(uint16_t conn_handle, uint16_t start_handle,
STATS_INC(ble_gattc_stats, disc_all_chrs_fail);
}

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
return rc;
}

Expand Down Expand Up @@ -2626,7 +2630,7 @@ ble_gattc_disc_chrs_by_uuid(uint16_t conn_handle, uint16_t start_handle,
STATS_INC(ble_gattc_stats, disc_chrs_uuid_fail);
}

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
return rc;
}

Expand Down Expand Up @@ -2836,7 +2840,7 @@ ble_gattc_disc_all_dscs(uint16_t conn_handle, uint16_t start_handle,
STATS_INC(ble_gattc_stats, disc_all_dscs_fail);
}

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
return rc;
}

Expand Down Expand Up @@ -2971,7 +2975,7 @@ ble_gattc_read(uint16_t conn_handle, uint16_t attr_handle,
STATS_INC(ble_gattc_stats, read_fail);
}

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
return rc;
}

Expand Down Expand Up @@ -3135,7 +3139,7 @@ ble_gattc_read_by_uuid(uint16_t conn_handle, uint16_t start_handle,
STATS_INC(ble_gattc_stats, read_uuid_fail);
}

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
return rc;
}

Expand Down Expand Up @@ -3331,7 +3335,7 @@ ble_gattc_read_long(uint16_t conn_handle, uint16_t handle, uint16_t offset,
STATS_INC(ble_gattc_stats, read_long_fail);
}

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
return rc;
}

Expand Down Expand Up @@ -3555,7 +3559,7 @@ ble_gattc_read_mult_internal(uint16_t conn_handle, const uint16_t *handles,
STATS_INC(ble_gattc_stats, read_mult_fail);
}

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
return rc;
}

Expand Down Expand Up @@ -3730,7 +3734,7 @@ ble_gattc_write(uint16_t conn_handle, uint16_t attr_handle,
/* Free the mbuf in case the send failed. */
os_mbuf_free_chain(txom);

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
return rc;
}

Expand Down Expand Up @@ -4048,7 +4052,7 @@ ble_gattc_write_long(uint16_t conn_handle, uint16_t attr_handle,
/* Free the mbuf in case of failure. */
os_mbuf_free_chain(txom);

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
return rc;
}

Expand Down Expand Up @@ -4337,7 +4341,7 @@ ble_gattc_write_reliable(uint16_t conn_handle,
attrs[i].om = NULL;
}

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
return rc;
}

Expand Down Expand Up @@ -4709,7 +4713,7 @@ ble_gatts_indicate_custom(uint16_t conn_handle, uint16_t chr_val_handle,
/* Tell the application that an indication transmission was attempted. */
ble_gap_notify_tx_event(rc, conn_handle, chr_val_handle, 1);

ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
os_mbuf_free_chain(txom);
return rc;
}
Expand Down Expand Up @@ -4777,7 +4781,7 @@ ble_gattc_rx_mtu(uint16_t conn_handle, uint16_t cid, int status, uint16_t chan_m
proc = ble_gattc_extract_first_by_conn_cid_op(conn_handle, BLE_L2CAP_CID_ATT, BLE_GATT_OP_MTU);
if (proc != NULL) {
ble_gattc_mtu_cb(proc, status, 0, chan_mtu);
ble_gattc_process_status(proc, BLE_HS_EDONE);
ble_gattc_process_status(proc, BLE_HS_EDONE, false);
}
}

Expand All @@ -4800,7 +4804,7 @@ ble_gattc_rx_find_info_idata(uint16_t conn_handle, uint16_t cid,
BLE_GATT_OP_DISC_ALL_DSCS);
if (proc != NULL) {
rc = ble_gattc_disc_all_dscs_rx_idata(proc, idata);
ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
}
}

Expand All @@ -4822,7 +4826,7 @@ ble_gattc_rx_find_info_complete(uint16_t conn_handle, uint16_t cid, int status)
BLE_GATT_OP_DISC_ALL_DSCS);
if (proc != NULL) {
rc = ble_gattc_disc_all_dscs_rx_complete(proc, status);
ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
}
}

Expand All @@ -4845,7 +4849,7 @@ ble_gattc_rx_find_type_value_hinfo(uint16_t conn_handle, uint16_t cid,
BLE_GATT_OP_DISC_SVC_UUID);
if (proc != NULL) {
rc = ble_gattc_disc_svc_uuid_rx_hinfo(proc, hinfo);
ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
}
}

Expand All @@ -4867,15 +4871,15 @@ ble_gattc_rx_find_type_value_complete(uint16_t conn_handle, uint16_t cid, int st
BLE_GATT_OP_DISC_SVC_UUID);
if (proc != NULL) {
rc = ble_gattc_disc_svc_uuid_rx_complete(proc, status);
ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
}
}

/**
* Dispatches an incoming "attribute data" entry from a read-by-type-response
* to the appropriate active GATT procedure.
*/
void
int
ble_gattc_rx_read_type_adata(uint16_t conn_handle, uint16_t cid,
struct ble_att_read_type_adata *adata)
{
Expand All @@ -4892,8 +4896,15 @@ ble_gattc_rx_read_type_adata(uint16_t conn_handle, uint16_t cid,
&rx_entry);
if (proc != NULL) {
rc = rx_entry->cb(proc, adata);
ble_gattc_process_status(proc, rc);
/* We need to put procedure on head of the queue since caller expects
* to process the same proc again.
*/
ble_gattc_process_status(proc, rc, true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shoudn't this be put back only on success?

} else {
rc = -1;
}

return rc;
}

/**
Expand All @@ -4916,7 +4927,7 @@ ble_gattc_rx_read_type_complete(uint16_t conn_handle, uint16_t cid, int status)
&rx_entry);
if (proc != NULL) {
rc = rx_entry->cb(proc, status);
ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
}
}

Expand All @@ -4939,7 +4950,7 @@ ble_gattc_rx_read_group_type_adata(uint16_t conn_handle, uint16_t cid,
BLE_GATT_OP_DISC_ALL_SVCS);
if (proc != NULL) {
rc = ble_gattc_disc_all_svcs_rx_adata(proc, adata);
ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
}
}

Expand All @@ -4961,7 +4972,7 @@ ble_gattc_rx_read_group_type_complete(uint16_t conn_handle, uint16_t cid, int st
BLE_GATT_OP_DISC_ALL_SVCS);
if (proc != NULL) {
rc = ble_gattc_disc_all_svcs_rx_complete(proc, status);
ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
}
}

Expand All @@ -4985,7 +4996,7 @@ ble_gattc_rx_read_rsp(uint16_t conn_handle, uint16_t cid, int status, struct os_
&rx_entry);
if (proc != NULL) {
rc = rx_entry->cb(proc, status, om);
ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
}
}

Expand All @@ -5008,7 +5019,7 @@ ble_gattc_rx_read_blob_rsp(uint16_t conn_handle, uint16_t cid, int status,
BLE_GATT_OP_READ_LONG);
if (proc != NULL) {
rc = ble_gattc_read_long_rx_read_rsp(proc, status, om);
ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
}
}

Expand All @@ -5032,7 +5043,7 @@ ble_gattc_rx_read_mult_rsp(uint16_t conn_handle, uint16_t cid, int status,
proc = ble_gattc_extract_first_by_conn_cid_op(conn_handle, cid, op);
if (proc != NULL) {
ble_gattc_read_mult_cb(proc, status, 0, om);
ble_gattc_process_status(proc, BLE_HS_EDONE);
ble_gattc_process_status(proc, BLE_HS_EDONE, false);
}
}

Expand All @@ -5053,7 +5064,7 @@ ble_gattc_rx_write_rsp(uint16_t conn_handle, uint16_t cid)
BLE_GATT_OP_WRITE);
if (proc != NULL) {
ble_gattc_write_cb(proc, 0, 0);
ble_gattc_process_status(proc, BLE_HS_EDONE);
ble_gattc_process_status(proc, BLE_HS_EDONE, false);
}
}

Expand All @@ -5079,7 +5090,7 @@ ble_gattc_rx_prep_write_rsp(uint16_t conn_handle, uint16_t cid, int status,
&rx_entry);
if (proc != NULL) {
rc = rx_entry->cb(proc, status, handle, offset, om);
ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
}
}

Expand All @@ -5102,7 +5113,7 @@ ble_gattc_rx_exec_write_rsp(uint16_t conn_handle, uint16_t cid, int status)
ble_gattc_rx_exec_entries, &rx_entry);
if (proc != NULL) {
rc = rx_entry->cb(proc, status);
ble_gattc_process_status(proc, rc);
ble_gattc_process_status(proc, rc, false);
}
}

Expand All @@ -5123,7 +5134,7 @@ ble_gatts_rx_indicate_rsp(uint16_t conn_handle, uint16_t cid)
BLE_GATT_OP_INDICATE);
if (proc != NULL) {
ble_gatts_indicate_rx_rsp(proc);
ble_gattc_process_status(proc, BLE_HS_EDONE);
ble_gattc_process_status(proc, BLE_HS_EDONE, false);
}
}

Expand Down
Loading