LE Audio: Broadcast Assistant should discover PACS

[Thalley]

Is your enhancement proposal related to a problem? Please describe. A BAP broadcast assistant should discover PACS on the Broadcast Sink/Scan Delegator device to determine if it supports a broadcast source. Currently this isn't possible with the API.

Describe the solution you'd like Similar to how the bt_bap_unicast_client_discover discover PACS, the bt_bap_broadcast_assistant should also discover PACS on the remote device.

Describe alternatives you've considered N/A

Additional context N/A

[jthm-ot]

I can take it

[Chris-StJohn-Bose]

@jthm-ot @Thalley please be aware that there are proposals to extend PACS in the near future. I'm running IOP with a modified version of pacs.c and bap_unicast_client.c. It would be nice if your refactor could make extending PACS a little easier.

[Thalley]

@jthm-ot Any plans for this in the near future? Since this will likely conflict with https://github.com/zephyrproject-rtos/zephyr/issues/70826 I'm like to have at least 1 of the 2 implemented in the near future

[Chris-StJohn-Bose]

@jthm-ot @Thalley please be aware that there are proposals to extend PACS in the near future. I'm running IOP with a modified version of pacs.c and bap_unicast_client.c. It would be nice if your refactor could make extending PACS a little easier.

To elaborate on my comment about "making it easier" - here's a snippet of the discovery for a client I'm working on. It uses a single discovery callback method rather than a chain of them. Just an idea...

        if (params->type == BT_GATT_DISCOVER_PRIMARY) {
            /* Start discovering characteristics */
            inst->discover_params.type = BT_GATT_DISCOVER_CHARACTERISTIC;
            inst->discover_params.uuid = NULL;
            struct bt_gatt_service_val *svc = attr->user_data;
            inst->discover_params.start_handle = attr->handle+1;
            inst->discover_params.end_handle = svc->end_handle;

            err = bt_gatt_discover(conn, &inst->discover_params);
            if (!err) {
                /* Stop the service discovery and continue with characteritics */
                return BT_GATT_ITER_STOP;
            }
            /* fall through and fail check: release, callback and stop */

        } else if (params->type == BT_GATT_DISCOVER_CHARACTERISTIC) {
            struct bt_gatt_chrc *chrc = attr->user_data;
            struct bt_gatt_subscribe_params *sub_params = NULL;

            if (bt_uuid_cmp(chrc->uuid, BT_UUID_XXXX) == 0) {
                inst->xxxx_handle = chrc->value_handle;
                sub_params = &inst->xxxx_sub_params;
                sub_params->disc_params = &inst->xxxx_sub_disc_params;
            }
            if (bt_uuid_cmp(chrc->uuid, BT_UUID_YYYY) == 0) {
                inst->yyyy_handle = chrc->value_handle;
                sub_params = &inst->yyyy_sub_params;
                sub_params->disc_params = &inst->yyyy_sub_disc_params;
            }
            if (bt_uuid_cmp(chrc->uuid, BT_UUID_ZZZZ) == 0) {
                inst->zzzz_handle = chrc->value_handle;
            }

            /* Subscribe? */
            err = 0;
            if (sub_params != NULL) {
                sub_params->value = BT_GATT_CCC_NOTIFY;
                sub_params->value_handle = chrc->value_handle;
                sub_params->ccc_handle = 0;
                sub_params->end_handle = params->end_handle;
                sub_params->notify = ahs_gatt_notify;
                err = bt_gatt_subscribe(conn, sub_params);
            }

            if (!err) {
                /* Continue discovery of characteristics in this service */
                return BT_GATT_ITER_CONTINUE;
            }
            /* fall through and fail check: release, callback and stop */
        }

[Thalley]

It uses a single discovery callback method rather than a chain of them. Just an idea...

We actually used to have that, but we changed it to a chain.

There are a couple of reasons for this: 1) If you only discover a single characteristic at a time using the UUID (compared to a general discovery for BT_GATT_DISCOVER_CHARACTERISTIC, then you don't get much value out of having a combine callback fucntion, as nearly nothing will be shared. 2) If you do a more general discover, then this function may be called multiple time from the same read request response, as it it called per handle. This is great for optimization, but one needs to be aware of that, especially if you want to do calls to bt_gatt_subscribe, as bt_gatt_subscribe is a blocking operation, and you e.g. get 5 handles in a response, then you may end up calling bt_gatt_subscribe 5 times in the same response, and if you run out of ATT buffers, then your call to bt_gatt_subscribe will fail, and you won't subscribe to all characteristics, which is mandatory in some cases (like BAP). The only way to circumvent this, is to have enough buffers, but since you may find up to 255 sink ASEs and 255 source ASEs, that's too many buffers to handle worst case. Discovering them one by one limits it to only require a single buffer. 3) Finally to subscribe you need to know the CCC handle. The CONFIG_BT_GATT_AUTO_DISCOVER_CCC feature is pretty neat for this, but it requires a discovery parameter. Similar to the issue in 2), doing parallel discoveries will require additional discover parameters. Discovering it in a chain will only require 1 such parameter, and you can even re-use the discover parameter that was used to find the characteristic you want to subscribe to in the first place. This will however only work if you always return BT_GATT_ITER_STOP if you want to reused the discover paramters.

So in short, we did use that approach earlier and it could speed up discovery, but at the cost of (significant) memory. It is how e.g. the TBS client works today (https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/bluetooth/audio/tbs_client.c#L1554-L1719), but where TBS needs up to 11 ATT buffers for discovery, and that's only if no other ATT operation is done at the same time.