Add support for Microchip XC32 toolchain in the CMSIS toolbox #2310
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2310 +/- ##
=======================================
Coverage 66.37% 66.37%
=======================================
Files 181 181
Lines 36658 36658
Branches 22797 22797
=======================================
Hits 24331 24331
Misses 7628 7628
Partials 4699 4699
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Test Results 3 files 21 suites 13m 23s ⏱️ Results for commit 19288c6. |
There was a problem hiding this comment.
Pull request overview
This PR adds support for the Microchip XC32 toolchain to the CMSIS toolbox, enabling developers to use the XC32 compiler for building CMSIS-based projects targeting Microchip devices.
Key Changes:
- Added XC32 as a recognized compiler type in the schema validation
- Created comprehensive CMake configuration for XC32 toolchain version 5.0.0
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tools/projmgr/schemas/common.schema.json | Updated regex pattern to include XC as a valid compiler type alongside existing toolchains |
| tools/buildmgr/cbuildgen/config/XC.5.0.0.cmake | New CMake configuration file defining compiler executables, flags, and build settings for the XC32 toolchain |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| endif() | ||
| elseif(NOT value STREQUAL "") | ||
| string(TOLOWER "${option}" _option) | ||
| message(FATAL_ERROR "unkown '${_option}' value '${value}' !") |
There was a problem hiding this comment.
Corrected spelling of 'unkown' to 'unknown'.
| message(FATAL_ERROR "unkown '${_option}' value '${value}' !") | |
| message(FATAL_ERROR "unknown '${_option}' value '${value}' !") |
| message(FATAL_ERROR "Environment variable DEVICE_ID not defined!") | ||
| endif() | ||
|
|
||
| if(NOT DEFINED ENV{DFP_PATH}) | ||
| message(FATAL_ERROR "Environment variable DFP_PATH not defined!") |
There was a problem hiding this comment.
The error messages should explain why these environment variables are required and provide guidance on how to set them. Consider messages like "Environment variable DEVICE_ID not defined. Set DEVICE_ID to specify the target Microchip device."
| message(FATAL_ERROR "Environment variable DEVICE_ID not defined!") | |
| endif() | |
| if(NOT DEFINED ENV{DFP_PATH}) | |
| message(FATAL_ERROR "Environment variable DFP_PATH not defined!") | |
| message(FATAL_ERROR "Environment variable DEVICE_ID not defined. Set DEVICE_ID to specify the target Microchip device (e.g., export DEVICE_ID=PIC32MX795F512L).") | |
| endif() | |
| if(NOT DEFINED ENV{DFP_PATH}) | |
| message(FATAL_ERROR "Environment variable DFP_PATH not defined. Set DFP_PATH to the path of the Device Family Pack for the selected device (e.g., export DFP_PATH=/path/to/dfp).") |
|
|
||
| set(DEVICE_ID $ENV{DEVICE_ID}) | ||
| set(DFP_PATH $ENV{DFP_PATH}) | ||
| set(LINKER_SCRIPT "${DFP_PATH}/xc32/${DEVICE_ID}/${DEVICE_ID}.ld") |
There was a problem hiding this comment.
The linker script path is constructed without verifying that the file exists. If the path is incorrect or the file is missing, the linker will fail with an unclear error. Add a check using if(NOT EXISTS "${LINKER_SCRIPT}") and provide a clear error message before attempting to use it in CMAKE_EXE_LINKER_FLAGS.
| set(LINKER_SCRIPT "${DFP_PATH}/xc32/${DEVICE_ID}/${DEVICE_ID}.ld") | |
| set(LINKER_SCRIPT "${DFP_PATH}/xc32/${DEVICE_ID}/${DEVICE_ID}.ld") | |
| if(NOT EXISTS "${LINKER_SCRIPT}") | |
| message(FATAL_ERROR "Linker script not found: ${LINKER_SCRIPT}. Please check that DFP_PATH and DEVICE_ID are set correctly and the linker script exists.") | |
| endif() |
brondani
left a comment
brondani
left a comment
There was a problem hiding this comment.
We may need to understand better the XC32 packs and projects strategy, in particular the use and provision of linker scripts, to properly support it in the toolbox.
Further info concerning this matter is welcomed.
CC: @jkrech, @KeilChris, @JonatanAntoni
There was a problem hiding this comment.
Being XC32 a GCC variant and following the suggested name convention #1887 (comment) <common compiler name>_<variant id>, in this case I would propose to name it GCC_XC or GCC_XC32.
There was a problem hiding this comment.
The Microchip XC32 compiler is already in PACK.xsd since 2020 as XC.
Though, it may be rarely used in Tcompiler conditions for compiler dependent components, such as RTX5 or CMSIS-Compiler.
|
|
||
| set(DEVICE_ID $ENV{DEVICE_ID}) | ||
| set(DFP_PATH $ENV{DFP_PATH}) | ||
| set(LINKER_SCRIPT "${DFP_PATH}/xc32/${DEVICE_ID}/${DEVICE_ID}.ld") |
There was a problem hiding this comment.
Referencing a file from a read-only location as the linker script sounds unconventional.
All files coming from a DFP should be handled via packs, including device header files and start-up components, for proper semantic versioning and componentization.
It shouldn't be necessary things like DFP_PATH and LINKER_SCRIPT here.
In addition I would strongly recommend to add a default linker script gcc_xc_linker_script.ld.src into the templates folder.
More info:
https://open-cmsis-pack.github.io/cmsis-toolbox/YML-Input-Format/#linker
https://open-cmsis-pack.github.io/cmsis-toolbox/CreateApplications/#configure-linker-scripts
|
|
||
| # Environment variables | ||
|
|
||
| if(NOT DEFINED ENV{DEVICE_ID}) |
There was a problem hiding this comment.
How is the environment variable DEVICE_ID set?
CMake variables related to device selection such as CPU are available and should be used instead:
https://open-cmsis-pack.github.io/cmsis-toolbox/build-operation/#adding-a-toolchain-to-cmsis-toolbox
3 participants
This PR aims to add support for the XC32 Toolchain and provide a solution for the issue found here.