Add Settings Utility and Example App #2178

TimJTi · 2023-11-04T13:43:13Z

Last year @fjpanag offered to donate some code to the repo. This is my attempt at making his "settings" functions NuttX compatible along with an example app to check it out
Impact

None. This is new.
Testing

On my custom board (SAMA5D27C-D1G). It seems to work!

There are probably still some style and/or NuttX-convention errors/inconsistencies so I have left it as a draft for now.

TimJTi · 2023-11-04T14:12:11Z

I have no idea why this is failing CI? HELP!

TimJTi · 2023-11-04T15:30:00Z

OK - I assume it's because a created a new "top level" folder (utils) for the settings functions rather than placing it under some pre-existing folder?

If someone can propose a good folder for it to be re-homed to I can do it, or the new folder needs to be added to the scripts, I think?

xiaoxiang781216 · 2023-11-05T05:35:06Z

how about system/ folder? Many utility libraries put there.

TimJTi · 2023-11-05T13:34:51Z

@xiaoxiang781216 It still fails CI - I HAVE NO IDEA!!!!!!!!!!!

raiden00pl · 2023-11-06T08:28:32Z

@TimJTi when CI fails and you don't know what's going on, the most common problem is some error in Kconfig. For me configure with this PR shows:

CMake Warning at CMakeLists.txt:348 (message):
  Kconfig Configuration Error: warning: the default selection
  EXAMPLES_SETTINGS_CREATE_RAMDISK (undefined) of <choice> (defined at
  /home/raiden00/git/RTOS/nuttx/apps/examples/settings/Kconfig:31) is not
  contained in the choice

You're probably using kconfig-frontends which doesn't catch as many errors as kconfiglib used by CI

TimJTi · 2023-11-06T10:32:18Z

@TimJTi when CI fails and you don't know what's going on, the most common problem is some error in Kconfig. For me configure with this PR shows:
CMake Warning at CMakeLists.txt:348 (message):
  Kconfig Configuration Error: warning: the default selection
  EXAMPLES_SETTINGS_CREATE_RAMDISK (undefined) of <choice> (defined at
  /home/raiden00/git/RTOS/nuttx/apps/examples/settings/Kconfig:31) is not
  contained in the choice
You're probably using kconfig-frontends which doesn't catch as many errors as kconfiglib used by CI

Thank you! I will check.

It's a long time since I set up my Linux build environment but I will certainly look to see whether I use kconfig-frontends or kconfiglib and changeover if I can.

TimJTi · 2023-11-06T11:57:01Z

@raiden00pl Tests are running now - thank you.

I edit Kconfig files in VS Code, and have a syntax checker extension, and have kconfig-frontends as per NuttX install/setup guides for Ubuntu.

I can pip install kconfiglib, but am not sure how to make use of it. Can you share your Kconfig edit/create workflow with me so I can avoid these errors in the future?

I am actually thinking VS Code may be a hindrance not a help - I find a build will fail within VSCode for reasons I can never see, usually to do with apps, and end up having to "force save" a new .config from make menuconfig and build from a terminal prompt before it all works OK again in VS Code.

May be time for me to jump ship to a different edit/build/debug workflow completely!!!!!!!!!!

raiden00pl · 2023-11-06T12:32:32Z

In my case it was just installing python-kconfiglib package for my OS (I'm avoid pip installs when possible). Build system should auto-detect kconfiglib if available.
kconfiglib should make the menuconfig command available in your system, so you can check it with command -v menuconfig command which should return path to menuconfig command:

[raiden00:~/git/RTOS/nuttx/nuttx]$ command -v menuconfig                                                                               
/usr/bin/menuconfig

If this doesn't work, then probably the packages installed with pip (usually ~/.local/bin) are not in system PATH

TimJTi · 2023-11-06T14:07:57Z

@raiden00pl

In my case it was just installing python-kconfiglib package for my OS (I'm avoid pip installs when possible). Build system should auto-detect kconfiglib if available. kconfiglib should make the menuconfig command available in your system, so you can check it with command -v menuconfig command which should return path to menuconfig command:

Thank you - installed and working!

system/settings/Kconfig

system/settings/storage_bin.c

system/settings/storage_text.c

xiaoxiang781216 · 2023-11-06T17:46:19Z

system/settings/settings.c

+      if ((g_settings.store[i].file[0] != '\0') &&
+           g_settings.store[i].save_fn)
+        {
+          ret = g_settings.store[i].save_fn(g_settings.store[i].file);


it isn't safe to call fs function in signal handler

@xiaoxiang781216 - OK - will look tomorrow as need a clear mind for it :)

here: https://man7.org/linux/man-pages/man7/signal-safety.7.html.

Found that as soon as I replied a few minutes ago so deleted it. Thanks :)

@xiaoxiang781216 If I have understood this right:

Calls to the save function are obviously asynchronous

Each call to the save function sets a a new timer

That timer could trigger before the processing of a previous timer has completed. And that potentially violates the signal safety concern?

I have proven this to be an issue, to satisfy my curiosity by adding a sleep into the the save function and calling it a few times before the function awakens. It crashes, so - I hope - I've understood it?

@fjpanag Thoughts?

@fjpanag @xiaoxiang781216
Maybe the save() function should only set a new timer if g_settings.wrpend == 0?

I am obviously struggling a little here, sorry :(

Hmmm...been reading more....the save and/or dump functions need to become reentrant. Will take another look tomorrow, but still open to suggestions :)

@fjpanag @xiaoxiang781216
The more I think about this the more I wondering whether the "dump_cache" function should be removed completely?

The code as-is presumes a write to a block device (e.g. SD card) but that may not be what's wanted. In my case, I actually need to write the data to EEPROM, and only bytes that have changed, so that needs a separate and unrelated thread to handle it.

This code currently has the ability to signal other threads that c hange has occured. Is that not enough? Off load any async file saving away from this settings function?

I am also inclined to move away from a signal and move to a poll method instead?

What do you both think?

Otherwise, I think I should change it back to a draft PR - or even close it completely and forgot it? If it's not doing things "right" then I don't want to use it, nor will others, and it certainly isn't going to be merged of course.

fjpanag · 2023-11-07T10:47:18Z

@TimJTi I would like to review this, if you don't mind, in case any change causes any issue already known to me.
I will try to check the app today, for proper usage of the API, and possibly tomorrow the actual settings code.

@xiaoxiang781216 How can I become a reviewer of the PR? Do I just leave comments inside of the commit page?