Skip to content

Fix bluetooth shell issue #90065

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

Merged
merged 3 commits into from
May 28, 2025
Merged

Fix bluetooth shell issue #90065

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b8159d5
Bluetooth: Shell: Remove redundant spaces in string.
CanWang001 May 16, 2025
a8f1c98
Bluetooth: Shell: Fix issue that BR security level cannot be set to 4.
CanWang001 May 16, 2025
ab43e18
Bluetooth: Shell: Fix issue that BR connection is not selected.
CanWang001 May 16, 2025
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
29 changes: 20 additions & 9 deletions subsys/bluetooth/host/shell/bt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -809,12 +809,7 @@ static void disconnected_set_new_default_conn_cb(struct bt_conn *conn, void *use
}

if (info.state == BT_CONN_STATE_CONNECTED) {
char addr_str[BT_ADDR_LE_STR_LEN];

default_conn = bt_conn_ref(conn);

bt_addr_le_to_str(info.le.dst, addr_str, sizeof(addr_str));
bt_shell_print("Selected conn is now: %s", addr_str);
}
}

Expand All @@ -826,17 +821,33 @@ static void disconnected(struct bt_conn *conn, uint8_t reason)
bt_shell_print("Disconnected: %s (reason 0x%02x)", addr, reason);

if (default_conn == conn) {
struct bt_conn_info info;
enum bt_conn_type conn_type = BT_CONN_TYPE_LE;

if (IS_ENABLED(CONFIG_BT_CLASSIC)) {
conn_type |= BT_CONN_TYPE_BR;
}

bt_conn_get_info(conn, &info);
bt_conn_unref(default_conn);
default_conn = NULL;

/* If we are connected to other devices, set one of them as default */
bt_conn_foreach(BT_CONN_TYPE_LE, disconnected_set_new_default_conn_cb, NULL);
bt_conn_foreach(info.type, disconnected_set_new_default_conn_cb, NULL);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't it be more accurate like this?:

Suggested change
bt_conn_foreach(info.type, disconnected_set_new_default_conn_cb, NULL);
bt_conn_foreach(BT_CONN_TYPE_LE | BT_CONN_TYPE_BR, disconnected_set_new_default_conn_cb, NULL);

I believe it's more accurate because we don't really care about what the type of the disconnected connection is for the purpose of selecting a different connection.

I don't know what the difference between BT_CONN_TYPE_BR and BT_CONN_TYPE_SCO is, so it may be SCO is correct instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi alwa-nordic, as the comments from Thalley, he expects to set a new default conn with the same type first.
Consider a requirement: If there are 4 connections: 2 LE and 2 BR/EDR, and the disconnected one is a BR/EDR, and we want to select the other BR/EDR connection.

SCO disconnection will not reach this callback and will use its own callback.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK. I'll defer to @Thalley's decision.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alternatively we could remove the "select next" feature.

The reasoning is that if I was doing some GATT operations on an LE default_conn, and then it disconnects, then I'd assume I could either continue with GATT operations, or not do any operations.

Perhaps the "select next" feature isn't that useful for multiple different types of connections?

Copy link
Contributor Author

@CanWang001 CanWang001 May 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If this PR #90123 is merged, and we use bt select and br select shell command to select different types of connections. After that, the "select next" may be removed.
Anyway, the "select next" provide the possibility to be unnecessary to enter bt select or br select.

if (default_conn == NULL) {
bt_conn_foreach(conn_type, disconnected_set_new_default_conn_cb, NULL);
}

if (default_conn != NULL) {
conn_addr_str(default_conn, addr, sizeof(addr));
bt_shell_print("Selected conn is now: %s", addr);
}
}
}

static bool le_param_req(struct bt_conn *conn, struct bt_le_conn_param *param)
{
bt_shell_print("LE conn param req: int (0x%04x, 0x%04x) lat %d to %d",
bt_shell_print("LE conn param req: int (0x%04x, 0x%04x) lat %d to %d",
param->interval_min, param->interval_max,
param->latency, param->timeout);

Expand Down Expand Up @@ -3847,7 +3858,7 @@ static int cmd_security(const struct shell *sh, size_t argc, char *argv[])
sec = *argv[1] - '0';

if ((info.type == BT_CONN_TYPE_BR &&
(sec < BT_SECURITY_L0 || sec > BT_SECURITY_L3))) {
(sec < BT_SECURITY_L0 || sec > BT_SECURITY_L4))) {
shell_error(sh, "Invalid BR/EDR security level (%d)", sec);
return -ENOEXEC;
}
Expand Down Expand Up @@ -5121,7 +5132,7 @@ SHELL_STATIC_SUBCMD_SET_CREATE(bt_cmds,
SHELL_CMD_ARG(oob, NULL, HELP_NONE, cmd_oob, 1, 0),
SHELL_CMD_ARG(clear, NULL, "[all] ["HELP_ADDR_LE"]", cmd_clear, 2, 1),
#if defined(CONFIG_BT_SMP) || defined(CONFIG_BT_CLASSIC)
SHELL_CMD_ARG(security, NULL, "<security level BR/EDR: 0 - 3, "
SHELL_CMD_ARG(security, NULL, "<security level BR/EDR: 0 - 4, "
"LE: 1 - 4> [force-pair]",
cmd_security, 1, 2),
SHELL_CMD_ARG(bondable, NULL, HELP_ONOFF, cmd_bondable,
Expand Down