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/gatts: Modify client supported features READ #1661

Merged
merged 1 commit into from
Dec 22, 2023

Conversation

Roshan23699
Copy link
Contributor

  1. Fix read on client-supported features.
  2. Read cannot return the local client-supported features, it should return the client-supported features for the reading connection. (This change is needed for PTS).

@Roshan23699 Roshan23699 force-pushed the bugfix/read_on_csfc branch 2 times, most recently from 8b7e13c to 0a009b1 Compare December 13, 2023 04:03
@Roshan23699
Copy link
Contributor Author

@sjanc could you PTAL ?

@sjanc
Copy link
Contributor

sjanc commented Dec 14, 2023

Hi,

which test was affected by this?

@Roshan23699
Copy link
Contributor Author

Roshan23699 commented Dec 14, 2023

GATT/SR/GAS/BV-03-C
Maintaining Client-Supported Features Characteristic Instance for Each Client.

* error.
*
*/
int ble_gatts_get_peer_cl_sup_feat(uint16_t conn_handle, uint8_t *out_supported_feat);
Copy link
Contributor

Choose a reason for hiding this comment

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

please name it ble_gatts_peer_cl_sup_feat_get(), also there should be len parameter for how many bytes are available in out_supported_feat array

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

goto done;
}

memcpy(out_supported_feat, conn->bhc_gatt_svr.peer_cl_sup_feat,
Copy link
Contributor

Choose a reason for hiding this comment

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

this should copy no more than N bytes of mentioned length parameter

Copy link
Contributor

Choose a reason for hiding this comment

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

this should copy no more than N bytes of mentioned length parameter

and maybe N should be checked as N < BLE_GATT_CHR_CLI_SUP_FEAT_SZ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with either, if buffer is smaller we can fill just up to its size and ignore rest, currently only 3 bits are defined so it will be 1 for some time anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@Roshan23699 Roshan23699 force-pushed the bugfix/read_on_csfc branch 2 times, most recently from ea27765 to 822025d Compare December 22, 2023 06:56
@sjanc sjanc merged commit 44e0ccc into apache:master Dec 22, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants