Adding in ability to support a library for freertos_config and a custom port #571

phelter · 2022-09-30T00:41:00Z

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.

…om freertos_kernel_port (FreeRTOS#558)

codecov · 2022-10-04T22:28:24Z

Codecov Report

Base: 94.30% // Head: 94.30% // No change to project coverage 👍

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.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

CMakeLists.txt

portable/CMakeLists.txt

CMakeLists.txt

FreeRTOS#571)

phelter · 2022-10-11T19:32:10Z

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).

phelter · 2022-10-13T20:46:17Z

Note - I have a follow-on PR for fixing the compiler warnings here: phelter#1 will change target to master once this PR is completed.

phelter · 2022-10-23T19:14:20Z

@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

…efault PORT selection for native POSIX and MINGW platforms.

phelter · 2022-10-30T23:21:07Z

@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

CMakeLists.txt

paulbartell

IMO, defining and linking a target similar to freertos_config should be left to the user.

phelter · 2022-11-01T17:12:26Z

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:

defining a target similar to freertos_config - is left to the user. DONE
linking a target similar to freertos_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 Configure time (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

… it from a FREERTOS_CONFIG_FILE_DIRECTORY.

phelter · 2022-11-12T02:19:30Z

@AniruddhaKanhere , @aggarg or @archigup - Ping again. Would appreciate a review of the changes requested by @paulbartell

phelter · 2022-11-18T07:01:28Z

@AniruddhaKanhere , @aggarg or @archigup - Ping once again. Just so you know this has been opened closing in on almost 2 months now.

AniruddhaKanhere · 2022-11-18T08:59:59Z

@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

Adding in ability to support a library for freertos_config and a cust…

1342407

…om freertos_kernel_port (FreeRTOS#558)

phelter requested a review from a team as a code owner September 30, 2022 00:41

Using single name definition for libraries everywhere. (FreeRTOS#558)

1822d8e

aggarg reviewed Oct 11, 2022

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

CMakeLists.txt Show resolved Hide resolved

CMakeLists.txt Outdated Show resolved Hide resolved

portable/CMakeLists.txt Outdated Show resolved Hide resolved

archigup reviewed Oct 11, 2022

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

Supporting backwards compatibility with FREERTOS_CONFIG_FILE_DIRECTORY (

50a29ef

FreeRTOS#571)

phelter and others added 2 commits October 12, 2022 03:15

Removing compiler warnings for GNU and Clang. (FreeRTOS#571)

06f9314

Merge branch 'main' into feature/cmake-updates

b625d2a

archigup previously approved these changes Oct 12, 2022

View reviewed changes

Merge branch 'main' into feature/cmake-updates

ae8eb33

phelter mentioned this pull request Oct 13, 2022

Fixing clang and gnu compiler warnings. phelter/FreeRTOS-Kernel#1

Open

phelter added 2 commits October 23, 2022 12:14

Merge branch 'main' into feature/cmake-updates

855d6d5

Added in documentation on how to consume from a main project. Added d…

a3db429

…efault PORT selection for native POSIX and MINGW platforms.

phelter dismissed archigup’s stale review via a3db429 October 23, 2022 23:46

phelter mentioned this pull request Oct 28, 2022

Support for CMake builds. Added Linux stub port for now to allow cross compilation FreeRTOS/Lab-Project-FreeRTOS-FAT#35

Merged

Merge branch 'main' into feature/cmake-updates

2310f17

AniruddhaKanhere previously approved these changes Oct 31, 2022

View reviewed changes

archigup previously approved these changes Oct 31, 2022

View reviewed changes

paulbartell reviewed Oct 31, 2022

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

paulbartell reviewed Oct 31, 2022

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

paulbartell reviewed Oct 31, 2022

View reviewed changes

CMakeLists.txt Outdated Show resolved Hide resolved

paulbartell suggested changes Oct 31, 2022

View reviewed changes

Only adding freertos_config if it exists. Removing auto generation of…

b2c564c

… it from a FREERTOS_CONFIG_FILE_DIRECTORY.

phelter dismissed archigup’s stale review via b2c564c November 4, 2022 23:30

Merge branch 'main' into feature/cmake-updates

971d58e

AniruddhaKanhere previously approved these changes Nov 18, 2022

View reviewed changes

AniruddhaKanhere added 2 commits November 18, 2022 14:30

Merge branch 'main' into feature/cmake-updates

ef7139d

Update actions to use 20.04 ubuntu

7c2e999

AniruddhaKanhere dismissed stale reviews from paulbartell and themself via 7c2e999 November 18, 2022 09:12

AniruddhaKanhere added 3 commits November 18, 2022 14:48

Update ci.yml

e03ee48

Update kernel-checks.yml

3ca8528

Make unit-test run on 20.04 ubuntu

cb4b792

AniruddhaKanhere approved these changes Nov 18, 2022

View reviewed changes

paulbartell approved these changes Nov 18, 2022

View reviewed changes

paulbartell merged commit 5f7ca3a into FreeRTOS:main Nov 18, 2022

paulbartell mentioned this pull request Dec 21, 2022

[Feature Request] CMakeLists.txt improvements for better Project integration and maintenance of portable(s) #558

Closed

phelter mentioned this pull request Feb 9, 2023

Feature/fixing clang gnu compiler warnings #620

Merged

Adding in ability to support a library for freertos_config and a custom port #571

Adding in ability to support a library for freertos_config and a custom port #571

Uh oh!

Conversation

phelter commented Sep 30, 2022

Description

Test Steps

Uh oh!

codecov bot commented Oct 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

phelter commented Oct 11, 2022

Uh oh!

phelter commented Oct 13, 2022

Uh oh!

phelter commented Oct 23, 2022

Uh oh!

phelter commented Oct 30, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

paulbartell left a comment

Choose a reason for hiding this comment

Uh oh!

phelter commented Nov 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phelter commented Nov 12, 2022

Uh oh!

phelter commented Nov 18, 2022

Uh oh!

AniruddhaKanhere commented Nov 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Oct 4, 2022 •

edited

Loading

phelter commented Nov 1, 2022 •

edited

Loading