Skip to content

Conversation

@graynode
Copy link
Contributor

While conducting large data transfers over rfcomm I found our device would disconnect after 3-400kb. Looking deeper I found it was possible for the tx credit to go negative which is what was causing the l2cap connection to be disconnected by the other device as this violates the standard.

see attached HCI log
bumble-rfcomm-hci-negative-tx-credit.txt

@google-cla
Copy link

google-cla bot commented Oct 23, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@graynode graynode force-pushed the rfcomm-tx-credit-goes-negative-fix branch from e71f3c3 to dae3ec5 Compare October 24, 2025 03:09
@graynode graynode changed the title Fixed bug where it's possible for tx_credit to go negative resulting in l2cap disconnect from peripheral Fixed bug where it's possible for rfcomm tx_credit to go negative resulting in l2cap disconnect from peripheral Oct 24, 2025
bumble/rfcomm.py Outdated
# Send anything we can (or an empty frame if we need to send rx credits)
rx_credits_needed = self.rx_credits_needed()
while (self.tx_buffer and self.tx_credits > 0) or rx_credits_needed > 0:
while (self.tx_buffer and self.tx_credits > 0) or (rx_credits_needed > 0 and self.tx_credit > 0):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This fix does indeed prevent tx_credit from going negative (in the case where rx_credits_needed is not 0 but tx_credit is 0), but it goes a bit too far: it prevents sending rx credits in that case, even though it may be desirable to do so.
I think the right fix here is to keep the while condition as it was (i.e "either there's some data that can be sent, or there's a need to send credits), but then in the if rx_credits_needed > 0 case, only include data and set tx_credit_spent if we have tx credits. Something like this maybe:

if rx_credits_needed > 0:
    # Send rx credits
    chunk = bytes([rx_credits_needed])
    self.rx_credits += rx_credits_needed
    if self.tx_buffer and self.tx_credits > 0:
        # Send some data along with the credits
        chunk += self.tx_buffer[: self.mtu - 1]
        self.tx_buffer = self.tx_buffer[len(chunk) - 1 :]
        tx_credits_spent = True
    else:
        tx_credits_spent = False

Copy link
Contributor Author

@graynode graynode Oct 24, 2025

Choose a reason for hiding this comment

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

Good catch. I just tested it locally and it works. I've updated the PR to reflect these changes.

@barbibulle barbibulle merged commit 578f7f0 into google:main Oct 26, 2025
57 checks passed
@graynode graynode deleted the rfcomm-tx-credit-goes-negative-fix branch October 28, 2025 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants