Skip to content

tools/rp{2040,23xx}: Ensure that picotool is found or built #16855

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 21, 2025
Merged

tools/rp{2040,23xx}: Ensure that picotool is found or built #16855

merged 1 commit into from
Aug 21, 2025

Conversation

nmaggioni
Copy link
Contributor

@nmaggioni nmaggioni commented Aug 14, 2025 •
edited
Loading

Summary

The current Makefile logic failed silently on some systems, maybe because of a different default shell (ZSH, in my case)? This resulted in builds that completed successfully but generated invalid UF2 files, which were then refused by the uC bootloader.

Now the check is properly enforced and picotool is either found in the $PATH or built from the pico-sdk source.

Impact

No more silently corrupted UF2 files if neither picotool nor pico-sdk are found.

Testing

These changes were tested on a Linux host, targeting a custom Cortex-M0+ (RP2040) board using today's NuttX master.

I would anyway encourage other people to test these changes to ensure that they do not break anyone's workspace:

  1. Follow the NuttX docs for the RP2040 but neither install picotool nor set the PICO_SDK_PATH environment variable. The build should fail explicitly.
  2. Set the PICO_SDK_PATH environment variable and retry. The build should emit a warning, compile picotool, and then succeed.
  3. Restart from scratch and have picotool available somewhere in the $PATH. The build should succeed without any warnings and without compiling any additional tools.

@github-actions github-actions bot added Area: Tooling Size: S The size of the change in this PR is small labels Aug 14, 2025
@nmaggioni
Copy link
Contributor Author

Is the build failing because of the Warning: fpu.c:57:8: warning: #warning "FPU test not built; Only available in the flat build (CONFIG_BUILD_FLAT)" [-Wcpp] warnings on RISC-V targets? I can't replicate the failure, and they seem benign.

I don't think that the issue is caused by my changes anyway?

@xiaoxiang781216
Copy link
Contributor

@nmaggioni what's about this error:

====================================================================================
Configuration/Tool: raspberrypi-pico-2-rv/usbnsh
2025-08-14 18:18:55
------------------------------------------------------------------------------------
  Cleaning...
  Configuring...
  Building NuttX...
tools/Unix.mk:556: *** "Generating UF2 files requires picotool to be available in $PATH, or $PICO_SDK_PATH must be specified".  Stop.
/github/workspace/sources/nuttx/tools/testbuild.sh: line 385: /github/workspace/sources/nuttx/../nuttx/nuttx.manifest: No such file or directory
  [1/1] Normalize raspberrypi-pico-2-rv/usbnsh
====================================================================================
Configuration/Tool: raspberrypi-pico-2-rv/nsh
2025-08-14 18:19:47
------------------------------------------------------------------------------------
  Cleaning...
  Configuring...
  Building NuttX...
tools/Unix.mk:556: *** "Generating UF2 files requires picotool to be available in $PATH, or $PICO_SDK_PATH must be specified".  Stop.
/github/workspace/sources/nuttx/tools/testbuild.sh: line 385: /github/workspace/sources/nuttx/../nuttx/nuttx.manifest: No such file or directory
  [1/1] Normalize raspberrypi-pico-2-rv/nsh

@nmaggioni
Copy link
Contributor Author

@xiaoxiang781216 That means my patch is working as intended and stopping the build if the SDK is missing :)
I thought that pico-sdk was available in CI but apparently not, were broken test binaries being built until now? I'll try to add it, please let me know if I've interpreted the build pipeline correctly.

Let's see if the failure of the risc-v-06 block will still happen or if it's just a weird CI race condition or something.

@github-actions github-actions bot added Area: CI Size: M The size of the change in this PR is medium labels Aug 17, 2025
@nmaggioni nmaggioni force-pushed the nm_rp2040_ensure_picosdk_picotool branch from e3c64a6 to 325d900 Compare August 17, 2025 15:37
xiaoxiang781216
xiaoxiang781216 reviewed Aug 18, 2025
View reviewed changes
jerpelea
jerpelea previously approved these changes Aug 18, 2025
View reviewed changes
@nmaggioni nmaggioni mentioned this pull request Aug 18, 2025
Merged
@nmaggioni nmaggioni force-pushed the nm_rp2040_ensure_picosdk_picotool branch from 325d900 to cf4a2dc Compare August 18, 2025 10:56
@github-actions github-actions bot removed the Area: CI label Aug 18, 2025
@nmaggioni
Copy link
Contributor Author

@xiaoxiang781216 ACK, changes to CI tooling were moved to a dedicated PR.

@nmaggioni nmaggioni marked this pull request as draft August 18, 2025 10:57
@xiaoxiang781216
Copy link
Contributor

@xiaoxiang781216 ACK, changes to CI tooling were moved to a dedicated PR.

@nmaggioni please rebase this pr again.

@nmaggioni nmaggioni force-pushed the nm_rp2040_ensure_picosdk_picotool branch from cf4a2dc to a075737 Compare August 19, 2025 18:41
@nmaggioni nmaggioni marked this pull request as ready for review August 19, 2025 18:41
@nmaggioni
Copy link
Contributor Author

I'll rebase on top of #16876 when possible, that should fix CI.

@simbit18
Copy link
Contributor

@nmaggioni please rebase this pr again.

@nmaggioni
tools/rp{2040,23xx}: Ensure that picotool is found or built
53c5282 
The current logic failed silently on some systems, maybe because of a
different default shell? This resulted in builds that completed
successfully but generated invalid UF2 files, which were refused by
the uC bootloader.

Now the check is properly enforced and picotool is either found in
the $PATH or built from the pico-sdk source.

Signed-off-by: Niccolò Maggioni <nicco.maggioni+nuttx@gmail.com>
@nmaggioni nmaggioni force-pushed the nm_rp2040_ensure_picosdk_picotool branch from a075737 to 53c5282 Compare August 21, 2025 10:47
@xiaoxiang781216 xiaoxiang781216 merged commit 86c466b into apache:master Aug 21, 2025
39 checks passed
@nmaggioni nmaggioni deleted the nm_rp2040_ensure_picosdk_picotool branch August 24, 2025 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jerpelea jerpelea jerpelea approved these changes

@xiaoxiang781216 xiaoxiang781216 xiaoxiang781216 approved these changes

Assignees
No one assigned
Labels
Area: Tooling Size: M The size of the change in this PR is medium Size: S The size of the change in this PR is small
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nmaggioni @xiaoxiang781216 @simbit18 @jerpelea