Skip to content
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 support of timestamped value for Observe-Composite. #1089

Closed
sbernard31 opened this issue Sep 6, 2021 · 17 comments
Closed

Add support of timestamped value for Observe-Composite. #1089

sbernard31 opened this issue Sep 6, 2021 · 17 comments
Labels
new feature New feature from LWM2M specification server Impact LWM2M server
Milestone

Comments

@sbernard31
Copy link
Contributor

This feature is needed to support clients which are using Notification Storing When Disabled or Offline feature.

Currently,
Leshan client does not support this feature (see #596)
Leshan server is able to handle "classic" observe response from client using this feature.
Leshan server is not able to handle "composite" observe response from client using this feature.

@sbernard31
Copy link
Contributor Author

We should keep this (#1218 (comment)) in mind when we implement this.

@JaroslawLegierski
Copy link
Contributor

I prepared following PoC with very limited support for Observe-Composite (only in leshan client) with timestamped data
(mainly for better understunding observe-composite concept in Leshan)

What do you think about it - is this the correct direction?
Currently we have 2 different methods in encoder:

  • encode - for classical nodes
  • encodeTimestampedNode - for timestamped nodes

Do we need 3'rd encoder for encoding mixed nodes (timestamped with classical into one notification) ?

@sbernard31
Copy link
Contributor Author

sbernard31 commented Nov 16, 2023

I looked at it and don't think this is the right direction.

  1. There is no need to add new encoder/decoder TimestampedMultiNodeEncoder/TimestampedMultiNodeDecoder should be enough
  2. Maybe TimestampedMultiNodeDecoder.decodeTimestampedNodes(byte[], LwM2mModel) should be adapted as expected (Add Timestamped Nodes support to Send Operation at Server side. #1218 (comment))
  3. No need to add client code to test it, just create a dedicated integration tests like ObserveTimeStampTest
  4. (not directly linked I guess) I didn't get why you add .getId() to LwM2mRoot() ?

HTH 🙂

@JaroslawLegierski
Copy link
Contributor

I looked at it and don't think this is the right direction.

1. There is no need to add new encoder/decoder `TimestampedMultiNodeEncoder`/`TimestampedMultiNodeDecoder` should be enough
2. Maybe `TimestampedMultiNodeDecoder.decodeTimestampedNodes(byte[], LwM2mModel)` should be adapted as expected ([Add Timestamped Nodes support to Send Operation at Server side. #1218 (comment)](https://github.com/eclipse-leshan/leshan/pull/1218#discussion_r824550092))

OK then I will focus on adapting TimestampedMultiNodeDecoder.decodeTimestampedNodes(byte[], LwM2mModel)

3. No need to add client code to test it, just create a dedicated integration tests like `ObserveTimeStampTest`

As I understand it, I should start by writing dedicated interation test instead of modifying the client/leshan-client-demo code ?

4. (not directly linked I guess)  I didn't get why you add `.getId(`) to `LwM2mRoot()`  ?

It was the mistake on my side - already corrected. Thank you very much for help.

@sbernard31
Copy link
Contributor Author

OK then I will focus on adapting TimestampedMultiNodeDecoder.decodeTimestampedNodes(byte[], LwM2mModel)

Let me know, if there is unclear point.

As I understand it, I should start by writing dedicated interation test instead of modifying the client/leshan-client-demo code ?

Yep I think this will be easier and adding a integration tests is a good idea anyway.

@JaroslawLegierski
Copy link
Contributor

JaroslawLegierski commented Nov 30, 2023

Please find here PoC of Observe Composite with ts values support on server side.

The following points are not clear for me:

  1. Currently leshan SENML_JSON encoder can encode following records:
[
	{
		"bn": "/3303/0/5700",
		"bt": 1699877805.766,
		"v": -128.8
	},
	{
		"bn": "/6/0/0",
		"bt": 1699877805.800,
		"v": -39.0
	}
]

can we change this format to following one: ?

[
	{
		"bn": "/3303/0/5700",
		"bt": 1699877805.766,
		"v": -128.8
	},
	{
		"bn": "/6/0/0",
		"t": 0.101,
		"v": -39.0
	}
]
  1. Support of mixed data (Multiple Measurements) when we have data with and without timestamps in one record e.g.:
[
	{
		"bn": "/3303/0/5700",
		"bt": 1699877805.766,
		"v": -128.8
	},
	{
		"bn": "/6/0/0",
		"v": -39.0
	}
]

Writing test that produces such data requires modifying the encoder (on the client side). Should I modify the existing encoder or create dedicated one only to be used by the integration test ?

@sbernard31
Copy link
Contributor Author

I looked at the POC quickly, I think this is the right direction. You can create a PR.

About SENML encoding, this is not related to this issue, so better to create a dedicated one.

@JaroslawLegierski
Copy link
Contributor

I looked at the POC quickly, I think this is the right direction. You can create a PR.

OK - PR #1552 created

About SENML encoding, this is not related to this issue, so better to create a dedicated one.

OK - done in #1554

@sbernard31
Copy link
Contributor Author

sbernard31 commented Dec 6, 2023

I look at #1552.

And I would like to agree on behavior before to go deeper.

Observer-Composite Behavior

For Observe-composite, I understand that each notification should contains all node requested by the Observe-Composite request.

If any of the conditions attached to one or more resources under observation meets the notification criteria a notify will be generated by the LwM2M client, which will include the value for each of the resources listed in the "Observe-Composite" operation. Hence, the resources that have not met the notify condition will still be included in the notification message.

(Source : http://www.openmobilealliance.org/release/LightweightM2M/V1_1_1-20190617-A/HTML-Version/OMA-TS-LightweightM2M_Core-V1_1_1-20190617-A.html#6-4-4-0-644-Observe-Composite-Operation)

E.g. : if you observe /1/0/1 and /3 each notification should contains both nodes.
Currently (without timestamp support) this looks like : Map<LwM2mPath, LwM2mNode> where :

// This is pseudo code representing Map<LwM2mPath, LwM2mNode>
{ 
    "/1/0/1" => LwM2mResource, // value can be null (if resource doesn't exist anymore)
    "/3"     => LwM2mObject, // value can be null (if resource doesn't exist anymore)
}

Examples of invalid structure :

// This is invalid
{ 
    "/1/0/1" => LwM2mResource,
    "/3/0/0"     => LwM2mResource
    "/3/0/1"     => LwM2mResource
}
// this is not valid too.
{ 
    "/1/0/1" => LwM2mResource,
    "/3"     => LwM2mObject
}

Observer-Composite Behavior with timestamp

I'm not sure but I understand that timestamp is mainly used for ` Notification Storing When Disabled or Offline" is enable.

Note: SenML time values (Base Time and Time) are represented in floating point as seconds and a missing time attribute is considered to have a value of zero. Base time and time are added together. In general, positive time values represent an absolute time relative to the Unix epoch, while negative values represent a relative time in the past from the current time. For details see [SENML].

Historical version of notifications are typically generated when "Notification Storing When Disabled or Offline" resource of the LwM2M Server Object is set to true (see Appendix E. LwM2M Objects defined by OMA (Normative)) and when the Device comes on line after having been disabled for a period of time.

(source : https://www.openmobilealliance.org/release/LightweightM2M/V1_1_1-20190617-A/HTML-Version/OMA-TS-LightweightM2M_Core-V1_1_1-20190617-A.html#7-4-5-0-745-SenML-JSON)

In that case that means each notification which should have been sent during "offline", should be timestamped, store and then will be sent later (on wake-up).

E.g. : still with /1/0/1 and /3 , the notification should contains contains both nodes for each timestamp.
For TimestampedLwM2mNodes, I think this should looks like :

// This is pseudo code representing Map<Instant, Map<LwM2mPath, LwM2mNode>>  (internal structure of TimestampedLwM2mNodes
{
  t1 => { 
      "/1/0/1" => LwM2mResource, // value can be null (if resource doesn't exist anymore)
      "/3"     => LwM2mObject // value can be null (if resource doesn't exist anymore)
  },
  t2 => { 
      "/1/0/1" => LwM2mResource,
      "/3"     => LwM2mObject
  },
  t3 => { 
      "/1/0/1" => LwM2mResource,
      "/3"     => LwM2mObject
  },
}

Examples of invalid structure :

// This is invalid
{
  t1 => { 
      "/1/0/1" => LwM2mResource, 
  },
  t2 => { 
      "/3"     => LwM2mObject
  },
  t3 => { 
      "/3/0/0"     => LwM2mResource
      "/3/0/1"     => LwM2mResource
  },
}

@JaroslawLegierski does it makes sense to you ?

@JaroslawLegierski
Copy link
Contributor

E.g. : still with /1/0/1 and /3 , the notification should contains contains both nodes for each timestamp.
For TimestampedLwM2mNodes, I think this should looks like :

As I understand in this case correct SenML format is as follow ?

[
	{
		"bn": "/1/0/1",
		"bt": 1701940138.352,
		"v": 3600
	},
	{
		"bn": "/3/",
		"bt": 1701940138.352,
		"n": "0/0",
		"vs": "Leshan Demo Device"
	},
	{
		"n": "0/1",
		"vs": "Model 500"
	},
	{
		"n": "0/2",
		"vs": "LT-500-000-0001"
	},
	{
		"n": "0/3",
		"vs": "1.0.0"
	},
	{
		"n": "0/9",
		"v": 7
	},
	{
		"n": "0/10",
		"v": 276427
	},
	{
		"n": "0/11/0",
		"v": 0
	},
	{
		"n": "0/13",
		"v": 1701937693
	},
	{
		"n": "0/14",
		"vs": "+01"
	},
	{
		"n": "0/15",
		"vs": "Europe/Belgrade"
	},
	{
		"n": "0/16",
		"vs": "U"
	},
	{
		"n": "0/17",
		"vs": "Demo"
	},
	{
		"n": "0/18",
		"vs": "1.0.1"
	},
	{
		"n": "0/19",
		"vs": "1.0.2"
	},
	{
		"n": "0/20",
		"v": 2
	},
	{
		"n": "0/21",
		"v": 296960
	}
]

@sbernard31
Copy link
Contributor Author

Yep SenML format could looks like this, but keep in mind that there a lot of way to express exactly same content in SenML.

Here I tried to focus on content not the form. The content will allow to define the API and the behavior. Then encoded it in one format or another will just be a formality.

@JaroslawLegierski
Copy link
Contributor

Yep SenML format could looks like this, but keep in mind that there a lot of way to express exactly same content in SenML.

Here I tried to focus on content not the form. The content will allow to define the API and the behavior. Then encoded it in one format or another will just be a formality.

Yes sure - I'm asking about content produced by this integration test in *** HACK *** section:

        List<LwM2mPath> paths = new ArrayList<>();
        paths.add(new LwM2mPath("/1/0/1"));
        paths.add(new LwM2mPath("/3"));

          ....
        
// *** HACK send time-stamped notification as Leshan client does not support it *** //
        // create time-stamped nodes

        TimestampedLwM2mNodes.Builder builder = new TimestampedLwM2mNodes.Builder();
        Map<LwM2mPath, LwM2mNode> currentValues = new HashMap<>();
        currentValues.put(paths.get(0), LwM2mSingleResource.newIntegerResource(1, 3600));
        currentValues.put(paths.get(1), new LwM2mObject(3,
                new LwM2mObjectInstance(0,
                LwM2mSingleResource.newStringResource(0, "Leshan Demo Device"),
                LwM2mSingleResource.newStringResource(1, "Model 500"),
                LwM2mSingleResource.newStringResource(2, "LT-500-000-0001"),
                LwM2mSingleResource.newStringResource(3, "1.0.0"),
                LwM2mSingleResource.newIntegerResource(9, 75),
                LwM2mSingleResource.newIntegerResource(10, 423455),
                LwM2mSingleResource.newIntegerResource(11, 0),
                LwM2mSingleResource.newDateResource(13, new Date(1367491215000L)),
                LwM2mSingleResource.newStringResource(14, "+01"),
                LwM2mSingleResource.newStringResource(15, "Europe/Belgrade"),
                LwM2mSingleResource.newStringResource(16, "U"),
                LwM2mSingleResource.newStringResource(17, "Demo"),
                LwM2mSingleResource.newStringResource(18, "1.0.1"),
                LwM2mSingleResource.newStringResource(19, "1.0.2"),
                LwM2mSingleResource.newIntegerResource(20, 4),
                LwM2mSingleResource.newIntegerResource(21, 20500736)
                )));
        builder.addNodes(Instant.ofEpochMilli(System.currentTimeMillis()), currentValues);
        TimestampedLwM2mNodes timestampednodes = builder.build();

@sbernard31
Copy link
Contributor Author

So do we agree on : #1089 (comment) 🙂 ?

@JaroslawLegierski
Copy link
Contributor

So do we agree on : #1089 (comment) 🙂 ?

Yes, I agree - I'm currently working on correcting the integration tests

@JaroslawLegierski
Copy link
Contributor

I just finished correction of the integration tests. PR #1552 has been updated.

@sbernard31
Copy link
Contributor Author

I will look at #1552 and test it and probably try to modify the code a bit.

sbernard31 added a commit that referenced this issue Jan 15, 2024
Co-authored-by: Simon Bernard <code@simonbernard.eu>
@sbernard31
Copy link
Contributor Author

This is implemented by #1552 and integrated in master. It should be available in next release 2.0.0-M15

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature from LWM2M specification server Impact LWM2M server
Projects
None yet
Development

No branches or pull requests

2 participants