minIni icon indicating copy to clipboard operation
minIni copied to clipboard

Glue for SPIFFS

Open timothyjohncarney opened this issue 7 years ago • 5 comments

I have adapted this project for use with SPIFFS.

https://github.com/pellepl/spiffs

In order to do so, I needed to make a few changes.

  1. Statically allocate the LocalBuffers to reduce call stack usage.
  2. Remove case-insensitive search as it is not C99 compatible.
  3. Add a wrapper function to SPIFFS file read API to behave like fgets.

I am willing to share my changes, but I am unsure if these changes are of interest. I believe the first change will break the reentrancy. Some locking mechanism is necessary to guarantee reentrancy after moving the buffers off of the call stack. Furthermore, removing support for case-insensitive search seems like a step in the wrong direction.

Please advise and thank you for your work.

-Tim

timothyjohncarney avatar Jun 05 '18 16:06 timothyjohncarney

Hello Tim,

I am certainly interested in the SPIFFS wrapper functions. Re-entrancy is only an issue when using a multitasking kernel. I think that people using a multitasking kernel, will also use a microcontroller with sufficient RAM for all task stacks. Which functions are not C99 compatible? An easy fix would be to add portable implementations of those functions in the minIni source (conditionally compiled, so that we use the library functions when available).

Regards, Thiadmer Riemersma

On Tue, Jun 5, 2018 at 6:49 PM, Tim [email protected] wrote:

I have adapted this project for use with SPIFFS.

https://github.com/pellepl/spiffs

In order to do so, I needed to make a few changes.

  1. Statically allocate the LocalBuffers to reduce call stack usage.
  2. Remove case-insensitive search as it is not C99 compatible.
  3. Add a wrapper function to SPIFFS file read API to behave like fgets.

I am willing to share my changes, but I am unsure if these changes are of interest. I believe the first change will break the reentrancy. Some locking mechanism is necessary to guarantee reentrancy after moving the buffers off of the call stack. Furthermore, removing support for case-insensitive search seems like a step in the wrong direction.

Please advise and thank you for your work.

-Tim

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/compuphase/minIni/issues/9, or mute the thread https://github.com/notifications/unsubscribe-auth/AK7TQ4XerrtHvDY8Nc3f-ChPJ0MPhww1ks5t5rajgaJpZM4UbMBp .

compuphase avatar Jun 06 '18 18:06 compuphase

Thiadmer,

The string-insensitive comparison was not C99 compatible, but I did find PORTABLE_STRNICMP since my last message, so I do not think that is a concern.

I am using FreeRTOS, but I still have limited RAM so I am unable to support the large buffers on the call stack. I have declared the buffers as static and that allows me to run without allocating as much RAM to the call stack of each task I wish to use with minIni.

-Tim

timothyjohncarney avatar Jun 06 '18 20:06 timothyjohncarney

Thiadmer,

I am happy to push the changes I made to a feature branch and create a pull request. Would you please grant me push access to the repository so that I may contribute those files?

Thanks, Tim

timothyjohncarney avatar Jun 08 '18 13:06 timothyjohncarney

Hello Tim,

I have added the branch SPIFFS to minIni. I have also added you as a collaborator to the repository. I think it means that you have access to all projects (and all branches). I did not find out how to restrict the push access to just a specific branch. I think you need an "organization account" to do that.

Regards, Thiadmer Riemersma

On Fri, Jun 8, 2018 at 3:48 PM, Tim [email protected] wrote:

Thiadmer,

I am happy to push the changes I made to a feature branch and create a pull request. Would you please grant me push access to the repository so that I may contribute those files?

Thanks, Tim

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/compuphase/minIni/issues/9#issuecomment-395766090, or mute the thread https://github.com/notifications/unsubscribe-auth/AK7TQ6GLw2vqCfumMAXoyTgrOKfJwi1Xks5t6oCmgaJpZM4UbMBp .

compuphase avatar Jun 12 '18 12:06 compuphase

Thiadmer,

I have pushed my changes for review.

Thank you, Tim

timothyjohncarney avatar Jun 12 '18 14:06 timothyjohncarney