pico_btstack/CMakeLists.txt makes PICO_BTSTACK_PATH unusable #1553

Open
opened 2023-11-20 15:14:53 -06:00 by usedbytes · 4 comments
usedbytes commented 2023-11-20 15:14:53 -06:00 (Migrated from github.com)

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 https://github.com/usedbytes/picow_ds4/issues/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 https://github.com/usedbytes/picow_ds4/issues/1

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 https://github.com/usedbytes/picow_ds4/issues/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 https://github.com/usedbytes/picow_ds4/issues/1
peterharperuk commented 2023-11-20 15:35:50 -06:00 (Migrated from github.com)

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

Yes. If there was a cmake file in btstack we would use it.
peterharperuk commented 2023-11-21 04:16:56 -06:00 (Migrated from github.com)

@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 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 commented 2023-11-22 07:16:07 -06:00 (Migrated from github.com)

@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.

@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 commented 2024-06-21 14:47:13 -05:00 (Migrated from github.com)

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

Yeah, note some libraries just provide a CMake file which defines a variable with all the filenames
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-raspberrypi/pico-sdk#1553
No description provided.