Skip to content
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.

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

FreeRTOS CMake file update #202

Merged
merged 6 commits into from
Jun 25, 2021
Merged

FreeRTOS CMake file update #202

merged 6 commits into from
Jun 25, 2021

Conversation

robamu
Copy link
Contributor

@robamu robamu commented Jun 6, 2021

  1. Added more warnings if paths are not found
  2. Added new path suffixes because of changed FreeRTOS kernel structure

1. Added more warnings if paths are not found
2. Added new path suffixes because of changed FreeRTOS kernel structure
@g-arjones
Copy link
Contributor

Unfortunately it looks like this is no longer being maintained. I have a few more changes as well... If you are willing to maintain a fork maybe we could merge it all...

@robamu
Copy link
Contributor Author

robamu commented Jun 7, 2021

I'm not sure whether it is not maintained anymore or it just takes a long time.. In any case, the fastest solution probably would be to have a fork for now.. I already forked the project for my personal account, but I could fork the project for my own organization (which allows to me add people as well), merge your PRs and then merge my own PRs. What do you think?

@g-arjones
Copy link
Contributor

Sounds good to me...

@atsju
Copy link
Collaborator

atsju commented Jun 7, 2021

It's maintained but slow. I already discussed with @Hish15 and we would like to help maintaining the repo to get PR merged faster. But we never took time to write to @ObKo to ask if is possible and how we could contribute. We will definitely do it but it will not be immediately.

@robamu
Copy link
Contributor Author

robamu commented Jun 7, 2021

Sounds good, that would be nice. If the repository is transferred to an organization (free one is okay as well), you can also assign more people to be able to merge pull requests. If all PRs I require are merged, I will definitely switch back to the upstream again

Copy link
Collaborator

@atsju atsju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor format remark. Appart from that it seems OK.

cmake/FindFreeRTOS.cmake Outdated Show resolved Hide resolved
cmake/FindFreeRTOS.cmake Outdated Show resolved Hide resolved
robamu and others added 4 commits June 25, 2021 09:31
Done

Co-authored-by: Julien Staub <atsju2@yahoo.fr>
Done

Co-authored-by: Julien Staub <atsju2@yahoo.fr>
@atsju atsju requested a review from Hish15 June 25, 2021 07:45
NO_DEFAULT_PATH
)

if(FreeRTOS_COMMON_INCLUDE MATCHES "FreeRTOS_COMMON_INCLUDE-NOTFOUND")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep the same coding style what about :

Suggested change
if(FreeRTOS_COMMON_INCLUDE MATCHES "FreeRTOS_COMMON_INCLUDE-NOTFOUND")
if(NOT FreeRTOS_COMMON_INCLUDE)

Comment on lines +81 to +84
"portable/GCC/${PORT}/r0p1"
"portable/GCC/${PORT}"
"Source/portable/GCC/${PORT}"
"Source/portable/GCC/${PORT}/r0p1"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alphabetical order

Suggested change
"portable/GCC/${PORT}/r0p1"
"portable/GCC/${PORT}"
"Source/portable/GCC/${PORT}"
"Source/portable/GCC/${PORT}/r0p1"
"portable/GCC/${PORT}"
"portable/GCC/${PORT}/r0p1"
"Source/portable/GCC/${PORT}"
"Source/portable/GCC/${PORT}/r0p1"

NO_DEFAULT_PATH
)

if(FreeRTOS_${PORT}_PATH MATCHES "FreeRTOS_${PORT}_PATH-NOTFOUND")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if(FreeRTOS_${PORT}_PATH MATCHES "FreeRTOS_${PORT}_PATH-NOTFOUND")
if(NOT FreeRTOS_${PORT}_PATH)

@Hish15 Hish15 merged commit e77a5ae into ObKo:master Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants