Skip to content

Bluetooth: Mesh: Fix regression in PB-ADV #36498

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

Merged
merged 1 commit into from
Jun 27, 2021

Conversation

trond-snekvik
Copy link
Collaborator

After #35702, the provisioner is unable to mark a link as closed, as
it depends on the send_end callback to be called, so it can start its
timer. PB-Adv keeps a reference to the buffers of reliable messages,
which prevents this callback to be invoked, as the buffer destructor is
never called.

Move scheduling of the retransmit timer to the initial transmission, and
replace the timer based LINK_CLOSE message tx duration with a message
counting solution.

Signed-off-by: Trond Einar Snekvik [email protected]

@trond-snekvik trond-snekvik requested a review from LingaoM June 23, 2021 15:38
@trond-snekvik trond-snekvik added bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug labels Jun 23, 2021
@trond-snekvik
Copy link
Collaborator Author

I'm not 100 sure about this solution, as it's changing the retransmit logic to not take the time spent sending the messages into account, which can make retransmits come earlier than necessary.

I also believe I owe you a beer, @jhedberg, you pointed out that this might happen, and I dismissed it 🙁

@LingaoM
Copy link
Collaborator

LingaoM commented Jun 24, 2021

I'm not 100 sure about this solution, as it's changing the retransmit logic to not take the time spent sending the messages into account, which can make retransmits come earlier than necessary.

I also believe I owe you a beer, @jhedberg, you pointed out that this might happen, and I dismissed it 🙁

Sorry for this, i thought bsim would automatically catch these problems, or it was a mistake. Is it necessary for us to add a check for the available buffer under bsim?

@trond-snekvik
Copy link
Collaborator Author

Sorry for this, i thought bsim would automatically catch these problems, or it was a mistake. Is it necessary for us to add a check for the available buffer under bsim?

That's how I discovered it, actually. #35446 fails because of this bug. I'll rebase it after this goes in, and we'll have coverage.

After zephyrproject-rtos#35702, the provisioner is unable to mark a link as closed, as
it depends on the send_end callback to be called, so it can start its
timer. PB-Adv keeps a reference to the buffers of reliable messages,
which prevents this callback to be invoked, as the buffer destructor is
never called.

Move scheduling of the retransmit timer to the initial transmission, and
replace the timer based LINK_CLOSE message tx duration with a message
counting solution.

Signed-off-by: Trond Einar Snekvik <[email protected]>
@jhedberg jhedberg merged commit 534177b into zephyrproject-rtos:main Jun 27, 2021
@trond-snekvik trond-snekvik deleted the prov_refcount branch June 27, 2021 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Mesh area: Bluetooth bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants