Closed bolddp closed 6 years ago
Hi, As I understand the spec, the way you propose is correct. I think a discover on 3/0/5 should work too (205 for supported, 4.04 for not supported), but it's not so clear in the specification. You should ask at OMA.
Currently the Leshan Client discover implementation is only based on model description. (See BaseObjectEnabler). So using a custom model is a possible workaround. (See ObjectInitializer ) If you want a more dynamic behavior, you should implement your own LwM2mObjectEnabler.
As far as I know nobody is working on that. If you want to provide a PR for Leshan you're welcome :). I suppose a discover method can be added to LwM2mInstanceEnabler and should used in ObjectEnabler. The questions are :
should we keep the default behavior with model if user didn't implement discover in LwM2mInstanceEnabler
?
JFYI :
LwM2mObjectEnabler
is the interface which must be implemented. This is the real contract.BaseObjectEnabler
is an abstract class which can be used as base for a LwM2mObjectEnabler
implementation.ObjectEnabler
is a possible implementation which chose to match LwM2m object instance concept to LwM2mInstanceEnabler
interface.HTH
@sbernard31 Thx for the detailed feedback! We are using the LeshanClient for a simulator at the moment, and it is in our interest that it can discover on instance level so it behaves according to spec. When we implement this, I will try to solve it in a way that is general enough for a PR.
Ok good to know :)
is it an open source simulator ?
Unfortunately not, it's proprietary. It's designed to simulate Lwm2m clients that comply with a messaging protocol that we're building on top of Lwm2m.
I'm ready to have a new go at this now, it doesn't seem like anybody else is working on it? What you wrote above still applies, as far as I can see, e.g. Discover is currently just based on the ObjectModel and would need to be extended all the way down to LwM2mInstanceEnabler
.
After looking into the specification a little more, I will also need to take a look at the handling of attributes on object and instance level, since they should be in Discover on all levels.
Sry for delayed response.
Yep, nobody is currently working on that, so you can go on it. About "attributes", currently the clients does support any attributes except the version one you added. Am I wrong ?
You're right, and since the attributes must be reported in the Discover response along with the links, I will also have a look at any improvements that can be made to the attribute handling.
What exists is basically the ObserveSpec class, that represents the NOTIFICATION class attributes in the Lwm2m spec. But it's more optimized for serving the CoAP layer, e.g. through the toQueryParams() method.
ObserveSpec is also used in the WriteAttributesRequest constructors. It serves as a wrapper around some rules that are mentioned in the spec, e.g. throw an error if pmin > pmax.
I will open a pull request as soon as I have something worth looking at and then work on from there.
Do you need support of attributes too ? I mean we can envisage a better support of Discover at instance level without support of attributes in a first time.
If you need both and those 2 features are not too interwoven. It will be easier to review if we make this step by step.
You might be right that it would be better to split these features up, but then I suppose we should reverse the order and start with attribute support, which is perhaps what you mean as well?
I'm just a little afraid that it will be hard to understand the attribute requirements fully without looking into the Discover functionality requirements at the same time. For instance, if you perform Discover on a resource, the attributes on both the object level and instance level should be inherited and included in the DiscoverResponse. This means that a first implementation of only Discover on instance level may be hard to pull off too.
But I will try to find a way to split it up into separate units of work.
I suppose we should reverse the order and start with attribute support
I don't know. To be honest I'm not sure to see the real interest about attributes. Especially as we don't support writteAttributes
at client side.
But I let you investigate that :)
I've looked into it a little further now. We rely heavily on observe in our solution, and to be able to control the data volumes that are generated it's imperative for us to be able to set the pmin and pmax attributes. And because of that it's also very important for us to be able to simulate both WriteAttributes and full Discover.
Please consider the following interface and implementations to handle attributes in a consistent way:
public interface Attribute {
String getCoRELinkParam();
Attachment getAttachment();
Set<AssignationLevel> getAssignationLevels();
AttributeClass getAttributeClass();
AccessMode getAccessMode();
Object getValue();
boolean isWritable();
boolean canBeAssignedTo(AssignationLevel assignationLevel);
}
Abstract base implementation
public abstract class BaseAttribute implements Attribute {
private final String coRELinkParam;
private final Attachment attachment;
private final EnumSet<AssignationLevel> assignationLevels;
private final AttributeClass attributeClass;
private final AccessMode accessMode;
private final Object value;
protected BaseAttribute(String coRELinkParam, Attachment attachment, EnumSet<AssignationLevel> assignationLevels,
AttributeClass attributeClass, AccessMode accessMode, Object value) {
this.coRELinkParam = coRELinkParam;
this.attachment = attachment;
this.assignationLevels = assignationLevels;
this.attributeClass = attributeClass;
this.accessMode = accessMode;
this.value = value;
}
public String getCoRELinkParam() {
return coRELinkParam;
}
public Attachment getAttachment() {
return attachment;
}
public Set<AssignationLevel> getAssignationLevels() {
return assignationLevels;
}
public AttributeClass getAttributeClass() {
return attributeClass;
}
public AccessMode getAccessMode() {
return accessMode;
}
public Object getValue() {
return value;
}
@Override
public boolean isWritable() {
return accessMode == AccessMode.W || accessMode == AccessMode.RW;
}
@Override
public boolean canBeAssignedTo(AssignationLevel assignationLevel) {
return assignationLevels.contains(assignationLevel);
}
}
And then one implementation for each supported attribute:
public class MinimumPeriodAttribute extends BaseAttribute {
protected MinimumPeriodAttribute(Integer value) {
super("pmin", Attachment.RESOURCE,
EnumSet.of(AssignationLevel.OBJECT, AssignationLevel.INSTANCE, AssignationLevel.RESOURCE),
AttributeClass.NOTIFICATION, AccessMode.RW, value);
}
}
My idea then is that when you create a WriteAttributesRequest, you inject a group of parameters in the constructor, and a rule engine makes sure that they're writable and that they're consistent, e.g. pmin < pmax etc. You would also be able to validate that the attributes are written on the correct level, e.g. throw an error if there is an attempt to write "ver" on a resource.
So you want to define a kind of model for Attributes (I means objects containing metadata about attributes) A bit like we do for LwM2mModel ? I'm not totally convinced but let's see what it will looks like.
FMPOV if we go in those way the model should not contains the value (or this instance should not contains the metadata). I mean we need only 1 instance in memory which describes what is the MinimumPeriodAttribute metadata and several instance of MinimumPeriodAttribute with theirs value. Do you see what I mean ?
OK, I see your point about duplicated metadata. I'll take that into consideration moving forward.
Pull request for this... https://github.com/eclipse/leshan/pull/478
+1
The orginal topic was about "Discover on instance level not implemented?". I think this is done in #532.
About write attributes work is still in progress so I create an issue for this #534.
If my Lwm2m server implementation needs to check if the connected device supports the Factory Reset resource on the Device object, my understanding is that I should perform a Discover on the Device Instance, e.g. /3/0. Then I need to examine the returned resource links to see if /3/0/5 is present. Is this correct?
If the answer is Yes, Discover doesn't seem to be implemented on the Instance level, e.g. in the LwM2mInstanceEnabler interface. Is this correct, or am I looking in the wrong place?
If it isn't implemented yet, is anybody working on it? If not, I might have a look at it myself and then some pointers on where to call the LwM2mInstanceEnabler.discover method would be much appreciated!