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.

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 samples: Broadcast code handling #75470

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jthm-ot
Copy link
Contributor

@jthm-ot jthm-ot commented Jul 4, 2024

The broadcast sink sample no longer treats the broadcast_code_received semaphore as a boolean.

Fixes #75469

The broadcast sink sample no longer treats the broadcast_code_received
semaphore as a boolean.

Fixes zephyrproject-rtos#75469

Signed-off-by: Jens Rehhoff Thomsen <[email protected]>
Comment on lines -852 to -853
/* Use the semaphore as a boolean */
k_sem_reset(&sem_broadcast_code_received);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the broadcast sink without a broadcast source get stuck at

		/* sem_broadcast_code_received is also given if the
		 * broadcast is not encrypted
		 */
		printk("Waiting for broadcast code\n");
		err = k_sem_take(&sem_broadcast_code_received, SEM_TIMEOUT);
		if (err != 0) {
			printk("sem_broadcast_code_received timed out, resetting\n");
			continue;
		}

now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean without a broadcast code? If encrypted then sink gets stucked until code provided via broadcast_code_cb. If not encrypted then semaphore is given on syncable_cb

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right. You are still giving the semaphore.

Since the semaphore is

K_SEM_DEFINE(sem_broadcast_code_received, 0U, 1U);

then it could never be given twice, so what does this PR actually change?

Doesn't

	k_sem_reset(&sem_broadcast_code_received);
	k_sem_give(&sem_broadcast_code_received);

and

	k_sem_give(&sem_broadcast_code_received);

both always set the semaphore count to 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the sem is reset then wait in the main loop hits an error and does a continue => reset

	printk("Waiting for broadcast code\n");
	err = k_sem_take(&sem_broadcast_code_received, SEM_TIMEOUT);
	if (err != 0) {
		printk("sem_broadcast_code_received timed out, resetting\n");
		continue;
	}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, so if k_sem_reset while k_sem_take is waiting, then it returns an error? Was not aware of that, but yeah I can see that from the documentation now. Nice catch

@Thalley Thalley added this to the v3.7.0 milestone Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

Bluetooth: samples: bap_broadcast_sink sample resets on broadcast code received
5 participants