search

eclipse-leshan / leshan

Java Library for LWM2M
https://www.eclipse.org/leshan/
BSD 3-Clause "New" or "Revised" License
647 stars 407 forks source link

Add support of write attributes at client side. #534

Open sbernard31 opened 6 years ago

sbernard31 commented 6 years ago

This means :

  1. be able to handle write request attributes and store attributes
  2. use this attributes to raise notification
  3. show attributes in Discover response

There is an ongoing work about this ~(#484)~ : #1514

The LWM2M v1.1.1 specification about that :

It could be worth to also check LWM2M v1.2.x, just for clarification (not for new feature) :

LWM2M v1.1.x also refer to IETF draft-ietf-core-dynlink-07 as [DynLink], so it could also make to look at it to clarify some ambiguities. Since draft 14, Conditional Attributes section was moved to draft-ietf-core-conditional-attributes. LWM2M v1.2.x refer to this last one draft-ietf-core-conditional-attributes-06 as [Cond_Attr]. So it could also make to look at it to clarify some ambiguities.

There is also some old discussion at OMA Developer repository which could maybe help (when we don't find answer in official specification) :

rkimsb2 commented 6 years ago

+1

henris42 commented 4 years ago

Hi, any news on this one? I have an urgent need to somehow handle PMAX/PMIN attribute requests in the client. The client now answers with 500 Internal Error, which is not tolerated by our server - we cannot observe anything.

I'm happy also with ANY workaround, PMAX/PMIN do not need to actually work in this application, we just need to get the server the expected response (204 I believe)

sbernard31 commented 4 years ago

Clearly not a priority for us now. This could need some modification at californium side, meaning this could be a not so easy task. The only hope : an unintended contribution. So I won't bet on this at short term.

Anyway a possible workaround: You need a custom ObjectEnabler which do what you want on WriteAttributesRequest E.g : 

public class CustomObjectEnabler extends ObjectEnabler {

    public CustomObjectEnabler(int id, ObjectModel objectModel, Map<Integer, LwM2mInstanceEnabler> instances,
            LwM2mInstanceEnablerFactory instanceFactory, ContentFormat defaultContentFormat) {
        super(id, objectModel, instances, instanceFactory, defaultContentFormat);
    }

    @Override
    public synchronized WriteAttributesResponse writeAttributes(ServerIdentity identity,
            WriteAttributesRequest request) {
        return WriteAttributesResponse.success();
    }
}

Then if you are using ObjectsInitializer you can create a custom one too : 

public class CustomObjectsInitializer extends ObjectsInitializer {

    @Override
    protected ObjectEnabler createNodeEnabler(ObjectModel objectModel) {
        Map<Integer, LwM2mInstanceEnabler> instances = new HashMap<>();
        LwM2mInstanceEnabler[] newInstances = createInstances(objectModel);
        for (LwM2mInstanceEnabler instance : newInstances) {
            // set id if not already set
            if (instance.getId() == null) {
                int id = BaseInstanceEnablerFactory.generateNewInstanceId(instances.keySet());
                instance.setId(id);
            }
            instance.setModel(objectModel);
            instances.put(instance.getId(), instance);
        }
        return new CustomObjectEnabler(objectModel.id, objectModel, instances, getFactoryFor(objectModel),
                getContentFormat(objectModel.id));
    }
}

And this should be OK. HTH.

henris42 commented 4 years ago

Hi,

Yes I got it working with this, thanks!

dellzhui commented 1 year ago

Clearly not a priority for us now. This could need some modification at californium side, meaning this could be a not so easy task. The only hope : an unintended contribution. So I won't bet on this at short term.

Anyway a possible workaround: You need a custom ObjectEnabler which do what you want on WriteAttributesRequest E.g :

public class CustomObjectEnabler extends ObjectEnabler {

    public CustomObjectEnabler(int id, ObjectModel objectModel, Map<Integer, LwM2mInstanceEnabler> instances,
            LwM2mInstanceEnablerFactory instanceFactory, ContentFormat defaultContentFormat) {
        super(id, objectModel, instances, instanceFactory, defaultContentFormat);
    }

    @Override
    public synchronized WriteAttributesResponse writeAttributes(ServerIdentity identity,
            WriteAttributesRequest request) {
        return WriteAttributesResponse.success();
    }
}

Then if you are using ObjectsInitializer you can create a custom one too :

public class CustomObjectsInitializer extends ObjectsInitializer {

    @Override
    protected ObjectEnabler createNodeEnabler(ObjectModel objectModel) {
        Map<Integer, LwM2mInstanceEnabler> instances = new HashMap<>();
        LwM2mInstanceEnabler[] newInstances = createInstances(objectModel);
        for (LwM2mInstanceEnabler instance : newInstances) {
            // set id if not already set
            if (instance.getId() == null) {
                int id = BaseInstanceEnablerFactory.generateNewInstanceId(instances.keySet());
                instance.setId(id);
            }
            instance.setModel(objectModel);
            instances.put(instance.getId(), instance);
        }
        return new CustomObjectEnabler(objectModel.id, objectModel, instances, getFactoryFor(objectModel),
                getContentFormat(objectModel.id));
    }
}

And this should be OK. HTH.

Hi sbernard31, Your sample code solves the problem I had perfectly, it's 2022 now, why don't we put it in ObjectsInitializer and implement it

sbernard31 commented 1 year ago

Your sample code solves the problem I had perfectly, it's 2022 now, why don't we put it in ObjectsInitializer and implement it.

I'm sorry but I didn't understand what is your problem. You vaguely try to explain it at : https://github.com/eclipse/leshan/pull/1330#issuecomment-1283610038

You talk about implementing Write Attributes support at client side. And now you say that this little sample of code solves your problem.

I don't get it :thinking: because this is far to implement Write Attribute at client side. Implementing Write Attribute does not means get the request and answer with succeed. This means changing the way CoAP observe will behave and as I explain at https://github.com/eclipse/leshan/issues/534#issuecomment-680086944 : "This could need some modification at californium side, meaning this could be a not so easy task."

Maybe there is way to do that without californium modification but for sure this will not be easy at all.

it's 2022 now, why don't we put it in ObjectsInitializer and implement it.

This looks like : "This ticket was created in 2018, we are in 2022 why this is still not done ? This should be already implemented !"

Anyway, the reason is still the same :

Finally, if this sample of code solves your problem perfectly, I guess you don't really need to implement Write Attributes and so maybe just want to write some kind of code to test that a server send the WriteAttributeRequest correctly. And so the API already allow to do that like explain at : https://github.com/eclipse/leshan/issues/534#issuecomment-680086944

If I didn't understand you correctly please clarify your needs.

dellzhui commented 1 year ago

I just went through #484 in detail,now I know that it is not easy to implement writeAttributes at the client side。

I am working on a mqtt2lwm2m project, the goal is to connect home-assistant to lwm2m server, so I need to implement a lwm2m client.

The biggest hurdle I've run into is not being able to receive the pmax property set on the server side.

The setting of the pmax property has now been implemented as per the sample code you mentioned above.

But this is still far from the real implementation of writeAttributes, if there is spare time I want to join this job, but before that, I may have to read OMA-TS-LightweightM2M_Core-V1_2-20201110-A.

Thanks.

sbernard31 commented 1 year ago

but before that, I may have to read OMA-TS-LightweightM2M_Core-V1_2-20201110-A.

Note that currently Leshan v1.x targets LWM2M v1.0.x and Leshan v2.x targets LWM2M v1.1.x. At this day, there is no version in development which currently targets LWM2M v1.2.x

dellzhui commented 1 year ago

Note that currently Leshan v1.x targets LWM2M v1.0.x and Leshan v2.x targets LWM2M v1.1.x. At this day, there is no version in development which currently targets LWM2M v1.2.x

Got it, thanks.

Warmek commented 1 year ago

I would like to implement a write attribute at client side and I looked at #484, but I'm not sure where to start, as from what saw part of the fixes were in #533. Could you clarify what was done and what would be needed to implement this feature?

sbernard31 commented 1 year ago

What must be done is explained in this issue description (https://github.com/eclipse/leshan/issues/534#issue-341031759) :

  1. be able to handle write request attributes and store attributes
  2. use this attributes to raise notification
  3. show attributes in Discover response

(1.) and (3.) the easy part and was partially done in #533 but the critical part is (2.) (1.) and (3.) are useless without (2.). (2.) is about changing when notification must be sent depending of attributes values. I'm not sure if content of the notification must be changed too :thinking:. I read the specification again and I think content is only changed for #596.

Currently, I have no clear idea how to handle this with current californium API. If the feature is not clear enough you can read LWM2M-v1.1.1@core§Table: 5.1.2.-2 class Attributes.

You can explore to try to find a solution for (2.) but I feel this could be a not so easy task :grimacing:

Also note that I have some doubts about current observation abstraction at client side. (https://github.com/eclipse/leshan/pull/1323#issue-1408021027) Maybe once we move forward on #1373, we will be more confident on this part. @JaroslawLegierski did you try to implement observe at client side with java-coap(fork) ?

JaroslawLegierski commented 1 year ago

From what I remember I ended with the send function - I didn't implement the observations.

Warmek commented 1 year ago

@sbernard31 Could you explain this?

use this attributes to raise notification

Do you mean that attributes need to be sent in observations?

sbernard31 commented 1 year ago

Do you mean that attributes need to be sent in observations?

Nope I don't think we need to send attributes in notifications.

I meant :

(2.) is about changing when notification must be sent depending of attributes values. I'm not sure if content of the notification must be changed too thinking. I read the specification again and I think content is only changed for #596.

OR in others words : Implementing the behavior of each attributes defined in LWM2M specification.

Warmek commented 1 year ago

So when we send notifications should be influenced by attributes? For example, after exciding pmax? If so, could you point me to where notifications are triggered from after exciding said time?

sbernard31 commented 1 year ago

So when we send notifications should be influenced by attributes? For example, after exciding pmax? If so, could you point me to where notifications are triggered from after exciding said time?

Currently, there is no mechanism like this. Currently when a resource is modified the implementer is responsible to fire the event. Then this event is spread until transport sub layer and notification is send immediately.

To better understand the leshan listener/event model, you should probably take a look at :

Here some ideas I had very long time ago (https://github.com/eclipse/leshan/pull/484#issuecomment-374290085), I don't know if that still makes sense but could help to better understand the feature.

Warmek commented 1 year ago

Response to https://github.com/eclipse-leshan/leshan/issues/1488#issuecomment-1684155286

Maybe we can use : https://github.com/eclipse-leshan/leshan/issues/534 unless you see a good reason to not use it ?

I looked through it but decided it would be easier to write it myself

I looked at your poc quickly (so maybe I missed something) I see that ObservationTrigger is able to fire resources changes but this is not enough it should also prevent event to be raised. So it seems this is maybe not the right design or at least something is missing.

Regarding pmin, I created a method (putIntoQueue(LwM2mPath path)) to catch (and delay if needed) rising events but wasn't sure how to implement it (to be precise, I don't know where it would be best to catch raised events).

Just to know, is this a really needed feature for you ? or you just try to implement it for specification support completeness.

We need this feature

sbernard31 commented 1 year ago

I looked through it but decided it would be easier to write it myself

I was just talking about reusing the issue for our discussion, not the code.

Regarding pmin, I created a method (putIntoQueue(LwM2mPath path)) to catch (and delay if needed) rising events but wasn't sure how to implement it (to be precise, I don't know where it would be best to catch raised events).

Sorry, do not be more available, I try to prioritize my effort and put this one pretty low in the list. (@JaroslawLegierski) Guys, do not hesitate to let me know what is the higher priority for you. So I can try to adapt a little my effort.

We need this feature

That means you are using the leshan-client, could you share for what/ how you use it ? I'm curious :slightly_smiling_face:

sbernard31 commented 1 year ago

@Warmek, I explore it since few days and I begin to get clearer idea but this will lead to some deep modification at client side. I let you know as soon as I get something acceptable.

Warmek commented 1 year ago

I explore it since few days and I begin to get clearer idea but this will lead to some deep modification at client side. I let you know as soon as I get something acceptable.

Thank you for letting me know. Are those changes regarding my PoC, or are you looking into your own way to implement write attributes?

sbernard31 commented 1 year ago

Are those changes regarding my PoC, or are you looking into your own way to implement write attributes?

For now I didn't reuse your code. As expected this is a very impacting feature because this tweaks behavior which is generally handled by the coap library.

sbernard31 commented 12 months ago

@Warmek, I create a draft PR, so you can look at my work : https://github.com/eclipse-leshan/leshan/pull/1514

Let me know your opinion about it and if with that you think you can move forward on this ? If you have question you can use review comment to ask for clarification about some part of the code.

Warmek commented 11 months ago

I looked through the code and other than the multi-threading "issue" described by https://github.com/eclipse-leshan/leshan/pull/1514#issuecomment-1726792151, I didn't have any issues.

Also, it might be a good idea to be more clear as to how is multi-threading handled in Leshan project as it can have a significant impact on how the code is written.

sbernard31 commented 11 months ago

other than the multi-threading "issue" described by https://github.com/eclipse-leshan/leshan/pull/1514#issuecomment-1726792151, I didn't have any issues.

I tried to answer to this question with https://github.com/eclipse-leshan/leshan/pull/1514#issuecomment-1727193592. I don't now if there is still a concern about that ? If, yes I need more concern explanation.

Also, it might be a good idea to be more clear as to how is multi-threading handled in Leshan project as it can have a significant impact on how the code is written.

I'm not sure how I can answer to this question. :thinking: Leshan client is NOT a single thread application.
When we need to schedule task we generally use ScheduledExecutorService. When there is state shared by several thread, we need to use classic java way to handle concurrent access.

If this is not expected answer, please clarify what you have in mind :grimacing:

sbernard31 commented 7 months ago

I moved forward on : https://github.com/eclipse-leshan/leshan/pull/1514 But still not enough for a minimal viable feature. Writing some test I see to fully support write attribute in discover we first need to implement :

Then :

sbernard31 commented 6 months ago

Minimal Viable Feature is available at : #1514

If anybody can play with it ? feedback / comments are welcome.

sbernard31 commented 5 months ago

Minimal Viable Feature is now integrated in master and will be available in next release 2.0.0-M15.

sbernard31 commented 5 months ago

Missing known feature epmin / epmax.

(Still no support of write attribute for Composite-Observe)