Skip to content

Conversation

@jason-fox
Copy link
Contributor

Updates the documentation to explicitly include GeoJSON examples. This means that examples for the following options are now available:

  • NGSI-LD with legacy - (which still relies on the current conversion function used in NGSI-LD)
  • NGSI-LD with JEXL
  • NGSI-v2 with JEXL

Related #1150 , supersedes #1020

@mapedraza
Copy link
Collaborator

mapedraza commented Nov 23, 2021

We would suggest to reduce the scope of this PR to only documentation changes (including the examples derived from #1050 and additional fixes).

With regards to code changes, if we have understand correctly, you are just removing old commented codes in the geoproperties autocasting part, but not implementing autocast as in line with PR #1020 (paying special attention to this comment from @fgalan #1020 (comment)). Even though with JEXL you can perform practically all the changes needed for any casting, having a library with certain common changes can be helpful for the users, especially those who have no knowledge of JEXL, as well as simplifying the devices provisioning.

In sum: reduce this PR only to documentation changes and continue the code work in #1020 (or a fresh new PR if that one is to old) towards a JEXL-independent and full-type (i.e. not only geoproperties) autocast feature.

@jason-fox
Copy link
Contributor Author

Fixed c5e3779 - this is now purely a documentation PR.

Comment on lines 497 to 511
| :------------------------------------------- | :--------------- |
| `" a "|trim` | `a` |
| `name|length` | `8` |
| `name|indexOf("e")` | `1` |
| `name|substring(0,name|indexOf("e")+1)` | `"De"` |

The following are some expressions not supported by the legacy expression language:
The following are some examples of **JEXL** expressions not supported by the **legacy** expression language:

| Expression | Expected outcome |
| :-------------------------------------------------- | :---------------------------------- |
| `value == 6? true : false` | `true` |
| <code>value == 6 && name&vert;indexOf("e")>0</code> | `true` |
| `array[1]+1` | `3` |
| `object.name` | `"John"` |
| `{type:"Point",coordinates: [value,value]}` | `{type:"Point",coordinates: [6,6]}` |
| Expression | Expected outcome | Format |
| :------------------------------------------ | :---------------------------------- | ------- |
| `value == 6? true : false` | `true` | Boolean |
| `value == 6 && name&vert;indexOf("e")>0` | `true` | Integer |
| `array[1]+1` | `3` | Number |
| `object.name` | `"John"` | String |
| `{type:"Point",coordinates: [value,value]}` | `{type:"Point",coordinates: [6,6]}` | Object |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem rendering the simbol | placed into a code block, inside a table. Almost this part need to be reverted using : <code> ... </code> and &vert; for vertical bar |

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed 54b1a53 - it looks like GitHub's Markdown parser is less sophisticated than MkDocs/ReadTheDocs

Copy link
Collaborator

@mapedraza mapedraza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

- [Trusts concept](https://docs.openstack.org/keystone/stein/user/trusts)
- [Trusts API](https://docs.openstack.org/keystone/stein/api_curl_examples.html#post-v3-os-trust-trusts)

### NGSI-LD `GeoProperty` support
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the ToC at the beginning of the file I see:

[GeoJSON support NGSI-LD only](#geojson-support-ngsi-ld-only)

So it seems section title and corresponding entry in ToC are not aligned...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ef3dd11

| `${@pressure * 20}` | `1040` | Integer |
| `${trim(@pressure)}` | `52` | Integer |
| `${@consumption * 20}` | `8.8` | Float |
| `${trim(@consumption)}` | `0.44` | Integer |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0.44 is not an Integer...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ef3dd11

| `${@pressure * 20}` | `1040` | Integer |
| `${@active * 20}` | `null` | None |
| `${trim(@active)` | `null` | None |
| `${trim(@consumption)}` | `0.44` | Integer |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ef3dd11

| Expression | Expected outcome | Format |
| :------------------------------------------ | :---------------------------------- | ------- |
| `value == 6? true : false` | `true` | Boolean |
| <code>value == 6 && name&vert;indexOf("e")>0</code> | `true` | Integer |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Integer also in true case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed ef3dd11

Copy link
Member

@fgalan fgalan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@fgalan fgalan merged commit 735c269 into telefonicaid:master Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants