bluetooth: services: ras: Add initial folder structure for ranging service #18503

alexstanoev-nordic · 2024-11-04T11:47:54Z

This adds the base folder structure and headers to be used in the
implementation of the Ranging Service draft specification.

The ranging requester and ranging responder implementation will follow shortly,
this PR adds common headers and folders between the two modules.

NordicBuilder · 2024-11-04T11:49:58Z

CI Information

^{To view the history of this post, clich the 'edited' button above}
Build number: 6

Inputs:

Sources:

sdk-nrf: PR head: 76d2287b0635dfe0fa34748d5ad344a7f83c29f1

more details

sdk-nrf:

PR head: 76d2287b0635dfe0fa34748d5ad344a7f83c29f1
merge base: 8729fa975dd0e32f79ded7e9af85002d68c8ed32
target head (main): cd64ee95a0387b14cefdf94e31dd65a3f70bc6eb
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 (11)

CODEOWNERS
include
│  ├── bluetooth
│  │  ├── services
│  │  │  │ ras.h
subsys
│  ├── bluetooth
│  │  ├── services
│  │  │  ├── CMakeLists.txt
│  │  │  ├── Kconfig
│  │  │  ├── ras
│  │  │  │  ├── CMakeLists.txt
│  │  │  │  ├── Kconfig.ras
│  │  │  │  ├── ras_internal.h
│  │  │  │  ├── rreq
│  │  │  │  │  │ Kconfig.ras_rreq
│  │  │  │  ├── rrsp
│  │  │  │  │  ├── CMakeLists.txt
│  │  │  │  │  ├── Kconfig.ras_rrsp
│  │  │  │  │  │ ras_rrsp.c

Outputs:

Toolchain

Version: b44b7a08c9
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:b44b7a08c9_912848a074

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

◻️ Toolchain - Skipped: existing toolchain is used
✅ Build twister
- sdk-nrf test count: 783
✅ Integration tests
- ✅ desktop52_verification
- ✅ test_ble_nrf_config
- ✅ test-fw-nrfconnect-ble_samples
- ✅ test-sdk-find-my

Disabled integration tests

- doc-internal
- test-fw-nrfconnect-apps
- test-fw-nrfconnect-ble_mesh
- test-fw-nrfconnect-boot
- test-fw-nrfconnect-chip
- test-fw-nrfconnect-fem
- 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_thingy91
- test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
- test-fw-nrfconnect-nrf_crypto
- test-fw-nrfconnect-proprietary_esb
- test-fw-nrfconnect-rpc
- test-fw-nrfconnect-rs
- test-fw-nrfconnect-tfm
- test-fw-nrfconnect-thread
- test-fw-nrfconnect-zigbee
- test-low-level
- test-sdk-audio
- test-sdk-dfu
- test-sdk-mcuboot
- test-sdk-pmic-samples
- test-sdk-sidewalk
- test-sdk-wifi

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

subsys/bluetooth/services/CMakeLists.txt

NordicBuilder · 2024-11-04T12:12:25Z

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.

include/bluetooth/services/ras.h

olivier-le-sage · 2024-11-04T12:39:55Z

include/bluetooth/services/ras.h

+#define BT_UUID_RAS_RD_READY       BT_UUID_DECLARE_16(BT_UUID_RAS_RD_READY_VAL)
+#define BT_UUID_RAS_RD_OVERWRITTEN BT_UUID_DECLARE_16(BT_UUID_RAS_RD_OVERWRITTEN_VAL)
+
+#define BT_RAS_MAX_SUBEVENTS_PER_PROCEDURE 32


this and the macro below are just needed to calculate the buffer size, right? So that we support the case where there are 256 steps in a procedure, spread across 32 subevents, such that each subevent has 8 steps ? And if I'm not mistaken this assumes a ranging data size of about ~10kB per procedure, including headers?

Maybe we should make this configurable ?

Yes, it's used for buffer sizing and it does end up at around 10kB (9476 bytes):

#define BT_RAS_PROCEDURE_MEM (BT_RAS_RANGING_HEADER_LEN \ + (BT_RAS_MAX_SUBEVENTS_PER_PROCEDURE * BT_RAS_SUBEVENT_HEADER_LEN) \ + (BT_RAS_MAX_STEPS_PER_PROCEDURE * BT_RAS_STEP_MODE_LEN) \ + (BT_RAS_MAX_STEPS_PER_PROCEDURE * BT_RAS_MAX_STEP_DATA_LEN))

The spec says

The RAS Server shall have memory to record at least one complete Ranging Data for one CS Procedure.

But as long as the defaults satisfy this, I think making it configurable makes sense.
I'll remove the macros for now and bring the configs with the rest of the RRSP implementation.

I think that sentence is still satisfied even without the maximum possible procedure size, don't you think so ? "one complete Ranging Data for one CS procedure" can also be a CS procedure with a single subevent and a small number of steps (at least the way I read it), as this would be complete ranging data

It's a bit vague as to what "complete" means, I read it as any valid ranging data procedure the peer might send, including one with the maximum number of steps.
I still agree it makes sense for this to be configurable, since the app will have better expectations on the data length it expects.
Do you think the default should not be 32/256?

include/bluetooth/services/ras.h

subsys/bluetooth/services/ras/ras_internal.h

nordicjm · 2024-11-04T15:00:21Z

subsys/bluetooth/services/ras/rreq/ras_rreq.c

+/*
+ * Copyright (c) 2024 Nordic Semiconductor ASA
+ *
+ * SPDX-License-Identifier: LicenseRef-Nordic-5-Clause


can this file go?

is it causing any issues ? We're trying to merge this in parts/stages

Add the file when you add the contents of the file, it doesn't make sense to add an empty file then add the contents in a different PR

The two files are coming shortly in separate PRs; this has been split for ease of review.
If I don't bring them in then I won't have anything to put in zephyr_library_sources_ifdef for now so it'll be two empty CMakeLists.txt files instead.

I don't have a strong preference, can remove them and have empty cmakelists for now too - in either case the next PRs will follow as soon as this has been merged.

if the cmake files would be empty, they can go, and be added in the PR adding the file contents

Removed ras_rreq.c and the corresponding cmake file

nordicjm · 2024-11-04T15:00:32Z

subsys/bluetooth/services/ras/rrsp/ras_rrsp.c

@@ -0,0 +1,5 @@
+/*
+ * Copyright (c) 2024 Nordic Semiconductor ASA
+ *


Added implementation in b9bfd7f

olivier-le-sage · 2024-11-04T15:15:27Z

include/bluetooth/services/ras.h

+struct ras_subevent_header {
+	/** Starting ACL connection event count for the results reported in the event */
+	uint16_t start_acl_conn_event;
+	/** Frequency compensation value in units of 0.01 ppm (15-bit signed integer) */


here if possible I would suggest referencing BT_HCI_LE_CS_SUBEVENT_RESULT_FREQ_COMPENSATION_NOT_AVAILABLE (or mentioning that 0xC000 is a special value). Otherwise I think the user might not realize they need to check for this value before converting this to a signed integer

…rvice This adds the base folder structure and headers to be used in the implementation of the Ranging Service draft specification. Signed-off-by: Aleksandar Stanoev <[email protected]>

ncs-dragoon is the only codeowner for RAS during development. Signed-off-by: Aleksandar Stanoev <[email protected]>

Create RAS primary service and support reading RRSP features. The initial RAS implementation will not support any optional features. Signed-off-by: Aleksandar Stanoev <[email protected]>

alexstanoev-nordic requested review from a team as code owners November 4, 2024 11:47

alexstanoev-nordic force-pushed the develop/ras_folder_structure branch from 0aab5d1 to 1459343 Compare November 4, 2024 11:55