leshan icon indicating copy to clipboard operation
leshan copied to clipboard

Add support to "/" for Read-Composite.

Open sbernard31 opened this issue 4 years ago • 6 comments

Currently this is not supported :

[{"n":"/"}]

(source : http://www.openmobilealliance.org/release/LightweightM2M/V1_1_1-20190617-A/HTML-Version/OMA-TS-LightweightM2M_Core-V1_1_1-20190617-A.html#Table-745-10-SenML-JSON-payload-in-the-request-to-all-objects)

sbernard31 avatar Mar 23 '21 16:03 sbernard31

Together with @JaroslawLegierski we came up with an implementation for this case. Let me know what you think about this kind of solution.

adamsero avatar Jul 14 '22 13:07 adamsero

I'm not sure I get it :thinking:

It looks like a kind of HACK at leshan-server-demo side as you change a ReadCompositeRequest on / by ReadCompositeRequest on all specific object supported by the client (https://github.com/eclipse/leshan/commit/1a782cefd25679864c57814ad513f95c8e8bd96c#diff-8f94b871acdc39fb01247b73d4d7814389187176c1af3b0a6628efe7e1f4f62e) The idea is more to add support of this feature in the library itself. I mean we want to be able to create a new ReadCompositeRequest("/");

:warning: I don't think so much about this feature but I guess the harder part is about making Encoder/Decoder compatible with it. This needs not so easy changes because current API is not adapted.

E.g for decoder :

Map<LwM2mPath, LwM2mNode> decodeNodes(byte[] content,
                                      ContentFormat format, List<LwM2mPath> paths, LwM2mModel model)
                                 throws CodecException;

when you call it with paths = ["/3","/2/0","/5/0/1"] You should get something like a map { "/3"=> LwM2mObject, "2/0"=>LwM2mObjectInstance, "5/0/1"=>LwM2mSingleResource}

And so when you call it with paths = ["/3","/2/0","/5/0/1"] you should get {"/"=> LwM2mRoot}

But a LwM2mRoot which extends LwM2mNode does not exist for now.

sbernard31 avatar Jul 18 '22 15:07 sbernard31

Finally I was able to create a rough PoC, you can find it here. There's obviously a lot to improve in the code, but let me know if you even like my approach.

adamsero avatar Aug 01 '22 16:08 adamsero

I looked at this quickly and I feel it goes in the right direction.

The main design issue is how we should handle LwM2mRoot.getId() ?

(By the way I will be unavailable until 22th August)

sbernard31 avatar Aug 05 '22 14:08 sbernard31

@sbernard31 do you think we could advance with this issue?

adamsero avatar Sep 06 '22 11:09 adamsero

I currently have hard time working on Transport Layer abstraction. :sweat_smile:

But I can review PR or code and give some feedback is needed from time to time.

Keep in mind that I will not integrate new feature in master before to get transport layer integrated. (bug fix can be integrated)

sbernard31 avatar Sep 06 '22 13:09 sbernard31

@sbernard31 Here is a proposal of displaying read-composite on root path. I’ve implemented it as a list, below is a link to changes. https://github.com/eclipse-leshan/leshan/compare/master...JaroslawLegierski:leshan:opl/composite_on_root

mgdlkundera avatar Aug 24 '23 11:08 mgdlkundera

I look at you work and with a Leshan client demo this looks like : Capture d’écran de 2023-08-28 18-06-42

That changes a lot of other composite display.

Should we try to have something which looks more like others : Capture d’écran de 2023-08-28 18-06-26

It's also totally OK if some feature supported by Leshan library is not in demo OR have no UI in demo. So are we sure we want a UI for it ?

Also maybe better to first focus on a PR about adding support of "/" to read composite in library with some integration tests. Then once this is done, focus on the UI ?

sbernard31 avatar Aug 28 '23 16:08 sbernard31

@sbernard31 I added integration tests for root case. Could you take a look on it?

mgdlkundera avatar Sep 20 '23 06:09 mgdlkundera

I looked at it quickly, I think this is the right direction. Maybe you can review your code, clean it if needed, remove everything about demo then create a PR about it ? :slightly_smiling_face:

sbernard31 avatar Sep 20 '23 08:09 sbernard31

This is implemented by #1534 and will be available in next release 2.0.0-M14.

sbernard31 avatar Nov 10 '23 16:11 sbernard31