LwM2M send operation with multiple timestamped values fails

[etiennedm]

Describe the bug Sending multiple timestamped values for the same resource results in only the latest one being stored (and discarding the timestamp for this single value). In general, not only for send operations, but for read/write/notify as well, SenML values with timestamps should be honored.

This is related to #10135 where a similar request was made. I assumed the integration of Leshan 2.0.0-M14 had been done with this in mind, but it looks like there are still some improvements to be made.

Your Server Environment

• own setup
  • Deployment: monolith
  • Deployment type: dev build
  • ThingsBoard Version: on master 3d7128a1b6
  • Community Edition
  • Ubuntu 22.04

Your Device

• custom device running Zephyr and LwM2M client

To Reproduce Steps to reproduce the behavior:

1. Run the ThingsBoard server with a device provisioned for LwM2M communication
2. Trigger a Send operation from the client with multiple values for the same resource as a payload [{"bn":"/3303/1/","bt":1717886813,"n":"5700","v":20.0},{"n":"5700","v":21.0,"t":10},{"n":"5700","v":22.0,"t":20},{"n":"5700","v":23.0,"t":30},{"n":"5700","v":24.0,"t":40},{"n":"5700","v":25.0,"t":50},{"n":"5700","v":26.0,"t":60},{"n":"5700","v":27.0,"t":70},{"n":"5700","v":28.0,"t":80},{"n":"5700","v":29.0,"t":90}]

Expected behavior 10 values for the resource /3303/1/5700 should be stored with timestamps 1717886813, 1717886823, ..., 1717886903

Additional context

    @Override
    public void onUpdateValueWithSendRequest(Registration registration, SendRequest sendRequest) {
        for(var entry : sendRequest.getTimestampedNodes().getNodes().entrySet()) {
            // At this point the 10 timestamped values have been effectively reduced
            // to a single one by calling getNodes()
            ...
        }
    }

I'd be happy to test anything you need. Thanks for the great work, ThingsBoard has been working flawlessly for me so far!

[ViacheslavKlimov]

Hi @etiennedm,

Please provide detailed steps to reproduce the issue (device profile configuration, client details, commands, etc.)

[etiennedm]

Hi,

Surely you must have a test suite for LwM2M requests. It is as simple as sending a send operation with a payload like the one I pasted above (as SenML-JSON) and stepping through with the debugger (and it should exhibit the same behaviour with notify and write operations also I believe). You'll see in DefaultLwM2mUplinkMsgHandler.java in onUpdateValueWithSendRequest that the call to sendRequest.getTimestampedNodes().getNodes() reduces the set of values to a single value (the one with the newest timestamp).

This is the expected behaviour from Leshan as described in the comment for getNodes:

Get all collected nodes as LwM2mPath-LwM2mNode map ignoring timestamp information. In case of the same path conflict the most recent one is taken. Null timestamp is considered as most recent one.

Instead, on the Thingsboard side, the iteration should be over getTimestamps(), getting values with getNodesAt(), and then inserting as suitable as attributes and telemetry values.

[etiennedm]

Hi,

Here is a way I was able to reproduce the issue using Leshan's demo client:

1. Start up the Thingsboard instance and provision a LwM2M device, with an instance of the 3303 object, with e.g. test_ep endpoint client name.
2. Start the LeshanClientDemo, pointing it to the Thingsboard instance -u localhost:5685 -n test_ep.
3. Repeatedly run the command collect /3303/0/5700 in the Leshan CLI, to collect several samples.
4. Send the collected samples with send -c SENML_JSON collected-value.

You should then be able to observe the issue described above.

Hope this helps!

[etiennedm]

Hi, any update on this issue? This makes the LwM2M transport severely limited compared to the other transports for sleepy devices that connect only a few times a day: they cannot upload timestamped data acquired in between connections.

[mhuybrechts]

This was for us the main reason to upgrade to 3.7. It is very unfortunate it doesn't work as we'd expected.

In our case the devices are battery-powered ones and they use NB-IoT/LTE-M to send their collected data. This is for us a need-to-have to be able to get a long battery life.

[etiennedm]

Hi @ViacheslavKlimov, any news on this issue? Are you able to reproduce with the steps outlined above?

[ashvayka]

Hi @etiennedm . I am almost sure you are completely correct. Thank you for raising this issue. We will attempt to fix in 3.7.1

[nickAS21]

Hi @etiennedm . Thanks for reporting, the issue has been reproduced and confirmed, this issue will be resolved shortly.

[etiennedm]

Great news, thanks for the update. I can test my (limited) use case if you point me to a branch with the fix when it is ready.

[ViacheslavKlimov]

The fix is already in the master branch, you may test your use case there :)

[nickAS21]

Hi @etiennedm . I await your test results on this issue

[etiennedm]

Hi,

The issue is not fixed: this is a step in the right direction, I can see that all the values are now processed (e.g. they show up in the debug data of the rule chain) but they still all have the same timestamp attached, not the one that is in the LwM2M payload.

This makes sense since your change in DefaultLwM2mUplinkMsgHandler.java is not actually using the instant value that contains the data point timestamp. I believe there needs to be some changes so that in the end in updateAttrTelemetry the timestamps are provided to sendParametersOnThingsboardTelemetry.

Thanks, and I'm ready to test again when you are!

[etiennedm]

Hi, did you get a chance to review the results above from my limited tests?

[nickAS21]

Hi, did you get a chance to review the results above from my limited tests?

Hi @etiennedm . I await your test results on this issue

[etiennedm]

Hi @nickAS21, I tested the fix on your branch and it still uses the current timestamp to save the data points.

I see 2 potential problems:

1. Why use PROPERTY_KEY_TS_LATEST_MAP in updateAttrTelemetry? It looks like it should be a key otherwise keyTsLatestMap.putIfAbsent(key, new AtomicLong(tsNow)) in getTsByKey will return null.
2. I'm not sure what is going on in compareAndSwapOrIncrementTsAtomically() but could it cause issues? Even after patching the code to avoid problem 1 I'm still getting the current timestamp.

Are you testing with Leshan's demo client as detailed above? It should show this behavior.

Thanks!

[nickAS21]

Hi @etiennedm . I carefully studied your objections, made the appropriate changes to the code. Please check the code with the changes made # lwm2m fix bug #11706 lwm2m fix bug thingsboard/thingsboard-pe#2870

Thanks!

[etiennedm]

Hi @nickAS21, thanks for the changes. It works for my simple use case. I haven't had the chance to perform more extensive tests, but the data is correctly timestamped now. Since most of the heavy lifting of parsing the coap packet and generating the right timestamp is done by Leshan, I think you can go ahead and merge.

Thanks for implementing this!