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: L2CAP_BR: Enable Retransmission and Flow control #78879

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

lylezhu2012
Copy link
Contributor

Enable the retransmission, flow control, enhance retransmission, and streaming feature for BR/EDR L2cap.

Comment on lines 249 to 251
#define BT_L2CAP_BR_FCS_NO 0x00
/** Frame Check Sequence type. 16-bit FCS. */
#define BT_L2CAP_BR_FCS_16BIT 0x01
Copy link
Member

Choose a reason for hiding this comment

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

No tabs for alignment, please (only for indentation)

/** Endpoint Link Mode.
* The value is defined as BT_L2CAP_BR_LINK_MODE_*
*/
uint8_t mode;
Copy link
Member

Choose a reason for hiding this comment

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

Same here and anywhere else if you did the same. Use just spaces for alignment and tabs for indentation (or to put it in another way: the only place for tabs is in the very beginning of a line - as long as you have some non-tab character on a line there should not be any more tabs anywhere later in the line)

Copy link
Member

Choose a reason for hiding this comment

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

Note: I know there are inconsistencies wrt this in the Bluetooth code base, but we should try to move toward the convention that I just described.

Comment on lines 30 to 35
config BT_L2CAP_RET
bool "Bluetooth L2CAP restransmission mode [EXPERIMENTAL]"
select EXPERIMENTAL
help
This option enables Bluetooth L2CAP restransmission mode

Copy link
Member

Choose a reason for hiding this comment

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

Please add some CI test cases for this. Right now it seems like the new c-code could be invalid garbage, and yet CI would show all green for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. How about to add test code to shell?

Copy link
Member

Choose a reason for hiding this comment

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

OK. How about to add test code to shell?

If there's some convenient and intuitive way to test this through the shell, then sure, why not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. It is done.

Enable signaling channel configuration for Retransmission
and Flow control feature.

Send I-frame and S-frame. Support retransmission and Flow
control for sending.
Receive and handle I-frame and S-frame.

Signed-off-by: Lyle Zhu <[email protected]>
Add mode optional support for BR l2cap connect initiator role.

If `chan->rx.optional` is true, set the mode to basic mode instead of
return error code `-ENOTSUP`.

Signed-off-by: Lyle Zhu <[email protected]>
Add a sub-command set `l2cap` for command set `br`.

Move command `l2cap-register` to sub-command set. And rename it to
`register`.

Add command `connect`, `disconnect`, and `send` for command set
`l2cap`.

Remove original net buffer pool from `data_pool` to `data_rx_pool`.
Add a net buffer pool `data_tx_pool` to for command `send`.

Do not wait anymore if no net buffer can be allocated from
`data_rx_pool`.

Dump all received data in L2CAP data received callback.

Signed-off-by: Lyle Zhu <[email protected]>
Add more parameters to command `connect` and `register`, including mode,
mode_optional, extended_control, and hold_credit.

Add command `credits` to give the rx credit.

Signed-off-by: Lyle Zhu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants