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

nimble/host: move att_truncate_to_mtu to ble_att_svr_tx_rsp #1879

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

piotrnarajowski
Copy link
Contributor

No description provided.

@github-actions github-actions bot added host size/S Small PR labels Sep 25, 2024
@piotrnarajowski
Copy link
Contributor Author

#AutoPTS run mynewt GATT/SR --test_case_limit 10

@piotrnarajowski piotrnarajowski marked this pull request as ready for review September 25, 2024 12:05
@piotrnarajowski
Copy link
Contributor Author

@rymanluk please review

@rymanluk rymanluk requested a review from sjanc September 25, 2024 14:05
Copy link
Contributor

@rymanluk rymanluk left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me

@piotrnarajowski
Copy link
Contributor Author

@sjanc please approve workflow

conn = ble_hs_conn_find(conn_handle);
if (conn == NULL) {
hs_status = BLE_HS_ENOTCONN;
goto done;
Copy link
Contributor

Choose a reason for hiding this comment

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

this return with locked hs lock...

also I think ble_hs_conn_chan_find_by_scid() and ble_att_truncate_to_mtu() should also be under lock

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Move att_truncate_to_mtu from ble_eatt_tx to ble_att_svr_tx_rsp.
This could cause some issues when used by client.
@sjanc sjanc merged commit 027a56d into apache:master Oct 28, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DNM do not merge host size/S Small PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants