treewide: restructure includes #191

eivindj-nordic · 2025-06-27T10:35:50Z

Restructure includes.

Dependent on:
nrfconnect/sdk-zephyr#3008

hermabe

I am thinking of moving all the SoftDevice files into its own subfolder for easier automated deploys, but that doesnt block this move

hermabe · 2025-06-27T10:41:27Z

include/nrf_error.h

Is this correct? The file is called nrf_error.h. Also, would be nice to not encode the SoftDevice variant number in the path. Then this will have to be updated to work with other variants.

No, this should be nrf_error.h.

Would be nice to not have to specify the full path indeed. I'm looking at a way to include the softdevice headers with a softdevice prefix , so it can be referenced by #include <softdevice/nrf_error.h>, though I haven't gotten it to work yet with zephyr_include_directories.

hermabe · 2025-06-27T10:42:24Z

include/nrf_error.h

Why change to having the comment on a different line? The previous one was much more readable.

Code style and maximum line length.

Why change to having the comment on a different line? The previous one was much more readable.

This file is supposed to be a generic error codes header for the whole Bare Metal (when SD not in use), so it needs to comply with the same coding style.

bjarki-andreasen · 2025-06-27T15:10:37Z

A suggestion to avoid clashing with users of ncs bare metal would be to namespace ncs bare metal, as a whole, that is, move every include into

include/nrf/
include/bm/

and since this matches the namespacing of most bm and nrf stuff from what I see, so bm_timer would go in

include/bm/timer.h

and be included like

#include <bm/timer.h>

and nrf_sde.h would go in

include/nrf/sde.h

etc.

Also I believe reserving the namespace ble is way to restrictive on our users, at least hide our ble stuff behind nrf_ble so users can prefix their ble stuff with ble without conflict :) (bm should probably also be behind nrf, since bm could easily clash with users, whereas nrf is not very likely to)

nordicjm · 2025-06-30T06:36:52Z

Yes please as @bjarki-andreasen says, the idea is that users should be able to have this repo or even extend it themselves with their own modules and ncs and bm should work without clashes so namespacing is important to avoid that

eivindj-nordic · 2025-06-30T08:29:18Z

A suggestion to avoid clashing with users of ncs bare metal would be to namespace ncs bare metal, as a whole, that is, move every include into

I agree that might be good to do.

As of SoftDevice headers starting with ble_ that is out of our scope. We need a way to include those without specifying the variant (s115 etc. ) and leave room for more variants to come in the future. If there is a good way with CMake to go from from include/bm/softdevice/s115/ble_gap.h to e.g. #include <bm/softdevice/ble_gap.h> that would be nice. One way could be to do include/bm/softdevice/s115/softdevice/ble_gap.h, though that is a bit long.

bjarki-andreasen · 2025-06-30T10:22:39Z

A suggestion to avoid clashing with users of ncs bare metal would be to namespace ncs bare metal, as a whole, that is, move every include into

I agree that might be good to do.

As of SoftDevice headers starting with ble_ that is out of our scope. We need a way to include those without specifying the variant (s115 etc. ) and leave room for more variants to come in the future. If there is a good way with CMake to go from from include/bm/softdevice/s115/ble_gap.h to e.g. #include <bm/softdevice/ble_gap.h> that would be nice. One way could be to do include/bm/softdevice/s115/softdevice/ble_gap.h, though that is a bit long.

Here's how you do it: Create a seperate include folder, somewhere, it does not need to be in the root of the project for example, I will suggest adding the following tree /subsys/softdevice/s155/include/nrf/softdevice/ Then include that folder by updating

sdk-nrf-bm/subsys/softdevice/CMakeLists.txt

Line 7 in c73d76e

zephyr_include_directories(${ZEPHYR_NRF_BM_MODULE_DIR}/include/s115/)

to

zephyr_include_directories(s155/include)

and place your s155 specific headers like https://github.com/nrfconnect/sdk-nrf-bm/blob/main/include/s115/ble.h in /subsys/softdevice/s155/include/nrf/softdevice/, so /subsys/softdevice/s155/include/nrf/softdevice/ble.h

You can now include

#include <nrf/softdevice/ble.h>

and it will automatically include the correct header based on the CMakeLists.txt entry :)

bjarki-andreasen

Blocking until issues addressed :)

hermabe · 2025-07-01T13:17:54Z

west.yml

Adding some comments for https://github.com/nrfconnect/sdk-zephyr/pull/3008/files here to keep it internal for now:

SD_PPI_* is not defined for hardware with DPPI so that code doesnt work on 54L

The fact that you have to change to #include <softdevice/* is an example of everyone expecting to be able to just include the SoftDevice headers without any prefix. Existing customers will then have to update their code for no gain.

This entry in zephyr was added a couple of months ago for nRF52 devices, so updating that is not an issue.
Though, in the broader context of existing customers then yes, this will be an extra change for them to do. On the other hand, if we want to do this to clarify what is part of the SoftDevice, now is the time for it. What do you think @bjarki-andreasen?

We have spent effort to not make api changes unless necessary so that it is easy to port applications to the new SoftDevice. The reason for this whole project is to make it easy for existing bare-metal 52 customers to move to 54L.

But then again, existing customers are unlikely to use this repo since it doesnt fit into the nrf5 sdk and cannot be used with other RTOSes.

With the suggestion I made the include path to nrf_sd_def.h would be #include <nrf/softdevice/nrf_sd_def.h> in BM, we could also forego nrf for the softdevice (though I would prefer nrf/) and the path would be the same as sdk-zephyr in this case.

How important is it that these paths are identical between the two (bm and sdk-zephyr)?

With the suggestion I made the include path to nrf_sd_def.h would be #include <nrf/softdevice/nrf_sd_def.h>

Which is what would break existing users and I am thinking we should reconsider.

How important is it that these paths are identical between the two (bm and sdk-zephyr)?

It would be unexpected if the different repos had the same header with different include paths. But this is maybe the only place where a SoftDevice header is included in sdk-zephyr.

However, none of this is that important, and assuming no existing user will use this repo the renamings are ok.

Restructure includes. Update Bluetooth -> ble Signed-off-by: Eivind Jølsgard <[email protected]>

b-gent

Doc updates looking ok

eivindj-nordic self-assigned this Jun 27, 2025

eivindj-nordic requested review from a team as code owners June 27, 2025 10:35

eivindj-nordic force-pushed the includes branch 3 times, most recently from 2b7b7fb to 7f56946 Compare June 27, 2025 10:41

hermabe reviewed Jun 27, 2025

View reviewed changes

eivindj-nordic force-pushed the includes branch from 7f56946 to 1b96eb2 Compare June 27, 2025 10:56

eivindj-nordic requested a review from a team as a code owner June 27, 2025 10:56

eivindj-nordic force-pushed the includes branch 3 times, most recently from b964767 to d50c90c Compare June 27, 2025 11:39

eivindj-nordic force-pushed the includes branch 2 times, most recently from ec3e10f to 18b9520 Compare June 30, 2025 08:42

eivindj-nordic requested a review from a team as a code owner June 30, 2025 08:42

eivindj-nordic force-pushed the includes branch 2 times, most recently from eaaae3a to 30aed4a Compare June 30, 2025 08:58

bjarki-andreasen requested changes Jun 30, 2025

View reviewed changes

eivindj-nordic force-pushed the includes branch 2 times, most recently from 1824182 to 3eb1599 Compare July 1, 2025 08:22

eivindj-nordic requested a review from a team July 1, 2025 08:22

eivindj-nordic force-pushed the includes branch from 3eb1599 to 03d0b71 Compare July 1, 2025 12:45

hermabe reviewed Jul 1, 2025

View reviewed changes

treewide: restructure includes and file paths

0211ed6

Restructure includes. Update Bluetooth -> ble Signed-off-by: Eivind Jølsgard <[email protected]>

eivindj-nordic force-pushed the includes branch from 03d0b71 to 0211ed6 Compare July 1, 2025 13:55

b-gent approved these changes Jul 5, 2025

View reviewed changes

eivindj-nordic mentioned this pull request Jul 22, 2025

drivers: LPUARTE #202

Merged

hermabe mentioned this pull request Aug 14, 2025

softdevice: Move all SoftDevice files to separate folder #228

Merged

eivindj-nordic added this to the v0.9.0 milestone Aug 15, 2025

eivindj-nordic closed this Aug 18, 2025

lemrey removed this from the v0.9.0 milestone Sep 12, 2025

Uh oh!

treewide: restructure includes #191

treewide: restructure includes #191

Uh oh!

Conversation

eivindj-nordic commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hermabe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eivindj-nordic Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MirkoCovizzi Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjarki-andreasen commented Jun 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nordicjm commented Jun 30, 2025

Uh oh!

eivindj-nordic commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bjarki-andreasen commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bjarki-andreasen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bjarki-andreasen Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

b-gent left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

eivindj-nordic commented Jun 27, 2025 •

edited

Loading

eivindj-nordic Jun 27, 2025 •

edited

Loading

MirkoCovizzi Jun 27, 2025 •

edited

Loading

bjarki-andreasen commented Jun 27, 2025 •

edited

Loading

eivindj-nordic commented Jun 30, 2025 •

edited

Loading

bjarki-andreasen commented Jun 30, 2025 •

edited

Loading

bjarki-andreasen Jul 1, 2025 •

edited

Loading