Skip to content

Conversation

bobeal
Copy link
Contributor

@bobeal bobeal commented Aug 5, 2020

Hi,

Creating this PR in draft mode, as it is obviously not ready yet to be merged. The purpose is to launch the discussion about what is inside.

The point is the following:

  • Currently, when a sensor sends a new measure, the whole corresponding entity is replaced in the CB (via a call to batch upsert endpoint)
  • This is semantically not correct, as we are actually just updating the value of one or more properties (along with their observedAt properties)
  • What's more, it makes hard to record the temporal evolution of a property (which is a major feature for a sensor), as the entity is replaced on each new measure

So here is a draft that uses the partial attribute update endpoint instead of the batch upsert.

To not break any compatibility with existing deployments, maybe we could make this configurable (something like config.contextBroker.usePartialAttributeUpdate)?

What are your thoughts about this?

…ensor values

Signed-off-by: Benoit Orihuela <benoit.orihuela@eglobalmark.com>
@fgalan
Copy link
Member

fgalan commented Aug 7, 2020

I'm afraid the subtleness of NGSI-LD is beyond my scope :) so I cannot provide an opinion on that. Maybe @jason-fox could provide good feedback on this.

However, with regards to

To not break any compatibility with existing deployments, maybe we could make this configurable (something like config.contextBroker.usePartialAttributeUpdate)?

The work in feature/842_ngsi_ld should be understood as experimental (as the CNR says: "Add basic NGSI-LD support as experimental feature") so no need of introduce this new parameter from my point of view.

@jason-fox
Copy link
Contributor

jason-fox commented Aug 10, 2020

Can you explain further the reasoning behind why upsert needs replacing with a series of attribute updates? From my understanding of the 1.3.1 spec

  • post/ngsi-ld/v1/entityOperations/upsert/ (array of one or more items)

Is functionally equivalent to a series of operations of the form:

  • patch /ngsi-ld/v1/entities/urn:ngsi-ld:WeatherStation:ws4/attrs

As it is, the existing NGSI-LD implementation aligns with the upsert operations used in the v2 and v1 equivalent classes and involves less HTTP traffic. It also allows for anonymous measures (where the device is not known and therefore the measure is an operations which creates the entity). Has the amendment been tested with anonymous measures and multi-measures?

If it is necessary to make multiple individual entity updates, then I would assume that that code would lie within the IoT Agent itself, not the library. The library itself needs to remain compatible with all IoT Agents.

Just trying to understand the reasoning behind the proposal.

@jason-fox
Copy link
Contributor

jason-fox commented Aug 10, 2020

From the NGSI-LD 1.2.1 spec (section 5.6.8 Batch Entity Creation or Update (Upsert) )

For each of the NGSI-LD Entities included in the input Array implementations shall:

  • Retain the concerned Entity under the corresponding Entity collection if it does not exist yet (i.e. no
    Entity with the same Entity Id is present).
  • If there were an existing Entity with the same Entity Id, it shall be completely replaced by the new Entity
    content provided, if the requested update mode is replace.
  • If there were an existing Entity with the same Entity Id, it shall be executed the behaviour defined by
    clause 5.6.3, if the requested update mode is update.

Now it could be the case that in the current implementation the update mode has been specified incorrectly in the request, it should be update not replace.

@bobeal
Copy link
Contributor Author

bobeal commented Aug 12, 2020

Hi Jason,

For each of the NGSI-LD Entities included in the input Array implementations shall:

  • Retain the concerned Entity under the corresponding Entity collection if it does not exist yet (i.e. no
    Entity with the same Entity Id is present).
  • If there were an existing Entity with the same Entity Id, it shall be completely replaced by the new Entity
    content provided, if the requested update mode is replace.
  • If there were an existing Entity with the same Entity Id, it shall be executed the behaviour defined by
    clause 5.6.3, if the requested update mode is update.

Now it could be the case that in the current implementation the update mode has been specified incorrectly in the request, it should be update not replace.

The problem is that the update mode says it must respect the behavior defined by clause 5.6.3, which is about appending entity attributes (not updating existing entity attributes). Thus entity attributes contained in the payload will replace the existing ones, and not update them.

Another problem related to this: the target entity and its properties may have changed in the CB (for instance, one could have added a new relationship to the property holding the sensor values). By using the batch upsert operation, the IoT Agent overrides the existing attribute, thus potentially erasing some information because it has no knowledge of this change (well, he could do some preliminary synchronization before upserting but I think that goes out of its scope).

@bobeal
Copy link
Contributor Author

bobeal commented Aug 12, 2020

As it is, the existing NGSI-LD implementation aligns with the upsert operations used in the v2 and v1 equivalent classes and involves less HTTP traffic. It also allows for anonymous measures (where the device is not known and therefore the measure is an operations which creates the entity). Has the amendment been tested with anonymous measures and multi-measures?

It has been tested with multi-measures (we are using this with Sigfox based smart meters that send around 10 measures on each update).

But it has not been tested with anonymous measures. Of course, a patch update on a non existing entity will fail with a 404. So that would probably require to fallback to the "create initial entity" flow if the device is not known?

@jason-fox
Copy link
Contributor

jason-fox commented Aug 13, 2020

Another problem related to this: the target entity and its properties may have changed in the CB (for instance, one could have added a new relationship to the property holding the sensor values

This is a modelling question which @fgalan and the Telefonica team should have an opinion on. Based on the current functionality of v1 and v2 should the be IoT Agent solely responsible for the Device entity?

For example, I can already set static attributes within a device or set of devices.

curl -s -o /dev/null -X POST \
  'http://iot-agent:4041/iot/services' \
  -H 'Content-Type: application/json' \
  -H 'fiware-service: openiot' \
  -H 'fiware-servicepath: /' \
  -d '{
 "services": [
   {
     "apikey":      "3314136",
     "cbroker":     "http://orion:1026",
     "entity_type": "Lamp",
     "resource":    "/iot/d",
     "protocol":    "PDI-IoTA-UltraLight",
     "transport":   "HTTP",
     "timezone":    "Europe/Berlin",
     "commands": [ 
        {"name": "on","type": "command"},
        {"name": "off","type": "command"}
     ],
     "attributes": [
        {"object_id": "s", "name": "state", "type":"Text"},
        {"object_id": "l", "name": "luminosity", "type":"Integer"}
     ],
     "static_attributes": [
          {"name": "category", "type":"Text", "value": ["actuator","sensor"]},
          {"name": "controlledProperty", "type": "Text", "value": "light"},
          {"name": "function", "type": "Text", "value":["onOff", "sensing"]},
          {"name": "supportedProtocol", "type": "Text", "value": ["ul20"]},
          {"name": "supportedUnits", "type": "Text", "value": "CDL"}
     ]
   }
 ]
}'

I can already set or amend relationships (e.g. controlledAsset) within an individual device (when provisioning):

curl -s -o /dev/null -X POST \
  'http://iot-agent:4041/iot/devices' \
  -H 'Content-Type: application/json' \
  -H 'fiware-service: openiot' \
  -H 'fiware-servicepath: /' \
  -d '{
 "devices": [
    {
      "device_id": "lamp001",
      "entity_name": "Lamp:001",
      "entity_type": "Lamp",
      "endpoint": "http://iot-sensors:3001/iot/lamp001",
       "static_attributes": [
          {"name": "controlledAsset", "type": "Relationship","value": "urn:ngsi-ld:Room:101"}
      ]
    }
  ]
}
'

So my assumption here is that following the current implementation of v1, v2 the IoT Agent maintains the device - full stop.

Now imagine you have the Room urn:ngsi-ld:Room:101 entity. It would hold attributes such as:

{ 
  "name": "luminosity", 
   "type":"Property",
  "value": 10,
  "unitCode" : "CDL",
  "providedBy" : "urn:ngsi-ld:Lamp:001"
}

Where the property-of-a-property unitCode and relationship-of-a-property providedBy are coming from Lamp.
(at the moment this could be created using a subscription:

curl -L -X POST 'http://localhost:1027/v2/subscriptions/' \
-H 'Content-Type: application/json' \
-H 'fiware-service: openiot' \
--data-raw '{
  "description": "Notify Subscription Listener of Lamp context changes",
  "subject": {
    "entities": [
      {
        "idPattern": "Lamp.*"
      }
    ],
    "condition": {
      "attrs": ["luminosity"]
    }
  },
  "notification": {
    "http": {
      "url": "http://tutorial:3000/device/subscription/luminosity"
    },
    "attrs": ["luminosity", "controlledAsset", "supportedUnits"]
  },
  "throttling": 5
}' 

... but I was thinking this workaround could be added to the IoT Agent code by adding an internal_attribute flag indicating that controlledAsset somehow was the indication of the other entity (and potentially a mapping of attribute name(s) to be copied). According to the Smart data models Device Model this is the reason for having controlledAsset as an attribute.

This would mean that Device remains literally just the thing controlled by the IoT Agent. You'd only need to read from it if you needed more details (e.g. batteryLevel) whereas Room or whatever else you wanted to annotate and use would have a linked data attribute.

Any thoughts? 🤔 -What about anonymous measures?

@bobeal
Copy link
Contributor Author

bobeal commented Aug 18, 2020

So my assumption here is that following the current implementation of v1, v2 the IoT Agent maintains the device - full stop.

OK, I get the point. But, practically, that means an IoT Agent is (and must act as) a (NGSI-LD) context source, which is the only sure way for it to say it maintains the device and no one else can modify it (unless you add some authorization in the CB). Why not, but it has to be known and clearly stated (especially when integrating with a CB). Btw, I saw some code in IoT agent node lib calling the context source registrations API but I was not clear about the when and what for ...

That is somewhat inline with the examples given in the Distributed Architecture (6.3.3) section of the NGSI-LD API specification: "a certain Context Source registers that it can provide the indoor temperature for Building A and Building B or that it can provide the speed of cars in a geographic region covering the centre of a city."

But, what confuses me is that this is not in line with the current way of (batch) creating the devices in the CB (a context source is not supposed to do that) ...

Or maybe it is an acceptable risk that the entity can be modified directly in the CB (one is not supposed to break things ...)?

... but I was thinking this workaround could be added to the IoT Agent code by adding an internal_attribute flag indicating that controlledAsset somehow was the indication of the other entity (and potentially a mapping of attribute name(s) to be copied). According to the Smart data models Device Model this is the reason for having controlledAsset as an attribute.

So, If I understand it correctly, upon the reception of a new measure, the IoT Agent would update the corresponding property on the "controlled" entity in the CB?

Any thoughts? thinking -What about anonymous measures?

Well, I'm still not totally clear on the better way to manage the devices provisioning ...

To get back to the original subject, I see two ways to communicate new sensor values:

  • Use the partial attribute update (5.6.4), even the attribute update (5.6.2) do a replacement (unless we consider the replacement as a full update ...)
  • Add a new attribute instance on a temporal entity (5.6.12)

Not sure that helps a lot ... at least, it reflects some of our current thoughts on devices provisioning and management wrt the NGSI-LD API 😄

@jason-fox
Copy link
Contributor

From the ETSI spec:

6.15.3.1 POST
This method is associated to the operation "Batch Entity Creation or Update (Upsert)" and shall exhibit the behaviour
defined by clause 5.6.8. Figure 6.15.3.1-1 shows the operation interaction and table 6.15.3.1-1 describes the request
body and possible responses.
The "options" query parameter for this request can take the following values:
• "replace". Indicates that all the existing Entity content shall be replaced (default mode).
• "update". Indicates that existing Entity content shall be updated.

Looking at the sendUpdateValueNgsiLD() here you can see I am not setting the options=update query parameter. Therefore we are getting the default (replace) behaviour.

If this were to be altered to use:

const url = '/ngsi-ld/v1/entityOperations/upsert/?options=update';

Would this be sufficient?

@bobeal
Copy link
Contributor Author

bobeal commented Sep 11, 2020

Hello Jason,

Sorry for this late answer!

And, yes, I thought about this and a batch upsert in update mode (which allows the CB and potential subscribers to interpret them as updates of existing properties) would be sufficient.

I guess I can close this draft PR now 😄

@jason-fox
Copy link
Contributor

Maybe you could raise a PR to add options=update?

@bobeal
Copy link
Contributor Author

bobeal commented Sep 11, 2020

Maybe you could raise a PR to add options=update?

Good idea 😄

I'll do it before end of next week.

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