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.

Sign up for GitHub

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: rfcomm: Support dynamic channel allocation #83785

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Bluetooth: rfcomm: Support dynamic channel allocation #83785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 11 additions & 1 deletion include/zephyr/bluetooth/classic/rfcomm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ enum {
BT_RFCOMM_CHAN_HSP_AG,
BT_RFCOMM_CHAN_HSP_HS,
BT_RFCOMM_CHAN_SPP,
BT_RFCOMM_CHAN_DYNAMIC_START,
};

struct bt_rfcomm_dlc;
Expand Down Expand Up @@ -109,7 +110,16 @@ struct bt_rfcomm_dlc {
};

struct bt_rfcomm_server {
/** Server Channel */
/** Server Channel
*
* Possible values:
* 0 A dynamic value will be auto-allocated when bt_rfcomm_server_register() is
* called.
*
* 0x01 - 0x1e Dynamically allocated. May be pre-set by the application before server
* registration (not recommended however), or auto-allocated by the stack
* if the 0 is passed.
*/
uint8_t channel;

/** Server accept callback
Expand Down
29 changes: 23 additions & 6 deletions subsys/bluetooth/host/classic/rfcomm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,32 @@ rfcomm_sessions_lookup_bt_conn(struct bt_conn *conn)

int bt_rfcomm_server_register(struct bt_rfcomm_server *server)
{
if (server->channel < RFCOMM_CHANNEL_START ||
Copy link
Contributor

Choose a reason for hiding this comment

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

`RFCOMM_CHANNEL_START' definition can be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is used to support the dynamic channel allocation if the passed channel is zero.

server->channel > RFCOMM_CHANNEL_END || !server->accept) {
if (server->channel > RFCOMM_CHANNEL_END || !server->accept) {
return -EINVAL;
}

/* Check if given channel is already in use */
if (rfcomm_server_lookup_channel(server->channel)) {
LOG_DBG("Channel already registered");
return -EADDRINUSE;
if (!server->channel) {
uint8_t chan = (uint8_t)BT_RFCOMM_CHAN_DYNAMIC_START;

for (; chan <= RFCOMM_CHANNEL_END; chan++) {
/* Check if given channel is already in use */
if (!rfcomm_server_lookup_channel(chan)) {
server->channel = chan;
LOG_DBG("Allocated channel 0x%02x for new server", chan);
break;
}
}
Comment on lines +209 to +216
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to add one lock to avoid that the same channel is allocated to two servers because higher priority task may preempt after the if.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the style of the Bleutooth stack. For example, the l2cap psm allocation of BLE does not have such protection. And servers is not protected for each updating.

So I think the case your description is not including in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK. Maybe it can be improved later.


if (!server->channel) {
LOG_WRN("No free dynamic rfcomm channels available");
return -EADDRNOTAVAIL;
}
} else {
/* Check if given channel is already in use */
if (rfcomm_server_lookup_channel(server->channel)) {
LOG_WRN("Channel already registered");
return -EADDRINUSE;
}
}

LOG_DBG("Channel 0x%02x", server->channel);
Expand Down
Loading