-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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.
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
Add implementation for Public Broadcast Profile and dedicated sample apps #60777
Add implementation for Public Broadcast Profile and dedicated sample apps #60777
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @andreeaDumitrache, and thank you very much for your first pull request to the Zephyr project!
A project maintainer just triggered our CI pipeline to run it against your PR and ensure it's compliant and doesn't cause any issues. You might want to take this opportunity to review the project's Contributor Expectations and make any updates to your pull request if necessary. 😊
4cc7de3
to
265b80e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests/bsim/bluetooth/audio/test_scripts/pbp.sh
Remember to give execute permissions to that new script
265b80e
to
0c72825
Compare
include/zephyr/bluetooth/audio/pbp.h
Outdated
enum bt_pbp_encryption { | ||
BT_PBP_STREAMS_NOT_ENCRYPTED = 0, | ||
BT_PBP_STREAMS_ENCRYPTED = 1, | ||
}; | ||
|
||
enum bt_pbp_quality { | ||
BT_PBP_AUDIO_CONFIG_NOT_PRESENT = 0, | ||
BT_PBP_AUDIO_CONFIG_PRESENT = 1, | ||
}; | ||
|
||
enum bt_pbp_announcement_features { | ||
BT_PBP_ANNOUNCEMENT_FEATURE_ENCRYPTION = BIT(0), | ||
BT_PBP_ANNOUNCEMENT_FEATURE_STANDARD_QUALITY = BIT(1), | ||
BT_PBP_ANNOUNCEMENT_FEATURE_HIGH_QUALITY = BIT(2), | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing Doxygen comments?
d93b9ff
to
036ceac
Compare
samples/bluetooth/public_broadcast_sink/src/bap_broadcast_sink.c
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've only reviewed the public header file so far.
I don't think the functions supplied in that file really warrant a pbp.c
implementation, as the functions do not really implement any requirements from the PBP spec, but do require significant amount of memory for very simple things that probably are best left up to the application.
Let's discuss and resolve my comments before I start reviewing the samples, tests and implementation
3372312
to
0417393
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good now. Some final comments regarding the passing and generation of the PBA
0417393
to
12f6104
Compare
9bc7642
to
cbec895
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor comments to the header file, otherwise LGTM
cbec895
to
a6f71a4
Compare
PBP API allows sources to create a Public Broadcast Announcement. PBP API to parse a Public Broadcast Announcement. public_broadcast_source application starts extended advertising and includes a Public Broadcast Announcement. The advertised broadcast audio stream quality will cycle between high and standard quality. public_broadcast_sink application scans for broadcast sources and synchronizes to the first found source which defines a Public Broadcast Announcement including a High Quality Public Broadcast Audio Stream configuration. Add bsim tests for Public Broadcast Profile APIs. Add shell implementation for Public Broadcast Profile APIs. Signed-off-by: Daniela Andreea Dumitrache <[email protected]>
a6f71a4
to
c335578
Compare
@kartben Will you approve or remove your change request so we can get this merged please? |
@kartben please re-review or dismiss your review :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to see Bluetooth samples using zephyr:code-sample
sphinx directive!
Missed this before the holidays, sorry |
Hi @andreeaDumitrache! To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge. Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁 |
PBP API allows sources to create a Public Broadcast Announcement.
PBP API to parse a Public Broadcast Announcement.
public_broadcast_source application starts extended advertising and
includes a Public Broadcast Announcement. The advertised broadcast
audio stream quality will cycle between high and standard quality.
public_broadcast_sink application scans for broadcast sources and
synchronizes to the first found source which defines a Public Broadcast
Announcement including a High Quality Public Broadcast Audio Stream
configuration.
Add bsim tests for Public Broadcast Profile APIs.
Add shell implementation for Public Broadcast Profile APIs.
fixes #41226