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.

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

Jump to bottom

driver: creating a cs multiantenna callback #20283

Merged
merged 1 commit into from
Feb 17, 2025
Merged

driver: creating a cs multiantenna callback #20283

merged 1 commit into from
Feb 17, 2025

Conversation

thomas-m-johansen
Copy link
Contributor

in order to allow customers to use their own antenna switches for cs they need a way to link their antenna switching functions to the SDC.

adding the callback for the antenna switch currently supported as well as linking the new callback to the SDC.

@thomas-m-johansen thomas-m-johansen requested a review from a team as a code owner February 11, 2025 07:37
@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 Feb 11, 2025
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Feb 11, 2025 •
edited
Loading

CI Information

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

Inputs:

Sources:

sdk-nrf: PR head: 8830cc57499900a18d94406db4e79dbe03dc970a

more details

sdk-nrf:

PR head: 8830cc57499900a18d94406db4e79dbe03dc970a
merge base: 3b2e0b5a67428f69fdd067c1379e33ad977a9b53
target head (main): 87fef230aa647b5dec2ad7770ef04e0c43f374bf
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 (4) 
subsys
│  ├── bluetooth
│  │  ├── controller
│  │  │  ├── CMakeLists.txt
│  │  │  ├── cs_antenna_switch.c
│  │  │  ├── cs_antenna_switch.h
│  │  │  │ hci_driver.c

Outputs:

Toolchain

Version: 342151af73
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:342151af73_bbe5b33786

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

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister - Skipped: Skipping Build & Test as it succeeded in a previous run: 9
  • ✅ Integration tests
    • ✅ desktop52_verification - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test_ble_nrf_config
    • ✅ test-fw-nrfconnect-ble_samples
    • ✅ test-fw-nrfconnect-chip - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-fw-nrfconnect-zigbee - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-sdk-find-my - Skipped: Job was skipped as it succeeded in a previous run
    • ✅ test-sdk-sidewalk - Skipped: Job was skipped as it succeeded in a previous run
Disabled integration tests
    • doc-internal
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-boot
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_mosh
    • 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-rs
    • 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

olivier-le-sage
olivier-le-sage reviewed Feb 11, 2025
View reviewed changes
subsys/bluetooth/controller/cs_antenna_switch.h

#include <stdint.h>

/** @brief Antenna switching Callback for use in Channel Sounding.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/** @brief Antenna switching Callback for use in Channel Sounding.
/** @brief Antenna switching callback for use in Channel Sounding.

@thomas-m-johansen thomas-m-johansen force-pushed the add_cs_antenna_switching_callback branch from 1d30341 to 0909b64 Compare February 11, 2025 09:18
@thomas-m-johansen thomas-m-johansen requested a review from a team as a code owner February 11, 2025 09:18
nordicjm
nordicjm requested changes Feb 11, 2025
View reviewed changes
subsys/bluetooth/controller/cs_antenna_switch.c Outdated
Comment on lines 7 to 8
#include <stdint.h>
#include "cs_antenna_switch.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

odd indent

subsys/bluetooth/controller/cs_antenna_switch.c
Comment on lines +12 to +14
#define DEFAULT_CS_ANTENNA_GPIO_PORT NRF_P1
#define DEFAULT_CS_ANTENNA_BASE_PIN (11)
#define DEFAULT_CS_ANTENNA_PIN_MASK (0xF << DEFAULT_CS_ANTENNA_BASE_PIN)
Copy link
Contributor

Choose a reason for hiding this comment

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

there should be no default in a C file, it should come from dts

Copy link
Contributor

@olivier-le-sage olivier-le-sage Feb 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, I agree with you that, eventually, this will come from DTS. That is actually what we have planned in our team.

However in this first step we're keeping our existing hard-coded controller code (just moving it from the compiled library to these open source files) so that we can make this API change, which is a multi-step process.

We have not yet settled on what exactly the DTS configuration or its docs will look like and may need to collect more customer feedback before implementing it, so there is no point delaying this PR for that.

@thomas-m-johansen thomas-m-johansen force-pushed the add_cs_antenna_switching_callback branch from 0909b64 to 603a8f6 Compare February 12, 2025 08:06
@thomas-m-johansen
Copy link
Contributor Author

thomas-m-johansen commented Feb 12, 2025 •
edited
Loading

had to add a init function for the pins being used by the antenna switch, while these are currently being set up by the SDC, it's better to assume that the SDC will not have this responsibility indefinitely.

@thomas-m-johansen thomas-m-johansen force-pushed the add_cs_antenna_switching_callback branch from 603a8f6 to ee91c0a Compare February 12, 2025 11:54
@thomas-m-johansen thomas-m-johansen force-pushed the add_cs_antenna_switching_callback branch from ee91c0a to e7660ed Compare February 12, 2025 13:22
@thomas-m-johansen thomas-m-johansen force-pushed the add_cs_antenna_switching_callback branch from e7660ed to 7b5ecfe Compare February 13, 2025 07:08
@thomas-m-johansen
Copy link
Contributor Author

rebased in hope that it would fix CI errors

@olivier-le-sage
Copy link
Contributor

The SDC manifest has been updated: #20321

@thomas-m-johansen
driver: creating a cs multiantenna callback
8830cc5 
in order to allow customers to use their own antenna switches for cs
they need a way to link their antenna switching functions to the SDC.

adding the callback for the antenna switch currently supported as well
as linking the new callback to the SDC.

also added a function to enable the pins used for antenna switching so
the GPIO has the correct DIR set.

Signed-off-by: Thomas Johansen <thomas.johansen@nordicsemi.no>
@thomas-m-johansen thomas-m-johansen force-pushed the add_cs_antenna_switching_callback branch from 7b5ecfe to 8830cc5 Compare February 14, 2025 12:45
@nordicjm nordicjm merged commit c223901 into nrfconnect:main Feb 17, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nordicjm nordicjm nordicjm approved these changes

@olivier-le-sage olivier-le-sage olivier-le-sage approved these changes

Assignees
No one assigned
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@thomas-m-johansen @NordicBuilder @olivier-le-sage @nordicjm