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: Add support for ext adv param v2 HCI command #1851

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

rahult-github
Copy link
Contributor

BLE spec version 5.4 introduced support for host to pass primary and secondary phy options in set extended advertising parameters. For this a new HCI command "HCI_LE_Set_Extended_Advertising_Parameters v[2] " with OCF 0x007F got introduced.

This patch adds support in host for this HCI command.

@rahult-github
Copy link
Contributor Author

rahult-github commented Aug 14, 2024

Hi @sjanc , @andrzej-kaczmarek please take a look and review

@rahult-github
Copy link
Contributor Author

Hi @sjanc , please review.

@rahult-github
Copy link
Contributor Author

Hi @sjanc , @andrzej-kaczmarek , please let me know in case of any further comments

@sjanc
Copy link
Contributor

sjanc commented Aug 20, 2024

until now we didn't have need for V2 (or more) HCI commands in host but I think we should start tracking controller supported commands.

I believe we should be able to simply extend host API and if no preference is selected we can fallback to V1 if V2 is not supported, in case of preference set by app, we fail if V2 is not supported

@rahult-github
Copy link
Contributor Author

until now we didn't have need for V2 (or more) HCI commands in host but I think we should start tracking controller supported commands.

I believe we should be able to simply extend host API and if no preference is selected we can fallback to V1 if V2 is not supported, in case of preference set by app, we fail if V2 is not supported

So current implementation to read supported commands is only for event mask2 . I have extended it to use all commands bits ( so that future implementation can use it as needed).

@rahult-github rahult-github force-pushed the feat/add_ext_adv_param_v2 branch 3 times, most recently from 9e78666 to b3c89da Compare August 23, 2024 12:07
@github-actions github-actions bot added the size/M Medium PR label Aug 23, 2024
nimble/include/nimble/hci_common.h Outdated Show resolved Hide resolved
nimble/host/include/host/ble_gap.h Outdated Show resolved Hide resolved
nimble/host/src/ble_gap.c Show resolved Hide resolved
@rahult-github rahult-github force-pushed the feat/add_ext_adv_param_v2 branch 4 times, most recently from 4fb8ee9 to 28906f6 Compare August 26, 2024 13:05
@sjanc
Copy link
Contributor

sjanc commented Sep 6, 2024

looks good to me! but please fix those few style issues reported by CI

@rahult-github
Copy link
Contributor Author

looks good to me! but please fix those few style issues reported by CI

Thank you. Have pushed again.

Copy link
Contributor

@sjanc sjanc left a comment

Choose a reason for hiding this comment

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

CI failures are due to regression in newt tool

@sjanc sjanc merged commit 7af0b74 into apache:master Sep 9, 2024
15 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
host size/M Medium PR size/S Small PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants