telefonicaid / iotagent-node-lib

Module to enable IoT Agent developers to build custom agents for their devices that can easily connect to NGSI Context Brokers
https://iotagent-node-lib.rtfd.io/
GNU Affero General Public License v3.0
60 stars 86 forks source link

Adding NGSI-LD support #842

Closed jason-fox closed 3 years ago

jason-fox commented 4 years ago

It is necessary to add support for NGSI-LD to all IoT Agents. This is a large amount of work and needs to be split according. This issue is to discuss how the code can be incorporated and link the relevant PRs.

The main issue here is that there is a lot of work to review, so I've tried to split it into smaller PRs

1) [x] #839 - NGSI-LD relies very heavily on metadata support (a.k.a. Properties of Properties) - whilst adding the code it was noticed that aliasing didn't work nicely with metadata. Effectively this is a bug fix. 2) [x] #840 - This is a simple Grep to be able to distinguish between NGSI-v2 and NGSI-LD tests. Very minor change across lots of files - I split it out to reduce the size of the PR 3) [x] #841 - This is the main body of work for measures support 4) [x] #843 - Temporal and GeoProperty support 5) [x] #847 - Add NGSI-LD Multimeasures support 6) [x] #848 - Add NGSI-LD Command support 7) [x] #850 - Update the NGSI interactions to use ES-6

Dependencies

jason-fox commented 4 years ago

Things which haven't been done yet (which I'd prefer to raise as separate smaller PRs)

The thing is that this is still experimental, but would be massive if it is all collected into one PR.

Basically need to discuss how this can be reviewed, tested and added into the codebase in a timely manner.

jason-fox commented 4 years ago

The following repo provides a test harness for the NGSI-LD support.

https://github.com/jason-fox/tutorials.IoT-Agent/tree/test

To run the test harness just type:

./services start

Load the Postman collection into Postman and run interactively.

Alternatively use the following cUrl commands:

Provision Service Group

curl -L -X POST 'http://localhost:4041/iot/services' \
-H 'fiware-service: openiot' \
-H 'fiware-servicepath: /' \
-H 'Content-Type: application/json' \
--data-raw '{
 "services": [
   {
     "apikey":      "4jggokgpepnvsb2uv4s40d59ov",
     "cbroker":     "http://orion:1026",
     "entity_type": "Motion",
     "resource":    "/iot/d",
     "trust": "08ca25ba8ea0f322f50244c413fce2e32feed43d"
   }
 ]
}'

Provision Device

curl -L -X POST 'http://localhost:4041/iot/devices' \
-H 'fiware-service: openiot' \
-H 'fiware-servicepath: /' \
-H 'Content-Type: application/json' \
--data-raw '{
 "devices": [
   {
     "device_id":   "motion001",
     "entity_name": "urn:ngsi-ld:Motion:001",
     "entity_type": "Motion",
     "timezone":    "Europe/Berlin",
     "attributes": [
       { "object_id": "c", "name":"count", "type":"Integer"}
      ],
      "static_attributes": [
         {"name":"refStore", "type": "Relationship","value": "urn:ngsi-ld:Store:001"}
      ]
   }
 ]
}
'

Dummy Device Measure

curl -L -X POST 'http://localhost:7896/iot/d?k=4jggokgpepnvsb2uv4s40d59ov&i=motion001' \
-H 'Content-Type: text/plain' \
--data-raw 'c|1'

Read Measure as NSGI-LD Data

curl -L -X GET 'http://localhost:1026/ngsi-ld/v1/entities/urn:ngsi-ld:Motion:001' \
-H 'fiware-service: openiot' \
-H 'fiware-servicepath: /' \
-H 'Link: <https://fiware.github.io/tutorials.Step-by-Step/tutorials-context.jsonld>; rel="http://www.w3.org/ns/json-ld#context"; type="application/ld+json"'
fgalan commented 4 years ago

As a general comment, I think the idea of working the functionality in several branches (each one branching from the previous), creating and merging PRs one by one is a good idea.

PRs #839 and #840 seems to be small and straighforward. I have provided some feedback on them and I think once addresses they can be merged (I'll check with @AlvaroVega for a second opinion, anyway).

With regards to #841 and #843 (and all the other that may come in that line) I think it will be a good idea to create a branch from master ("prelanding-ngsi-ld" for instance) then merge the PRs on that branch and run CI on that branch (it's simpler for us run CI testing when the branch lives in our repository). We have used this approach other times for big works (e.g. NGIv2 implementation).

If you agree, I'll take care of creating that "prelanding-ngsi-ld" (I'd do so after de #839 and #840 merges).

fgalan commented 4 years ago

If you agree, I'll take care of creating that "prelanding-ngsi-ld" (I'd do so after de #839 and #840 merges).

PR #839 and PR #840 have been just merged. Should I create "prelanding-ngsi-ld" "feature/842_ngsi_ld" branch from master and change to it the base branch for PR #841 and RB #843?

jason-fox commented 4 years ago

That is fine by me .

fgalan commented 4 years ago

Branch has been created (at the end the name is: feature/842_ngsi_ld and PR #841and PR #843 has been changed to use it as base branch.

fgalan commented 4 years ago

Basic implementation should be completed in PR #849, which now steps to CI e2e phase before merging.

However, we would like to propose a modification for the future (with the category of "technical debt" in order not blocking the merge of PR #849).

Instead of having a global setting for the NGSI version (v1, v2 or LD) the parameter can be specified at three different levels:

  1. Global setting (in config.js or equivalent env var)
  2. Per configuration group
  3. Per device

being the last one preferred over the formers. I mean, device takes precedence over configuration group and configuration group takes preference over global setting.

2 and 3 will need an slight modification of the provisioning API to include the new field in the provision API (suggestion: "ngsiVersion": "v1|v2|ld").

I'd like to remark this pattern has been already used for other configurable aspects in IOTA. For instance, timestamp setting that can be defined globally, per configuration group and per device). A similar approach is used in the JEXL contribution and explicitAttrs contribution.

Making NGSI version selectable at global/configuration/device level is important given that, as you know, NGSIv2-NGSILD interoperability is a main requirement and this is a way in which we could have the same IOTA instance running both versions of the API at the same time, in a very configurable and flexible manner.

jason-fox commented 4 years ago

Whereas this does make sense for attributes or changing the payload, I don't think that a per-device switch pushing two data formats into a single context broker makes much sense for NGSI-LD/NGSI v2. There are multiple NGSI-LD brokers (five or six at last count) - only one also has an NGSI-v2 interface, and that is missing a lot of recent changes. I won't implement something like this with the current round of development - it is too much of a moving target - best to add it as a backlog issue to technical debt if you feel it is important.

It should be remembered that combined NGSI v1/v2 into one broker was not added when the NGSI v1/v2 messaging was added to the IoT Agents - and NGSI v1/v2/LD follows the same paradigm - this makes messaging a bit different to the other attribute switch use cases you have cited.

Personally for combined v2/LD, I would use two separate IoT Agent instances and connect them to separate NGSI v2 and NGSI-LD brokers. This keeps my backing databases clean, and would allow me to switch in and out other NGSI-LD brokers as necessary. In the case that I need to duplicate context data my preferred solution/ workaround for now would be to shadow device data using subscriptions.

jason-fox commented 3 years ago

Merge completed.

fgalan commented 3 years ago

Two items in https://github.com/telefonicaid/iotagent-node-lib/issues/842#issuecomment-583404601 are still unchecked.

Maybe some specif issues should be created for them?

jason-fox commented 3 years ago

969 should cover the documentation of basic measures interactions. Commands and Lazy Attributes are still subject to amendment by ETSI CIM - you may wish to add an issue on that.