settings subsystem enhancements

[jhedberg]

It'd be nice to have the following improvements to the settings API:

• 1. Remove SETTINGS_EXPORT_SHOW and the tgt parameter to the export callback
• 2. Reduce stack consumption
• 3. Remove 256-byte value length limitation
• 4. Provide stream-style encoding and decoding to/from flash, so the the API only requires a pointer to binary data, and the settings implementation takes care of encoding/decoding to/from base64 and writing/reading to/from flash on the fly. This would eliminate the need of a separate base64 value buffer on the app-side, thereby further contributing to the stack footprint reduction.
• 5. Provide a way to clear an entire sub-path of key-value pairs, e.g. the equivalent of calling settings_save_one(key, NULL) for every key under bt/mesh/ could then be accomplished with something like settings_clear("bt/mesh/"), and settings_clear(NULL) would clear the entire settings storage. If we want to make this API multi-purpose, it could interpret a key ending in / as a sub-path and one without it as a key in itself, i.e. the equivalent of calling settings_save_one(key, NULL) (but more intuitive looking).
• 6. Consider removing enum settings_type and have all app-side data treated as a byte array (i.e. what's currently SETTINGS_BYTES)
• 7. Add support for performing an export on a sub-path. E.g. I'm interested in storing all pending mesh values, i.e. those under the sub-path bt/mesh/. Right now I'm having to do the settings_save_one() calls like I would in the export callback, however doing these through the export callback (invoked through a new settings_save_path("bt/mesh/") call or similar) may be more efficient just like the existing export procedure is - e.g. with NFFS there's then just a single file open and a single file close, instead of multiple ones.

[carlescufi]

CC @lemrey @oyvindronningstad

[Laczen]

@nvlsianpu Item 4 could be done by moving the encoding to the file backend (the settings part of the file backend). The data representation on "disk" can then be different for different file backends: it can be a string containing the name and data for fcb and nffs. Or a string containing the name and itemid and a itemid-data pair for NVS. In this case NVS could drop the stream encoding completely.

[nvlsianpu]

@jhedberg I take this on my desk.

[jhedberg]

One other thing I forgot: we discussed with @carlescufi that since settings_load() is done by the app, it'd be consistent that settings_subsys_init() is also done by the app (especially since you need to be sure that it's not called after someone has already tried to register a handler). I realized we merged a version for Bluetooth where settings_subsys_init() is called through bt_enable() and was just about to write a patch to move it back out to the app. However, one thing occurred to me: there might be kernel-side subsystems with initializers that are run even before the app's main() is run. Would it then make sense to make settings_subsys_init() an internal detail that's called by registering it with SYS_INIT()?

[Laczen]

Hi @nvlsianpu, I am currently looking into adding a NVS backend for the settings module. Are you going to address all items as proposed ? If not, what items will you be removed from the list.

For the NVS backend I am thinking of doing it in the following way: settings items are stored with ID above 0x8000, at 0x8000 the highest item ID that is used is stored. For each stored setting the name+value is added as an item. Reading all settings is easy: step through item numbers from 0x8001 to 0x8000+value stored at 0x8000 and do cb(...). Writing needs to step through all item numbers as well and compare based upon the name (and stop if name is reached) and then write the new name+value at the found item number. I think this will be the easiest integration in settings, what's your opinion ?

[nvlsianpu]

Would it then make sense to make settings_subsys_init() an internal detail that's called by registering it with SYS_INIT()?

@jhedberg analyzed this when I had been introducing the settings - not possible to do so as the initialization required readiness of FS+flash_driver or FCB+flash_map+flash-driver.For FCB-backend it would be possible if more priority level of Subsystem initialization were available. For FC-backend it would required additionally mounting the FS before (automatically).

[nvlsianpu]

@Laczen So far I'm working on points 1,2,3,4,6 - it will change a loot. Especial that a backed will need to provide function of type int (*write_cb)(void *ctx, off_t off, char const *buf, size_t len) for write an setting item in fragments. Please let me know whether you need to write 'name' separately from value (i don't understand how you gonna to implement backend from conversation above - but remember what was discused on the NVS PR"

[Laczen]

@nvlsianpu, my original idea was to make one item that contains a link between a name and a item. I now think this is not so good because this one item will get to large. The backend could then also be similar to the "file" backend, and this is where I would start from. The name would not be separated from the value.

Writing a setting in fragments is a big change, but a nice challenge. Can I interprete the off_t off as the following representation of the data: name:off:value so two separators, the first ends the name, the second ends the offset and an empty offset means it has reached the end of the data ?

[nvlsianpu]

@Laczen here is preveiew of the patch. All encoding works is performed by f. settings_line_write.

[lemrey]

I agree with @Laczen
that keeping a setting's name together with its value in a single nvs entry would be a better solution than keeping the name,value table in a entry. It would reduce the number of writes (when booting the first time) and we would avoid the problem of such table growing too large entirely.

Additionally, 'updating' a setting and 'writing' a new setting would then become the same operation:

• search for all entries that contain a setting for a given setting name
• if a setting with the given name exists, write an updated entry with the same ID
• otherwise, write a new entry with a new ID

The last (largest) setting ID should be read on bootup and kept in RAM, in order to avoid having to read all the entries twice each time we write a setting.

[nvlsianpu]

Fixes will be provided by #9521 PR