Add support for JSON RPC protocol

[mikee47]

This PR takes a look at how ConfigDB might be used to support processing and generation of JSON RPC messages.

All messages require a standard "jsonrpc": "2.0" and id property (except notifications).
A request requires method and params properties.
A Response requires result on success, and error on failure.

The general idea is that the application defines a ConfigDB schema defining the structure for params, result and error objects. A union (oneOf) is appropriate since any message may contain only one of these items.

For parsing, it is necessary to first scan the message to extract the standard fields and establish what kind of message it is.

Request

The params is itself a union, and the tag corresponds to the method.
If params appears before method then a second pass is required.

Response

Processing responses is tricker as there is nothing in the message to indicate how result is structured. Presumably the application would keep a note of outgoing request {id: method} mappings and use that to determine the expected result. That mapping might include a callback for handling the response.

Error

The code and message fields are standard, but data is variable and so is likely to be application-specific.

TODO:

• Revise JSON export so that properties are emitted before objects

[pljakobs]

I've been playing with this for a bit this week, I still have to get to terms with how to build a "correct" schema that is modular and not overly complex, but this does already look pretty promising.

The part that I didn't consider is that, for reading, the json objects / properties can come in any order and if the "method" comes after the "params" section, that makes linear parsing impossible - I still have to look at your two pass solution for this.

also, I notice that my json-rpc implementation is probably quite wrong, as I have not properly implemented the response format in the past.

[pljakobs]

so I created a sample that creates a few of the messages I use in my protocol, including requests, responses and notifications. It took a bit of getting my head around as my schema has a oneOf as the root element - which I think is necessary to not get a named envelope?
The actual schema is nicely abstracted, allowing me to generate both the json-rpc message as well as as the contained data object for http responses.
I have, at this point, not yet thought about decoding received json, for now, I'll try to replace the most expensive json renerating in my firmware with this.

I wonder if there is a (sensible) way of abstracting the message generation, technically, I should be able to write a function that takes the message type (eg colorRequest, colorResponse or colorEvent, or maybe one for Request, Response and Event taking a paramter for the method) along with the parameters union (or actually a pointer to it) that the calling code would pass the fitting parameter structure.

[edit]
that last sentence was as convoluted as my thoughts about this.
The key question is this:
looking at this code:

[[maybe_unused]] bool generateRawColorRequest(Jsonrpc& db, int id)
{
        {
                Jsonrpc::Root root(db);
                if(auto update = root.update()) {
                        auto colorRequest = update.toColorRequest();
                        colorRequest.setId(id);
                        colorRequest.setMethod("color");

                        auto rawRequest = colorRequest.params.toRawColor(); // make this a color request
                        rawRequest.raw.setR(1023);
                        rawRequest.raw.setG(512);
                        rawRequest.raw.setB(128);
                        rawRequest.raw.setWw(64);
                        rawRequest.raw.setCw(32);
                        printMessage(colorRequest);
                        colorRequest.clearDirty();
                } else
                        return false;
        }
        return true;
}

within ConfigDB, as far as I understand, there is a structure defined as containing r,g,b,ww and cw.
It is part of a union with all other params for a colorRequest, which itself is a member of a union with all messages defined in the schema.
What I wonder is:

• are those datatypes available to the application? that is: can I define a variable to be of the union type for color request params? (or maybe, depending on how the schema is built, general params)
• does this actually make sense? It seems that this would create a ton of potential errors on passing parameter structs to a function

Maybe, the manual construction of messages is the right way, as passing parameters to a function would also mean additional stack use.

[edit 2]
re-reading your initial comment, one small correction:
requests require json-rpc, id and method, as far as I an see, params is optional for requests
response requires json-rpc, id and (result xor error) and
notifications require json-rpc, method and may have params

[pljakobs]

another thing:
I have an error object that is defined like this:

"error": {
      "type": "object",
      "properties": {
        "code": {
          "$ref": "value-types/$defs/error-code"
        },
        "message": {
          "$ref": "value-types/$defs/string-value"
        },
        "data": {
          "$ref": "value-types/$defs/string-value"
        }
      },
      "required": [
        "code",
        "message"
      ],
      "additionalProperties": false
    }

that get's wrapped into an error response envelope:

    {
        "type":"object",
        "title":"error-response",
        "properties": {
            "jsonrpc": { "$ref": "value-types/$defs/jsonrpc-version" },
            "id": { "$ref": "value-types/$defs/jsonrpc-id" },
            "error": { "$ref": "params/$defs/error" }
        },
        "required": ["jsonrpc", "id", "error"],
        "additionalProperties": false
    }

I use this function to generate any sort of error response:

[[maybe_unused]] bool generateErrorResponse(Jsonrpc& db, int id, const ErrorType error, String data=NULL)
{
	{
		Jsonrpc::Root root(db);
		if(auto update = root.update()) {
			auto message = update.toErrorResponse();
			message.setId(id);
			auto result = message.error;
			switch(error) {
				case ParseError:
					result.setCode(-32700);
					result.setMessage("Parse error");
					break;
				case InvalidRequest:
					result.setCode(-32600);
					result.setMessage("Invalid Request");
					break;
				case MethodNotFound:
					result.setCode(-32601);
					result.setMessage("Method not found");
					break;
				case InvalidParams:
					result.setCode(-32602);
					result.setMessage("Invalid params");
					break;
				case InternalError:
					result.setCode(-32603);
					result.setMessage("Internal error");
					break;
				case ApplicationError1:
					result.setCode(-32000);
					result.setMessage("Application error 1");
					if(data!=NULL)
						result.setData(data);
					break;
				case ApplicationError2:
					result.setCode(-32001);
					result.setMessage("Application error 2");
					if(data!=NULL)
						result.setData(data);
					break;
				case ApplicationError3:
					result.setCode(-32002);
					result.setMessage("Application error 3");
					if(data!=NULL)
						result.setData(data);
					break;
			}
			printMessage(message);
			root.clearDirty();
		} else {
			return false;
		}
	}
	return true;
}

if I call the function with a data string, it correctly generates a valid json-rpc mesage:

{
  "error": {
    "code": -32001,
    "message": "Application error 2",
    "data": "something went horribly wrong"
  },
  "jsonrpc": "2.0",
  "id": 2
}

but when I omit the data string, it still includes the data property as a NULL value

{
  "error": {
    "code": -32601,
    "message": "Method not found",
    "data": null
  },
  "jsonrpc": "2.0",
  "id": 2
}

I guess while that's syntactically okay and would make sense when using the api to unset a value, in this case, it would be preferrable to just not generate the data property. Maybe we can use the required array to track which properties should be generated unconditionally?

[mikee47]

Maybe we can use the required array to track which properties should be generated unconditionally?

That would require implementing change tracking in ConfigDB. That's currently work in progress, should have a draft PR for that in a while.

My feeling is that we shouldn't be attempting to address any of the protocol elements of JsonRPC in ConfigDB.
The importMessage and exportMessage functions are intended to be a basic starting point and so just use a String, but the final implementation would do this within custom streams. The fly in the ointment with that is the possibility of requiring double-parsing, although many stream types (including MemoryDataStream) support seeking so it would only be the very longest messages that could be problematic.

NB. In the JsonRPC sample the RpcData.cfgb error definition includes code and message, even though they are mandatory and required for all error messages. That was more for convenience: logically they should probably just go into the Message structure.

This approach would help to simplify the RGBWWJson schema considerably.

[mikee47]

Note: ConfigDB doesn't recognise or do anything with const schema values

[pljakobs]

Note: ConfigDB doesn't recognise or do anything with const schema values

noted, although, I think it would be nice to just code it in flash rather than hold a ram value for it.

[mikee47]

within ConfigDB, as far as I understand, there is a structure defined as containing r,g,b,ww and cw.
It is part of a union with all other params for a colorRequest, which itself is a member of a union with all messages defined in the schema.
What I wonder is:

• are those datatypes available to the application? that is: can I define a variable to be of the union type for color request params? (or maybe, depending on how the schema is built, general params)
• does this actually make sense? It seems that this would create a ton of potential errors on passing parameter structs to a function

So the Struct type definitions represent the in-memory layout of an object and so not really intended for API use. One issue is that string and array fields are stored as indexes into internal structures managed by the database, which makes direct access unsafe and error prone. Currently the tag for unions is also included (as it's a property) but that's about to change (PR #102).

Stuff like colour information is just numbers so there's no issue with using the generated Structs if it's convenient. But of course what's missing is a way to update an object using the struct instead of individual fields. That would be pretty easy to do since internally it's just a memcpy, but again this becomes dangerous if arrays or strings are involved. A compromise might be to generate update methods for any struct types which contain only integral values.

Example for ContainedRaw::Struct:

struct __attribute__((packed)) Struct {
    uint16_t r{0};
    uint16_t g{0};
    uint16_t b{0};
    uint16_t ww{0};
    uint16_t cw{0};
};

We could generate a method within ContainedRaw:

const Struct getStruct() const;

and within RawUpdater:

void setStruct(const Struct& value);

[pljakobs]

this morning, I was trying to implement receive messages, which turns out to be more difficult than I initially thought, as it isn't immediately obvious to me how I would select a schema.
I guess the easiest is for "error" messages - the have an "id" and an "error" property.
If we're receiving a request, we need to map the "method" to a schema, which can either be done in the application or, if the code generator would build that, in generated code (probably the best option).
If we're receiving a response, even the generator doesn't necessarily have enough information, as the method of aresponse is implied by the id that mapps to the request id and thus the request method defines the method schema for the response.
My Schema at this point does not create a dependable link between request objects and the corresponding response form,
I wonder if that could be solved by defining the schema differently - maybe request, response and notification for a given API endpoint have to be within one oneOf structure like this:

{
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "$defs": {
    "info": {
      "oneOf": [
        {
          "ctype": "InfoRequest",
          "type": "object",
          "title": "info-request",
          "properties": {
            "jsonrpc": { "$ref": "value-types/$defs/jsonrpc-version" },
            "id": { "$ref": "value-types/$defs/jsonrpc-id" },
            "method": { "$ref": "value-types/$defs/info-method" }
          },
          "required": ["jsonrpc", "id", "method"],
          "additionalProperties": false
        },
        {
          "ctype": "InfoResponse",
          "type": "object",
          "title": "info-response",
          "properties": {
            "jsonrpc": { "$ref": "value-types/$defs/jsonrpc-version" },
            "id": { "$ref": "value-types/$defs/jsonrpc-id" },
            "result": {
              "oneOf": [
                { "$ref": "params/$defs/info-v1-params" },
                { "$ref": "params/$defs/info-v2-params" }
              ]
            }
          },
          "required": ["jsonrpc", "id", "result"],
          "additionalProperties": false
        },
        {
          "ctype": "InfoEvent",
          "type": "object",
          "title": "info-event",
          "properties": {
            "jsonrpc": { "$ref": "value-types/$defs/jsonrpc-version" },
            "method": { "$ref": "value-types/$defs/info-method" },
            "params": {
              "oneOf": [
                { "$ref": "params/$defs/info-v1-params" },
                { "$ref": "params/$defs/info-v2-params" }
              ]
            }
          },
          "required": ["jsonrpc", "method", "params"],
          "additionalProperties": false
        }
      ]
    }
  }
}

ConfigDB does not use anchor today, right?

[mikee47]

I've moved the JsonRPC module into the library and reworked your sample code. Does any of that make sense?