FreeRTOS-Kernel
FreeRTOS-Kernel copied to clipboard
Adding in ability to support a library for freertos_config and a custom port
Description
DEPRECATE: CMake: FREERTOS_CONFIG_FILE_DIRECTORY configuration and replace it with expected library freertos_config
Rationale - some configuration settings can now be added to the freertos_config target eg:
target_compile_definitions(freertos_config
INTERFACE
projCOVERAGE_TEST=0
)
Added support for FREERTOS_PORT=A_CUSTOM_PORT - this allows the inclusion of a port as a library as well that can be linked to the FreeRTOS-Kernel appropriately.
Assorted fixes:
- spelling:
Compiller->Compiler - missing link to Threads::Threads (pthreads) for GCC_POSIX build.
Test Steps
Setup a local cmake project and use the following project file to include FreeRTOS-Kernel
FetchContent_Declare( freertos_kernel
GIT_REPOSITORY [email protected]:phelter/FreeRTOS-Kernel.git
GIT_TAG feature/cmake-updates
)
FetchContent_Declare( freertos_plus_tcp
GIT_REPOSITORY [email protected]:phelter/FreeRTOS-Plus-TCP.git
GIT_TAG cmake-support
GIT_SUBMODULES "" # Don't grab any submodules since not latest
)
add_library(freertos_config INTERFACE)
add_library(FreeRTOS::Config ALIAS freertos_config)
target_sources(freertos_config
INTERFACE
inc/FreeRTOSConfig.h
inc/FreeRTOSIPConfig.h
)
target_include_directories(freertos_config SYSTEM
INTERFACE
inc
)
target_compile_definitions(freertos_config
INTERFACE
projCOVERAGE_TEST=0
)
# -----------------------------------------------------------------------------
set( FREERTOS_HEAP "4" CACHE STRING "" FORCE)
set( FREERTOS_PORT "GCC_POSIX" CACHE STRING "" FORCE)
if (CMAKE_CROSSCOMPILING)
set(FREERTOS_PORT "GCC_ARM_CA9" CACHE STRING "" FORCE)
endif()
FetchContent_MakeAvailable(freertos_kernel)
# -----------------------------------------------------------------------------
set(FREERTOS_PLUS_TCP_BUFFER_ALLOCATION "1" CACHE STRING "" FORCE)
set(FREERTOS_PLUS_TCP_COMPILER "" CACHE STRING "" FORCE)
set( FREERTOS_PLUS_TCP_NETWORK_IF "POSIX" CACHE STRING "" FORCE)
if (CMAKE_CROSSCOMPILING)
set(FREERTOS_PLUS_TCP_NETWORK_IF "ZYNQ" CACHE STRING "" FORCE)
endif()
set(FREERTOS_PLUS_TCP_TEST_CONFIGURATION "CUSTOM" CACHE STRING "" FORCE)
FetchContent_MakeAvailable(freertos_plus_tcp)
Related Issue
-----------
See #558
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Codecov Report
Base: 94.30% // Head: 94.30% // No change to project coverage :thumbsup:
Coverage data is based on head (
cb4b792) compared to base (cd1f51c). Patch has no changes to coverable lines.
Additional details and impacted files
@@ Coverage Diff @@
## main #571 +/- ##
=======================================
Coverage 94.30% 94.30%
=======================================
Files 6 6
Lines 2370 2370
Branches 579 579
=======================================
Hits 2235 2235
Misses 85 85
Partials 50 50
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 94.30% <ø> (ø) |
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
Thanks for your comments . I've responded with all of them, and have allowed the FREERTOS_CONFIG_FILE_DIRECTORY to be backwards compatible - but with a warning to update when used.
If you like I can separate the ports each into separate CMakeLists.txt as well which would help simplify and limit the case statements in the port directory to just one (which one it selects).
Note - I have a follow-on PR for fixing the compiler warnings here: https://github.com/phelter/FreeRTOS-Kernel/pull/1 will change target to master once this PR is completed.
@paulbartell @aggarg - Any chance we can move the needle on this PR?
I've performed all of the changes you've requested. And am waiting for this one to be merged to start the next one.
Thanks
@paulbartell @aggarg - Ping again.
Need a maintainer to approve running the workflows for this PR and would appreciate some time to identify any further issues that need to be resolved.
Thanks
IMO, defining and linking a target similar to freertos_config should be left to the user.
@paulbartell - You've requested a change. I am unclear as to what that change is that will get approval for this PR.
With no changes:
defininga target similar tofreertos_config- is left to the user. DONElinkinga target similar tofreertos_config- is done inside the FreeRTOS-Kernel/CMakeLists.txt to:- provide a standardized name of the target, so that it is known by the user what is needed and required for integration
- error out at
CMake Configuretime (initial call of cmake that configures the build environment ) if those integration requirements are NOT met.
For Linking - with this PR - if freertos_config is not defined, the CMake Configure will fail to complete and provide a detailed description of what is needed by the user to integrate FreeRTOS-Kernel.
Are you asking NOT to link freertos_config to freertos_kernel ?
If this is the case - then the user will have NO indication that there is missing configuration required while performing the CMake Configure, and will it will only fail when they attempt the build the FreeRTOS-Kernel with missing header files - an no means of telling the user (while building) what the problem really is. This also goes against the programming methodology of Fail-Early-Fail-Fast.
If you are still requiring a change here, can you please provide a proposal of how to provide a standardized way a user is to link a target similar to the freertos_config and provide more details as to what they must do to integrate their config with freertos_kernel.
I am unclear as to what must be done next to move this PR along.
Thanks
@AniruddhaKanhere , @aggarg or @archigup - would it be possible for one of you to approve this PR now that all of the issues/changes requested by @paulbartell are now resolved.
Thanks.
@AniruddhaKanhere , @aggarg or @archigup - Ping again. Would appreciate a review of the changes requested by @paulbartell
@AniruddhaKanhere , @aggarg or @archigup - Ping once again. Just so you know this has been opened closing in on almost 2 months now.
@phelter, apologies for the delay, I was out of office and was travelling. I approve of your change!
Thank you for your patience and your efforts in creating this PR.
Thanks, Aniruddha