iotagent-node-lib
iotagent-node-lib copied to clipboard
telefonicaid
Question: Adding NGSI-LD 1.6.1 PATCH support
Unlike #1258 adding merge PATCH support will prove a bit tricker.
- Not all IoT Agents will want to support the merge PATCH endpoint at all (though it is useful for advanced actuation)
- Not all IoT Agents will want to support
nullto indicate the removal of a target member - see line 2689
None of the members described admit a null value directly, as when a JSON-LD processor
encounters null, the associated entry or value is always removed when expanding the JSON-LD
document. However in the context of a update or merge operation (see clauses 5.5.8 and 5.5.12), a
null encoded as a JSON literal {"@type": "@json", "@value": null } is a valid JSON token and
shall be used to indicate the removal of a target member. In all other cases implementations shall
raise an error of type BadRequestData if a null or JSON literal null value is encountered.
Before raising a PR to cover this, I thought I'd raise an issue for discussion how best to handle this. I guess they are new config parameters for the server:
contextBroker: {
host: '192.168.1.1',
port: '1026',
ngsiVersion: 'ld',
jsonLdContext: 'http://context.json-ld'
},
server: {
port: 4041
},
Becomes
contextBroker: {
host: '192.168.1.1',
port: '1026',
ngsiVersion: 'ld',
jsonLdContext: 'http://context.json-ld'
},
server: {
port: 4041,
supportNull: true,
supportMerge: true
},
If so I assume that is just another Docker ENV param and nothing is needed for provisioning devices or services.
Probably separate supportNull and supportMerge into different sections makes sense since it is a NGSL-LD specific behavior in the configuration. In case of using mixed mode this can be confusing (it would be supported by v2 endpoint?).
I propose to use a new key into config file. As an example, ldFlavours or ldConfig as name for the new key.
The point is that this is actually the north port NGSI-LD capability of the IoT Agent itself ( server ) and by implication the underlying IoT Agent in NGSI-LD mode. Something like this:
server: {
port: 4041,
ldCapability : {
supportNull: true,
supportMerge: true
}
},
Imagine you have something like an NGSI-LD OneM2M bridge or an NGSI-LD ROS2 IoT Agent. In this case it makes sense that the removal of a target member aligns to removing the relevant container in OneM2M or unsubscribing from a given ROS2 topic. Merge Patch would need to be
- get the latest state from the southport as JSON
- apply IETF JSON Merge Patch
- push the actuation back down into the southport.
Now this sort of complex operation doesn't make any sense for simpler protocols, so wouldn't need to be supported, but would be really, really useful in robotics.
The point is that this is actually the north port NGSI-LD capability of the IoT Agent itself (
server) and by implication the underlying IoT Agent in NGSI-LD mode. Something like this:server: { port: 4041, ldCapability : { supportNull: true, supportMerge: true } },
This alternative is fine as long as it does not impact existing codebase for v2 endopoint.
Next question on advanced actuations. The NGSI-LD supports multiple values for the same attribute separated by a datasetId for example:
{
"lampColor": [
{
"type": "Property",
"value": ["green", "55 secs"],
"datasetId": "urn:ngsi-ld:Property:do-this"
},
{
"type": "Property",
"value": ["red", "10 secs"],
"datasetId": "urn:ngsi-ld:Property:then-do-this"
}
],
}
Could be used to send sequential commands to an endpoint. At the moment each Attribute is defined as:
attributes.push({
type: 'Property',
value,
name: attribute
});
Could this be extended to add an optional datasetId? Is this something to be added to the NGSI-v2 handlers as well?
Second question on advanced actuations. There are some commands in robotics which rely on the metadata as well as the command itself, for example move by X,Y,Z given set of a reference coordinates:
{
"moveTo": {
"type": "Property",
"value": [1,2,3],
"qos": { "type": "Property", "value": 1},
"referenceCoords": { "type": "Property", "value": [1,2,3]},
"format": { "type": "Property", "value": "W3C"}
}
}
Could Attribute be extended to add a metadata element. Since metadata is supported by the v2 API, I assume this is something to be added to the NGSI-v2 handler as well.
{
"moveTo": {
"type": "Array",
"value": [1,2,3],
"metadata": {
"qos": { "type": "Number", "value": 1},
"referenceCoords": { "type": "Array", "value": [1,2,3]},
"format": { "type": "Text", "value": "W3C"}
}
}
}
Basically re-defining each attribute as:
attributes.push({
type: 'Property',
value,
name,
datasetId,
metadata
});
I've done some investigations here and I've discovered something:
- NGSI-v2 already partially supports
metadata- I've added a test here: e7a7bb2 -
metadatasupport was missing from NGSI-v2 notifications - I've fixed it and added a test here: ca27860
So I don't think metadata is controversial.
datasetId is an NGSI-LD only thing, so there will be no need to add support to the NGSI-v2 handlers I guess. The datasetId attribute in each value can be left as undefined which conveniently enough aligns to the NGSI-LD concept of the default datasetId anyway, which is the 90% use case.
Once I've got notification based NGSI-LD commands extended as well, all that remains is to clearly document the difference in the docs stating that sending sequential commands to an endpoint is NGSI-LD only.
Was this issue covered by PR https://github.com/telefonicaid/iotagent-node-lib/pull/1266?
- Add: datasetId and metadata support for NGSI-LD PATCH+PUT operations (commands)
Or anything is pending?
Still pending. At the moment the supportNulls flag discussed above is hardcoded. There should be also be a supportDatasetId flag too. supportMerge now exists (kind of) since the correct Unsupported Operation error response is returned if no mergeHandler has been set on the agent, and this aligns with the mechanism already in use with other handlers like queryHandler or commandHandler.
The docs should be updated to describe all these handlers, but they are already out of date since commandHandler and configurationHandler appear to be missing from the list, and not all signatures for the IoT Agent's library interface are listed correctly - not specifically a Merge-Patch issue in this case, just a lack of proper consistency in maintenance.
It doesn't help that there is no proper way of defining which bits of the existing interface actually are properly public and not going to change and which parts happen to be publicly accessible and are still subject to alteration or removal - this is due to the age of the codebase - a class based structure would do a better job of data hiding.
The main part still missing is an ability to reject null and/or datasetId on purpose - i.e. "This IoT Agent can't understand that payload" - and make that configurable through config.js and environment variables.