-
Notifications
You must be signed in to change notification settings - Fork 51
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
Support for CMake builds. Added Linux stub port for now to allow cross compilation #35
Conversation
c0ab845
to
5f06b6b
Compare
Hey, this seems not to be formatted; Could you format the c files with with uncrustify and the config in tools/uncrustify.cfg? |
Please go ahead and add the formatting. I typically use clang-format for my formatting and do not have |
portable/common/ff_ramdisk.c
Outdated
FF_PRINTF( "TotalSectors %8lu\n", (unsigned long) pxIOManager->xPartition.ulTotalSectors ); | ||
FF_PRINTF( "SecsPerCluster %8lu\n", (unsigned long) pxIOManager->xPartition.ulSectorsPerCluster ); | ||
FF_PRINTF( "Size %8lu KB\n", (unsigned long) ulTotalSizeKB ); | ||
FF_PRINTF( "FreeSize %8lu KB ( %d perc free )\n", (unsigned long) ulFreeSizeKB, iPercentageFree ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In stead of casting to unsigned long
, the +FAT library normally uses unsigned
in combination with %u
:
- FF_PRINTF( "TotalSectors %8lu\n", (unsigned long) pxIOManager->xPartition.ulTotalSectors );
+ FF_PRINTF( "TotalSectors %8u\n", (unsigned) pxIOManager->xPartition.ulTotalSectors );
It also removes the warning and it works well on 64-bit platforms as well.
Thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Until now, I didn't comment on this PR ( except about FF_PRINTF()
) because I am totally inexperienced with cmake ( I only use GNU make ).
I checked out your branch and typed "cmake", which doesn't work obviously.
Would you mind to give a short description of how I can compile the project for dummies?
Also, if you open this PR, you will see that "This branch is out-of-date with the base branch", with a button "Update branch". Could you update it with the master branch?
I will talk to the team and see who can further review this PR. Sorry for the long delay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Short description on how to compile the project:
I've added a build check in ci/cd environment similar to the FreeRTOS-Plus-TCP
repo (the .github directory is a copy and modify from that).
I've also updated the main README.md as to how to integrate with CMake in a higher level project.
- Branch is now up-to-date with master in FreeRTOS repo.
- As for the FF_PRINTF - all but one of the ff_ramdisk.c files are using
%8lu
rather than what you requested:
See: https://github.com/FreeRTOS/Lab-Project-FreeRTOS-FAT/search?q=%22TotalSectors%22
Leaving as defined since it matches others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As for the FF_PRINTF - all but one of the ff_ramdisk.c files are using %8lu rather than what you requested
That is possibly true, but we would like to get rid of all %ld
and %lu
formats, both in FreeRTOS+TCP and in FreeRTOS+TCP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@htibosch - My goal was to add the necessary code to have a linux port build similar to other FreeRTOS git repos (Kernel and Plus-TCP) and follow the same coding style and usage of the existing ports inside this Plus-FAT
repo. Changing some code to reflect an arbitrary request to get rid of a particular format in this and other repos is out of scope (from my perspective) of this particular PR.
Since this is an unrelated issue to any of the CMake build support (a different feature/clean-up task), I'll add it to a follow on PR associated with fixing compile warnings of this repo instead (and I'll change all of the various portable code instead of just this new Linux port).
See similar ones for:
FreeRTOS-Kernel
: Fixing clang and gnu compiler warnings. phelter/FreeRTOS-Kernel#1FreeRTOS-Plus-TCP
: phelter/FreeRTOS-Plus-TCP@feature/cmake-support...feature/fixing-clang-gnu-compiler-warningsFreeRTOS-Plus-FAT
: - will do once have this PR completed.
I intend to update the above ones and provide them, but have been blocked in continuing with this effort due to the outstanding pull requests.
…and added documentation on how to integrate with cmake.
…ments related to (FreeRTOS#35)
* Support for CMake builds. Added Linux stub port for now to allow cross compilation. * Fixing misuse of doxygen documentation * Converting prints from %lu to %u where possible as per request in comments related to (#35) * Fixing clang compiler warnings. * Fixing build test to work regardless of configuration. * Adding in for Zynq port dependency on uncached_memory target. * Excluding build_test from all build - must request it explicitly. * Ensuring XPAR_XSDPS_0_IS_CACHE_COHERENT is 1 for this build since using a modified xsdps driver that requires it. * Updating compiler warnings. * Fixing documentation, signed/unsigned conversion. Bugfix for time based code for removing failure in 2032 due to uint32_t used for time. * Fixing error creation using FF_createERR everywhere missed. * Fixing build_test in ci.yml * Uncrustify: triggered by comment. --------- Co-authored-by: Nikhil Kamath <110539926+amazonKamath@users.noreply.github.com> Co-authored-by: GitHub Action <action@github.com>
Adding CMake support with a separate build.
device support
-seeffconfigDEV_SUPPORT
FREERTOS_PLUS_FAT_PORT=A_CUSTOM_PORT
Note only tested compiling with Linux so far.
To integrate into your overall project can now use: