Hello all,

The `sys/config` package lets an app persist and restore data in
permanent storage.  It is documented here:
<http://mynewt.apache.org/latest/os/modules/config/config.html>.

This package does its job well and its design is sound.  It has one
minor problem though (in my opinion, in case that needs saying) - it is
nearly impossible to use!  Here is an example:
<https://github.com/apache/mynewt-core/blob/master/boot/split/src/split_config.c>.
This file implements a single 8-bit setting called `split/status`.  This
is not an extreme example; it is how all `sys/config` client code looks.
Speaking for myself, it is a major implementation effort whenever I need
to add a new setting, or worse, a new setting group.  As basic and
fundamental as data persistence is to an embedded application, this
should be an easier task.

The problem is obvious: the config package is very powerful, and with
that power comes a very complex API.

I think most application code would be fine with a less powerful
library.  Such a library would allow all settings to be loaded,
individual settings to be saved on demand, and very little else.  It
would not be possible to commit several settings at once or to execute
custom code when a setting is saved or loaded.  With this simplified set
of requirements in mind, I came up with a wish list for an API:

1. It should be easy to create a new setting group.  Ideally you could
just copy and paste from existing code.

2. Creating a new setting should be as simple as appending an element to
an array.

3. Persisting a setting should be possible with a single function call.

So I took that and implemented... something.  The library is called
"scfg" (short for "simple config").  I'll go into more details below,
but first, here is how that `split/status` configuration would be done
using the new library: <https://github.com/apache/mynewt-core/pull/2183>
[1].  I think this is a marked improvement, but please judge for
yourselves.

### Implementation

I had the idea that we could just build a simple library on top of the
existing `sys/config` library.  Unfortunately, `sys/config` doesn't lend
itself well to this kind of a wrapper library.  The problem is that the
`conf_handler` callbacks don't accept a `void *arg` parameter, meaning
every config group must define a dedicated set of callback functions.
This is an issue because scfg needs to define a generic set of callbacks
for all config groups.  So I had to modify `sys/config` so that the
callbacks accepted an extra generic argument, and this had to be done in
a backwards compatible way.  It's not pretty, but this is what I came up
with: <https://github.com/apache/mynewt-core/pull/2181>.

The scfg library required a second change to `sys/config`: Support for
unsigned integer types.  Only signed integer types are currently
supported.  Before my change, when a config setting needed to use an
unsigned type, the handler would work around this limitation by
specifying a signed integer type that is slightly wider than the actual
data.  Then the conf handler callbacks would manually convert between
the unsigned and signed types.  Since scfg does not allow for arbitrary
code to run during save and restore operations, the library needs to
handle unsigned types natively.  I made this change here:
<https://github.com/apache/mynewt-core/pull/2180>.

Finally, here is the PR for the scfg library itself:
<https://github.com/apache/mynewt-core/pull/2182>.

### Questions

1. Do we want to enable extended callbacks in `sys/config` by default,
or should this require a syscfg setting to be enabled?  This change adds
eight bytes to every config group which is a bummer.  I prefer that we
enable this change unconditionally.  I think a simplified config
interface is valuable enough that developers should be able to assume it
is available.

2. Does scfg need some sort of "on-loaded" callback for each setting?
My feeling is no, we don't need that.  If applications need to know when
settings are restored, then we can modify `sys/config` to allow
callbacks to be registered.  These callbacks would be executed after
`conf_load()` completes.

3. Do we need support for binary data?  `sys/config` only allows text
values for settings.  Binary blobs must be converted to text using
something like base64 before they are saved.  This is a nuissance for
large settings (e.g., nimble host security data).  I would love it if we
could support raw binary values, but my impression is that this would
require massive changes to the sys/config library.  I think this change
would be quite valuable, but it is a feature in itself and it should be
considered separately.

All comments are welcome.

[1] It seems the split image feature is broken again.  In light of that,
this probably wasn't the best group to use as an example!

Thanks for reading,
Chris

Reply via email to