"// pico_cmake_set" not seen in included header files.

[Memotech-Bill]

If a board header file includes other header files, then CMake does not see any pico_cmake_set directives in the secondary header file.

This particularly affects the vgaboard.h header file, which includes pico.h. None of the pico settings are seen by CMake. This does not matter in this case as all the settings are default values anyway.

However, it is not possible to simply change pico.h to pico2.h for a VGA board with pico2 installed, as CMake will still think it is compiling for RP2040 rather than RP2350.

[lurch]

Interesting. pico_cmake_set (and pico_cmake_set_default) is implemented in https://github.com/raspberrypi/pico-sdk/blob/develop/cmake/generic_board.cmake so I guess it might be a bit tricky to update it to also look at any #inclue files?
ping @will-v-pi to see if he has any ideas, as he recently tweaked this in #2034

[lurch]

FWIW, https://github.com/raspberrypi/pico-sdk/blob/develop/tools/check_board_header.py does look at #inclue files 😆

[will-v-pi]

We could add parsing for the #include lines to generic_board.cmake - for example create a PICO_BOARD_HEADER_FILES variable and run the current if (EXISTS ${PICO_BOARD_HEADER_FILE}) on all of them in order?

[lurch]

I think the search for #include lines would need to be recursive though, since you might well have nested #includes?

[will-v-pi]

That's true - in which case maybe add the parsing into the while(HEADER_FILE_CONTENTS) loop, and append to HEADER_FILE_CONTENTS when you see an include directive. This would also have the effect of processing the cmake_set_defaults from the included header after the new header, which I think is the preferred way round (so you can override them in the new header)?

[lurch]

I guess appending to HEADER_FILE_CONTENTS (rather than doing an "in-place insert") might theoretically cause a problem if you had something like:

file1.h

// pico_cmake_set VAR1=VAL1
// pico_cmake_set VAR2=VAL2
#include "file2.h"
// pico_cmake_set VAR3=VAL3

file2.h

// pico_cmake_set VAR2=VAL4
// pico_cmake_set VAR3=VAL5
// pico_cmake_set VAR4=VAL6

I think you'd "expect" VAR1=VAL1, VAR2=VAL4, VAR3=VAL3, VAR4=VAL6 but if you only appended to HEADER_FILE_CONTENTS when encountering an #include then you'd actually end up with VAR1=VAL1, VAR2=VAL4, VAR3=VAL5, VAR4=VAL6 ?
But this may not actually be a problem, and doing a simple append-only approach might be "good enough" for the cases where pico_cmake_set is actually used?

[will-v-pi]

That's very true (with the opposite expected for cmake_set_default) - so it would be better to pre-pend the new file to HEADER_FILE_CONTENTS (the same as inserting in-place as we're popping from the front), which should be simple enough.

I'll steal your regex from check_board_header.py and whip up a PR

[Memotech-Bill]

I have wondered whether BOARD could optionally be a list, rather than a single filename.

In that way, a pico2_w on a VGA board might be defined as:

BOARD=pico2_w,vgaboard

That would save having to have multiple definitions for any of the boards which will accept any flavour of pico.

[lurch]

I have wondered whether BOARD could optionally be a list, rather than a single filename.

Ugh no. That might lead to people thinking that BOARD=pico2,pico_w is the same thing as BOARD=pico2_w.

I guess some concept of CARRIER_BOARD might be useful (see e.g. #696 and #837 ), but I guess that's a much bigger change.

[lurch]

@Memotech-Bill Would you like to give #2113 a try please, and see if it behaves as you expect?

[Memotech-Bill]

@Memotech-Bill Would you like to give #2113 a try please, and see if it behaves as you expect?

Yes, on a quick test it seems to work as expected.

vgaboard_cut_2w.h

/*
 * Copyright (c) 2020 Raspberry Pi (Trading) Ltd.
 *
 * SPDX-License-Identifier: BSD-3-Clause
 */

// Revised version for when the links between GPIOs 20 & 21 and SD Data 1 & 2
// have been broken, to allow both the default UART and the SD card to be used,
// the latter in 1-bit or SPI modes.
// Memotech-Bill, March 2022.

// -----------------------------------------------------
// NOTE: THIS HEADER IS ALSO INCLUDED BY ASSEMBLER SO
//       SHOULD ONLY CONSIST OF PREPROCESSOR DIRECTIVES
// -----------------------------------------------------

#ifndef _BOARDS_VGABOARD_H
#define _BOARDS_VGABOARD_H

// For board detection
#define RASPBERRYPI_VGABOARD_CUT

// Audio pins. I2S BCK, LRCK are on the same pins as PWM L/R.
// - When outputting I2S, PWM sees BCK and LRCK, which should sound silent as
//   they are constant duty cycle, and above the filter cutoff
// - When outputting PWM, I2S DIN should be low, so I2S should remain silent.
#define VGABOARD_I2S_DIN_PIN 26
#define VGABOARD_I2S_BCK_PIN 27
#define VGABOARD_I2S_LRCK_PIN 28

#define VGABOARD_PWM_L_PIN 28
#define VGABOARD_PWM_R_PIN 27

#define VGABOARD_VGA_COLOR_PIN_BASE 0
#define VGABOARD_VGA_SYNC_PIN_BASE 16

// Note DAT1/2 are shared with UART TX/RX (pull jumpers off header to access
// UART pins and disconnect SD DAT1/2)
#define VGABOARD_SD_CLK_PIN 5
#define VGABOARD_SD_CMD_PIN 18
#define VGABOARD_SD_DAT0_PIN 19

// Note buttons are shared with VGA colour LSBs -- if using VGA, you can float
// the pin on VSYNC assertion and sample on VSYNC deassertion
#define VGABOARD_BUTTON_A_PIN 0
#define VGABOARD_BUTTON_B_PIN 6
#define VGABOARD_BUTTON_C_PIN 11

#ifndef PICO_DEFAULT_UART
#define PICO_DEFAULT_UART 1
#endif

#ifndef PICO_DEFAULT_UART_TX_PIN
#define PICO_DEFAULT_UART_TX_PIN 20
#endif

#ifndef PICO_DEFAULT_UART_RX_PIN
#define PICO_DEFAULT_UART_RX_PIN 21
#endif

#define PICO_SCANVIDEO_COLOR_PIN_BASE VGABOARD_VGA_COLOR_PIN_BASE
#define PICO_SCANVIDEO_SYNC_PIN_BASE VGABOARD_VGA_SYNC_PIN_BASE

#define PICO_SD_CLK_PIN VGABOARD_SD_CLK_PIN
#define PICO_SD_CMD_PIN VGABOARD_SD_CMD_PIN
#define PICO_SD_DAT0_PIN VGABOARD_SD_DAT0_PIN

// 1 or 4
#ifndef PICO_SD_DAT_PIN_COUNT
#define PICO_SD_DAT_PIN_COUNT 1
#endif

// 1 or -1
#define PICO_SD_DAT_PIN_INCREMENT 1

#define PICO_AUDIO_I2S_DATA_PIN VGABOARD_I2S_DIN_PIN
#define PICO_AUDIO_I2S_CLOCK_PIN_BASE VGABOARD_I2S_BCK_PIN

#define PICO_AUDIO_PWM_L_PIN VGABOARD_PWM_L_PIN
#define PICO_AUDIO_PWM_R_PIN VGABOARD_PWM_R_PIN

#define PICO_VGA_BOARD

// vgaboard has a Pico on it, so default anything we haven't set above

#include "boards/pico2_w.h"

#endif

Start of build process, confirming that RP2350 has been selected:

pi@raspberrypi:~/pico/PicoBB/console/pico_w $ make BOARD=vgaboard_cut_2w
rm -rf build
mkdir -p build
cd build && PICO_SDK_PATH=/home/pi/pico/pico-sdk \
	cmake -DPICO_BOARD=vgaboard_cut_2w -DSTDIO=USB+UART -DLFS=Y -DFAT=N -DSOUND=SDL \
		-DSTACK_CHECK=4 -DMIN_STACK=Y -DPRINTER= -DSERIAL_DEV=1 \
		-DCYW43=BACKGROUND -DBBC_SRC=../../BBCSDL -DGRAPH= -DOS_RAM= -DUSB_CON=0 \
		-DNET_HEAP=1 -DLFS_ORIG=1024  --no-warn-unused-cli -S ../../../src/pico -B .
Not searching for unused variables given on the command line.
Using PICO_SDK_PATH from environment ('/home/pi/pico/pico-sdk')
PICO_SDK_PATH is /home/pi/pico/pico-sdk
Target board (PICO_BOARD) is 'vgaboard_cut_2w'.
Using board configuration from /home/pi/pico/PicoBB/boards/vgaboard_cut_2w.h
Auto-converting non-specific PICO_PLATFORM='rp2350' to 'rp2350-arm-s'
Defaulting platform (PICO_PLATFORM) to 'rp2350-arm-s' based on PICO_BOARD setting.
-- Defaulting build type to 'Release' since not specified.
Defaulting compiler (PICO_COMPILER) to 'pico_arm_cortex_m33_gcc' since not specified.
Configuring toolchain based on PICO_COMPILER 'pico_arm_cortex_m33_gcc'
Defaulting PICO_GCC_TRIPLE to 'arm-none-eabi'
-- The C compiler identification is GNU 8.3.1
-- The CXX compiler identification is GNU 8.3.1
-- The ASM compiler identification is GNU
-- Found assembler: /usr/bin/arm-none-eabi-gcc
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/arm-none-eabi-gcc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/arm-none-eabi-g++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
Build type is Release
-- Found Python3: /usr/bin/python3.9 (found version "3.9.2") found components: Interpreter 
TinyUSB available at /home/pi/pico/pico-sdk/lib/tinyusb/src/portable/raspberrypi/rp2040; enabling build support for USB.
BTstack available at /home/pi/pico/pico-sdk/lib/btstack
cyw43-driver available at /home/pi/pico/pico-sdk/lib/cyw43-driver
Pico W Bluetooth build support available.
lwIP available at /home/pi/pico/pico-sdk/lib/lwip
Pico W Wi-Fi build support available.
mbedtls available at /home/pi/pico/pico-sdk/lib/mbedtls
Using PICO_EXAMPLES_PATH from environment ('/home/pi/pico/pico-examples')
-- PICO_CHIP set to rp2350
-- BBC BASIC Upstream source from ../../BBCSDL
-- Custom command to build /home/pi/pico/PicoBB/console/pico_w/build/pico_stub.c
-- Custom command to build /home/pi/pico/PicoBB/console/pico_w/build/sympico.h
-- Using upstream expression evaluation code with REDUCE_STACK_SIZE
-- Using memory protection to detect stack overrun
-- Pico W CYW43 background support

And confirmation that the resulting executable is for RP2350:

pi@raspberrypi:~/pico/PicoBB/console/pico_w $ picotool info bbcbasic_pwc.uf2 
File bbcbasic_pwc.uf2 family ID 'rp2350-arm-s':

Program Information
 name:          bbcbasic
 description:   BBC BASIC for Pico 2W Console v0.47, Build Nov 28 2024, USB Console, UART Console, Flash Filesystem, cyw43=background,
                SDL Sound, /dev/uart, Min Stack, Stack Check 4, RTC
 features:      UART stdin / stdout
                UART stdout
                UART stdin
                USB stdin / stdout
 binary start:  0x10000000
 binary end:    0x100997a0
 target chip:   RP2350
 image type:    ARM Secure

I will have to wait until the change reaches the master branch of the SDK before I can make use of it in my repositories. Otherwise anyone else attempting to build my code will fail to get the intended CMake configuration.

[kilograham]

If a board header file includes other header files, then CMake does not see any pico_cmake_set directives in the secondary header file.

This particularly affects the vgaboard.h header file, which includes pico.h. None of the pico settings are seen by CMake. This does not matter in this case as all the settings are default values anyway.

However, it is not possible to simply change pico.h to pico2.h for a VGA board with pico2 installed, as CMake will still think it is compiling for RP2040 rather than RP2350.

sort of collapsing replies to lots of things above:

This is a known issue (perhaps not well documented)... w.r.t. vgaboard (and others), we intend to make a PICO_CARRIER_BOARD (or CARRIED board) to cover those, which could have helped, but will think about recursion too... In this case probably a good idea

[Memotech-Bill]

we intend to make a PICO_CARRIER_BOARD (or CARRIED board) to cover those, which could have helped, but will think about recursion too... In this case probably a good idea

This should perhaps be PICO_ADDON_BOARDS, and should probably have the capability to be a list. Unlike the RPi, Pico addon boards do not have configuration EEPROMs, so it is straightforward to stack multiple addon boards.

I can't call them HATs as the usually go underneath the Pico, not on top.