-
Notifications
You must be signed in to change notification settings - Fork 91
Add support for NGSI-LD valueType
and more NGSI-LD sub-property types.
#1723
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
Conversation
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.
This file (and the rest of similar ones) are under ngsiv2/ structure.
Maybe they are no longer used but we forgot to remove them? In that case, thanks for the clean up :)
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.
Remove: autocast (including env var IOTA_AUTOCAST
) (#1498) was part of 4.0.0 release
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
@@ -222,7 +221,7 @@ function updateEntityNgsi2(deviceData, updatedDevice, callback) { | |||
) { | |||
options.json[constants.TIMESTAMP_ATTRIBUTE] = { | |||
type: constants.TIMESTAMP_TYPE_NGSI2, | |||
value: moment() | |||
value: new Date().toISOString() |
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.
Really needed or a kind of improvement?
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.
Moment.js is deprecated, or rather there are better ways of achieving the same result without the use of an unnecessary extra package. In this case Date
gives us everything we need from moment.
https://momentjs.com/docs/#/-project-status/
Ideally I'd like to remove both moment and moment-timezone entirely at some point
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.
Sounds good :) NTC
Co-authored-by: Fermín Galán Márquez <[email protected]>
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
Let's wait for @AlvaroVega to merge this PR.
doc/admin.md
Outdated
@@ -105,6 +105,7 @@ It configures the connection parameters to stablish a connection to the Context | |||
host: '192.168.56.101', | |||
port: '1026', | |||
ngsiVersion: 'ld', | |||
valueType: 'valueType', |
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.
Given that value type is only used by NGSI-LD and taking into account that other LD-related config use the "Ld" token in the name (eg jsonLdContext
, maybe it shoulb be named ldValueType
?
In that case, the env var could be renamed to IOTA_CB_NGSILD_VALUE_TYPE
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 b50a691
Moved to IOTA_LD_SUPPORT_DATA_TYPE
with config value server.ldSupport.datatype
- this aligns to the other LD only config values.
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
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
As defined in the 1.9.1 NGSI-LD specification, there are now two mechanisms available for supporting the equivlent of NGSI-v2
type
attribute holding an attribute's datatype.This PR updates NGSI-LD processing to allow for three types of valueType processing to add datatype metadata
valueType
@type
The previous 1.8.1 default (and recommendation) was
@type
, but this should change sincevalueType
was created to simplifyvalue
and make it queriable, The new default (for now) should be nothing, sincevalueType
is not widely supported by context brokers yet.Note that
valueType
alwayd optional - it is still possible to define attributes asProperty
,GeoProperty
orRelationship
. Furthermore the additional sub-types ofProperty
such asJsonProperty
,VocabProperty
andListProperty
are also now supported.