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

Remove JSON format for object model definition? #707

Closed
sbernard31 opened this issue Jun 24, 2019 · 11 comments
Closed

Remove JSON format for object model definition? #707

sbernard31 opened this issue Jun 24, 2019 · 11 comments
Labels
discussion Discussion about anything

Comments

@sbernard31
Copy link
Contributor

OMA defines an xml file format to describe an object model.

By the past, there was legal issues with this format which avoid us to reuse it in Leshan.
To workaround this issue we created a format based on json to describe object model.

Since more than 2 years now, this legal issue is fixed and we can now use xml format from OMA. There is no needs to support another format like the one we created (based json)

Currently both format are supported, only XML is documented.
This is maybe time to remove code about this json format ?
WDYT ?

@sbernard31 sbernard31 added the discussion Discussion about anything label Jun 24, 2019
@sbernard31
Copy link
Contributor Author

sbernard31 commented Jun 24, 2019

@dherykw, @mofosyne, I see you were using this json format by the past maybe, you have an opinion about this ?

@mofosyne
Copy link

mofosyne commented Jun 24, 2019

The company I work for is still using the json format. I haven't been tasked to transition it yet. But I'll pass the message along to the cloud team that you guys are considering depreciating and removing this json feature. (FYI, I'm in the embedded device team)

I've tried to do some cursory look into doing the transition myself, but found the lack of documentation of the json format too much of a hassle at the time (And lower priority compared to my primary task).

Also in my own personal opinion, json at least to me is easier to edit as well.

@sbernard31
Copy link
Contributor Author

You can look at ObjectModelSerDes, ResourceModelSerDes classes (this could help to understand the format)

A short term solution could be to just duplicate our classes and keep using this format on your side ? (even if I think it's preferable to move to XML format)

If you guys write a kind of converter (json => xml), do not hesitate to share it. This could maybe help other users.

@sophokles73
Copy link
Contributor

IMHO we should drop it, if possible. Supporting two ways to do the "same" thing causes unnecessary effort and poses the risk of introducing inconsistencies. Even if we want to support some additional features that are not in the OMA schema, I would rather try to do it based on the original schema, e.g. extending it.

@dherykw
Copy link

dherykw commented Jun 25, 2019

First thing that I want to say is thanks for asking I really apreciate that users are asked about this type of important changes.

We are currently using the json approach and it will be a pain in the neck to migrate to xml.

We use the .json objectspect also in the user interface to validate inputs before send the message to the server, so we had to pass xml to .json again.

We did not have even in the roadmap to migrate to xml since right now it does not give us any benefit. If you decided to remove the .json we would be stuck in the latest .json version for a long time.

So, if we had to answer directly the question asked in this issue, my answer would be.
No Please.

And I would try to introduce another question: is it worthy to change a format that user will need to change again in the UI if they want to do prevalidation?

Best

@GiacomoGenovese
Copy link

Hi,
thanks for asking.
From our side, we are not using JSON format for models since long time.
However, as @dherykw wrote, we are still using JSON objectspect also to register device's objects in our external application.
I also agree with @sophokles73 about inconsistence for models.
So, in my honest opinion, I would drop json for models and keep the json objectspect interface.

Thanks again for asking and thanks again for the great job you do.

Best

@dherykw
Copy link

dherykw commented Jun 25, 2019

If as @GiacomoGenovese sais, we can call the API and receive the definition of objectspect.json ... the integration could be more smooth.

@sbernard31
Copy link
Contributor Author

sbernard31 commented Jun 27, 2019

I will try to answer about some concerns, in case we decide to remove json format support from leshan-core :

If you decided to remove the .json we would be stuck in the latest .json version for a long time.

If you are talking about the file containing the model, I'm afraid there is no more file like this since a long time now. (except maybe the one used to test the JSON serializer).

If you are talking about the json format itself, there is no reason this format change even if we remove it from Leshan as this is strongly linked to the OMA XML schema which should not change anymore for the version 1.0 of the LWM2M specification.

We are currently using the json approach and it will be a pain in the neck to migrate to xml.

  1. If we remove it, nothing will prevent you to load your model from JSON file. You would have to duplicate ObjectModelSerDes, ResourceModelSerDes and do something like :
// load file
InputStream inputStream = ObjectModelSerDesTest.class.getResourceAsStream("/model.json");
ByteArrayOutputStream result = new ByteArrayOutputStream();
byte[] buffer = new byte[1024];
int length;
while ((length = inputStream.read(buffer)) != -1) {
    result.write(buffer, 0, length);
}
String smodel = result.toString("UTF-8");

// deserialize JSON
ObjectModelSerDes serDes = new ObjectModelSerDes();
JsonValue json = Json.parse(smodel);
List<ObjectModel> models = serDes.deserialize(json.asArray());

// Create server
LeshanServerBuilder builder = new LeshanServerBuilder();
LwM2mModelProvider modelProvider = new StaticModelProvider(models);
builder.setObjectModelProvider(modelProvider);
LeshanServer lwServer = builder.build();
lwServer.start();
  1. I think writing a converter from json to xml should not be so hard. It just misses the xml serializer. (As the json parser is already there). I think we talk about a simple class as 100/200 lines. If someone do that please share it here. 🙏 !

we are still using JSON objectspect also to register device's objects in our external application.

This is the case for our demo too, so ObjectModelSerDes, ResourceModelSerDes would probably moved in leshan-server-demo

So, in my honest opinion, I would drop json for models and keep the json objectspect interface.

Are you talking about the leshan-server-demo part ? (I'm a bit confused here)

And I would try to introduce another question: is it worthy to change a format that user will need to change again in the UI if they want to do prevalidation?

The idea is :
Models are generally provided by OMA in a DDF(xml) format.
So Leshan must be able to parse it and create an in memory model. (LwM2mModel)
For some use cases (E.g. leshan-server-demo), this memory model could be serialized in json at so a json serializer could make sense. But this is a bit out of scope of this project.
So, currently we have something like XML => LwM2mModel => JSON.

When we used JSON format as input we haved JSON => LwM2mModel => JSON.
This was pretty mush the same : 1 parsing 1 serialization.

@mofosyne
Copy link

Just a quick update. I checked with the guy whose maintaining the JSON models. They are fine with it's removal.

@sbernard31
Copy link
Contributor Author

@mofosyne thx for feedback ! 🙏

@sbernard31
Copy link
Contributor Author

#742 is now integrated in master, meaning that from now our custom json format is not more available in Leshan production ready libraries.

But the code is still available in leshan-server-demo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion about anything
Projects
None yet
Development

No branches or pull requests

5 participants