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

Jump to bottom

applications: nrf5340_audio: Rail the presentation delay. #15035

Merged
merged 1 commit into from
May 21, 2024
Merged

applications: nrf5340_audio: Rail the presentation delay. #15035

merged 1 commit into from
May 21, 2024

Conversation

gWacey
Copy link
Contributor

@gWacey gWacey commented Apr 23, 2024

OCT--2976

Ensure the preferred presentation delay is within the range of the min and max delays, rail if not.
Search for the presentation delay function updated to search over the end points for the direction.

@gWacey gWacey requested a review from a team as a code owner April 23, 2024 21:46
@gWacey gWacey requested review from koffes, andvib, rick1082 and alexsven and removed request for a team April 23, 2024 21:46
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Apr 23, 2024
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Apr 23, 2024 •
edited
Loading

Test specification

CI/Jenkins/NRF

  • Integration Platforms

CI/Jenkins/integration

Test Module File based changes Manually selected West overwrite

Detailed information of selected test modules

Note: This message is automatically posted and updated by the CI

@gWacey gWacey force-pushed the OCT-2976-Presentation-delay-should-be-clamped-between-pres_min-pres_max branch 4 times, most recently from eb97d33 to 48857e8 Compare April 29, 2024 11:45
@gWacey gWacey requested a review from alexsven April 29, 2024 11:45
@gWacey gWacey force-pushed the OCT-2976-Presentation-delay-should-be-clamped-between-pres_min-pres_max branch from 48857e8 to 7316250 Compare April 30, 2024 12:17
@gWacey gWacey requested a review from tejlmand as a code owner April 30, 2024 12:17
@gWacey gWacey requested a review from alexsven April 30, 2024 12:17
@NordicBuilder
Copy link
Contributor

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

@gWacey gWacey force-pushed the OCT-2976-Presentation-delay-should-be-clamped-between-pres_min-pres_max branch from 7316250 to a5b1780 Compare May 2, 2024 08:13
@gWacey gWacey requested a review from alexsven May 2, 2024 08:14
@gWacey gWacey force-pushed the OCT-2976-Presentation-delay-should-be-clamped-between-pres_min-pres_max branch 2 times, most recently from bec232b to d2c05af Compare May 7, 2024 10:16
@gWacey gWacey force-pushed the OCT-2976-Presentation-delay-should-be-clamped-between-pres_min-pres_max branch 4 times, most recently from 01ebc99 to 345a01c Compare May 14, 2024 08:57
@gWacey gWacey force-pushed the OCT-2976-Presentation-delay-should-be-clamped-between-pres_min-pres_max branch from 345a01c to 6f906f0 Compare May 14, 2024 15:02
@gWacey gWacey requested a review from koffes May 14, 2024 15:04
@gWacey gWacey force-pushed the OCT-2976-Presentation-delay-should-be-clamped-between-pres_min-pres_max branch from 6f906f0 to 74007dd Compare May 15, 2024 07:30
koffes
koffes approved these changes May 15, 2024
View reviewed changes
applications/nrf5340_audio/src/bluetooth/bt_stream/unicast/unicast_client.c Outdated
Comment on lines 194 to 248
if (pref_pd_min == 0) {
*pres_dly_us = pd_min;
} else if (pref_pd_min <= pd_max) {
*pres_dly_us = pref_pd_min;
} else {
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
if (pref_pd_min == 0) {
*pres_dly_us = pd_min;
} else if (pref_pd_min <= pd_max) {
*pres_dly_us = pref_pd_min;
} else {
if (pref_pd_min == 0) {
*pres_dly_us = pd_min;
} else if (pref_pd_min = pd_max) {
*pres_dly_us = pref_pd_min;
} else if (pref_pd_min < pd_max) {
*pres_dly_us = pref_pd_min;
LOG_WRN("....");
} else {

Copy link
Contributor

Choose a reason for hiding this comment

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

And the same below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure I follow the above, why the warning?

@koffes koffes self-requested a review May 15, 2024 14:27
@gWacey gWacey force-pushed the OCT-2976-Presentation-delay-should-be-clamped-between-pres_min-pres_max branch from 74007dd to b24bd3c Compare May 16, 2024 08:32
@gWacey gWacey removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label May 16, 2024
@gWacey gWacey force-pushed the OCT-2976-Presentation-delay-should-be-clamped-between-pres_min-pres_max branch from b24bd3c to b9db25b Compare May 16, 2024 11:23
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label May 16, 2024
@gWacey gWacey force-pushed the OCT-2976-Presentation-delay-should-be-clamped-between-pres_min-pres_max branch 5 times, most recently from 5527b39 to d7805c4 Compare May 16, 2024 15:59
@gWacey gWacey removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label May 21, 2024
@gWacey
applications: nrf5340_audio: Rail the presentation delay.
98b8007 
	Ensure the preferred presentation delay is within the range of the
	min and max delays, rail if not.
	Search for the presentation delay function updated to search over
	the end points for the direction.

Signed-off-by: Graham Wacey <[email protected]>
@gWacey gWacey force-pushed the OCT-2976-Presentation-delay-should-be-clamped-between-pres_min-pres_max branch from d7805c4 to 98b8007 Compare May 21, 2024 06:55
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label May 21, 2024
@nordicjm nordicjm merged commit 5b5956d into nrfconnect:main May 21, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexsven alexsven alexsven approved these changes

@rick1082 rick1082 rick1082 approved these changes

@koffes koffes koffes approved these changes

@andvib andvib Awaiting requested review from andvib andvib is a code owner automatically assigned from nrfconnect/ncs-audio

@tejlmand tejlmand Awaiting requested review from tejlmand

Assignees
No one assigned
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@gWacey @NordicBuilder @koffes @andvib @rick1082 @alexsven @nordicjm