mikee47 / ConfigDB

Configuration database for Sming
GNU General Public License v3.0
3 stars 1 forks source link

code generation or schema issue? #50

Open pljakobs opened 3 days ago

pljakobs commented 3 days ago

I guess there's something wrong with my schema. While make configdb-rebuild passes correctly, building the app fails with errors like:

/opt/Sming/Sming/Components/ConfigDB/src/include/ConfigDB/Union.h:112:12: error: could not convert '<brace-enclosed initializer list>()' from '<brace-enclosed initializer list>' to 'AppData::HSVUpdater'
  112 |    return {};
      |            ^
      |            |
      |            <brace-enclosed initializer list>
/opt/Sming/Sming/Components/ConfigDB/src/include/ConfigDB/Union.h:114:10: error: no matching function for call to 'AppData::HSVUpdater::HSVUpdater(ConfigDB::Object&, const ConfigDB::PropertyInfo&, unsigned int)'
  114 |   return Item(*parent, typeinfo().getObject(tag), dataRef + propinfo().offset);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

my intent is for a preset like this to fit the schema:

    {
      "name": "sunset",
      "favorite":true,
      "color": {
        "hsv": {
          "h": 30,
          "s": 80,
          "v": 90
        }
      }
    }

a schema validator tells me that this was valid against more than one schema from "oneOf" and further digging indicates that since none of the color definitions require any properties, an empty list would fit either. But here, I have "color":{"hsv":{}} which I think should match the HSV definition...

any thoughts?

schema:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "type": "object",
  "store":"json",
  "$defs":{
    "RAW": {
      "type": "object",
      "properties": {
        "raw":{
          "type":"object",
          "properties":{
            "ww": {
                "type": "integer",
                "minimum": 0,
                "maximum": 1023
            },
            "r": {
                "type": "integer",
                "minimum": 0,
                "maximum": 1023
            },
            "b": {
                "type": "integer",
                "minimum": 0,
                "maximum": 1023
            },
            "cw": {
                "type": "integer",
                "minimum": 0,
                "maximum": 1023
            },
            "g": {
                "type": "integer",
                "minimum": 0,
                "maximum": 1023
            }
          }
        }
      }
    },
    "HSV": {
      "type": "object",
      "properties": {
        "hsv":{
          "type": "object",
          "properties": {
            "s": {
                "type": "number",
                "minimum": 0,
                "maximum": 100
            },
            "v": {
                "type": "number",
                "minimum": 0,
                "maximum": 100
            },
            "h": {
                "type": "number",
                "minimum": 0,
                "maximum": 359
            }
          }
        }
      }
    },
    "color":{
      "oneOf":
        [
          {"$ref":"#/$defs/HSV"},
          {"$ref":"#/$defs/RAW"}
      ]
    },
    "channel": {
        "type":"object",
        "properties":{
            "pin":{
                "type":"integer",
                "minimum":0,
                "maximum":255
            },
            "name":{
                "type":"string"
            }
        }
    }
  },
  "properties": {
    "presets": {
      "type": "array",
      "store":"json",
      "items": {
        "type": "object",
        "properties": {
          "name": {
            "type": "string"
          },
          "color": {
            "$ref":"#/$defs/color"
          },
          "ts": {
            "type": "integer"
          },
          "id":{
            "type":"integer"
          },
          "favorite":{
            "type":"boolean",
            "default":false
          }
        }
      }
    },
    "controllers": {
      "type": "array",
      "store":"json",
      "items": {
        "type": "object",
        "properties": {
          "id":{
            "type":"integer"
          },
          "ip": {
            "type": "string"
          },
          "name": {
            "type": "string"
          },
          "last-seen": {
            "type": "integer"
          }
        }
      }
    },
    "scenes":{
      "type":"array",
      "store":"json",
      "items":{
        "type":"object",
        "properties":{
          "name":{
            "type":"string"
          },
          "settings":{
            "type":"array",
            "items":{
              "type":"object",
              "properties":{
                "controller_id":{
                  "type":"integer"
                },
                "color": {
                  "$ref":"#/$defs/color"
                }
              }
            }
          }
        }
      }
    }
  }
}
mikee47 commented 3 days ago

There is one issue, maybe unrelated, but you have defined scenes as a store which must be an object - an array will lead to undefined behaviour. I need to add a check for that in the code generator.

mikee47 commented 3 days ago

Also, for me the error you give above is way down the list of compile errors so should be disregarded until earlier errors are fixed. The first error is:

AppData.cpp:69:38: error: ‘ContainedColor’ in ‘class AppData::ContainedPresetsItem’ does not name a type

Which looks like an issue with the generator. I'll look into that.

mikee47 commented 3 days ago

OK, I've raised a PR which allows this schema to be parsed and built, with the exception of the store annotation on array items. This is an interesting schema as it has identified a failure of the code generation due to its complex nested nature!

I do wonder though, why you have a RAW object definition with a single raw object property, looks a bit odd...

pljakobs commented 3 days ago

oh, I wasn't aware that an array by itself can't be a store. I was thinking of all those arrays being a store.

pljakobs commented 3 days ago

so to my understanding, the RAW schema object doesn't become itself an object, but in my resulting preset, I want either the raw or the hsv object to be present, not just it's children, as I need to detect whether it's, well, a raw or a hsv preset. I could drop the color object, though, as this never has more children than either raw or hsv

in other words: there are two different valid preset objects:

{
  "name":"blue",
  "favorite":false,
  "color": {
    "raw": {
      "r":0,
      "g":0,
      "b":1023,
      "ww":0,
      "cw":0
    }
  }
}

and



{
  "name":"red",
  "favorite":false,
  "color": {
    "hsv": {
      "h":0,
      "s":100,
      "v":100
    }
  }
}
pljakobs commented 3 days ago

oh wait, you're asking why do I define RAW as a definition but only use it once? Actually, here's an interesting question: can the definition be shared over two schemata? Can I refer to a definition in an external file? I saw that you had an include statement, but I think that was to include c++ headers in the generated code, right?

mikee47 commented 3 days ago

oh, I wasn't aware that an array by itself can't be a store. I was thinking of all those arrays being a store.

Yep, it's because an array represents dynamic storage and whilst those all live in a separate pool attached to the Store, the ArrayId identifier needs somewhere to live.

mikee47 commented 3 days ago

oh wait, you're asking why do I define RAW as a definition but only use it once?

No, I'll elaborate on this in a sec.

Actually, here's an interesting question: can the definition be shared over two schemata? Can I refer to a definition in an external file?

Not at present, but no reason that can't be added. Trying to keep things simple!

I saw that you had an include statement, but I think that was to include c++ headers in the generated code, right?

Correct.

pljakobs commented 3 days ago

Not at present, but no reason that can't be added. Trying to keep things simple!

would be useful here, as the RAW and HSV definitions of app-config and app-data should be identical - not that they'd change a whole lot, but right now, I have them in both schemas

mikee47 commented 3 days ago

so to my understanding, the RAW schema object doesn't become itself an object,

Yes, it does. It has "type": "object" so produces an object of type RAW.

but in my resulting preset, I want either the raw or the hsv object to be present, not just it's children, as I need to detect whether it's, well, a raw or a hsv preset.

Your definition of color definition uses oneOf, which generates a Union for you. The color.getTag() method tells you which type is stored there.

mikee47 commented 3 days ago

oh wait, you're asking why do I define RAW as a definition but only use it once?

You have this:

    "$defs":{
      "RAW": {
        "type": "object",
        "properties": {
          "raw":{
            "type":"object",
            "properties":{

So a RAW object containing a single raw object, not necessary, just do this:

    "$defs": {
        "RAW": {
            "type": "object",
            "properties": {

Remember, a Union overlays objects not properties, like this:

struct Color {
  uint8_t tag;
  union {
    struct HSV hsv;
    struct RAW raw;
  };
};
pljakobs commented 3 days ago

Your definition of color definition uses oneOf, which generates a Union for you. The color.getTag() method tells you which type is stored there.

okay, and that resolves to a json object "raw"? (or "RAW") specifically the presets store, for now, isn't even data the controller will interfere with, it's purely the backend storage for a front end application. I'm considering synchronizing presets between controllers (hence the time stamp field) but otherwise, the controller won't do anything with it (I just don't see an advantage in addig an endpoint to change the color setting to a preset name when I can just read the preset and send the color settings) So, I was only looking at this from the json side, and it seemed that the example presets I gave did require the RAW definition to contain the raw object. hmmm..

mikee47 commented 3 days ago

Your definition of color definition uses oneOf, which generates a Union for you. The color.getTag() method tells you which type is stored there.

okay, and that resolves to a json object "raw"? (or "RAW")

Yes. The types will be named RAW but a property name or variable instance will be called raw. Strictly speaking it should be Raw for correct CamelCase type names - the convention could be changed and we could even explicitly specify the identifiers used but that just adds complexity we probably don't need.

If in doubt, just write/copy/paste a minimal schema and see what happens when it's compiled. (make configdb-rebuild is handy for this.)

specifically the presets store, for now, isn't even data the controller will interfere with, it's purely the backend storage for a front end application. I'm considering synchronizing presets between controllers (hence the time stamp field) but otherwise, the controller won't do anything with it (I just don't see an advantage in addig an endpoint to change the color setting to a preset name when I can just read the preset and send the color settings)

So why use ConfigDB at all? Just treat them as opaque files. Maybe the file timestamp would suffice?

So, I was only looking at this from the json side, and it seemed that the example presets I gave did require the RAW definition to contain the raw object. hmmm..

If you find examples which don't compile the way you expect, please post them - it could be a bug!

mikee47 commented 3 days ago

oh, I wasn't aware that an array by itself can't be a store. I was thinking of all those arrays being a store.

Yep, it's because an array represents dynamic storage and whilst those all live in a separate pool attached to the Store, the ArrayId identifier needs somewhere to live.

I'm having a look at this, might be simple to implement...

pljakobs commented 3 days ago

So why use ConfigDB at all? Just treat them as opaque files. Maybe the file timestamp would suffice?

I think I just went for it as it keeps all paths open and makes it really simple, but you're right, storing a file might just be enough.