From a3cf21a8ef87104e3c0eea51d100e8a9d3c31977 Mon Sep 17 00:00:00 2001 From: Emil Gydesen Date: Tue, 10 Dec 2024 11:16:26 +0100 Subject: [PATCH] Bluetooth: TBS: Ensure sending notifications The existing implemented only attempted to send all notifications, but if host was out of ATT TX buffers the notifications would fail and the client may miss out on important information, and would be a spec violation. This commit refactors notificatios in TBS so that they are always sent. Signed-off-by: Emil Gydesen --- subsys/bluetooth/audio/Kconfig.tbs | 8 + subsys/bluetooth/audio/tbs.c | 825 +++++++++++++++++++--- tests/bsim/bluetooth/audio/src/tbs_test.c | 4 +- 3 files changed, 741 insertions(+), 96 deletions(-) diff --git a/subsys/bluetooth/audio/Kconfig.tbs b/subsys/bluetooth/audio/Kconfig.tbs index 14729499bee461..1194ced263b52c 100644 --- a/subsys/bluetooth/audio/Kconfig.tbs +++ b/subsys/bluetooth/audio/Kconfig.tbs @@ -51,6 +51,14 @@ config BT_TBS_MAX_SCHEME_LIST_LENGTH help Sets the maximum length of the URI scheme list. +config BT_TBS_LOCK_TIMEOUT + int "Milliseconds of timeout when handling concurrent operations" + range 0 1000 + default 10 + help + The number of milliseconds that the TBS implementation will maximum wait before rejecting + an write request if another is already in progress in another thread. + endif # BT_TBS diff --git a/subsys/bluetooth/audio/tbs.c b/subsys/bluetooth/audio/tbs.c index fb94678375fa25..cff011e271d605 100644 --- a/subsys/bluetooth/audio/tbs.c +++ b/subsys/bluetooth/audio/tbs.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -34,10 +35,26 @@ #include "audio_internal.h" #include "tbs_internal.h" +#include "common/bt_str.h" LOG_MODULE_REGISTER(bt_tbs, CONFIG_BT_TBS_LOG_LEVEL); #define BT_TBS_VALID_STATUS_FLAGS(val) ((val) <= (BIT(0) | BIT(1))) +#define MUTEX_TIMEOUT K_MSEC(CONFIG_BT_TBS_LOCK_TIMEOUT) + +struct tbs_flags { + bool bearer_provider_name_changed: 1; + bool bearer_technology_changed: 1; + bool bearer_uri_schemes_supported_list_changed: 1; + bool bearer_signal_strength_changed: 1; + bool bearer_list_current_calls_changed: 1; + bool status_flags_changed: 1; + bool incoming_call_target_bearer_uri_changed: 1; + bool call_state_changed: 1; + bool termination_reason_changed: 1; + bool incoming_call_changed: 1; + bool call_friendly_name_changed: 1; +}; /* A service instance can either be a GTBS or a TBS instance */ struct tbs_inst { @@ -57,8 +74,6 @@ struct tbs_inst { struct bt_tbs_terminate_reason terminate_reason; struct bt_tbs_call calls[CONFIG_BT_TBS_MAX_CALLS]; - bool notify_current_calls; - bool notify_call_states; bool pending_signal_strength_notification; struct k_work_delayable reporting_interval_work; @@ -68,6 +83,22 @@ struct tbs_inst { size_t attr_count; bool authorization_required; + + struct k_mutex mutex; + /* Flags for each client. Access and modification of these shall be guarded by the mutex */ + struct tbs_flags flags[CONFIG_BT_MAX_CONN]; + + /* Control point notifications are handled separately from other notifications - We will not + * accept any new control point operations while a notification is pending + */ + struct cp_ntf { + struct bt_tbs_call_cp_notify notification; + + uint8_t conn_index; /* The conn index that triggered the request */ + bool pending: 1; + } cp_ntf; + + struct k_work_delayable notify_work; }; static struct tbs_inst svc_insts[CONFIG_BT_TBS_BEARER_COUNT]; @@ -303,6 +334,126 @@ static struct tbs_inst *lookup_inst_by_uri_scheme(const uint8_t *uri, uint8_t ur return NULL; } +static void disconnected(struct bt_conn *conn, uint8_t reason) +{ + /* Clear pending notifications */ + for (size_t i = 0U; i < ARRAY_SIZE(svc_insts); i++) { + const uint8_t conn_index = bt_conn_index(conn); + int err; + + err = k_mutex_lock(&svc_insts[i].mutex, MUTEX_TIMEOUT); + if (err != 0) { + LOG_WRN("Failed to take mutex: %d", err); + /* In this case we still need to clear the data, so continue and hope for + * the best + */ + } + + if (svc_insts[i].cp_ntf.pending && conn_index == svc_insts[i].cp_ntf.conn_index) { + memset(&svc_insts[i].cp_ntf, 0, sizeof(svc_insts[i].cp_ntf)); + } + + memset(&svc_insts[i].flags[conn_index], 0, sizeof(svc_insts[i].flags[conn_index])); + + if (err == 0) { /* if mutex was locked */ + err = k_mutex_unlock(&svc_insts[i].mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + } + } +} + +BT_CONN_CB_DEFINE(conn_cb) = { + .disconnected = disconnected, +}; + +static int notify(struct bt_conn *conn, const struct bt_uuid *uuid, + const struct bt_gatt_attr *attrs, const void *value, size_t value_len) +{ + const uint8_t att_header_size = 3; /* opcode + handle */ + const uint16_t att_mtu = bt_gatt_get_mtu(conn); + + __ASSERT(att_mtu > att_header_size, "Could not get valid ATT MTU"); + const uint16_t maxlen = att_mtu - att_header_size; /* Subtract opcode and handle */ + + if (maxlen < value_len) { + LOG_DBG("Truncating notification to %u (was %u)", maxlen, value_len); + value_len = maxlen; + } + + /* Send notification potentially truncated to the MTU */ + return bt_gatt_notify_uuid(conn, uuid, attrs, value, value_len); +} + +struct tbs_notify_cb_info { + struct tbs_inst *inst; + struct bt_gatt_attr *attr; + void (*value_cb)(struct tbs_flags *flags); +}; + +static void set_value_changed_cb(struct bt_conn *conn, void *data) +{ + struct tbs_notify_cb_info *cb_info = data; + struct tbs_inst *inst = cb_info->inst; + struct tbs_flags *flags = &inst->flags[bt_conn_index(conn)]; + struct bt_gatt_attr *attr = cb_info->attr; + struct bt_conn_info info; + int err; + + err = bt_conn_get_info(conn, &info); + __ASSERT(err == 0, "Failed to get conn info: %d", err); + + if (info.state != BT_CONN_STATE_CONNECTED) { + /* Not connected */ + return; + } + + if (!bt_gatt_is_subscribed(conn, attr, BT_GATT_CCC_NOTIFY)) { + /* Not subscribed */ + return; + } + + /* Set the specific flag based on the provided callback */ + cb_info->value_cb(flags); + + /* We may schedule the same work multiple times, but that is OK as scheduling the same work + * multiple times is a no-op + */ + err = k_work_schedule(&inst->notify_work, K_NO_WAIT); + __ASSERT(err >= 0, "Failed to schedule work: %d", err); +} + +static void set_value_changed(struct tbs_inst *inst, void (*value_cb)(struct tbs_flags *flags), + const struct bt_uuid *uuid) +{ + struct tbs_notify_cb_info cb_info = { + .inst = inst, + .value_cb = value_cb, + .attr = bt_gatt_find_by_uuid(inst->attrs, 0, uuid), + }; + + __ASSERT(cb_info.attr != NULL, "Failed to look attribute for %s", bt_uuid_str(uuid)); + bt_conn_foreach(BT_CONN_TYPE_LE, set_value_changed_cb, &cb_info); +} + +/** + * 1) When value is changed, set a bit, and schedule work if not already + * 2) +Adopt MCS ways of scheduling notifications in TBS +Add truncation of values +Add long read dirty bit +Need to reject certain operations while notifications are pending + BT_TBS_RESULT_CODE_OPERATION_NOT_POSSIBLE + */ + +static void set_terminate_reason_changed_cb(struct tbs_flags *flags) +{ + if (flags->termination_reason_changed) { + LOG_DBG("pending notification replaced"); + } + + flags->termination_reason_changed = true; +} + static void tbs_set_terminate_reason(struct tbs_inst *inst, uint8_t call_index, uint8_t reason) { inst->terminate_reason.call_index = call_index; @@ -310,8 +461,7 @@ static void tbs_set_terminate_reason(struct tbs_inst *inst, uint8_t call_index, LOG_DBG("Index %u: call index 0x%02x, reason %s", inst_index(inst), call_index, bt_tbs_term_reason_str(reason)); - bt_gatt_notify_uuid(NULL, BT_UUID_TBS_TERMINATE_REASON, inst->attrs, - (void *)&inst->terminate_reason, sizeof(inst->terminate_reason)); + set_value_changed(inst, set_terminate_reason_changed_cb, BT_UUID_TBS_TERMINATE_REASON); } /** @@ -481,34 +631,291 @@ static void net_buf_put_current_calls(const struct tbs_inst *inst, struct net_bu } } -static int inst_notify_calls(const struct tbs_inst *inst) +static void set_call_state_changed_cb(struct tbs_flags *flags) +{ + if (flags->call_state_changed) { + LOG_DBG("pending notification replaced"); + } + + flags->call_state_changed = true; +} + +static void set_list_current_calls_changed_cb(struct tbs_flags *flags) +{ + if (flags->bearer_list_current_calls_changed) { + LOG_DBG("pending notification replaced"); + } + + flags->bearer_list_current_calls_changed = true; +} + +static int inst_notify_calls(struct tbs_inst *inst) { + set_value_changed(inst, set_call_state_changed_cb, BT_UUID_TBS_CALL_STATE); + set_value_changed(inst, set_list_current_calls_changed_cb, BT_UUID_TBS_LIST_CURRENT_CALLS); + + return 0; +} + +/** + * @brief Attempt to move a call in an instance from dialing to alerting + * + * This function will look through the state of an instance to see if there are any calls in the + * instance that are in the dialing state, and move them to the dialing state if we do not have any + * pending call state notification. The reason for this is that we do not have an API for the + * application to change from dialing to alterting state at this point, but the qualification tests + * require us to do this state change. + * Since we only notify the latest value, we need to notify dialing first for both current calls and + * call states, and then switch to the alerting state for the call and then notify again. + * + * @param inst The instance to attempt the state change on + * @retval true There was a state change + * @retval false There was not a state change + */ +static bool try_change_dialing_call_to_alerting(struct tbs_inst *inst) +{ + bool state_changed = false; + + /* If we still have pending state change notifications, we cannot change the state + * autonomously + */ + for (size_t i = 0U; i < ARRAY_SIZE(inst->flags); i++) { + const struct tbs_flags *flags = &inst->flags[i]; + + if (flags->bearer_list_current_calls_changed || flags->call_state_changed) { + return false; + } + } + + /* Check if we have any calls in the dialing state */ + for (size_t i = 0U; i < ARRAY_SIZE(inst->calls); i++) { + if (inst->calls[i].state == BT_TBS_CALL_STATE_DIALING) { + inst->calls[i].state = BT_TBS_CALL_STATE_ALERTING; + state_changed = true; + break; + } + } + + if (state_changed) { + inst_notify_calls(inst); + } + + return state_changed; +} + +static void notify_handler_cb(struct bt_conn *conn, void *data) +{ + struct tbs_inst *inst = data; + struct tbs_flags *flags = &inst->flags[bt_conn_index(conn)]; + struct bt_conn_info info; int err; - if (inst->notify_call_states) { - net_buf_put_call_states(inst, &read_buf); + err = bt_conn_get_info(conn, &info); + __ASSERT(err == 0, "Failed to get conn info: %d", err); - err = bt_gatt_notify_uuid(NULL, BT_UUID_TBS_CALL_STATE, inst->attrs, read_buf.data, - read_buf.len); - if (err != 0) { - return err; + if (info.state != BT_CONN_STATE_CONNECTED) { + /* Not connected */ + return; + } + + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to take mutex: %d", err); + goto reschedule; + } + + if (flags->bearer_provider_name_changed) { + LOG_DBG("Notifying Bearer Provider Name: %s", inst->provider_name); + + err = notify(conn, BT_UUID_TBS_PROVIDER_NAME, inst->attrs, inst->provider_name, + strlen(inst->provider_name)); + if (err == 0) { + flags->bearer_provider_name_changed = false; + } else { + goto fail; + } + } + + if (flags->bearer_technology_changed) { + LOG_DBG("Notifying Bearer Technology: %s (0x%02x)", + bt_tbs_technology_str(inst->technology), inst->technology); + + err = notify(conn, BT_UUID_TBS_TECHNOLOGY, inst->attrs, &inst->technology, + sizeof(inst->technology)); + if (err == 0) { + flags->bearer_technology_changed = false; + } else { + goto fail; + } + } + + if (flags->bearer_uri_schemes_supported_list_changed) { + LOG_DBG("Notifying Bearer URI schemes supported list: %s", inst->uri_scheme_list); + + err = notify(conn, BT_UUID_TBS_URI_LIST, inst->attrs, &inst->uri_scheme_list, + strlen(inst->uri_scheme_list)); + if (err == 0) { + flags->bearer_uri_schemes_supported_list_changed = false; + } else { + goto fail; } } - if (inst->notify_current_calls) { + if (flags->bearer_signal_strength_changed) { + LOG_DBG("Notifying Bearer Signal Strength: 0x%02x", inst->signal_strength); + + err = notify(conn, BT_UUID_TBS_SIGNAL_STRENGTH, inst->attrs, &inst->signal_strength, + sizeof(inst->signal_strength)); + if (err == 0) { + flags->bearer_signal_strength_changed = false; + } else { + goto fail; + } + } + + if (flags->bearer_list_current_calls_changed) { + LOG_DBG("Notifying Bearer List Current Calls"); + net_buf_put_current_calls(inst, &read_buf); + err = notify(conn, BT_UUID_TBS_LIST_CURRENT_CALLS, inst->attrs, read_buf.data, + read_buf.len); + if (err == 0) { + flags->bearer_list_current_calls_changed = false; + } else { + goto fail; + } - err = bt_gatt_notify_uuid(NULL, BT_UUID_TBS_LIST_CURRENT_CALLS, inst->attrs, - read_buf.data, read_buf.len); - if (err != 0) { - return err; + if (try_change_dialing_call_to_alerting(inst)) { + goto reschedule; } } - return 0; + if (flags->status_flags_changed) { + LOG_DBG("Notifying Status Flags: 0x%02x", inst->status_flags); + + err = notify(conn, BT_UUID_TBS_STATUS_FLAGS, inst->attrs, &inst->status_flags, + sizeof(inst->status_flags)); + if (err == 0) { + flags->status_flags_changed = false; + } else { + goto fail; + } + } + + if (flags->incoming_call_target_bearer_uri_changed) { + LOG_DBG("Notifying Incoming Call Target Bearer URI: call index 0x%02x, URI %s", + inst->incoming_uri.call_index, inst->incoming_uri.uri); + + err = notify(conn, BT_UUID_TBS_INCOMING_URI, inst->attrs, &inst->incoming_uri, + sizeof(inst->incoming_uri.call_index) + + strlen(inst->incoming_uri.uri)); + if (err == 0) { + flags->incoming_call_target_bearer_uri_changed = false; + } else { + goto fail; + } + } + + if (flags->call_state_changed) { + LOG_DBG("Notifying Call States"); + + net_buf_put_call_states(inst, &read_buf); + err = notify(conn, BT_UUID_TBS_CALL_STATE, inst->attrs, read_buf.data, + read_buf.len); + if (err == 0) { + flags->call_state_changed = false; + } else { + goto fail; + } + + if (try_change_dialing_call_to_alerting(inst)) { + goto reschedule; + } + } + + if (flags->termination_reason_changed) { + LOG_DBG("Notifying Bearer Provider Name: call_index 0x%02x reason 0x%02x", + inst->terminate_reason.call_index, inst->terminate_reason.reason); + + err = notify(conn, BT_UUID_TBS_TERMINATE_REASON, inst->attrs, + &inst->terminate_reason, sizeof(inst->terminate_reason)); + if (err == 0) { + flags->termination_reason_changed = false; + } else { + goto fail; + } + } + + if (flags->incoming_call_changed) { + LOG_DBG("Notifying Incoming Call: call index 0x%02x, URI %s", + inst->in_call.call_index, inst->in_call.uri); + + err = notify(conn, BT_UUID_TBS_INCOMING_CALL, inst->attrs, &inst->in_call, + sizeof(inst->in_call.call_index) + strlen(inst->in_call.uri)); + if (err == 0) { + flags->incoming_call_changed = false; + } else { + goto fail; + } + } + + if (flags->call_friendly_name_changed) { + LOG_DBG("Notifying Friendly Name: call index 0x%02x, URI %s", + inst->friendly_name.call_index, inst->friendly_name.uri); + + err = notify(conn, BT_UUID_TBS_FRIENDLY_NAME, inst->attrs, &inst->friendly_name, + sizeof(inst->friendly_name.call_index) + + strlen(inst->friendly_name.uri)); + if (err == 0) { + flags->call_friendly_name_changed = false; + } else { + goto fail; + } + } + + /* The TBS spec is a bit unclear on this, but the TBS test spec states that the control + * operation notification shall be sent after the current calls and call state + * notifications, this shall be triggered after those. + */ + if (inst->cp_ntf.pending && bt_conn_index(conn) == inst->cp_ntf.conn_index && + !flags->bearer_list_current_calls_changed && !flags->call_state_changed) { + const struct bt_tbs_call_cp_notify *notification = &inst->cp_ntf.notification; + + LOG_DBG("Notifying CCP: Call index %u, %s opcode and status %s", + notification->call_index, bt_tbs_opcode_str(notification->opcode), + bt_tbs_status_str(notification->status)); + + err = notify(conn, BT_UUID_TBS_CALL_CONTROL_POINT, inst->attrs, notification, + sizeof(*notification)); + if (err == 0) { + inst->cp_ntf.pending = false; + } else { + goto fail; + } + } + +fail: + if (err != 0) { + LOG_DBG("Notify failed (%d), retrying next connection interval", err); +reschedule: + err = k_work_reschedule(&inst->notify_work, + K_USEC(BT_CONN_INTERVAL_TO_US(info.le.interval))); + __ASSERT(err >= 0, "Failed to reschedule work: %d", err); + } + + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); } -static int notify_calls(const struct tbs_inst *inst) +static void notify_work_handler(struct k_work *work) +{ + struct tbs_inst *inst = + CONTAINER_OF(k_work_delayable_from_work(work), struct tbs_inst, notify_work); + + bt_conn_foreach(BT_CONN_TYPE_LE, notify_handler_cb, inst); +} + +static int notify_calls(struct tbs_inst *inst) { int err; @@ -690,7 +1097,6 @@ static void current_calls_cfg_changed(const struct bt_gatt_attr *attr, uint16_t if (inst != NULL) { LOG_DBG("Index %u: value 0x%04x", inst_index(inst), value); - inst->notify_current_calls = (value == BT_GATT_CCC_NOTIFY); } } @@ -795,22 +1201,9 @@ static void call_state_cfg_changed(const struct bt_gatt_attr *attr, uint16_t val if (inst != NULL) { LOG_DBG("Index %u: value 0x%04x", inst_index(inst), value); - inst->notify_call_states = (value == BT_GATT_CCC_NOTIFY); } } -static int notify_ccp(struct bt_conn *conn, const struct bt_gatt_attr *attr, uint8_t call_index, - uint8_t opcode, uint8_t status) -{ - const struct bt_tbs_call_cp_notify ccp_not = { - .call_index = call_index, .opcode = opcode, .status = status}; - - LOG_DBG("Notifying CCP: Call index %u, %s opcode and status %s", call_index, - bt_tbs_opcode_str(opcode), bt_tbs_status_str(status)); - - return bt_gatt_notify(conn, attr, &ccp_not, sizeof(ccp_not)); -} - static void hold_other_calls(struct tbs_inst *inst, uint8_t call_index_cnt, const uint8_t *call_indexes) { @@ -958,8 +1351,6 @@ static int originate_call(struct tbs_inst *inst, const struct bt_tbs_call_cp_ori hold_other_calls(inst, 1, &call->index); - notify_calls(inst); - call->state = BT_TBS_CALL_STATE_ALERTING; notify_calls(inst); LOG_DBG("New call with call index %u", call->index); @@ -1113,6 +1504,30 @@ static void notify_app(struct bt_conn *conn, struct tbs_inst *inst, uint16_t len } } +static bool is_valid_cp_len(uint16_t len, const union bt_tbs_call_cp_t *ccp) +{ + if (len < sizeof(ccp->opcode)) { + return false; + } + + switch (ccp->opcode) { + case BT_TBS_CALL_OPCODE_ACCEPT: + return len == sizeof(ccp->accept); + case BT_TBS_CALL_OPCODE_TERMINATE: + return len == sizeof(ccp->terminate); + case BT_TBS_CALL_OPCODE_HOLD: + return len == sizeof(ccp->hold); + case BT_TBS_CALL_OPCODE_RETRIEVE: + return len == sizeof(ccp->retrieve); + case BT_TBS_CALL_OPCODE_ORIGINATE: + return len >= sizeof(ccp->originate) + BT_TBS_MIN_URI_LEN; + case BT_TBS_CALL_OPCODE_JOIN: + return len >= sizeof(ccp->join) + 1; /* at least 1 call index */ + default: + return true; /* defer to future checks */ + } +} + static ssize_t write_call_cp(struct bt_conn *conn, const struct bt_gatt_attr *attr, const void *buf, uint16_t len, uint16_t offset, uint8_t flags) { @@ -1122,6 +1537,7 @@ static ssize_t write_call_cp(struct bt_conn *conn, const struct bt_gatt_attr *at uint8_t status; uint8_t call_index = 0; const bool is_gtbs = inst_is_gtbs(inst); + int err; if (!is_authorized(inst, conn)) { return BT_GATT_ERR(BT_ATT_ERR_AUTHORIZATION); @@ -1131,19 +1547,21 @@ static ssize_t write_call_cp(struct bt_conn *conn, const struct bt_gatt_attr *at return BT_GATT_ERR(BT_ATT_ERR_INVALID_OFFSET); } - if (len < sizeof(ccp->opcode)) { + if (!is_valid_cp_len(len, ccp)) { return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN); } LOG_DBG("Index %u: Processing the %s opcode", inst_index(inst), bt_tbs_opcode_str(ccp->opcode)); + err = k_mutex_lock(&inst->mutex, MUTEX_TIMEOUT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } + switch (ccp->opcode) { case BT_TBS_CALL_OPCODE_ACCEPT: - if (len != sizeof(ccp->accept)) { - return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN); - } - call_index = ccp->accept.call_index; if (is_gtbs) { @@ -1159,10 +1577,6 @@ static ssize_t write_call_cp(struct bt_conn *conn, const struct bt_gatt_attr *at status = accept_call(tbs, &ccp->accept); break; case BT_TBS_CALL_OPCODE_TERMINATE: - if (len != sizeof(ccp->terminate)) { - return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN); - } - call_index = ccp->terminate.call_index; if (is_gtbs) { @@ -1178,10 +1592,6 @@ static ssize_t write_call_cp(struct bt_conn *conn, const struct bt_gatt_attr *at status = terminate_call(tbs, &ccp->terminate, BT_TBS_REASON_CLIENT_TERMINATED); break; case BT_TBS_CALL_OPCODE_HOLD: - if (len != sizeof(ccp->hold)) { - return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN); - } - call_index = ccp->hold.call_index; if (is_gtbs) { @@ -1197,10 +1607,6 @@ static ssize_t write_call_cp(struct bt_conn *conn, const struct bt_gatt_attr *at status = tbs_hold_call(tbs, &ccp->hold); break; case BT_TBS_CALL_OPCODE_RETRIEVE: - if (len != sizeof(ccp->retrieve)) { - return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN); - } - call_index = ccp->retrieve.call_index; if (is_gtbs) { @@ -1218,10 +1624,6 @@ static ssize_t write_call_cp(struct bt_conn *conn, const struct bt_gatt_attr *at case BT_TBS_CALL_OPCODE_ORIGINATE: { const uint16_t uri_len = len - sizeof(ccp->originate); - if (len < sizeof(ccp->originate) + BT_TBS_MIN_URI_LEN) { - return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN); - } - if (is_gtbs) { tbs = lookup_inst_by_uri_scheme(ccp->originate.uri, uri_len); if (tbs == NULL) { @@ -1237,11 +1639,6 @@ static ssize_t write_call_cp(struct bt_conn *conn, const struct bt_gatt_attr *at } case BT_TBS_CALL_OPCODE_JOIN: { const uint16_t call_index_cnt = len - sizeof(ccp->join); - - if (len < sizeof(ccp->join) + 1) { /* at least 1 call index */ - return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN); - } - call_index = ccp->join.call_indexes[0]; if (is_gtbs) { @@ -1284,8 +1681,20 @@ static ssize_t write_call_cp(struct bt_conn *conn, const struct bt_gatt_attr *at call_index = 0; } - if (conn != NULL) { - notify_ccp(conn, attr, call_index, ccp->opcode, status); + if (conn != NULL && bt_gatt_is_subscribed(conn, attr, BT_GATT_CCC_NOTIFY)) { + struct bt_tbs_call_cp_notify *notification = &inst->cp_ntf.notification; + + notification->call_index = call_index; + notification->opcode = ccp->opcode; + notification->status = status; + inst->cp_ntf.pending = true; + inst->cp_ntf.conn_index = bt_conn_index(conn); + + /* We may schedule the same work multiple times, but that is OK as scheduling the + * same work multiple times is a no-op + */ + err = k_work_schedule(&inst->notify_work, K_NO_WAIT); + __ASSERT(err >= 0, "Failed to schedule work: %d", err); } /* else local operation; don't notify */ if (tbs != NULL && status == BT_TBS_RESULT_CODE_SUCCESS) { @@ -1293,6 +1702,10 @@ static ssize_t write_call_cp(struct bt_conn *conn, const struct bt_gatt_attr *at notify_app(conn, tbs, len, ccp, status, call_index); } + /* TODO: Should we unlock the mutex before the app callbacks? */ + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + return len; } @@ -1533,6 +1946,10 @@ static int tbs_inst_init_and_register(struct tbs_inst *inst, struct bt_gatt_serv inst->authorization_required = param->authorization_required; k_work_init_delayable(&inst->reporting_interval_work, signal_interval_timeout); + k_work_init_delayable(&inst->notify_work, notify_work_handler); + + ret = k_mutex_init(&inst->mutex); + __ASSERT(ret == 0, "Failed to initialize mutex"); ret = bt_gatt_service_register(svc); if (ret != 0) { @@ -1734,6 +2151,13 @@ int bt_tbs_accept(uint8_t call_index) int status = -EINVAL; const struct bt_tbs_call_cp_acc ccp = {.call_index = call_index, .opcode = BT_TBS_CALL_OPCODE_ACCEPT}; + int err; + + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } if (inst != NULL) { status = accept_call(inst, &ccp); @@ -1743,6 +2167,9 @@ int bt_tbs_accept(uint8_t call_index) notify_calls(inst); } + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + return status; } @@ -1754,10 +2181,23 @@ int bt_tbs_hold(uint8_t call_index) .opcode = BT_TBS_CALL_OPCODE_HOLD}; if (inst != NULL) { + int err; + + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } + status = tbs_hold_call(inst, &ccp); - } - notify_calls(inst); + if (status == BT_TBS_RESULT_CODE_SUCCESS) { + notify_calls(inst); + } + + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + } return status; } @@ -1770,10 +2210,23 @@ int bt_tbs_retrieve(uint8_t call_index) .opcode = BT_TBS_CALL_OPCODE_RETRIEVE}; if (inst != NULL) { + int err; + + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } + status = retrieve_call(inst, &ccp); - } - notify_calls(inst); + if (status == BT_TBS_RESULT_CODE_SUCCESS) { + notify_calls(inst); + } + + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + } return status; } @@ -1786,22 +2239,37 @@ int bt_tbs_terminate(uint8_t call_index) .opcode = BT_TBS_CALL_OPCODE_TERMINATE}; if (inst != NULL) { + int err; + + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } + status = terminate_call(inst, &ccp, BT_TBS_REASON_SERVER_ENDED_CALL); - } - notify_calls(inst); + if (status == BT_TBS_RESULT_CODE_SUCCESS) { + notify_calls(inst); + } + + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + } return status; } int bt_tbs_originate(uint8_t bearer_index, char *remote_uri, uint8_t *call_index) { - struct tbs_inst *tbs = inst_lookup_index(bearer_index); + struct tbs_inst *inst = inst_lookup_index(bearer_index); uint8_t buf[CONFIG_BT_TBS_MAX_URI_LENGTH + sizeof(struct bt_tbs_call_cp_originate)]; struct bt_tbs_call_cp_originate *ccp = (struct bt_tbs_call_cp_originate *)buf; size_t uri_len; + int err; + int ret; - if (tbs == NULL) { + if (inst == NULL) { LOG_DBG("Could not find TBS instance from index %u", bearer_index); return -EINVAL; } else if (!bt_tbs_valid_uri((uint8_t *)remote_uri, strlen(remote_uri))) { @@ -1809,12 +2277,28 @@ int bt_tbs_originate(uint8_t bearer_index, char *remote_uri, uint8_t *call_index return -EINVAL; } + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } + uri_len = strlen(remote_uri); ccp->opcode = BT_TBS_CALL_OPCODE_ORIGINATE; (void)memcpy(ccp->uri, remote_uri, uri_len); - return originate_call(tbs, ccp, uri_len, call_index); + ret = originate_call(inst, ccp, uri_len, call_index); + + /* In the case that we are not connected to any TBS clients, we won't notify and we can + * attempt to change state from dialing to alerting immediately + */ + (void)try_change_dialing_call_to_alerting(inst); + + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + + return ret; } int bt_tbs_join(uint8_t call_index_cnt, uint8_t *call_indexes) @@ -1831,11 +2315,26 @@ int bt_tbs_join(uint8_t call_index_cnt, uint8_t *call_indexes) } if (inst != NULL) { + int err; + + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } + ccp->opcode = BT_TBS_CALL_OPCODE_JOIN; (void)memcpy(ccp->call_indexes, call_indexes, MIN(call_index_cnt, CONFIG_BT_TBS_MAX_CALLS)); status = join_calls(inst, ccp, call_index_cnt); + + if (status == BT_TBS_RESULT_CODE_SUCCESS) { + notify_calls(inst); + } + + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); } return status; @@ -1845,6 +2344,8 @@ int bt_tbs_remote_answer(uint8_t call_index) { struct tbs_inst *inst = lookup_inst_by_call_index(call_index); struct bt_tbs_call *call; + int err; + int ret; if (inst == NULL) { return BT_TBS_RESULT_CODE_INVALID_CALL_INDEX; @@ -1856,13 +2357,26 @@ int bt_tbs_remote_answer(uint8_t call_index) return BT_TBS_RESULT_CODE_INVALID_CALL_INDEX; } + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } + if (call->state == BT_TBS_CALL_STATE_ALERTING) { call->state = BT_TBS_CALL_STATE_ACTIVE; notify_calls(inst); - return BT_TBS_RESULT_CODE_SUCCESS; + ret = BT_TBS_RESULT_CODE_SUCCESS; } else { - return BT_TBS_RESULT_CODE_STATE_MISMATCH; + LOG_DBG("Call with index %u Invalid state %s", call_index, + bt_tbs_state_str(call->state)); + ret = BT_TBS_RESULT_CODE_STATE_MISMATCH; } + + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + + return ret; } int bt_tbs_remote_hold(uint8_t call_index) @@ -1870,6 +2384,7 @@ int bt_tbs_remote_hold(uint8_t call_index) struct tbs_inst *inst = lookup_inst_by_call_index(call_index); struct bt_tbs_call *call; uint8_t status; + int err; if (inst == NULL) { return BT_TBS_RESULT_CODE_INVALID_CALL_INDEX; @@ -1881,6 +2396,12 @@ int bt_tbs_remote_hold(uint8_t call_index) return BT_TBS_RESULT_CODE_INVALID_CALL_INDEX; } + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } + if (call->state == BT_TBS_CALL_STATE_ACTIVE) { call->state = BT_TBS_CALL_STATE_REMOTELY_HELD; status = BT_TBS_RESULT_CODE_SUCCESS; @@ -1895,6 +2416,9 @@ int bt_tbs_remote_hold(uint8_t call_index) notify_calls(inst); } + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + return status; } @@ -1903,6 +2427,7 @@ int bt_tbs_remote_retrieve(uint8_t call_index) struct tbs_inst *inst = lookup_inst_by_call_index(call_index); struct bt_tbs_call *call; int status; + int err; if (inst == NULL) { return BT_TBS_RESULT_CODE_INVALID_CALL_INDEX; @@ -1914,6 +2439,11 @@ int bt_tbs_remote_retrieve(uint8_t call_index) return BT_TBS_RESULT_CODE_INVALID_CALL_INDEX; } + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } if (call->state == BT_TBS_CALL_STATE_REMOTELY_HELD) { call->state = BT_TBS_CALL_STATE_ACTIVE; status = BT_TBS_RESULT_CODE_SUCCESS; @@ -1928,6 +2458,9 @@ int bt_tbs_remote_retrieve(uint8_t call_index) notify_calls(inst); } + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + return status; } @@ -1939,14 +2472,53 @@ int bt_tbs_remote_terminate(uint8_t call_index) .opcode = BT_TBS_CALL_OPCODE_TERMINATE}; if (inst != NULL) { + int err; + + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } status = terminate_call(inst, &ccp, BT_TBS_REASON_REMOTE_ENDED_CALL); - } - notify_calls(inst); + if (status == BT_TBS_RESULT_CODE_SUCCESS) { + notify_calls(inst); + } + + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + } return status; } +static void set_incoming_call_target_bearer_uri_changed_cb(struct tbs_flags *flags) +{ + if (flags->incoming_call_target_bearer_uri_changed) { + LOG_DBG("pending notification replaced"); + } + + flags->incoming_call_target_bearer_uri_changed = true; +} + +static void set_incoming_call_changed_cb(struct tbs_flags *flags) +{ + if (flags->incoming_call_changed) { + LOG_DBG("pending notification replaced"); + } + + flags->incoming_call_changed = true; +} + +static void set_call_friendly_name_changed_cb(struct tbs_flags *flags) +{ + if (flags->call_friendly_name_changed) { + LOG_DBG("pending notification replaced"); + } + + flags->call_friendly_name_changed = true; +} + static void tbs_inst_remote_incoming(struct tbs_inst *inst, const char *to, const char *from, const char *friendly_name, const struct bt_tbs_call *call) { @@ -1966,23 +2538,19 @@ static void tbs_inst_remote_incoming(struct tbs_inst *inst, const char *to, cons inst->incoming_uri.call_index = call->index; (void)utf8_lcpy(inst->incoming_uri.uri, to, sizeof(inst->incoming_uri.uri)); - bt_gatt_notify_uuid(NULL, BT_UUID_TBS_INCOMING_URI, inst->attrs, &inst->incoming_uri, - local_uri_ind_len); - - bt_gatt_notify_uuid(NULL, BT_UUID_TBS_INCOMING_CALL, inst->attrs, &inst->in_call, - remote_uri_ind_len); + set_value_changed(inst, set_incoming_call_target_bearer_uri_changed_cb, + BT_UUID_TBS_INCOMING_URI); + set_value_changed(inst, set_incoming_call_changed_cb, BT_UUID_TBS_INCOMING_CALL); if (friendly_name) { inst->friendly_name.call_index = call->index; utf8_lcpy(inst->friendly_name.uri, friendly_name, sizeof(inst->friendly_name.uri)); friend_name_ind_len = strlen(from) + 1; - - bt_gatt_notify_uuid(NULL, BT_UUID_TBS_FRIENDLY_NAME, inst->attrs, - &inst->friendly_name, friend_name_ind_len); } else { inst->friendly_name.call_index = BT_TBS_FREE_CALL_INDEX; - bt_gatt_notify_uuid(NULL, BT_UUID_TBS_FRIENDLY_NAME, inst->attrs, NULL, 0); } + + set_value_changed(inst, set_call_friendly_name_changed_cb, BT_UUID_TBS_FRIENDLY_NAME); } int bt_tbs_remote_incoming(uint8_t bearer_index, const char *to, const char *from, @@ -1990,6 +2558,7 @@ int bt_tbs_remote_incoming(uint8_t bearer_index, const char *to, const char *fro { struct tbs_inst *inst = inst_lookup_index(bearer_index); struct bt_tbs_call *call = NULL; + int err; if (inst == NULL) { LOG_DBG("Could not find TBS instance from index %u", bearer_index); @@ -2007,6 +2576,11 @@ int bt_tbs_remote_incoming(uint8_t bearer_index, const char *to, const char *fro return -ENOMEM; } + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } BT_TBS_CALL_FLAG_SET_INCOMING(call->flags); /* Notify TBS*/ @@ -2021,15 +2595,24 @@ int bt_tbs_remote_incoming(uint8_t bearer_index, const char *to, const char *fro notify_calls(inst); + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + LOG_DBG("New call with call index %u", call->index); return call->index; } +static void set_bearer_provider_name_changed_cb(struct tbs_flags *flags) +{ + flags->bearer_provider_name_changed = true; +} + int bt_tbs_set_bearer_provider_name(uint8_t bearer_index, const char *name) { struct tbs_inst *inst = inst_lookup_index(bearer_index); const size_t len = strlen(name); + int err; if (len >= CONFIG_BT_TBS_MAX_PROVIDER_NAME_LENGTH || len == 0) { return -EINVAL; @@ -2041,16 +2624,31 @@ int bt_tbs_set_bearer_provider_name(uint8_t bearer_index, const char *name) return 0; } + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } + (void)utf8_lcpy(inst->provider_name, name, sizeof(inst->provider_name)); - bt_gatt_notify_uuid(NULL, BT_UUID_TBS_PROVIDER_NAME, inst->attrs, inst->provider_name, - strlen(inst->provider_name)); + set_value_changed(inst, set_bearer_provider_name_changed_cb, BT_UUID_TBS_PROVIDER_NAME); + + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + return 0; } +static void set_bearer_technology_changed_cb(struct tbs_flags *flags) +{ + flags->bearer_technology_changed = true; +} + int bt_tbs_set_bearer_technology(uint8_t bearer_index, uint8_t new_technology) { struct tbs_inst *inst = inst_lookup_index(bearer_index); + int err; if (new_technology < BT_TBS_TECHNOLOGY_3G || new_technology > BT_TBS_TECHNOLOGY_WCDMA) { return -EINVAL; @@ -2062,10 +2660,18 @@ int bt_tbs_set_bearer_technology(uint8_t bearer_index, uint8_t new_technology) return 0; } + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } + inst->technology = new_technology; - bt_gatt_notify_uuid(NULL, BT_UUID_TBS_TECHNOLOGY, inst->attrs, &inst->technology, - sizeof(inst->technology)); + set_value_changed(inst, set_bearer_technology_changed_cb, BT_UUID_TBS_TECHNOLOGY); + + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); return 0; } @@ -2099,9 +2705,15 @@ int bt_tbs_set_signal_strength(uint8_t bearer_index, uint8_t new_signal_strength return 0; } +static void set_status_flags_changed_cb(struct tbs_flags *flags) +{ + flags->status_flags_changed = true; +} + int bt_tbs_set_status_flags(uint8_t bearer_index, uint16_t status_flags) { struct tbs_inst *inst = inst_lookup_index(bearer_index); + int err; if (!BT_TBS_VALID_STATUS_FLAGS(status_flags)) { return -EINVAL; @@ -2113,18 +2725,33 @@ int bt_tbs_set_status_flags(uint8_t bearer_index, uint16_t status_flags) return 0; } + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } + inst->status_flags = status_flags; - bt_gatt_notify_uuid(NULL, BT_UUID_TBS_STATUS_FLAGS, inst->attrs, &status_flags, - sizeof(status_flags)); + set_value_changed(inst, set_status_flags_changed_cb, BT_UUID_TBS_STATUS_FLAGS); + + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + return 0; } +static void set_bearer_uri_schemes_supported_list_changed_cb(struct tbs_flags *flags) +{ + flags->bearer_uri_schemes_supported_list_changed = true; +} + int bt_tbs_set_uri_scheme_list(uint8_t bearer_index, const char **uri_list, uint8_t uri_count) { char uri_scheme_list[CONFIG_BT_TBS_MAX_SCHEME_LIST_LENGTH]; size_t len = 0; struct tbs_inst *inst; + int err; NET_BUF_SIMPLE_DEFINE(uri_scheme_buf, READ_BUF_SIZE); @@ -2159,13 +2786,19 @@ int bt_tbs_set_uri_scheme_list(uint8_t bearer_index, const char **uri_list, uint return 0; } + err = k_mutex_lock(&inst->mutex, K_NO_WAIT); + if (err != 0) { + LOG_DBG("Failed to lock mutex"); + return -EBUSY; + } + /* Store final result */ (void)utf8_lcpy(inst->uri_scheme_list, uri_scheme_list, sizeof(inst->uri_scheme_list)); LOG_DBG("TBS instance %u uri prefix list is now %s", bearer_index, inst->uri_scheme_list); - bt_gatt_notify_uuid(NULL, BT_UUID_TBS_URI_LIST, inst->attrs, &inst->uri_scheme_list, - strlen(inst->uri_scheme_list)); + set_value_changed(inst, set_bearer_uri_schemes_supported_list_changed_cb, + BT_UUID_TBS_URI_LIST); if (!inst_is_gtbs(inst)) { /* If the instance is different than the GTBS notify on the GTBS instance as well */ @@ -2188,9 +2821,13 @@ int bt_tbs_set_uri_scheme_list(uint8_t bearer_index, const char **uri_list, uint LOG_DBG("GTBS: URI scheme %.*s", uri_scheme_buf.len, uri_scheme_buf.data); - bt_gatt_notify_uuid(NULL, BT_UUID_TBS_URI_LIST, gtbs_inst.attrs, - uri_scheme_buf.data, uri_scheme_buf.len); + set_value_changed(>bs_inst, set_bearer_uri_schemes_supported_list_changed_cb, + BT_UUID_TBS_URI_LIST); } + + err = k_mutex_unlock(&inst->mutex); + __ASSERT(err == 0, "Failed to unlock mutex: %d", err); + return 0; } diff --git a/tests/bsim/bluetooth/audio/src/tbs_test.c b/tests/bsim/bluetooth/audio/src/tbs_test.c index ea96241db136c8..e56f27a50fee7d 100644 --- a/tests/bsim/bluetooth/audio/src/tbs_test.c +++ b/tests/bsim/bluetooth/audio/src/tbs_test.c @@ -194,7 +194,7 @@ static int test_answer_terminate(void) printk("Answering call\n"); err = bt_tbs_remote_answer(g_call_index); if (err != BT_TBS_RESULT_CODE_SUCCESS) { - FAIL("Could not accept call: %d\n", err); + FAIL("Could not remote answer: %d\n", err); return err; } @@ -223,7 +223,7 @@ static int test_hold_retrieve(void) err = bt_tbs_remote_answer(g_call_index); if (err != BT_TBS_RESULT_CODE_SUCCESS) { - FAIL("Could not accept call: %d\n", err); + FAIL("Could not remote answer: %d\n", err); return err; }