change triggers

[pljakobs]

please do not take this as a "must have" but
I started implementing ConfigDB in my existing code and had this idea:

some changes might trigger direct actions (such as ip-address configuration).

What do you think of the idea of being able to register callbacks on the db that are triggered when a specific item has changed?

I'm thinking something like

network.ipAddressRegisterCallback(&addressChangeCallback)

void addressChangeCallback(){
  // change ip address and restart if necessary

and the callback would be triggered on change, whether through an updater or through a stream import.

[mikee47]

Could manage it in a similar way to async. updates so callback can be registered on an object...

[pljakobs]

can I come back to this?

my project currently contains this code as part of updating the configuration from a http post:

            if (ip_updated) {
            	if (restart) {
					debug_i("ApplicationWebserver::onConfig ip settings changed - rebooting");
					app.delayedCMD(F("restart"), 3000); // wait 3s to first send response
					//json[F("data")] = "restart";
                }
            }
            if (ap_updated) {
				if (restart && WifiAccessPoint.isEnabled()) {
					debug_i("ApplicationWebserver::onConfig wifiap settings changed - rebooting");
					app.delayedCMD(F("restart"), 3000); // wait 3s to first send response
					//json[F("data")] = "restart";
				}
            }
            if (color_updated) {
                debug_d("ApplicationWebserver::onConfig color settings changed - refreshing");

                //refresh settings
                app.rgbwwctrl.setup();

                //refresh current output
                app.rgbwwctrl.refresh();

            }

Of course, I could do

String oldIP;
{
   AppConfig::Network network(*app.cfg);
   oldIP=network.getIpAddress();
}

// do the import
auto bodyStream = request.getBodyStream();
if (bodyStream) 
   ConfigDB::Status status = app.cfg->importFromStream(ConfigDB::Json::format, *bodyStream);
String newIP;

{
   AppConfig::Network network(*app.cfg);
   newIP=network.getIpAddress();
}
if (newIP!=oldIP) {
   if (restart) {
      debug_i("ApplicationWebserver::onConfig ip settings changed - rebooting");
      app.delayedCMD(F("restart"), 3000); // wait 3s to first send response
      }
}

but I wonder if there's a better, more elegant way to do this, whether by a call back or a flag. But then I think this would only require the stream Updater to compare every property value upon update which may make the whole operation slower for a comparably small benefit? (or at least check if there's a registered call back and then compare, which is faster but still one extra operation per property)
Also, the callback would have to be triggered after the whole dataset has been ingested, otherwise it would interrupt the update.

[mikee47]

It's going to be fairly efficient to trigger a callback during de-serialisation since each item has to be parsed in turn anyway. The callback would likely be very generic, receiving this level of information:

struct CallbackInfo {
    Object& parent;
    PropertyInfo& prop;
    void* currentData;
    unsigned currentDataSize;
    void* newData;
    unsigned newDataSize;
};

Note: this would also accommodate #38 to handle custom field encoding.

It's easy to make a generic comparison between existing data and the new data using memcmp.

It would be preferable to just invoke a registered callback for every property and have the application filter using the provided data.
This can be made more efficient if CallbackInfo is used internally during de-serialisation so it doesn't have to be assembled specially.

What would be expensive is filtering based on an entire JSONPath, though it would be fairly simple to maintain this path and include it in CallbackInfo. A further optimisation would be to have a bool return value from the callback (or enum if we need more options): returning true would incur additional callbacks for each child object, whilst false would suppress them.

[mikee47]

Also, the callback would have to be triggered after the whole dataset has been ingested, otherwise it would interrupt the update.

Not necessarily - as with interrupt processing the application just makes notes of any important changes (e.g. by setting some flags) and then perform any processing when the update has finished.

enum class Flag {
    ipChanged,
};
using Flags = BitSet<uint8_t, Flag>;

Flags flags;

...

void itemUpdateCallback(CallbackInfo& info)
{
    if(info.prop.name == "ipAddress") {
        if(info.valueChanged()) { // Does a memcmp of newData vs. currentData
            flags |= Flag::ipChanged;
        }
    }
}

...

void afterUpdate()
{
    if(flags[Flag::ipChanged]) {
        flags -= Flag::ipChanged;

        debug_i("ApplicationWebserver::onConfig ip settings changed - rebooting");
        app.delayedCMD(F("restart"), 3000); // wait 3s to first send response
    }
}

[mikee47]

@pljakobs I've made a change in PR #21 so that any error detected during import halts the import. That includes not in schema errors where an object or property key isn't found. Where updates are small and only for one store I think that's the correct approach. I should probably go further and not commit any changes if this happens: BUT if an update contains changes to multiple stores then it may have already made changes to other stores so is there any point? Maybe we just have a 'try to update as much as you can and ignore the rest' approach.

There's also the situation where the database schema has been modified, in which case not in schema might not be a fatal error.

Any thoughts?

[pljakobs]

honestly, I think silently discarding data that does not conform to the schema is perfectly fine behaviour as well - thinking about backward compatibility of older backends with newer frontends. If I update the schema in a newer version, the older systems would be required to handle any errors rather than just consuming what they know of.

I can see situations where proper error reporting / handling is required but as a default case, I tend to prefer an API to be tolerant.

For a structural error like malformed json, raising an error is probably the right thing to do. Rollback would certainly be nice to have, but I'd be a bit concerned about performance then.

Since we're talking about configuration data, what would be great to have is a way to store a small set of known configs, such as a last known good or factory defaults.

[pljakobs]

Since we're talking about configuration data, what would be great to have is a way to store a small set of known configs, such as a last known good or factory defaults.

thinking about this again, this could probably be handled by just copying the underlying directory / fileset, but that may or many not be in a consistent state.

[pljakobs]

(I should finish my thinking before writing something up!)
I believe what I just said is most easily achieved by just saving the config to different export files.
This is, I believe, also the best way to provide an initial configuration: just include a factory-config.json in the build that is loaded if there is no valid ConfigDB.

[mikee47]

Sorry, I posted this here because one possible approach to this is an application callback which you suggested elsewhere. In this situation it would probably be more useful as very specific information could be provided about the error.

[mikee47]

OK, fixed in develop. All property types have a default value stored as a string. Calling Property::setJsonValue(nullptr) resets to default. Other than for String types, the normal Updater objects cannot do this because there is no concept of null. We'd need a separate method to do that.

[pljakobs]

this makes me think, how much true database logic will ever be used here.
I think in order to implement a default configuration set, I would do what I described above: load from a file default.json that I can ship with the code. I think that's easier to handle than using the internal defaults, especially, it's easier to update and change if need be.

For limits: while at first glance, I love to be able to specify limits (for example, my rgb values are 0..1023, hue is 0-359, sad and val are 0-100 (float) - but I feel that it's extra effort to have it enforced in the DB - if a frontend sends a json update and the backend streams it, to really leverage this, we would have to return a json formatted error that shows what property was out of bounds and by how much or rather, what the limits are. I think, the returns are diminishing here, I would expect everybody can do this in the front end.

It might be different for applications that, for example, render the front end by generating html or use Sming graphics primitives, they could use the limits to limit the input range for fields, but I am not sure how much use that would find.

Since you asked for special data types: that's somewhat the same thing, in order for it to be useful, we'd have to have some sort of transparent use - but honestly, in my case, ip address is a text field and I sanity check it upon input (at least I think I do).

All more sophisticated data types, I guess, most folks of my proficiency will hardly ever use them. Yes, they'd be great for safer code, but they're also mental work. I don't think it's worth your time, necessarily.

I think what you've done is pretty amazing already, I've started implementing it in my rgbww firmware and I can see it getting so much more readable and (hopefully) memory efficient, I believe we've reached pareto for the majority of functionality.

[mikee47]

For limits: while at first glance, I love to be able to specify limits (for example, my rgb values are 0..1023, hue is 0-359, sad and val are 0-100 (float) - but I feel that it's extra effort to have it enforced in the DB - if a frontend sends a json update and the backend streams it, to really leverage this, we would have to return a json formatted error that shows what property was out of bounds and by how much or rather, what the limits are. I think, the returns are diminishing here, I would expect everybody can do this in the front end.

One way to deal with this is to have the update response return the same content as in the request, but with the actual values set. The client can then just compare the two JSON datasets to resync their view. This is fine for small updates since the entire thing can be read into memory, and the response constructed whilst that is being parsed. Larger updates it's a problem as the response cannot start streaming until the processing has finished (not with HTTP anyway AFAIK).

[pljakobs]

one thing that I just stumbled on:

there's a few ways to initialize the database, via defaults in the schema or, as I suggested above, by loading from file. There may also be situations when the application has to determine if the database is initialized already or if it has to update part of the configuration based on, for example, hardware (like setting the AP name to something including the chip id).

I can think of two ways: either a flag like "isNewDatabase" that is set when the database files were not present upon creating the object or, more elegant, a callback after the database has been initialzed from defaults for the first time so the application can take over and, for example, load content from file.

What do you think?

[pljakobs]

I resolved this by having a "is_initialized" boolean property in my schema that defaults to "false" - if I have to initialize the database beyond what is in the defaults, I can then set this to "true" and be done.