Skip to content

Commit

Permalink
Bluetooth: has: Fix preset list notifications after reconnection
Browse files Browse the repository at this point in the history 
This fixes sending proper Preset List notifications after
reconnection. The issue was observed when the last preset
known to the client has been removed.
As we do not hold the information about the deleted presets,
we need to use Generic Update procedure to:
 1. Notify the presets that have been removed in range
    (PrevIndex = current_preset_last, Index=previous_preset_last)
 2. Notify deletion of preset Index=previous_preset_last.

Signed-off-by: Mariusz Skamra <mariusz.skamra@codecoup.pl>
  • Loading branch information
@MariuszSkamra
MariuszSkamra committed Oct 12, 2023
1 parent f2bb600 commit 6efa790
Showing 1 changed file with 160 additions and 31 deletions.
191 changes: 160 additions & 31 deletions subsys/bluetooth/audio/has.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ LOG_MODULE_REGISTER(bt_has, CONFIG_BT_HAS_LOG_LEVEL);
#define FEATURE_IND_PRESETS_UNCHANGED(_new_value) \
!BITS_CHANGED(_new_value, ((has.features & BT_HAS_FEAT_INDEPENDENT_PRESETS) != 0 ? 1 : 0))
#define NOTIFY_AFTER_REBOOT \
(BIT(NOTIFY_ACTIVE_INDEX) | BIT(NOTIFY_CURRENT_PRESET_LIST) | BIT(NOTIFY_FEATURES))
(BIT(NOTIFY_ACTIVE_INDEX) | BIT(NOTIFY_PRESET_LIST) | BIT(NOTIFY_FEATURES))

static struct bt_has has;

Expand All @@ -53,7 +53,9 @@ static void active_preset_index_cfg_changed(const struct bt_gatt_attr *attr, uin
struct has_client;

static int read_preset_response(struct has_client *client);
static int list_current_presets(struct has_client *client);
static int preset_list_changed(struct has_client *client);
static int preset_list_changed_generic_update_tail(struct has_client *client);
static int preset_list_changed_record_deleted_last(struct has_client *client);
static ssize_t write_control_point(struct bt_conn *conn, const struct bt_gatt_attr *attr,
const void *data, uint16_t len, uint16_t offset, uint8_t flags);

Expand Down Expand Up @@ -164,8 +166,12 @@ static void notify_work_handler(struct k_work *work);
enum has_notify {
NOTIFY_ACTIVE_INDEX,
NOTIFY_READ_PRESET_RESPONSE,
NOTIFY_CURRENT_PRESET_LIST,
NOTIFY_FEATURES,

NOTIFY_PRESET_LIST,
NOTIFY_PRESET_LIST_GENERIC_UPDATE_TAIL,
NOTIFY_PRESET_LIST_RECORD_DELETED_LAST,

NOTIFY_NUM,
};

Expand All @@ -175,6 +181,9 @@ static struct client_context {

/* Pending notification flags */
ATOMIC_DEFINE(notify, NOTIFY_NUM);

/* Last notified preset index */
uint8_t last_preset_index_known;
} contexts[CONFIG_BT_MAX_PAIRED];

/* Connected client clientance */
Expand Down Expand Up @@ -396,15 +405,36 @@ static void notify_work_handler(struct k_work *work)
} else if (err < 0) {
LOG_ERR("Notify read preset response err %d", err);
}
} else if (atomic_test_and_clear_bit(client->context->notify, NOTIFY_CURRENT_PRESET_LIST)) {
err = list_current_presets(client);
} else if (atomic_test_and_clear_bit(client->context->notify, NOTIFY_PRESET_LIST)) {
err = preset_list_changed(client);
if (err == -ENOMEM) {
atomic_set_bit(client->context->notify, NOTIFY_PRESET_LIST);
notify_work_reschedule(client, K_USEC(BT_AUDIO_NOTIFY_RETRY_DELAY_US));
} else if (err < 0) {
LOG_ERR("Notify preset list changed err %d", err);
}
} else if (atomic_test_and_clear_bit(client->context->notify,
NOTIFY_PRESET_LIST_GENERIC_UPDATE_TAIL)) {
err = preset_list_changed_generic_update_tail(client);
if (err == -ENOMEM) {
atomic_set_bit(client->context->notify,
NOTIFY_PRESET_LIST_GENERIC_UPDATE_TAIL);
notify_work_reschedule(client, K_USEC(BT_AUDIO_NOTIFY_RETRY_DELAY_US));
} else if (err < 0) {
LOG_ERR("Notify preset list changed generic update tail err %d", err);
}
} else if (atomic_test_and_clear_bit(client->context->notify,
NOTIFY_PRESET_LIST_RECORD_DELETED_LAST)) {
err = preset_list_changed_record_deleted_last(client);
if (err == -ENOMEM) {
atomic_set_bit(client->context->notify, NOTIFY_CURRENT_PRESET_LIST);
atomic_set_bit(client->context->notify,
NOTIFY_PRESET_LIST_RECORD_DELETED_LAST);
notify_work_reschedule(client, K_USEC(BT_AUDIO_NOTIFY_RETRY_DELAY_US));
} else if (err < 0) {
LOG_ERR("Notify generic update all presets err %d", err);
LOG_ERR("Notify preset list changed recoed deleted last err %d", err);
}
}

#endif /* CONFIG_BT_HAS_PRESET_SUPPORT */

if (IS_ENABLED(CONFIG_BT_HAS_PRESET_SUPPORT) &&
Expand Down Expand Up @@ -685,7 +715,7 @@ static int control_point_send_all(struct net_buf_simple *buf)

if (client == NULL || client->conn == NULL) {
/* Mark preset changed operation as pending */
atomic_set_bit(context->notify, NOTIFY_CURRENT_PRESET_LIST);
atomic_set_bit(context->notify, NOTIFY_PRESET_LIST);
continue;
}

Expand Down Expand Up @@ -737,8 +767,8 @@ static void preset_changed_prepare(struct net_buf_simple *buf, uint8_t change_id
preset_changed->is_last = is_last;
}

static int bt_has_cp_generic_update(struct has_client *client, const struct has_preset *preset,
uint8_t is_last)
static int bt_has_cp_generic_update(struct has_client *client, uint8_t prev_index, uint8_t index,
uint8_t properties, const char *name, uint8_t is_last)
{
struct bt_has_cp_generic_update *generic_update;

Expand All @@ -749,10 +779,10 @@ static int bt_has_cp_generic_update(struct has_client *client, const struct has_
preset_changed_prepare(&buf, BT_HAS_CHANGE_ID_GENERIC_UPDATE, is_last);

generic_update = net_buf_simple_add(&buf, sizeof(*generic_update));
generic_update->prev_index = preset_get_prev_index(preset);
generic_update->index = preset->index;
generic_update->properties = preset->properties;
net_buf_simple_add_mem(&buf, preset->name, strlen(preset->name));
generic_update->prev_index = prev_index;
generic_update->index = index;
generic_update->properties = properties;
net_buf_simple_add_mem(&buf, name, strlen(name));

if (client) {
return control_point_send(client, &buf);
Expand Down Expand Up @@ -785,26 +815,100 @@ static int read_preset_response(struct has_client *client)
}

err = bt_has_cp_read_preset_rsp(client, preset, is_last);
if (err != 0 || is_last) {
if (err != 0) {
return err;
}

client->read_presets_req.start_index = preset->index + 1;
client->read_presets_req.num_presets--;
client->context->last_preset_index_known = preset->index;

atomic_set_bit(client->context->notify, NOTIFY_READ_PRESET_RESPONSE);
notify_work_reschedule(client, K_USEC(BT_AUDIO_NOTIFY_RETRY_DELAY_US));
if (!is_last) {
client->read_presets_req.start_index = preset->index + 1;
client->read_presets_req.num_presets--;

atomic_set_bit(client->context->notify, NOTIFY_READ_PRESET_RESPONSE);
notify_work_reschedule(client, K_USEC(BT_AUDIO_NOTIFY_RETRY_DELAY_US));
}

return 0;
}

static int list_current_presets(struct has_client *client)
static int bt_has_cp_preset_record_deleted(struct has_client *client, uint8_t index)
{
NET_BUF_SIMPLE_DEFINE(buf, sizeof(struct bt_has_cp_hdr) +
sizeof(struct bt_has_cp_preset_changed) + sizeof(uint8_t));

preset_changed_prepare(&buf, BT_HAS_CHANGE_ID_PRESET_DELETED, BT_HAS_IS_LAST);
net_buf_simple_add_u8(&buf, index);

if (client != NULL) {
return control_point_send(client, &buf);
} else {
return control_point_send_all(&buf);
}
}

static int preset_list_changed_generic_update_tail(struct has_client *client)
{
const struct has_preset *prev;
struct has_preset last = {
.index = client->context->last_preset_index_known,
.properties = 0x00,
.name = " ",
};
int err;

prev = SYS_SLIST_PEEK_TAIL_CONTAINER(&preset_list, prev, node);

err = bt_has_cp_generic_update(client, prev ? prev->index : 0x00, last.index,
last.properties, last.name, false);
if (err != 0) {
return err;
}

return 0;
}

static int preset_list_changed_record_deleted_last(struct has_client *client)
{
const struct has_preset *last;
int err;

err = bt_has_cp_preset_record_deleted(client, client->context->last_preset_index_known);
if (err != 0) {
return err;
}

last = SYS_SLIST_PEEK_TAIL_CONTAINER(&preset_list, last, node);

client->context->last_preset_index_known = last ? last->index : 0x00;

return 0;
}

static int preset_list_changed(struct has_client *client)
{
const struct has_preset *preset = NULL;
const struct has_preset *next = NULL;
bool is_last = true;
int err;

if (sys_slist_is_empty(&preset_list)) {
/* The preset list is empty. We need to indicate deletion of all presets */
atomic_set_bit(client->context->notify,
NOTIFY_PRESET_LIST_RECORD_DELETED_LAST);

err = preset_list_changed_generic_update_tail(client);
if (err == -ENOMEM) {
atomic_set_bit(client->context->notify,
NOTIFY_PRESET_LIST_GENERIC_UPDATE_TAIL);
notify_work_reschedule(client, K_USEC(BT_AUDIO_NOTIFY_RETRY_DELAY_US));
} else if (err < 0) {
LOG_ERR("Notify preset list changed generic update tail err %d", err);
}

return 0;
}

preset_foreach(client->preset_changed_index_next, BT_HAS_PRESET_INDEX_LAST,
preset_found, &preset);

Expand All @@ -816,13 +920,36 @@ static int list_current_presets(struct has_client *client)

is_last = next == NULL;

err = bt_has_cp_generic_update(client, preset, is_last);
if (err != 0 || is_last) {
if (is_last && preset->index < client->context->last_preset_index_known) {
/* The last preset known to the client has been removed.
* As we do not hold the information about the deleted presets, we need to use
* Generic Update procedure to:
* 1. Notify the presets that have been removed in range
* (PrevIndex = current_preset_last, Index=previous_preset_last)
* 2. Notify deletion of preset Index=previous_preset_last.
*/
atomic_set_bit(client->context->notify,
NOTIFY_PRESET_LIST_GENERIC_UPDATE_TAIL);
atomic_set_bit(client->context->notify,
NOTIFY_PRESET_LIST_RECORD_DELETED_LAST);
is_last = false;
}

err = bt_has_cp_generic_update(client, preset_get_prev_index(preset), preset->index,
preset->properties, preset->name, is_last);
if (err != 0) {
return err;
}

if (is_last) {
client->preset_changed_index_next = 0;
client->context->last_preset_index_known = preset->index;
return 0;
}

client->preset_changed_index_next = preset->index + 1;

atomic_set_bit(client->context->notify, NOTIFY_CURRENT_PRESET_LIST);
atomic_set_bit(client->context->notify, NOTIFY_PRESET_LIST);
notify_work_reschedule(client, K_USEC(BT_AUDIO_NOTIFY_RETRY_DELAY_US));

return 0;
Expand Down Expand Up @@ -913,7 +1040,8 @@ static int set_preset_name(uint8_t index, const char *name, size_t len)
preset->ops->name_changed(index, preset->name);
}

return bt_has_cp_generic_update(NULL, preset, BT_HAS_IS_LAST);
return bt_has_cp_generic_update(NULL, preset_get_prev_index(preset), preset->index,
preset->properties, preset->name, BT_HAS_IS_LAST);
}

static uint8_t handle_write_preset_name(struct bt_conn *conn, struct net_buf_simple *buf)
Expand Down Expand Up @@ -1193,15 +1321,14 @@ int bt_has_preset_register(const struct bt_has_preset_register_param *param)
return -ENOMEM;
}

return bt_has_cp_generic_update(NULL, preset, BT_HAS_IS_LAST);
return bt_has_cp_generic_update(NULL, preset_get_prev_index(preset), preset->index,
preset->properties, preset->name, BT_HAS_IS_LAST);
}

int bt_has_preset_unregister(uint8_t index)
{
struct has_preset *preset;

NET_BUF_SIMPLE_DEFINE(buf, sizeof(struct bt_has_cp_hdr) +
sizeof(struct bt_has_cp_preset_changed) + sizeof(uint8_t));
int err;

CHECKIF(index == BT_HAS_PRESET_INDEX_NONE) {
LOG_ERR("index is invalid");
Expand All @@ -1217,12 +1344,14 @@ int bt_has_preset_unregister(uint8_t index)
return -EADDRINUSE;
}

preset_changed_prepare(&buf, BT_HAS_CHANGE_ID_PRESET_DELETED, BT_HAS_IS_LAST);
net_buf_simple_add_u8(&buf, preset->index);
err = bt_has_cp_preset_record_deleted(NULL, preset->index);
if (err != 0) {
return err;
}

preset_free(preset);

return control_point_send_all(&buf);
return 0;
}

static int set_preset_availability(uint8_t index, bool available)
Expand Down

0 comments on commit 6efa790

Please sign in to comment.