From 4cb5c1ccf17b46f5908cf364174bf9647e13a30c Mon Sep 17 00:00:00 2001 From: J Date: Thu, 22 Aug 2024 12:30:26 -0400 Subject: [PATCH] bazel build: Fix compilation bugs for Pico-W support (#1797) * Add @pico-sdk prefix to bazel/config in lwip.BUILD Without this, we're trying to refer to a subpackage of the lwip directory called bazel/config, which doesn't exist. See similar references in this file. * bazelbuild: Fix compilation errors with pico_lwip and freertos This fixes two general problems. * pico_lwip_contrib_freertos misspelled several things (omitted contrib/ dir prefix, didn't have @pico-sdk in front of out references to pico-sdk targets) This is fixed simply by fixing the spellings. * Circular dependency between pico_lwip_core and pico_lwip_contrib_freertos. In NO_SYS=0 mode, lwip wants to include sys_arch.h. But sys_arch.h is defined in pico_lwip_contrib_freertos. sys_arch.c in turn wants to include lwip's opt.h and arch.h, among other things. So it needs to depend on pico_lwip_core. This is fixed by extracting all the headers into a common rule which can be depended on by both targets, then depending on it in the relevant targets. Additionally, for the LWIP+FreeRTOS build to work correctly, we need to actually depend on the pico_lwip_contrib_freertos rule from pico_lwip_core. This the purpose of the select in the deps of pico_lwip_core. * bazel+cyw43: Fix compilation errors. This fixes issues with the cyw43 driver build rules in Bazel: * Before this, the btstack would always be included even if it could not be used. If the user did not specify a btstack config, this would cause a compilation error. Now, we condition the linking and building of the btstack on whether there is a config for it. * Before, the btbus was not properly linked. * Implements code review feedback --- src/rp2_common/pico_cyw43_driver/BUILD.bazel | 36 ++++++++++++++---- .../pico_cyw43_driver/cyw43-driver.BUILD | 9 ++++- src/rp2_common/pico_lwip/lwip.BUILD | 38 ++++++++++++++----- 3 files changed, 64 insertions(+), 19 deletions(-) diff --git a/src/rp2_common/pico_cyw43_driver/BUILD.bazel b/src/rp2_common/pico_cyw43_driver/BUILD.bazel index f99018c9..965a0b28 100644 --- a/src/rp2_common/pico_cyw43_driver/BUILD.bazel +++ b/src/rp2_common/pico_cyw43_driver/BUILD.bazel @@ -16,16 +16,10 @@ cc_library( cc_library( name = "pico_cyw43_driver", srcs = [ - "btstack_chipset_cyw43.c", - "btstack_cyw43.c", - "btstack_hci_transport_cyw43.c", "cyw43_bus_pio_spi.c", "cyw43_driver.c", ], hdrs = [ - "include/pico/btstack_chipset_cyw43.h", - "include/pico/btstack_cyw43.h", - "include/pico/btstack_hci_transport_cyw43.h", "include/pico/cyw43_driver.h", ], includes = ["include"], @@ -33,7 +27,6 @@ cc_library( deps = [ ":cyw43_bus_pio", ":cyw43_configport", - "//bazel/config:PICO_BTSTACK_CONFIG", "//src/rp2_common:pico_platform", "//src/rp2_common/hardware_clocks", "//src/rp2_common/hardware_dma", @@ -41,10 +34,37 @@ cc_library( "//src/rp2_common/hardware_pio", "//src/rp2_common/hardware_sync", "//src/rp2_common/pico_async_context", + "//src/rp2_common/pico_unique_id", + "@cyw43-driver//:cyw43_driver", + ] + + # Only depend on the btstack if its configuration is set up. + select({ + "//bazel/constraint:pico_btstack_config_unset": [], + "//conditions:default": [":pico_cyw43_btstack"], + }), +) + +cc_library( + name = "pico_cyw43_btstack", + srcs = [ + "btstack_chipset_cyw43.c", + "btstack_cyw43.c", + "btstack_hci_transport_cyw43.c", + ], + hdrs = [ + "include/pico/btstack_chipset_cyw43.h", + "include/pico/btstack_cyw43.h", + "include/pico/btstack_hci_transport_cyw43.h", + ], + includes = ["include"], + deps = [ + ":cyw43_bus_pio", + ":cyw43_configport", + "//bazel/config:PICO_BTSTACK_CONFIG", "//src/rp2_common/pico_btstack:btstack_run_loop_async_context", "//src/rp2_common/pico_btstack:pico_btstack_base", "//src/rp2_common/pico_btstack:pico_btstack_flash_bank", - "//src/rp2_common/pico_unique_id", + "//src/rp2_common/pico_cyw43_driver/cybt_shared_bus:cybt_shared_bus_driver", "@cyw43-driver//:cyw43_driver", ], ) diff --git a/src/rp2_common/pico_cyw43_driver/cyw43-driver.BUILD b/src/rp2_common/pico_cyw43_driver/cyw43-driver.BUILD index 99b6faeb..58148d86 100644 --- a/src/rp2_common/pico_cyw43_driver/cyw43-driver.BUILD +++ b/src/rp2_common/pico_cyw43_driver/cyw43-driver.BUILD @@ -11,7 +11,14 @@ cc_library( "src/cyw43_stats.c", ], hdrs = glob(["**/*.h"]), - defines = ["CYW43_ENABLE_BLUETOOTH=1"], + defines = select({ + "@pico-sdk//bazel/constraint:pico_btstack_config_unset": [ + "CYW43_ENABLE_BLUETOOTH=0", + ], + "//conditions:default": [ + "CYW43_ENABLE_BLUETOOTH=1", + ], + }), includes = [ "firmware", "src", diff --git a/src/rp2_common/pico_lwip/lwip.BUILD b/src/rp2_common/pico_lwip/lwip.BUILD index 83e2be77..88a5ca17 100644 --- a/src/rp2_common/pico_lwip/lwip.BUILD +++ b/src/rp2_common/pico_lwip/lwip.BUILD @@ -2,18 +2,36 @@ load("@pico-sdk//bazel:defs.bzl", "incompatible_with_config") package(default_visibility = ["//visibility:public"]) +# Some of the LWIP sys_arch.h and the lwip headers depend circularly on one +# another. Include them all in the same target. cc_library( - name = "pico_lwip_core", - srcs = glob(["src/core/*.c"]), + name = "pico_lwip_headers", hdrs = glob(["**/*.h"]), - includes = ["src/include"], - target_compatible_with = incompatible_with_config( - "@pico-sdk//bazel/constraint:pico_lwip_config_unset", - ), + includes = [ + "contrib/ports/freertos/include/arch", + "src/include", + ], deps = [ "@pico-sdk//bazel/config:PICO_LWIP_CONFIG", "@pico-sdk//src/rp2_common/pico_lwip:pico_lwip_config", ], + visibility = ["//visibility:private"], +) + +cc_library( + name = "pico_lwip_core", + srcs = glob(["src/core/*.c"]), + target_compatible_with = incompatible_with_config( + "@pico-sdk//bazel/constraint:pico_lwip_config_unset", + ), + deps = [ + ":pico_lwip_headers", + ] + select({ + "@pico-sdk//bazel/constraint:pico_freertos_unset": [], + "//conditions:default": [ + ":pico_lwip_contrib_freertos", + ], + }), ) cc_library( @@ -151,13 +169,13 @@ cc_library( cc_library( name = "pico_lwip_contrib_freertos", - srcs = ["ports/freertos/sys_arch.c"], - includes = ["ports/freertos/include"], + srcs = ["contrib/ports/freertos/sys_arch.c"], + includes = ["contrib/ports/freertos/include"], target_compatible_with = incompatible_with_config( "@pico-sdk//bazel/constraint:pico_freertos_unset", ), deps = [ - ":pico_lwip_core", - "//bazel/config:PICO_FREERTOS_LIB", + ":pico_lwip_headers", + "@pico-sdk//bazel/config:PICO_FREERTOS_LIB", ], )