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

bluetooth: controller: option to defer unknown VS commands #18112

Closed

Conversation

JordanYates
Copy link
Contributor

@JordanYates JordanYates commented Oct 23, 2024

Add a Kconfig option to enable deferring the handling of unknown vendor specific commands to the application, instead of unconditionally dropping them.

The expected use case for this option is for re-using the already present HCI link between two separate microcontrollers (e.g. nRF91 + nRF52) as a communications channel. The obvious way to enable this is through a vendor specific command, but this requires the HCI shim to not unconditionally drop commands it does not know about.

An alternate implementation option is to register a callback function with a dedicated function (sdc_register_default_vs_handler or similar), instead of hardcoding the expected function name. The current approach does have precedence in the LTE modem fault handling code however.

Changelog will be added once implementation is confirmed

Add a Kconfig option to enable deferring the handling of unknown vendor
specific commands to the application, instead of unconditionally
dropping them.

Signed-off-by: Jordan Yates <[email protected]>
@JordanYates JordanYates requested review from a team as code owners October 23, 2024 06:18
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Oct 23, 2024
@NordicBuilder
Copy link
Contributor

Thank you for your contribution!
It seems you are not a member of the nrfconnect GitHub organization. External contributions are handled as follows:
Large contributions, affecting multiple subsystems for example, may be rejected if they are complex, may introduce regressions due to lack of test coverage, or if they are not consistent with the architecture of nRF Connect SDK.
PRs will be run in our continuous integration (CI) test system.
If CI passes, PRs will be tagged for review and merged on successful completion of review. You may be asked to make some modifications to your contribution during review.
If CI fails, PRs may be rejected or may be tagged for review and rework.
PRs that become outdated due to other changes in the repository may be rejected or rework requested.
External contributions will be prioritized for review based on the relevance to current development efforts in nRF Connect SDK. Bug fix PRs will be prioritized.
You may raise issues or ask for help from our Technical Support team by visiting https://devzone.nordicsemi.com/.

Note: This comment is automatically posted and updated by the Contribs GitHub Action.

@NordicBuilder NordicBuilder added the external External contribution label Oct 23, 2024
@nordicjm nordicjm added the CI-Requested Approves single commit for CI tests on Internal HW label Oct 23, 2024
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Oct 23, 2024

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 1

Inputs:

Sources:

sdk-nrf: PR head: 60b134a2e633c41f6b8d48df66eddf4c02759361

more details

sdk-nrf:

PR head: 60b134a2e633c41f6b8d48df66eddf4c02759361
merge base: 6f5194b6cac97d8bea615bcc0629fd43dfc679d8
target head (main): 6f5194b6cac97d8bea615bcc0629fd43dfc679d8
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (2)
subsys
│  ├── bluetooth
│  │  ├── controller
│  │  │  ├── Kconfig
│  │  │  │ hci_internal.c

Outputs:

Toolchain

Version: 3dd8985b56
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:3dd8985b56_912848a074

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 885
  • ❌ Integration tests
    • ❌ desktop52_verification
    • ✅ test_ble_nrf_config
    • ❌ test-fw-nrfconnect-ble_samples
    • ❌ test-fw-nrfconnect-chip
    • ❌ test-fw-nrfconnect-rs
    • ✅ test-fw-nrfconnect-fem
    • ❌ test-fw-nrfconnect-zigbee
    • ❌ test-sdk-find-my
    • ❌ test-sdk-sidewalk
Disabled integration tests
    • doc-internal
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-boot
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_cloud
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_mosh
    • test-fw-nrfconnect-nrf-iot_nrf_provisioning
    • test-fw-nrfconnect-nrf-iot_positioning
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-proprietary_esb
    • test-fw-nrfconnect-ps
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-tfm
    • test-fw-nrfconnect-thread
    • test-low-level
    • test-sdk-audio
    • test-sdk-dfu
    • test-sdk-mcuboot
    • test-sdk-pmic-samples
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

@NordicBuilder
Copy link
Contributor

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

Copy link
Contributor

@rugeGerritsen rugeGerritsen left a comment

Choose a reason for hiding this comment

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

I believe the usecase can already be fulfilled with the existing hci_internal_user_cmd_handler_register()

@JordanYates
Copy link
Contributor Author

So it is, although the hci_internal_user_cmd_handler_register function isn't part of a public header

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. CI-Requested Approves single commit for CI tests on Internal HW external External contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants