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

Merged
merged 1 commit into from
Jul 8, 2024
Merged
Changes from all commits
Commits
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
4 changes: 0 additions & 4 deletions samples/bluetooth/bap_broadcast_sink/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -849,8 +849,6 @@ static void syncable_cb(struct bt_bap_broadcast_sink *sink, const struct bt_iso_
k_sem_give(&sem_syncable);

if (!biginfo->encryption) {
/* Use the semaphore as a boolean */
k_sem_reset(&sem_broadcast_code_received);
Comment on lines -852 to -853
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

k_sem_give(&sem_broadcast_code_received);
}
}
Expand Down Expand Up @@ -1010,8 +1008,6 @@ static void broadcast_code_cb(struct bt_conn *conn,

(void)memcpy(sink_broadcast_code, broadcast_code, BT_AUDIO_BROADCAST_CODE_SIZE);

/* Use the semaphore as a boolean */
k_sem_reset(&sem_broadcast_code_received);
k_sem_give(&sem_broadcast_code_received);
}

Expand Down
Loading