Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bluetooth: pacs: Add dynamic PACS registration #83730

Merged
merged 1 commit into from
Jan 22, 2025
Merged

Bluetooth: pacs: Add dynamic PACS registration #83730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions doc/releases/migration-guide-4.1.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,12 @@ Bluetooth Audio
* :kconfig:option:`CONFIG_BT_PAC_SNK`
* :kconfig:option:`CONFIG_BT_PAC_SRC`

* PACS have been changed to support dynamic, runtime configuration. This means that PACS now has
to be registered with :c:func:`bt_pacs_register` before it can be used. In addition,
:c:func:`bt_pacs_register` also have to be called before :c:func:`bt_ascs_register` can be
be called. All Kconfig options still remain. Runtime configuration cannot override a disabled
Kconfig option. (:github:`83730`)

Bluetooth Classic
=================

Expand Down
53 changes: 53 additions & 0 deletions include/zephyr/bluetooth/audio/pacs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,43 @@ struct bt_pacs_cap {
sys_snode_t _node;
};

/** Structure for registering PACS */
struct bt_bap_pacs_register_param {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to also add support for registering initial values here? i.e. the initial contexts, locations and records?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could, but it could also make sense not to do it. The current content of struct bt_bap_pacs_register_param strictly necessary to run the service, and they cannot easily be changed, in fact they would need a reregistration to change.

Contexts, locations, and records can more easily change at runtime.

I would prefer to keep it as is for now - but if you really want this change Im happy to create an issue for future change.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure :) It makes sense to keep as is, with the caveat that the default values are invalid, so the user shall call the other functions afterwards (similar as they have to today)

#if defined(CONFIG_BT_PAC_SNK) || defined(__DOXYGEN__)
/**
* @brief Enables or disables registration of Sink PAC Characteristic.
*/
bool snk_pac;
#endif /* CONFIG_BT_PAC_SNK */

#if defined(CONFIG_BT_PAC_SNK_LOC) || defined(__DOXYGEN__)
/**
* @brief Enables or disables registration of Sink Location Characteristic.
*
* Registration of Sink Location is dependent on @ref bt_bap_pacs_register_param.snk_pac
* also being set.
*/
bool snk_loc;
#endif /* CONFIG_BT_PAC_SNK_LOC */

#if defined(CONFIG_BT_PAC_SRC) || defined(__DOXYGEN__)
/**
* @brief Enables or disables registration of Source PAC Characteristic.
*/
bool src_pac;
#endif /* CONFIG_BT_PAC_SRC */

#if defined(CONFIG_BT_PAC_SRC_LOC) || defined(__DOXYGEN__)
/**
* @brief Enables or disables registration of Source Location Characteristic.
*
* Registration of Source Location is dependent on @ref bt_bap_pacs_register_param.src_pac
* also being set.
*/
bool src_loc;
#endif /* CONFIG_BT_PAC_SRC_LOC */
};

/**
* @typedef bt_pacs_cap_foreach_func_t
* @brief Published Audio Capability iterator callback.
Expand All @@ -71,6 +108,22 @@ void bt_pacs_cap_foreach(enum bt_audio_dir dir,
bt_pacs_cap_foreach_func_t func,
void *user_data);

/**
* @brief Register the Published Audio Capability Service instance.
*
* @param param PACS register parameters.
*
* @return 0 in case of success or negative value in case of error.
*/
int bt_pacs_register(const struct bt_bap_pacs_register_param *param);

/**
* @brief Unregister the Published Audio Capability Service instance.
*
* @return 0 in case of success or negative value in case of error.
*/
Comment on lines +116 to +124
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, please use @retval instead of @return and specific the possible return values. It makes it easier for applications to know exactly which return values can be expected, and why

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to keep it as is, as it will forward error codes from other functions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to keep it as is, as it will forward error codes from other functions.

That's one of the reasons to use retval: Simply forwarding error will make it harder to handle cases as an application.

We do not have any requirement in the coding guidelines for this, so I won't block the PR on this

int bt_pacs_unregister(void);

/**
* @brief Register Published Audio Capability.
*
Expand Down
11 changes: 11 additions & 0 deletions samples/bluetooth/bap_broadcast_sink/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1272,8 +1272,13 @@ static struct bt_le_per_adv_sync_cb bap_pa_sync_cb = {
.term = bap_pa_sync_terminated_cb,
};


static int init(void)
{
const struct bt_bap_pacs_register_param pacs_param = {
.snk_pac = true,
.snk_loc = true,
};
int err;

err = bt_enable(NULL);
Expand All @@ -1284,6 +1289,12 @@ static int init(void)

printk("Bluetooth initialized\n");

err = bt_pacs_register(&pacs_param);
if (err) {
printk("Could not register PACS (err %d)\n", err);
return err;
}

err = bt_pacs_cap_register(BT_AUDIO_DIR_SINK, &cap);
if (err) {
printk("Capability register failed (err %d)\n", err);
Expand Down
13 changes: 13 additions & 0 deletions samples/bluetooth/bap_unicast_server/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -721,9 +721,16 @@ static int set_available_contexts(void)
return 0;
}


int main(void)
{
struct bt_le_ext_adv *adv;
const struct bt_bap_pacs_register_param pacs_param = {
.snk_pac = true,
.snk_loc = true,
.src_pac = true,
.src_loc = true
};
int err;

err = bt_enable(NULL);
Expand All @@ -734,6 +741,12 @@ int main(void)

printk("Bluetooth initialized\n");

err = bt_pacs_register(&pacs_param);
if (err) {
printk("Could not register PACS (err %d)\n", err);
return 0;
}

bt_bap_unicast_server_register(&param);
bt_bap_unicast_server_register_cb(&unicast_server_cb);

Expand Down
12 changes: 12 additions & 0 deletions samples/bluetooth/cap_acceptor/src/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,12 @@ static int init_cap_acceptor(void)
static const struct bt_audio_codec_cap lc3_codec_cap = BT_AUDIO_CODEC_CAP_LC3(
SUPPORTED_FREQ, SUPPORTED_DURATION, MAX_CHAN_PER_STREAM, MIN_SDU, MAX_SDU,
FRAMES_PER_SDU, (SINK_CONTEXT | SOURCE_CONTEXT));
const struct bt_bap_pacs_register_param pacs_param = {
.snk_pac = true,
.snk_loc = true,
.src_pac = true,
.src_loc = true
};
int err;

err = bt_enable(NULL);
Expand All @@ -267,6 +273,12 @@ static int init_cap_acceptor(void)

LOG_INF("Bluetooth initialized");

err = bt_pacs_register(&pacs_param);
if (err) {
LOG_ERR("Could not register PACS (err %d)", err);
return 0;
}

if (IS_ENABLED(CONFIG_BT_PAC_SNK)) {
static struct bt_pacs_cap sink_cap = {
.codec_cap = &lc3_codec_cap,
Expand Down
14 changes: 14 additions & 0 deletions samples/bluetooth/hap_ha/src/bap_unicast_sr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,20 @@ static struct bt_pacs_cap cap_source = {

int bap_unicast_sr_init(void)
{
const struct bt_bap_pacs_register_param pacs_param = {
.snk_pac = true,
.snk_loc = true,
.src_pac = true,
.src_loc = true
};
int err;

err = bt_pacs_register(&pacs_param);
if (err) {
printk("Could not register PACS (err %d)\n", err);
return err;
}

bt_bap_unicast_server_register(&param);
bt_bap_unicast_server_register_cb(&unicast_server_cb);

Expand Down
10 changes: 10 additions & 0 deletions samples/bluetooth/tmap_bmr/src/bap_broadcast_sink.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,8 +313,18 @@ static int reset(void)

int bap_broadcast_sink_init(void)
{
const struct bt_bap_pacs_register_param pacs_param = {
.snk_pac = true,
.snk_loc = true,
};
int err;

err = bt_pacs_register(&pacs_param);
if (err) {
printk("Could not register PACS (err %d)\n", err);
return err;
}

bt_bap_broadcast_sink_register_cb(&broadcast_sink_cbs);
bt_le_per_adv_sync_cb_register(&broadcast_sync_cb);

Expand Down
15 changes: 15 additions & 0 deletions samples/bluetooth/tmap_peripheral/src/bap_unicast_sr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,23 @@ static struct bt_pacs_cap cap = {
.codec_cap = &lc3_codec_cap,
};


Thalley marked this conversation as resolved.
Show resolved Hide resolved
int bap_unicast_sr_init(void)
{
const struct bt_bap_pacs_register_param pacs_param = {
.snk_pac = true,
.snk_loc = true,
.src_pac = true,
.src_loc = true
};
int err;

err = bt_pacs_register(&pacs_param);
if (err) {
printk("Could not register PACS (err %d)\n", err);
return err;
}

bt_bap_unicast_server_register(&param);
bt_bap_unicast_server_register_cb(&unicast_server_cb);

Expand Down
Loading
Loading