Fix Pico builds with the io umbrella manifest#1595
Open
HipsterBrown wants to merge 3 commits intoModdable-OpenSource:publicfrom
Open
Fix Pico builds with the io umbrella manifest#1595HipsterBrown wants to merge 3 commits intoModdable-OpenSource:publicfrom
HipsterBrown wants to merge 3 commits intoModdable-OpenSource:publicfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
On v8.0.0, including the umbrella
$(MODDABLE)/modules/io/manifest.jsonin a project targeted at-p picofails to build for the original RP2040 Raspberry Pi Pico (no Wi-Fi). This worked in v7.2.1.Repro:
manifest.json:{ "include": [ "$(MODDABLE)/examples/manifest_base.json", "$(MODDABLE)/modules/io/manifest.json" ], "modules": { "*": "./main" } }main.jsuses onlyembedded:io/pwm+timer.Errors seen:
fatal error: pico/cyw43_arch.h: No such file or directory(modNet.c)fatal error: lwip/tcp.h: No such file or directory(modResolve.c)MODDEF_AUDIOOUT_I2S_DATAOUT_PIN undeclared/MODDEF_AUDIOOUT_I2S_BCK_PIN undeclared/#error LR_PIN must be BCK_PIN + 1(audioout-pico.c)initialization of 'int16_t *' from incompatible pointer type 'uint8_t *'(audioout-pico.c:579)Fixes:
modules/io/manifest.json— barepicoplatform should not force networkingBefore, the bare
picoplatform unconditionally includedmanifests/pico/manifest_net.json, which pullsexamples/manifest_net.jsonand compilesmodNet.c/modResolve.c. Those requirepico/cyw43_arch.handlwip/*— headers that only exist when the build is configured for the CYW43 radio (Pico W / Pico 2 W). This has been resolved by pointing the defaultpicotarget to the original$(IO)/manifests/pico/manifest.jsonand the compatible wireless boards to the new manifest_net.json for the pico. This seems to be a unique case for the pico since the other platforms are "all or nothing" when it comes to wireless connectivity.modules/io/audioout/pico/audioout-pico.c— guardMODDEF_AUDIOOUT_I2S_*definesThe pico audioout source unconditionally references
MODDEF_AUDIOOUT_I2S_DATAOUT_PIN,MODDEF_AUDIOOUT_I2S_BCK_PIN,MODDEF_AUDIOOUT_I2S_LR_PIN, andMODDEF_AUDIOOUT_I2S_BITSPERSAMPLE, but no manifest provides defaults — so any project pulling in audioout without explicit pin overrides fails to compile.The esp32 audioout driver already follows the
#ifndef-with-default pattern, so this PR applies it to pico, defaulting to the pin macros pico-extras already exposes. Pico-extras conventionally setsPICO_AUDIO_I2S_DATA_PIN=9andPICO_AUDIO_I2S_CLOCK_PIN_BASE=10, matching the Pimoroni Pico Audio Pack pinout. Existing manifests that already define these macros are unaffected.modules/io/audioout/pico/audioout-pico.c:579— pointer-type mismatch indoWritebuffer->buffer->bytesisuint8_t *; assigning intoint16_t *is an-Wincompatible-pointer-typeserror under recent GCC. The arithmetic is intentionally byte-offset, so the fix is applying an explicit cast.I have tested these fixes only with my local test program on the Pico. I don't have a Pico W or Pico 2W to test with at the moment.