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

FatFS Integration #551

Merged
merged 5 commits into from
Dec 26, 2017
Merged

FatFS Integration #551

merged 5 commits into from
Dec 26, 2017

Conversation

networkfusion
Copy link
Member

Provides the base for integrating file system support for nanoFramework leveraging the FatFS library.

Description

Adds the option for FatFs to be built by extracting the FatFs sources and adding them as part of the build.

Motivation and Context

It is necessary for full filesystem support, especially handy when used with an SD card (not sure if enabling the SD should be tied to FS or a separate option that is dependent on it... TBD.

How Has This Been Tested?

Build without error so far. Further testing should be carried out before merging.

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@nfbot
Copy link
Member

nfbot commented Dec 21, 2017

Hi @networkfusion,

I'm nanoFramework bot.
Thank you for your contribution!

A human will be reviewing it shortly. 😉

@josesimoes josesimoes added Series: STM32xx Everything related specifically with STM32 targets Type: enhancement Status: in progress labels Dec 21, 2017
@@ -129,7 +129,7 @@
* @brief Enables the SDC subsystem.
*/
#if !defined(HAL_USE_SDC) || defined(__DOXYGEN__)
#define HAL_USE_SDC FALSE
#define HAL_USE_SDC TRUE
Copy link
Member

Choose a reason for hiding this comment

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

This define here should be moved to target_board.h like all the others that we are touching and making dependent of a cmake option. See line 34 above, for example, with the ADC submodule.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is where I am having trouble because FatFS can be used for both SD and USB (and possibly flash, but lets not go there) so I am trying to work out the best way of setting the options as some people may not want either SD or USB enabled... Should the file system turn these things on regardless???

Copy link
Member Author

Choose a reason for hiding this comment

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

anyway, I guess that is not too important right now... and will only need minor refactoring with the changes so I have changed as suggested (although it is target_platform.h not target_board.h)...

removed generated by automated tool header from file as it is not...
deleted unnecessary file.
@@ -3,11 +3,6 @@
// See LICENSE file in the project root for full license information.
//

//////////////////////////////////////////////////////////////////////////////
Copy link
Member

Choose a reason for hiding this comment

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

This comment block is really meant to be here. 😃
It will be in the generated output file: target_platform.h as a warning to someone browsing on the build folder and that could think that changing this file will have any lasting effect on the build.

Copy link
Member

@josesimoes josesimoes left a comment

Choose a reason for hiding this comment

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

Please add that comment block and this is good to merge.

@josesimoes josesimoes merged commit ce1899e into nanoframework:develop Dec 26, 2017
@networkfusion networkfusion deleted the FatFS branch November 17, 2018 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Series: STM32xx Everything related specifically with STM32 targets Type: enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants