-
Notifications
You must be signed in to change notification settings - Fork 91
Add NGSI-LD basic Command support #848
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add NGSI-LD basic Command support #848
Conversation
- Split out NGSI-v1, v2 and LD handlers into separate files. - Add support for PATCH incoming commands. - Amend tests ( replace `/v2/op/update/` with NGSI-LD PATCH) - Amend test expectations to NGSI-LD format
|
This may also be working with polling commands, but the tests haven't been updated yet. |
Merge branch 'feature/ngsi-ld-multimeasure' into feature/ngsi-ld-command
- Do not create additional context - Upsert responds with 204 - Pass context in the body not Link Header - Improve debug - display stringified responses from broker
Merge branch 'feature/ngsi-ld-multimeasure' into feature/ngsi-ld-command
- Amend handler - Enabler test - Amend expectations if necessary.
|
Once #847, I have change base branch in this PR to telefonicaid:feature/842_ngsi_ld Please updgrade jason-fox:feature/ngsi-ld-command with telefonicaid:feature/842_ngsi_ld and tune the usual things (CNR, minimal documentation if applicable, etc.) to make this PR ready to merge. |
|
I've documented the new configuration options only and left it at that for now. northboundinteractions.md is massively out of date and needs a full rewrite. All references to NGSI v1 need to be removed and replaced with v2 and LD equivalents. |
doc/installationguide.md
Outdated
| the IoTAgent runs in a single thread. For more details about multi-core functionality, please refer to the | ||
| [Cluster](https://nodejs.org/api/cluster.html) module in Node.js and | ||
| [this section](howto.md#iot-agent-in-multi-thread-mode) of the library documentation. | ||
| - **jsonLdContext**: the location of the NGSI-LD `@context` element which provides additional information allowing the computer to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference with contextBroker.jsonLdContext field? Is not the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation of contextBroker.jsonLdContext was missed in the previous PR - I've just added it in this PR instead. This is not another new configuration attribute, just adding the missing documentation for the existing one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same text is duplicated in L44-45. I'd suggest to avoid duplication and have it only in one place and that place should be L44-45 as it is the place closer to the configuration object in which jsonLdContext is being used (i.e. contextBroker).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed e099b84
| @@ -0,0 +1,183 @@ | |||
| var async = require('async'), | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing license header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed 263588b
| @@ -0,0 +1 @@ | |||
| [{"@context":"http://context.json-ld","id":"urn:ngsi-ld:Robot:r2d2","type":"Robot","position_status":{"type":"Property","value":{"@type":"commandStatus","@value":"UNKNOWN"}},"position_info":{"type":"Property","value":{"@type":"commandResult","@value":" "}}}] | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to have the .json in pretty printed form, as the others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed: cac4d5f
| }); | ||
|
|
||
| xdescribe('When a command update arrives to the IoT Agent for a device with polling', function() { | ||
| describe('When a command update arrives to the IoT Agent for a device with polling', function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the xdescribe() and xit() has been solved in this PR? Or is there any pending after it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are following PRs - I haven't completely duplicated v2 tests here. see - #848 (comment) - I'm not entirely sure how Nock would work on this test so better to leave a disabled test than delete the use case. Someone should check to see if polling is working manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this is because NGSI-LD is using entity upsert for both create and update so the expectations can't just be duplicated from v2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarification. NTC.
| var updateMiddlewaresNgsi1 = [ | ||
| middlewares.ensureType, | ||
| middlewares.validateJson(updateContextTemplateNgsi1), | ||
| handleUpdateNgsi1, | ||
| updateErrorHandlingNgsi1 | ||
| ], | ||
| updateMiddlewaresNgsi2 = [ | ||
| middlewares.ensureType, | ||
| middlewares.validateJson(updateContextTemplateNgsi2), | ||
| handleUpdateNgsi2, | ||
| updateErrorHandlingNgsi2 | ||
| ], | ||
| updateMiddlewaresNgsiLD = [ | ||
| middlewares.ensureType, | ||
| middlewares.validateJson(updateContextTemplateNgsiLD), | ||
| handleUpdateNgsiLD, | ||
| updateErrorHandlingNgsiLD | ||
| ], | ||
| queryMiddlewaresNgsi1 = [ | ||
| middlewares.ensureType, | ||
| middlewares.validateJson(queryContextTemplate), | ||
| handleQueryNgsi1, | ||
| queryErrorHandlingNgsi1 | ||
| ], | ||
| queryMiddlewaresNgsi2 = [ | ||
| handleQueryNgsi2, | ||
| queryErrorHandlingNgsi2 | ||
| ], | ||
| queryMiddlewaresNgsiLD = [ | ||
| handleQueryNgsiLD, | ||
| queryErrorHandlingNgsiLD | ||
| ], | ||
| updatePathsNgsi1 = [ | ||
| '/v1/updateContext', | ||
| '/NGSI10/updateContext', | ||
| '//updateContext' | ||
| ], | ||
| updatePathsNgsi2 = [ | ||
| '/v2/op/update', | ||
| '//op/update' | ||
| ], | ||
| updatePathsNgsiLD = [ | ||
| '/ngsi-ld/v1/entities/:id/attrs/:attr' | ||
| ], | ||
| queryPathsNgsi1 = [ | ||
| '/v1/queryContext', | ||
| '/NGSI10/queryContext', | ||
| '//queryContext' | ||
| ], | ||
| queryPathsNgsi2 = [ | ||
| '/v2/op/query', | ||
| '//op/query' | ||
| ], | ||
| queryPathsNgsiLD = [ | ||
| '/ngsi-ld/v1/entities/:id' | ||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where this routes configuration has been moved now? I have tried to find the new place, but I'm being unable...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The routes are defined in contextSever-NGSI-LD.js and the equivalent v1 and v2 files. Line 41,42 or thereabouts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NTC
Co-Authored-By: Fermín Galán Márquez <[email protected]>
fgalan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM


This PR adds basic Command and Lazy Attribute support
/v2/op/update/with NGSI-LD PATCH)/v2/op/query/with NGSI-LD GET)