From 6efa790dddbee988ce67644138ec4bc2b4941633 Mon Sep 17 00:00:00 2001 From: Mariusz Skamra Date: Thu, 12 Oct 2023 17:22:53 +0200 Subject: [PATCH] Bluetooth: has: Fix preset list notifications after reconnection 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 --- subsys/bluetooth/audio/has.c | 191 +++++++++++++++++++++++++++++------ 1 file changed, 160 insertions(+), 31 deletions(-) diff --git a/subsys/bluetooth/audio/has.c b/subsys/bluetooth/audio/has.c index 1141f748055583c..f45597bc1556025 100644 --- a/subsys/bluetooth/audio/has.c +++ b/subsys/bluetooth/audio/has.c @@ -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; @@ -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); @@ -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, }; @@ -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 */ @@ -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) && @@ -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; } @@ -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; @@ -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); @@ -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); @@ -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; @@ -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) @@ -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"); @@ -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)