Enabling CONFIG_BT_AUDIO with no (audio) services enabled causes build error

[Thalley]

Describe the bug
If CONFIG_BT_AUDIO is enabled, but none of the services (e.g. CONFIG_BT_VCS, CONFIG_BT_MICS) or clients are enabled, then we get a build error stating

  No SOURCES given to Zephyr library: subsys__bluetooth__audio

Because the directory's CMakeLists.txt only has conditionally enabled source files.

To Reproduce
Build e.g. the BT shell project with CONFIG_BT_AUDIO=y and none of the services/clients from subsys/bluetooth/audio/CMakeLists.txt enabled.

Expected behavior
If nothing BLE audio related is enabled but CONFIG_BT_AUDIO=y it should still build without error.

Impact
Very minor, as CONFIG_BT_AUDIO can just be disabled.

Environment (please complete the following information):

• Commit SHA or Version used: 57e7fba

[tejlmand]

Is this really related to #38403 ?

#38403 is because the cleanup in #37512 results in warning in several samples when building those in their default setup.

If nothing BLE audio related is enabled but CONFIG_BT_AUDIO=y it should still build without error.

It's not an error, it's a warning.

And in this case it sounds to me like the has only partially configured a feature.
User has enabled CONFIG_BT_AUDIO=y but not actually configured BT_AUDIO.

To me that sounds like No SOURCES given to Zephyr library: subsys__bluetooth__audio is very appropriate in this particular case because it informs the users that something has actually been forgotten with regards to Bluetooth audio.

A user who does CONFIG_BT_AUDIO=y might be under the impression that audio has been enabled, even if that is not the case. A warning in this case indicates that something is probably not correctly configured.

To me the only other good alternative seems to be if enabling BT_AUDIO can select a sane group of settings, but I doubt that in this case.

[Thalley]

Is this really related to #38403 ?

They are related in my opinion, even if they need different solutions.

To me the only other good alternative seems to be if enabling BT_AUDIO can select a sane group of settings, but I doubt that in this case.

That may be the case later, or perhaps BT_AUDIO will be a "hidden" config that is enabled if any of the subconfigs are enabled.

[tejlmand]

Is this really related to #38403 ?

They are related in my opinion, even if they need different solutions.

yes, I took a deeper look into this, and it is introduced with this commit: 2371284 from same PR, #37512.

So I agree they are then related.

Seems @dcpleung just went ahead and added zephyr_library() everywhere.
That is not good, as it makes this problem even worse:
#8826

What we should strive towards is some clear and logical libraries, and avoid using libzephyr.a as a catch-all library.

So #37512 is good in creating libs where things were placed in libzephyr.a (zephyr_sources()), but should not create new libs on each subdir, unless there is an explicit reason.

[tejlmand]

seems 2371284 was the only glitch in this regard.

[dcpleung]

Since all the indicated source files are compiled only if certain kconfigs are enabled, we can use the ALLOW_EMPTY property to suppress the warning.

As for subsys/bluetooth, I thought about having one big bluetooth subsys library. But since every subdirectories define their own library, I was simply following the convention there. Maybe it's time to group all subsys/bluetooth into one big library?

[tejlmand]

we can use the ALLOW_EMPTY property to suppress the warning.

This was exactly my concern raised here, if introducing such property:
#37910 (comment)

Such a property would be useful in the interrupt controller case, but a general concern is that users simply starts to just do:
zephyr_library_property(ALLOW_EMPTY) instead of fixing things in correct manner in Kconfig or CMake.

That property is intended for special cases, in general we should avoid such situations.
The warning of empty libraries are good and should in general show that something might not be properly configured.

[tejlmand]

As for subsys/bluetooth, I thought about having one big bluetooth subsys library. But since every subdirectories define their own library, I was simply following the convention there. Maybe it's time to group all subsys/bluetooth into one big library?

That was actually part of a big cleanup I tried to long time ago: 8a23d85 (from #8451)
but the elephant was a little big.

So i'm all in favor of continuing that process, but in smaller steps this time.
Fewer and logical libraries, so that it is easier to predict where source files end up.

and not:
#8826
#8825 (comment)

[tejlmand]

@Thalley not convinced this is actually an issue.

Printing a warning is not an error if it informs the user of a potential misconfiguration.

Because i'm not familiar with bluetooth audio I have some questions.

• If a user does CONFIG_BT_AUDIO=y and nothing else, will he be able to use bluetooth audio, or will there be missing pieces ?
• I notice a user might do:
  CONFIG_BT_AUDIO=y
CONFIG_BT_AUDIO_UNICAST=y
  in which case specific selects regarding bluetooth is activated:
  

  zephyr/subsys/bluetooth/audio/Kconfig

  Lines 22 to 30 in 9e2b9c7

  	config BT_AUDIO_UNICAST
  	bool "Bluetooth Unicast Audio Support"
  	select BT_SMP
  	select BT_L2CAP_DYNAMIC_CHANNEL
  	select BT_ISO_UNICAST
  	select BT_GATT_DYNAMIC_DB
  	select BT_GATT_CACHING
  	select BT_L2CAP_ECRED
  	select BT_EATT

  
  but to my understanding, this is just a pre-requisite for using unicast bluetooth audio, it doesn't directly enables the audio feature, correct ?
  At least I failed to find any places in the code that are activated by this particular option:
  grep -rw BT_AUDIO_UNICAST have no hits in source code / headers / elsewhere.
  Only Kconfig / conf files contains this setting.

The summary of the above indicates to me that printing a warning when BT_AUDIO is enabled but no files are placed in the library is correct, as bluetooth audio is not functional in the system in this case.

Please correct me if my observation is wrong.

If a downstream project is relying on CONFIG_BT_AUDIO=y and adding files to it's own downstream library, then that downstream project should probably be cleaned up, and use the zephyr_library_amend() function.

Similar to what is done in entropy, where Zephyr provides the library here:

zephyr/drivers/entropy/CMakeLists.txt

Line 3 in 9e2b9c7

zephyr_library()

but in the sdk-nrf downstream project, it may happen that no files are added to the lib by Zephyr itself, instead this is done in the downstream project, here:
https://github.com/nrfconnect/sdk-nrf/blob/f10756d98c5bec203971771d8700ad81aa0283d4/drivers/entropy/CMakeLists.txt#L6-L7

zephyr_library_amend()
zephyr_library_sources_ifdef(CONFIG_ENTROPY_CC3XX entropy_cc310.c)

If you have a similar use-case for audio, then ensure to add something to the subsys__bluetooth__audio library elsewhere.

[Thalley]

@tejlmand Your observation is correct.

BT_AUDIO is very loosely defined, and refers not only to the new LE audio GATT services such as VCS and MICS (which are upstream), but also to the new audio stream control (which is not yet upstream). Possibly we want to remove the BT_AUDIO Kconfig all together, but we probably need to clean up some additional Kconfigs in the topic-le-audio branch first.

We can probably leave this as is for now (unless the error is too much of a problem), but otherwise it will likely be fixed in the future when we start moving the topic-le-audio branch to the main branch.

[tejlmand]

@Thalley thanks for pointing my attention to the topic-le-audio branch.

And I do see the issue arising there, as BT_AUDIO is also used for adding files bluetooth_host_audio library, without necessarily having anything in the bluetooth_audio lib.

zephyr/subsys/bluetooth/host/CMakeLists.txt

Line 125 in e40e2df

add_subdirectory_ifdef(CONFIG_BT_AUDIO audio)

https://github.com/zephyrproject-rtos/zephyr/blob/topic-le-audio/subsys/bluetooth/host/audio/CMakeLists.txt

I assume the topic-le-audio branch is outside LTS scope ?

In that case I think we should close this issue as an issue because when it appears on zephyr/main it actually indicates a mis-configuration there.
We should of course ensure this is handled before merging the topic-le-audio branch, but question is if we should consider an overhaul of the library structure in the bluetooth subsystem, as discused here:
#38471 (comment)
#38471 (comment)

so that the bluetooth subsys has only a single, or few logically named libraries.
We actually have such task pending already: #8826
So maybe it's time to actually do that now. As bonus, such cleanup will avoid an empty audio library in this case.

[tejlmand]

unless the error is too much of a problem

It's still just a warning:

CMake Warning at /projects/github/ncs/zephyr/CMakeLists.txt:717 (message):
  No SOURCES given to Zephyr library: subsys__bluetooth__audio

  Excluding target from build.

and it is ignored by CI.
But generally speaking, warnings are bad and should be addressed, which we are doing right now.

[Thalley]

@tejlmand

I assume the topic-le-audio branch is outside LTS scope ?

Correct. I am fine by closing this issue if it causes conflicts with bug cleanup for the LTS release. It does, however, still serve as a reminder that something needs to be done w.r.t. the Kconfig an LE Audio, which we hope to get upstreamed to the main branch as soon as the merge window opens again.

[Thalley]

@tejlmand I was just pinged by @carlescufi for this issue.

What do you suggest we do about it? We can close it as you suggested, or we can fix it.

The easiest fix I can think of, is to make BT_AUDIO a hidden Kconfig that is set (select) whenever we enable any of the LE Audio (e.g. VCS or MICS) features. It deals with the issue at hand, but it doesn't scale very well though.

What are your thoughts?

[tejlmand]

@Thalley I think we should change this from a bug to something else, so that we can consider the appropriate way to handle this before topic-le-audio gets merged.

Not sure exactly what it should be turned into, as it's not a bug, neither an enhancement.
Mayde feature-request to be addressed as part of topic-le-audio ?

[Thalley]

Once BAPS have been upstreamed, we should revisit this and refactor the BT_AUDIO Kconfig.

[Thalley]

Will be fixed as part of upstreaming BAPS #41190