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

pico_btstack/CMakeLists.txt makes PICO_BTSTACK_PATH unusable #1553

Open
usedbytes opened this issue Nov 20, 2023 · 4 comments
Open

pico_btstack/CMakeLists.txt makes PICO_BTSTACK_PATH unusable #1553

usedbytes opened this issue Nov 20, 2023 · 4 comments
Assignees
Milestone

Comments

@usedbytes
Copy link
Contributor

usedbytes commented Nov 20, 2023

The pico_btstack library CMakeLists.txt hard-codes all of the btstack source files. This means that it's tightly coupled to a specific btstack version.

This is causing problems for my project, picow_ds4 (see usedbytes/picow_ds4#1) which submodules btstack in order to support Playstation 4 controllers (which have HID descriptors which are too big for the upstream default) - and uses PICO_BTSTACK_PATH to use that submoduled version. pico-sdk has updated its btstack, I have not, so now my project doesn't build with an updated pico-sdk.

Having all of the source files listed in a CMakeLists.txt which lives inside pico-sdk seems like a layering violation - it's effectively an unmanaged dependency to a specific version of btstack, which is somewhat fragile.

You're welcome to argue that the submodule provides the versioning :-) - if you have the same version of btstack as what's submoduled in to pico-sdk then there's no issue. However it does make it basically impossible for projects to use PICO_BTSTACK_PATH unless they also submodule pico-sdk, which I'd like to avoid if possible.

I wonder if it would be better to have the pico_btstack build done within btstack itself, as a port, rather than managed by the pico-sdk build directly?

Related to #1379 / #1380 which is the specific PR which caused the particular breakage in usedbytes/picow_ds4#1

usedbytes added a commit to usedbytes/picow_ds4 that referenced this issue Nov 20, 2023
This is needed to be compatible with "new" pico-sdk versions.
See raspberrypi/pico-sdk#1553 and #1

Unfortunately this will break compatibility with older pico-sdk
versions.
@peterharperuk
Copy link
Contributor

Yes. If there was a cmake file in btstack we would use it.

@peterharperuk
Copy link
Contributor

@mringwal Do you have an opinion on this? It would be nice to not hardcode the list of source files in the pico-sdk but just include it from btstack. I think I copied the list from Micropython, so they have the same issue. I imagine I could convert your Makefile.inc files to cmake files with some "magic" but I doubt that would be that robust.

@mringwal
Copy link

@peterharperuk this is mainly relevant while people try stuff that hasn't been used much in the past ;)
In general, I agree with the observations. I guess there are two main options here: a) is to provide a minimal CMakeLists.txt that contain all files from btstack/src that are currently used for inclusion (that would synchronize the list with the actual files). The fancier version is to fully embrace CMake. We started to consolidate our various CMake builds and have an early proof-of-concept (see global-cmake branch). I'm not sure yet, what path we'll use.

@kilograham
Copy link
Contributor

Yeah, note some libraries just provide a CMake file which defines a variable with all the filenames

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants