eclipse-archived / californium.core

Californium project
86 stars 69 forks source link

Add nextObserveNumber to enable subclassing #40

Closed boaks closed 9 years ago

boaks commented 9 years ago

bugs.eclipse.org #475812

Signed-off-by: Achim Kraus achim.kraus@bosch-si.com

mkovatsc commented 9 years ago

I do not understand for what and how this would be used... The actual value of the observe clock (unfortunately) does not provide any properties other than that it is strictly increasing between notifications. Hence, no assumptions may be made based on its value.

boaks commented 9 years ago

This enhancement should help to fix a leshan-client observe notification problem. Currently the leshan client sends all notifications with the observe option value 0 instead of an ordered value. This is caused by the ObjectResource in leshan-client-cf, which extends the CoapResource of californium. It uses its own “notifyObserverRelationsForResource“ and doesn’t call the “notifyObserverRelations“ of the base class CoapResource. Therefore the notificationOrderer in CoapResource is not incremented and „checkObserveRelation“ in CoapResource will place allways 0 in the observe option. Yes, the design of leshan ObjectResource may be discussed, but right now, the fix with the smallest impact would be to make the incrementation of the order possible for a subclass. Alternatives would be to overwrite “checkObserveRelation” and use a own counter. But this would cause more changes. Or a redesign of the client, with even more changes.

mkovatsc commented 9 years ago

What is the problem with notifyObserverRelations? Might be better to improve the mechanism provided by Cf... How do you do the reordering at the LWM2M Server where the notifications arrive? Currently, the reorderer is in the CoapClient API. When we pack the client-side relation also in the exchange, we could have it in the stack, which also makes more sense to me...

boaks commented 9 years ago

As already said: There is no problem with CoapResource.notifyObserverRelations, but there is a problem with the leshan client implementation! The leshan client tries to use the CoapResource as some kind of "light facade" pattern; it just build one CoapResource java object for all resources below a LWM2M object (all instances and resources). Therefore all observe relations (even those for a singe resource) are registered at the CoapResource representing the LWM2M object. Therefore nofifyObserverRelations would fire too much (e.g. changing one resource would fire all observer instances and resource of that LWM2M object) and the leshan client therefore does some extra filtering:

    for (ObserveRelation relation : observeRelations) {
        if (relation.getExchange().getRequest().getOptions().getUriPathString().equals(URI)) {
            relation.notifyObservers();
        }
    }

But doing so, notificationOrderer.getNextObserveNumber(); is not called.

There are sure alternatives, but they would imply more changes ...

mkovatsc commented 9 years ago

Okay, now I get it :) I will merge this for now. For the next version, we need to find a way to enable observe for such "parent resources" (i.e., those who handle their subtree programmatically without sub-resources).

sbernard31 commented 9 years ago

We started a discussion about enabling observe for such resources : https://dev.eclipse.org/mhonarc/lists/cf-dev/msg00181.html

boaks commented 9 years ago

Such a filter is a much better solution. Should I prepare a proposal / pull request?