diff --git a/subsys/bluetooth/audio/aics.c b/subsys/bluetooth/audio/aics.c index 9c87f718adc6..4e31fea35935 100644 --- a/subsys/bluetooth/audio/aics.c +++ b/subsys/bluetooth/audio/aics.c @@ -214,6 +214,8 @@ static void notify(struct bt_aics *inst, enum bt_aics_notify notify, const struc notify_work_reschedule(inst, notify, K_USEC(BT_AUDIO_NOTIFY_RETRY_DELAY_US)); } else if (err < 0 && err != -ENOTCONN) { LOG_ERR("Notify %s err %d", aics_notify_str(notify), err); + } else { + /* Notification sent successfully */ } } @@ -246,99 +248,160 @@ static void value_changed(struct bt_aics *inst, enum bt_aics_notify notify) #define value_changed(...) #endif /* CONFIG_BT_AICS */ -static ssize_t write_aics_control(struct bt_conn *conn, - const struct bt_gatt_attr *attr, - const void *buf, uint16_t len, - uint16_t offset, uint8_t flags) +static uint8_t valid_control_point_write(uint16_t len, uint16_t offset, + const struct bt_aics_gain_control *cp, + uint8_t change_counter) { - struct bt_aics *inst = BT_AUDIO_CHRC_USER_DATA(attr); - const struct bt_aics_gain_control *cp = buf; - bool notify = false; - - if (offset) { - return BT_GATT_ERR(BT_ATT_ERR_INVALID_OFFSET); + if (offset != 0) { + LOG_DBG("Invalid offset: %u", offset); + return BT_ATT_ERR_INVALID_OFFSET; } - if (!len || !buf) { - return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN); + if (len == 0 || cp == NULL) { + LOG_DBG("Invalid length (%u) or NULL data (%p)", len, cp); + return BT_ATT_ERR_INVALID_ATTRIBUTE_LEN; } /* Check opcode before length */ if (!VALID_AICS_OPCODE(cp->cp.opcode)) { LOG_DBG("Invalid opcode %u", cp->cp.opcode); - return BT_GATT_ERR(BT_AICS_ERR_OP_NOT_SUPPORTED); + return BT_AICS_ERR_OP_NOT_SUPPORTED; } if ((len < AICS_CP_LEN) || (len == AICS_CP_SET_GAIN_LEN && cp->cp.opcode != BT_AICS_OPCODE_SET_GAIN) || (len > AICS_CP_SET_GAIN_LEN)) { - return BT_GATT_ERR(BT_ATT_ERR_INVALID_ATTRIBUTE_LEN); + LOG_DBG("Invalid length: %u", len); + return BT_ATT_ERR_INVALID_ATTRIBUTE_LEN; } LOG_DBG("Opcode %u, counter %u", cp->cp.opcode, cp->cp.counter); - if (cp->cp.counter != inst->srv.state.change_counter) { - return BT_GATT_ERR(BT_AICS_ERR_INVALID_COUNTER); + if (cp->cp.counter != change_counter) { + LOG_DBG("Invalid counter: %u != %u", cp->cp.counter, change_counter); + return BT_AICS_ERR_INVALID_COUNTER; + } + + return BT_ATT_ERR_SUCCESS; +} + +static uint8_t handle_set_gain_op(struct bt_aics *inst, const struct bt_aics_gain_control *cp, + bool *state_change) +{ + LOG_DBG("Set gain %d", cp->gain_setting); + if (cp->gain_setting < inst->srv.gain_settings.minimum || + cp->gain_setting > inst->srv.gain_settings.maximum) { + return BT_AICS_ERR_OUT_OF_RANGE; + } + + if (BT_AICS_INPUT_MODE_SETTABLE(inst->srv.state.gain_mode) && + inst->srv.state.gain != cp->gain_setting) { + inst->srv.state.gain = cp->gain_setting; + *state_change = true; + } + + return BT_ATT_ERR_SUCCESS; +} + +static uint8_t handle_unmute_op(struct bt_aics *inst, bool *state_change) +{ + LOG_DBG("Unmute"); + if (inst->srv.state.mute == BT_AICS_STATE_MUTE_DISABLED) { + return BT_AICS_ERR_MUTE_DISABLED; + } + if (inst->srv.state.mute != BT_AICS_STATE_UNMUTED) { + inst->srv.state.mute = BT_AICS_STATE_UNMUTED; + *state_change = true; + } + + return BT_ATT_ERR_SUCCESS; +} + +static uint8_t handle_mute_op(struct bt_aics *inst, bool *state_change) +{ + LOG_DBG("Mute"); + + if (inst->srv.state.mute == BT_AICS_STATE_MUTE_DISABLED) { + return BT_AICS_ERR_MUTE_DISABLED; + } + + if (inst->srv.state.mute != BT_AICS_STATE_MUTED) { + inst->srv.state.mute = BT_AICS_STATE_MUTED; + *state_change = true; + } + + return BT_ATT_ERR_SUCCESS; +} + +static uint8_t handle_set_manual_mode_op(struct bt_aics *inst, bool *state_change) +{ + LOG_DBG("Set manual mode"); + + if (BT_AICS_INPUT_MODE_IMMUTABLE(inst->srv.state.gain_mode)) { + return BT_AICS_ERR_GAIN_MODE_NOT_ALLOWED; + } + + if (inst->srv.state.gain_mode != BT_AICS_MODE_MANUAL) { + inst->srv.state.gain_mode = BT_AICS_MODE_MANUAL; + *state_change = true; + } + + return BT_ATT_ERR_SUCCESS; +} +static uint8_t handle_set_automatic_mode_op(struct bt_aics *inst, bool *state_change) +{ + LOG_DBG("Set automatic mode"); + + if (BT_AICS_INPUT_MODE_IMMUTABLE(inst->srv.state.gain_mode)) { + return BT_AICS_ERR_GAIN_MODE_NOT_ALLOWED; + } + + if (inst->srv.state.gain_mode != BT_AICS_MODE_AUTO) { + inst->srv.state.gain_mode = BT_AICS_MODE_AUTO; + *state_change = true; + } + + return BT_ATT_ERR_SUCCESS; +} + +static ssize_t write_aics_control(struct bt_conn *conn, const struct bt_gatt_attr *attr, + const void *buf, uint16_t len, uint16_t offset, uint8_t flags) +{ + struct bt_aics *inst = BT_AUDIO_CHRC_USER_DATA(attr); + const struct bt_aics_gain_control *cp = buf; + bool state_change = false; + int ret; + + ret = valid_control_point_write(len, offset, cp, inst->srv.state.change_counter); + if (ret != BT_ATT_ERR_SUCCESS) { + return BT_GATT_ERR(ret); } switch (cp->cp.opcode) { case BT_AICS_OPCODE_SET_GAIN: - LOG_DBG("Set gain %d", cp->gain_setting); - if (cp->gain_setting < inst->srv.gain_settings.minimum || - cp->gain_setting > inst->srv.gain_settings.maximum) { - return BT_GATT_ERR(BT_AICS_ERR_OUT_OF_RANGE); - } - if (BT_AICS_INPUT_MODE_SETTABLE(inst->srv.state.gain_mode) && - inst->srv.state.gain != cp->gain_setting) { - inst->srv.state.gain = cp->gain_setting; - notify = true; - } + ret = handle_set_gain_op(inst, cp, &state_change); break; case BT_AICS_OPCODE_UNMUTE: - LOG_DBG("Unmute"); - if (inst->srv.state.mute == BT_AICS_STATE_MUTE_DISABLED) { - return BT_GATT_ERR(BT_AICS_ERR_MUTE_DISABLED); - } - if (inst->srv.state.mute != BT_AICS_STATE_UNMUTED) { - inst->srv.state.mute = BT_AICS_STATE_UNMUTED; - notify = true; - } + ret = handle_unmute_op(inst, &state_change); break; case BT_AICS_OPCODE_MUTE: - LOG_DBG("Mute"); - if (inst->srv.state.mute == BT_AICS_STATE_MUTE_DISABLED) { - return BT_GATT_ERR(BT_AICS_ERR_MUTE_DISABLED); - } - if (inst->srv.state.mute != BT_AICS_STATE_MUTED) { - inst->srv.state.mute = BT_AICS_STATE_MUTED; - notify = true; - } + ret = handle_mute_op(inst, &state_change); break; case BT_AICS_OPCODE_SET_MANUAL: - LOG_DBG("Set manual mode"); - if (BT_AICS_INPUT_MODE_IMMUTABLE(inst->srv.state.gain_mode)) { - return BT_GATT_ERR(BT_AICS_ERR_GAIN_MODE_NOT_ALLOWED); - } - if (inst->srv.state.gain_mode != BT_AICS_MODE_MANUAL) { - inst->srv.state.gain_mode = BT_AICS_MODE_MANUAL; - notify = true; - } + ret = handle_set_manual_mode_op(inst, &state_change); break; case BT_AICS_OPCODE_SET_AUTO: - LOG_DBG("Set automatic mode"); - if (BT_AICS_INPUT_MODE_IMMUTABLE(inst->srv.state.gain_mode)) { - return BT_GATT_ERR(BT_AICS_ERR_GAIN_MODE_NOT_ALLOWED); - } - if (inst->srv.state.gain_mode != BT_AICS_MODE_AUTO) { - inst->srv.state.gain_mode = BT_AICS_MODE_AUTO; - notify = true; - } + ret = handle_set_automatic_mode_op(inst, &state_change); break; default: - return BT_GATT_ERR(BT_AICS_ERR_OP_NOT_SUPPORTED); + ret = BT_AICS_ERR_OP_NOT_SUPPORTED; + } + + if (ret != BT_ATT_ERR_SUCCESS) { + return BT_GATT_ERR(ret); } - if (notify) { - inst->srv.state.change_counter++; + if (state_change) { + inst->srv.state.change_counter++; /* May overflow which is OK */ LOG_DBG("New state: gain %d, mute %u, gain_mode %u, counter %u", inst->srv.state.gain, inst->srv.state.mute, inst->srv.state.gain_mode, diff --git a/subsys/bluetooth/audio/aics_client.c b/subsys/bluetooth/audio/aics_client.c index bffbed944d38..3d999305ef52 100644 --- a/subsys/bluetooth/audio/aics_client.c +++ b/subsys/bluetooth/audio/aics_client.c @@ -32,6 +32,7 @@ #include #include "aics_internal.h" +#include "common/bt_str.h" LOG_MODULE_REGISTER(bt_aics_client, CONFIG_BT_AICS_CLIENT_LOG_LEVEL); @@ -60,8 +61,8 @@ uint8_t aics_client_notify_handler(struct bt_conn *conn, struct bt_gatt_subscrib { uint16_t handle = params->value_handle; struct bt_aics *inst; - struct bt_aics_state *state; - uint8_t *status; + const struct bt_aics_state *state; + const uint8_t *status; if (conn == NULL) { return BT_GATT_ITER_CONTINUE; @@ -80,7 +81,7 @@ uint8_t aics_client_notify_handler(struct bt_conn *conn, struct bt_gatt_subscrib if (handle == inst->cli.state_handle) { if (length == sizeof(*state)) { - state = (struct bt_aics_state *)data; + state = (const struct bt_aics_state *)data; LOG_DBG("Inst %p: Gain %d, mute %u, gain_mode %u, counter %u", inst, state->gain, state->mute, state->gain_mode, state->change_counter); @@ -94,7 +95,7 @@ uint8_t aics_client_notify_handler(struct bt_conn *conn, struct bt_gatt_subscrib } } else if (handle == inst->cli.status_handle) { if (length == sizeof(*status)) { - status = (uint8_t *)data; + status = (const uint8_t *)data; LOG_DBG("Inst %p: Status %u", inst, *status); if (inst->cli.cb && inst->cli.cb->status) { inst->cli.cb->status(inst, 0, *status); @@ -116,6 +117,8 @@ uint8_t aics_client_notify_handler(struct bt_conn *conn, struct bt_gatt_subscrib if (inst->cli.cb && inst->cli.cb->description) { inst->cli.cb->description(inst, 0, desc); } + } else { + LOG_DBG("Receive notification on unexpected handle 0x%04X", handle); } return BT_GATT_ITER_CONTINUE; @@ -127,7 +130,7 @@ static uint8_t aics_client_read_state_cb(struct bt_conn *conn, uint8_t err, { int cb_err = err; struct bt_aics *inst = lookup_aics_by_handle(conn, params->single.handle); - struct bt_aics_state *state = (struct bt_aics_state *)data; + const struct bt_aics_state *state = (const struct bt_aics_state *)data; memset(params, 0, sizeof(*params)); @@ -176,7 +179,8 @@ static uint8_t aics_client_read_gain_settings_cb(struct bt_conn *conn, uint8_t e { int cb_err = err; struct bt_aics *inst = lookup_aics_by_handle(conn, params->single.handle); - struct bt_aics_gain_settings *gain_settings = (struct bt_aics_gain_settings *)data; + const struct bt_aics_gain_settings *gain_settings = + (const struct bt_aics_gain_settings *)data; memset(params, 0, sizeof(*params)); @@ -223,7 +227,7 @@ static uint8_t aics_client_read_type_cb(struct bt_conn *conn, uint8_t err, const void *data, uint16_t length) { int cb_err = err; - uint8_t *type = (uint8_t *)data; + const uint8_t *type = (const uint8_t *)data; struct bt_aics *inst = lookup_aics_by_handle(conn, params->single.handle); memset(params, 0, sizeof(*params)); @@ -268,7 +272,7 @@ static uint8_t aics_client_read_status_cb(struct bt_conn *conn, uint8_t err, const void *data, uint16_t length) { int cb_err = err; - uint8_t *status = (uint8_t *)data; + const uint8_t *status = (const uint8_t *)data; struct bt_aics *inst = lookup_aics_by_handle(conn, params->single.handle); memset(params, 0, sizeof(*params)); @@ -352,7 +356,7 @@ static uint8_t internal_read_state_cb(struct bt_conn *conn, uint8_t err, { int cb_err = err; struct bt_aics *inst = lookup_aics_by_handle(conn, params->single.handle); - struct bt_aics_state *state = (struct bt_aics_state *)data; + const struct bt_aics_state *state = (const struct bt_aics_state *)data; memset(params, 0, sizeof(*params)); @@ -389,6 +393,9 @@ static uint8_t internal_read_state_cb(struct bt_conn *conn, uint8_t err, LOG_DBG("Invalid length %u (expected %zu)", length, sizeof(*state)); cb_err = BT_ATT_ERR_UNLIKELY; } + } else { + /* Since we return BT_GATT_ITER_STOP this should never happen */ + __ASSERT(false, "Unexpected NULL data without error"); } if (cb_err) { @@ -439,6 +446,8 @@ static void aics_client_write_aics_cp_cb(struct bt_conn *conn, uint8_t err, /* Wait for read callback */ return; } + } else { + /* Write failed, fallthrough and notify application */ } atomic_clear_bit(inst->cli.flags, BT_AICS_CLIENT_FLAG_CP_RETRIED); @@ -550,13 +559,74 @@ static bool valid_inst_discovered(struct bt_aics *inst) inst->cli.desc_handle; } +static int store_attr_handle_and_subscribe(struct bt_aics_client *client_inst, struct bt_conn *conn, + const struct bt_gatt_chrc *chrc) +{ + struct bt_gatt_subscribe_params *sub_params = NULL; + + if (bt_uuid_cmp(chrc->uuid, BT_UUID_AICS_STATE) == 0U) { + LOG_DBG("Audio Input state"); + client_inst->state_handle = chrc->value_handle; + sub_params = &client_inst->state_sub_params; + } else if (bt_uuid_cmp(chrc->uuid, BT_UUID_AICS_GAIN_SETTINGS) == 0U) { + LOG_DBG("Gain settings"); + client_inst->gain_handle = chrc->value_handle; + } else if (bt_uuid_cmp(chrc->uuid, BT_UUID_AICS_INPUT_TYPE) == 0U) { + LOG_DBG("Input type"); + client_inst->type_handle = chrc->value_handle; + } else if (bt_uuid_cmp(chrc->uuid, BT_UUID_AICS_INPUT_STATUS) == 0U) { + LOG_DBG("Input status"); + client_inst->status_handle = chrc->value_handle; + sub_params = &client_inst->status_sub_params; + } else if (bt_uuid_cmp(chrc->uuid, BT_UUID_AICS_CONTROL) == 0U) { + LOG_DBG("Control point"); + client_inst->control_handle = chrc->value_handle; + } else if (bt_uuid_cmp(chrc->uuid, BT_UUID_AICS_DESCRIPTION) == 0U) { + LOG_DBG("Description"); + client_inst->desc_handle = chrc->value_handle; + if ((chrc->properties & BT_GATT_CHRC_NOTIFY) != 0U) { + sub_params = &client_inst->desc_sub_params; + } + + if ((chrc->properties & BT_GATT_CHRC_WRITE_WITHOUT_RESP) != 0U) { + atomic_set_bit(client_inst->flags, BT_AICS_CLIENT_FLAG_DESC_WRITABLE); + } + } else { + __ASSERT(false, "Unexpected UUID %s discovered", bt_uuid_str(chrc->uuid)); + } + + if (sub_params != NULL) { + int err; + + sub_params->value = BT_GATT_CCC_NOTIFY; + sub_params->value_handle = chrc->value_handle; + /* + * TODO: Don't assume that CCC is at value handle + 1; + * do proper discovery; + */ + sub_params->ccc_handle = chrc->value_handle + 1; + sub_params->notify = aics_client_notify_handler; + atomic_set_bit(sub_params->flags, BT_GATT_SUBSCRIBE_FLAG_VOLATILE); + + err = bt_gatt_subscribe(conn, sub_params); + if (err != 0 && err != -EALREADY) { + LOG_ERR("Failed to subscribe: %d", err); + + return err; + } + } + + return 0; +} + static uint8_t aics_discover_func(struct bt_conn *conn, const struct bt_gatt_attr *attr, struct bt_gatt_discover_params *params) { - struct bt_aics_client *client_inst = CONTAINER_OF(params, - struct bt_aics_client, - discover_params); + struct bt_aics_client *client_inst = + CONTAINER_OF(params, struct bt_aics_client, discover_params); struct bt_aics *inst = CONTAINER_OF(client_inst, struct bt_aics, cli); + const struct bt_gatt_chrc *chrc; + int err; if (!attr) { LOG_DBG("Discovery complete for AICS %p", inst); @@ -566,7 +636,7 @@ static uint8_t aics_discover_func(struct bt_conn *conn, const struct bt_gatt_att atomic_clear_bit(inst->cli.flags, BT_AICS_CLIENT_FLAG_BUSY); if (inst->cli.cb && inst->cli.cb->discover) { - int err = valid_inst_discovered(inst) ? 0 : -ENOENT; + err = valid_inst_discovered(inst) ? 0 : -ENOENT; inst->cli.cb->discover(inst, err); } @@ -576,69 +646,21 @@ static uint8_t aics_discover_func(struct bt_conn *conn, const struct bt_gatt_att LOG_DBG("[ATTRIBUTE] handle 0x%04X", attr->handle); - if (params->type == BT_GATT_DISCOVER_CHARACTERISTIC) { - struct bt_gatt_subscribe_params *sub_params = NULL; - struct bt_gatt_chrc *chrc; + __ASSERT_NO_MSG(params->type == BT_GATT_DISCOVER_CHARACTERISTIC); - chrc = (struct bt_gatt_chrc *)attr->user_data; - if (inst->cli.start_handle == 0) { - inst->cli.start_handle = chrc->value_handle; - } - inst->cli.end_handle = chrc->value_handle; - - if (!bt_uuid_cmp(chrc->uuid, BT_UUID_AICS_STATE)) { - LOG_DBG("Audio Input state"); - inst->cli.state_handle = chrc->value_handle; - sub_params = &inst->cli.state_sub_params; - } else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_AICS_GAIN_SETTINGS)) { - LOG_DBG("Gain settings"); - inst->cli.gain_handle = chrc->value_handle; - } else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_AICS_INPUT_TYPE)) { - LOG_DBG("Input type"); - inst->cli.type_handle = chrc->value_handle; - } else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_AICS_INPUT_STATUS)) { - LOG_DBG("Input status"); - inst->cli.status_handle = chrc->value_handle; - sub_params = &inst->cli.status_sub_params; - } else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_AICS_CONTROL)) { - LOG_DBG("Control point"); - inst->cli.control_handle = chrc->value_handle; - } else if (!bt_uuid_cmp(chrc->uuid, BT_UUID_AICS_DESCRIPTION)) { - LOG_DBG("Description"); - inst->cli.desc_handle = chrc->value_handle; - if (chrc->properties & BT_GATT_CHRC_NOTIFY) { - sub_params = &inst->cli.desc_sub_params; - } + chrc = (const struct bt_gatt_chrc *)attr->user_data; + if (inst->cli.start_handle == 0U) { /* if start handle is unset */ + inst->cli.start_handle = chrc->value_handle; + } + inst->cli.end_handle = chrc->value_handle; - if (chrc->properties & BT_GATT_CHRC_WRITE_WITHOUT_RESP) { - atomic_set_bit(inst->cli.flags, BT_AICS_CLIENT_FLAG_DESC_WRITABLE); - } + err = store_attr_handle_and_subscribe(client_inst, conn, chrc); + if (err != 0) { + if (client_inst->cb && client_inst->cb->discover) { + client_inst->cb->discover(inst, err); } - if (sub_params) { - int err; - - sub_params->value = BT_GATT_CCC_NOTIFY; - sub_params->value_handle = chrc->value_handle; - /* - * TODO: Don't assume that CCC is at handle + 2; - * do proper discovery; - */ - sub_params->ccc_handle = attr->handle + 2; - sub_params->notify = aics_client_notify_handler; - atomic_set_bit(sub_params->flags, BT_GATT_SUBSCRIBE_FLAG_VOLATILE); - - err = bt_gatt_subscribe(conn, sub_params); - if (err != 0 && err != -EALREADY) { - LOG_ERR("Failed to subscribe: %d", err); - - if (inst->cli.cb && inst->cli.cb->discover) { - inst->cli.cb->discover(inst, err); - } - - return BT_GATT_ITER_STOP; - } - } + return BT_GATT_ITER_STOP; } return BT_GATT_ITER_CONTINUE;