Skip to content

bluetooth: audio: add bt_audio_get_chan_count #72849

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

Jump to bottom
Merged
merged 1 commit into from
May 24, 2024
Merged

bluetooth: audio: add bt_audio_get_chan_count #72849

merged 1 commit into from
May 24, 2024

Conversation

babrsn
Copy link
Contributor

@babrsn babrsn commented May 16, 2024

Implement a function bt_audio_get_chan_count that takes an enum bt_audio_location and returns the number of channels in that value.

@github-actions GitHub Actions
Copy link

Hello @bbaa-demant, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@pin-zephyr pin-zephyr assigned pin-zephyr and babrsn and unassigned pin-zephyr May 16, 2024
@babrsn babrsn marked this pull request as ready for review May 16, 2024 11:13
@Thalley
Copy link
Contributor

Thalley commented May 16, 2024

@bbaa-demant Please add a closing keyword for the issue so that this PR gets linked to it.

Did you consider the POPCOUNT as suggested by #69617 (comment)?

Please also go through samples/tests to see where this function is duplicated, and replace the individual implementations with the one introduced by this PR

@babrsn
Copy link
Contributor Author

babrsn commented May 16, 2024 •
edited
Loading

Did you consider the POPCOUNT as suggested by #69617 (comment)?

I am not sure if the POPCOUNT (or __builtin_popcount) works with other toolchains beside gcc, (count_bits could be also an option). I will investigate it...

@zephyrbot zephyrbot added the area: Samples Samples label May 17, 2024
@babrsn
Copy link
Contributor Author

babrsn commented May 22, 2024

@Thalley I think your comments have been resolved in my last commit (5 days ago)...

@Thalley
Copy link
Contributor

Thalley commented May 22, 2024

@Thalley I think your comments have been resolved in my last commit (5 days ago)...

Yeah, sorry, a lot of Github notifications has gone lost last week :s

Thalley
Thalley requested changes May 22, 2024
View reviewed changes
@Thalley
Copy link
Contributor

Thalley commented May 22, 2024

We could also consider adding a public API for counting bits in a value and have the optimization of the compiler exist in that.

kartben
kartben reviewed May 22, 2024
View reviewed changes
include/zephyr/bluetooth/audio/audio.h Outdated
*
* @param chan_allocation The channel allocation
*
* @retval The number of channels.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @retval The number of channels.
* @return The number of channels.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Thalley
Thalley requested changes May 22, 2024
View reviewed changes
Thalley
Thalley previously approved these changes May 23, 2024
View reviewed changes
Copy link
Contributor

@Thalley Thalley left a comment

Choose a reason for hiding this comment

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

Thanks for the implementation and for the quick updates.

I have a single comment left with some minor formatting that isn't required, but may be better.

LGTM

@pin-zephyr pin-zephyr requested a review from kartben May 23, 2024 11:48
Thalley
Thalley requested changes May 24, 2024
View reviewed changes
Copy link
Contributor

@Thalley Thalley left a comment

Choose a reason for hiding this comment

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

Look like you accidentally added a merge commit rather than rebased on main

@babrsn
Bluetooth: Audio: add bt_audio_get_chan_count
d889aec 
Implement a function bt_audio_get_chan_count that takes an enum
bt_audio_location and returns the number of channels in that value.

This PR fixes #69617
(#69617)

Signed-off-by: Babak Arisian <bbaa@demant.com>
@babrsn
Copy link
Contributor Author

babrsn commented May 24, 2024 •
edited
Loading

Look like you accidentally added a merge commit rather than rebased on main

Yes, but not accidentally! I needed to restart the builds/checks because there was an error (unknown to me) related to documentation build...

Thank you very much for your support.

@Thalley Thalley changed the title subsys: bluetooth: audio: add bt_audio_get_chan_count bluetooth: audio: add bt_audio_get_chan_count May 24, 2024
@MaureenHelm MaureenHelm merged commit b0dceff into zephyrproject-rtos:main May 24, 2024
@github-actions GitHub Actions
Copy link

Hi @bbaa-demant!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

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! 🪁

@babrsn babrsn deleted the bbaa_add_get_chan_count branch May 24, 2024 21:06
@babrsn babrsn mentioned this pull request Oct 24, 2024
Merged
dkalowsk pushed a commit that referenced this pull request Oct 28, 2024
@babrsn @dkalowsk
MAINTAINERS: Add babrsn to Bluetooth Audio
c903013 
I have contributed the following PRs:
#80000
#79360
#78846
#78751
#74950
#74946
#73845
#72849

and am attending the weekly LE Audio Zephyr meetings.

Signed-off-by: Babak Arisian <bbaa@demant.com>
coreboot-bot pushed a commit to coreboot/zephyr-cros that referenced this pull request Oct 31, 2024
@babrsn
MAINTAINERS: Add babrsn to Bluetooth Audio
249b80d 
I have contributed the following PRs:
zephyrproject-rtos/zephyr#80000
zephyrproject-rtos/zephyr#79360
zephyrproject-rtos/zephyr#78846
zephyrproject-rtos/zephyr#78751
zephyrproject-rtos/zephyr#74950
zephyrproject-rtos/zephyr#74946
zephyrproject-rtos/zephyr#73845
zephyrproject-rtos/zephyr#72849

and am attending the weekly LE Audio Zephyr meetings.

(cherry picked from commit c903013)

Original-Signed-off-by: Babak Arisian <bbaa@demant.com>
GitOrigin-RevId: c903013
Cr-Build-Id: 8732568629207591185
Cr-Build-Url: https://cr-buildbucket.appspot.com/build/8732568629207591185
Copybot-Job-Name: zephyr-main-copybot-downstream
Change-Id: Iab40058a1cc005e6454c07b4156fc271c0b84fca
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/5982745
Reviewed-by: Ting Shen <phoenixshen@chromium.org>
Commit-Queue: Ting Shen <phoenixshen@chromium.org>
Tested-by: ChromeOS Prod (Robot) <chromeos-ci-prod@chromeos-bot.iam.gserviceaccount.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Thalley Thalley Thalley approved these changes

@pin-zephyr pin-zephyr pin-zephyr approved these changes

@asbjornsabo asbjornsabo Awaiting requested review from asbjornsabo

@Casper-Bonde-Bose Casper-Bonde-Bose Awaiting requested review from Casper-Bonde-Bose

@fredrikdanebjer fredrikdanebjer Awaiting requested review from fredrikdanebjer

@jhedberg jhedberg Awaiting requested review from jhedberg

@kruithofa kruithofa Awaiting requested review from kruithofa

@larsgk larsgk Awaiting requested review from larsgk

@MariuszSkamra MariuszSkamra Awaiting requested review from MariuszSkamra

@sjanc sjanc Awaiting requested review from sjanc

@hermabe hermabe Awaiting requested review from hermabe

@nashif nashif Awaiting requested review from nashif

@Vudentz Vudentz Awaiting requested review from Vudentz

@kartben kartben Awaiting requested review from kartben

Assignees

@babrsn babrsn

Labels
area: Bluetooth Audio area: Bluetooth area: Samples Samples
Projects
Bluetooth LE Audio
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@babrsn @Thalley @kartben @pin-zephyr @MaureenHelm @zephyrbot