Skip to content

att over br modify patch #458

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

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

att over br modify patch #458

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3530ca4
gatts: fix logical exceptions when the registration service exceeds i…
liuX10 Feb 2, 2026
8fee435
gatts: fix an issue where `bt_conn_unref` was not called after using …
liuX10 Feb 2, 2026
2f1eecf
gatts: add `bt_conn_info_t` when att_over_br connected callbacks report.
liuX10 Feb 2, 2026
72f845e
gatts: add judge about conn_info is NULL.
liuX10 Feb 3, 2026
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
33 changes: 27 additions & 6 deletions service/stacks/zephyr/sal_gatt_server_interface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,10 +409,12 @@ static struct bt_sdp_record* gatt_sdp_create_record(struct bt_gatt_service* srv)

gatt_record->srv = srv;
gatt_record->record = record;
break;
return record;
}

return record;
free(attrs);
Copy link
Contributor

@zhongzhijie1 zhongzhijie1 Feb 12, 2026

Choose a reason for hiding this comment

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

suggest add err log

free(record);
return NULL;
}

static void gatt_sdp_delete_record(struct bt_sdp_record* record)
Expand Down Expand Up @@ -789,6 +791,7 @@ static bt_status_t do_gatts_disconnect(bt_controller_id_t id, bt_address_t* bd_a
}

err = bt_att_br_disconnect(conn);
bt_conn_unref(conn);
if (err) {
BT_LOGE("%s, disconnect fail err:%d", __func__, err);
return BT_STATUS_FAIL;
Expand All @@ -810,13 +813,16 @@ static void zblue_gatts_connected_callback(struct bt_conn* conn)
bt_conn_get_info(conn, &info);
bt_addr_set(&addr, info.br.dst->val);

slot = bt_conn_find(&addr, BT_TRANSPORT_BREDR);
slot = bt_conn_add(&addr, BT_TRANSPORT_BREDR);
if (!slot) {
BT_LOGE("%s, conn slot null", __func__);
Copy link
Contributor

@zhongzhijie1 zhongzhijie1 Feb 12, 2026

Choose a reason for hiding this comment

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

br conn

return;
}

slot->conn = conn;
if (!slot->role) {
slot->role |= GATT_ROLE_SERVER;
}
Comment on lines +823 to +825
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

slot->role is only updated when it is currently 0. If the connection slot already exists with GATT_ROLE_CLIENT set, this will skip setting GATT_ROLE_SERVER, leaving the role flags inconsistent for an ATT-over-BR server connection. Consider always OR-ing in GATT_ROLE_SERVER here (or using bt_conn_set_role() to merge roles consistently).

Suggested change
if (!slot->role) {
slot->role |= GATT_ROLE_SERVER;
}
slot->role |= GATT_ROLE_SERVER;

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@zhongzhijie1 zhongzhijie1 Feb 12, 2026

Choose a reason for hiding this comment

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

it is att over br, we have no gattc role to initial


if_gatts_on_connection_state_changed(&addr, PROFILE_STATE_CONNECTED);
bt_sal_cm_profile_connected_callback(&addr, PROFILE_GATTS, CONN_ID_DEFAULT);
Expand Down Expand Up @@ -1306,10 +1312,21 @@ bt_status_t bt_sal_gatt_server_send_response(bt_controller_id_t id, bt_address_t
if (!addr || request_id == REQUEST_ID_NORSP) {
return BT_STATUS_PARM_INVALID;
}

/* FIXME: If the LE address matches the BREDR address, only the LE connection will send rsp. */
conn = get_le_conn_from_addr(addr);
if (!conn) {
return BT_STATUS_NOT_FOUND;
bt_conn_info_t* info;
BT_LOGW("%s, le conn null", __func__);

info = bt_conn_find(addr, BT_TRANSPORT_BREDR);
conn = info ? info->conn : NULL;
Comment on lines +1319 to +1323
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

For BR/EDR-only operation, get_le_conn_from_addr() will commonly be NULL; logging this as a warning can spam logs on every response. Consider lowering this to debug, or only warning after both LE and BR lookups fail.

Copilot uses AI. Check for mistakes.
if (!conn) {
BT_LOGE("%s, br conn null", __func__);
return BT_STATUS_NOT_FOUND;
}
}

handle = REQUEST_ID_HANDLE(request_id);
op_type = REQUEST_ID_OP_TYPE(request_id);
switch (op_type) {
Expand Down Expand Up @@ -1387,9 +1404,11 @@ bt_status_t bt_sal_gatt_server_send_notification(bt_controller_id_t id, bt_addre
/* FIXME: If the LE address matches the BREDR address, only the LE connection will be notified. */
context.conn = get_le_conn_from_addr(addr);
if (!context.conn) {
bt_conn_info_t* info;
BT_LOGW("%s, le conn null", __func__);
Copy link
Contributor

@zhongzhijie1 zhongzhijie1 Feb 12, 2026

Choose a reason for hiding this comment

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

no le conn, find bredr conn


context.conn = bt_conn_lookup_addr_br((bt_addr_t*)addr);
info = bt_conn_find(addr, BT_TRANSPORT_BREDR);
context.conn = info ? info->conn : NULL;
Comment on lines +1407 to +1411
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

For BR/EDR-only operation, get_le_conn_from_addr() being NULL is expected; logging it as a warning can spam logs for frequent notifications. Consider lowering this to debug, or only warning after both LE and BR lookups fail.

Copilot uses AI. Check for mistakes.
if (!context.conn) {
BT_LOGE("%s, br conn null", __func__);
return BT_STATUS_FAIL;
Expand Down Expand Up @@ -1484,9 +1503,11 @@ bt_status_t bt_sal_gatt_server_send_indication(bt_controller_id_t id, bt_address
/* FIXME: If the LE address matches the BREDR address, only the LE connection will be indicated. */
context.conn = get_le_conn_from_addr(addr);
if (!context.conn) {
bt_conn_info_t* info;
BT_LOGW("%s, le conn null", __func__);
Copy link

@huangyulong3 huangyulong3 Feb 12, 2026

Choose a reason for hiding this comment

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

If this behavior is expected for BREDR, the LOG level here should not be set to warning; INFO is preferable.


context.conn = bt_conn_lookup_addr_br((bt_addr_t*)addr);
info = bt_conn_find(addr, BT_TRANSPORT_BREDR);
context.conn = info ? info->conn : NULL;
if (!context.conn) {
Comment on lines +1506 to 1511
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

For BR/EDR-only operation, get_le_conn_from_addr() being NULL is expected; logging it as a warning can spam logs for frequent indications. Consider lowering this to debug, or only warning after both LE and BR lookups fail.

Copilot uses AI. Check for mistakes.
BT_LOGE("%s, br conn null", __func__);
return BT_STATUS_FAIL;
Expand Down
Loading