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 write attributes at client side. #534

Open
sbernard31 opened this issue Jul 13, 2018 · 28 comments
Open

Add support of write attributes at client side. #534

sbernard31 opened this issue Jul 13, 2018 · 28 comments
Labels
client Impact LWM2M client new feature New feature from LWM2M specification

Comments

@sbernard31
Copy link
Contributor

sbernard31 commented Jul 13, 2018

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) :

@sbernard31 sbernard31 added new feature New feature from LWM2M specification client Impact LWM2M client labels Jul 13, 2018
@rkimsb2
Copy link
Contributor

rkimsb2 commented Jul 15, 2018

+1

@henris42
Copy link

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
Copy link
Contributor Author

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
Copy link

henris42 commented Aug 26, 2020

Hi,

Yes I got it working with this, thanks!

@dellzhui
Copy link

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
Copy link
Contributor Author

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 : #1330 (comment)

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 🤔 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 #534 (comment) : "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 :

  • not a priority for current development team.
  • not an easy task at all.
  • and no contributor we try to implement it in a proper way.

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 : #534 (comment)

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

@dellzhui
Copy link

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
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor

Warmek commented Mar 30, 2023

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
Copy link
Contributor Author

What must be done is explained in this issue description (#534 (comment)) :

  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 🤔. 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 😬

Also note that I have some doubts about current observation abstraction at client side. (#1323 (comment))
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
Copy link
Contributor

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

@Warmek
Copy link
Contributor

Warmek commented Jun 19, 2023

@sbernard31 Could you explain this?

use this attributes to raise notification

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

@sbernard31
Copy link
Contributor Author

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
Copy link
Contributor

Warmek commented Jun 19, 2023

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
Copy link
Contributor Author

sbernard31 commented Jun 20, 2023

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 :

For now, I don't know exactly how code should looks like.
I guess we will have a class which will be responsible to mute or delay events but I don't know exactly where (probably at BaseObjectEnabler level? ) ... 😬
(All the event/listener model is not so simple, maybe refactoring will be needed).

Here some ideas I had very long time ago (#484 (comment)), I don't know if that still makes sense but could help to better understand the feature.

@Warmek
Copy link
Contributor

Warmek commented Aug 22, 2023

Response to #1488 (comment)

Maybe we can use : #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
Copy link
Contributor Author

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 🙂

@sbernard31
Copy link
Contributor Author

@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
Copy link
Contributor

Warmek commented Sep 13, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor Author

@Warmek, I create a draft PR, so you can look at my work : #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
Copy link
Contributor

Warmek commented Sep 22, 2023

I looked through the code and other than the multi-threading "issue" described by #1514 (comment), 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
Copy link
Contributor Author

sbernard31 commented Sep 22, 2023

other than the multi-threading "issue" described by #1514 (comment), I didn't have any issues.

I tried to answer to this question with #1514 (comment).
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. 🤔
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 😬

@sbernard31
Copy link
Contributor Author

sbernard31 commented Jan 29, 2024

I moved forward on : #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 :

  • add support to write attribute to discover + tests
  • we probably need more tests
  • and implement some TODO.
  • objectEnabler.getAttributesFor() should be implemented by server (or at least the API should be adapted)
  • we also need to implement this for java-coap transport layer.

@sbernard31
Copy link
Contributor Author

Minimal Viable Feature is available at : #1514

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

sbernard31 added a commit that referenced this issue Apr 5, 2024
Co-authored-by: Magdalena Kundera <magdalena.kundera@orange.com>
@sbernard31
Copy link
Contributor Author

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

@sbernard31
Copy link
Contributor Author

Missing known feature epmin / epmax.

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

mgdlkundera added a commit to JaroslawLegierski/leshan that referenced this issue Apr 18, 2024
…erve to client.

Co-authored-by: Magdalena Kundera <magdalena.kundera@orange.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Impact LWM2M client new feature New feature from LWM2M specification
Projects
None yet
Development

No branches or pull requests

6 participants