jvermillard / leshan

OMA Lightweight M2M java implementation - LWM2M
40 stars 25 forks source link

Client IntegerLwM2mExchange fix #48

Closed ghost closed 9 years ago

ghost commented 9 years ago

Hello Julien, this is a fix for the IntegerLwM2mExchange in the Leshan client. The main idea is to remove the convertion to string and back again for Integer type transport - instead, transport integers according to LWM2M spec (Appendix C. Data Types), TLV Integer format. Makes the client also compatible with Wakaama (important for us) and the payload will be smaller.

Along with the minor fixes in the pom.xml (missing plugin version, incorrect version range).

jschloman commented 9 years ago

It seems this change has caused several of the integration tests in the leshan-integration-tests project to stop passing. The fix (such as in the can_access_mandatory_object_without_create() test) seems to be replicating the change to the tests such as changing

new LwM2mResource(IntegrationTestHelper.MANDATORY_SINGLE_RESOURCE_ID, Value.newStringValue(Integer .toString(helper.intResource.getValue()))));

to

new LwM2mResource(IntegrationTestHelper.MANDATORY_SINGLE_RESOURCE_ID, Value.newStringValue(new String(TlvEncoder.encodeInteger(helper.intResource.getValue())))));

sbernard31 commented 9 years ago

There are 4 data formats for transferring resources : text, opaque, tlv, json (see 6.3) As I understand the spec, text and opaque is used for singles resources, tlv and json for multiple ones. (json MAY be used for single resource too, so maybe tlv too but is not so clear)

The Appendix C. Data Types you reference should be used for tlv encoding, but if you request a single resource, format should be text, so the integer will be encoded as an UTF8 String.

This means the integer 12345 will be encoded as string, so 5 bytes instead of a 2 bytes if it was encoding as Integer. (not perfect :cry: but it seems this is specification.)

About wakaama compatibility, I'm not sure to understand, but maybe you talk about the data type encoding problem as I explain in my feedback 2) on the mailing list. Primitive type seems to be encoded in TLV the same way as Text encoding. @jschloman did you find time to try to reuse LwM2mNodeEncoder and LwM2mNode class hierarchy to solve this problem ?

ghost commented 9 years ago

@sbernard31 , thanks for the explanation! I wrongly assumed that TLVs can be used in the single resource context, and yes, I had the same issue of Wakaama in mind, that you mentioned... So, the right approach is to fix Wakaama's TLV encoding instead?

sbernard31 commented 9 years ago

I reread my last sentence, and this is not really clear, to precise my mind : The problem I talk about concern the leshan client, not wakaama. I did some test with lualwm2m based on wakaama and I had no particular problem with integer encoding. But it was not based on the last commit so maybe there are a regression.

jschloman commented 9 years ago

@sbernard31 : we've started doing work in that direction and using the LwM2mNodeEncoder/LwM2mNode does not seem to be a difficult change to enact (we just need to assure it is done exhaustively without us leaving any edge elements). In the next month we hope to be able to refocus our efforts to Leshan and so this one will be added as a priority.

ghost commented 9 years ago

@sbernard31, i think now i got it. reade single resource (like read /3/0/9 (battery level)) should read "100" as plain text for 100%, read multiple, like /3/0/6 (avail. power sources) should respond with binary, like byte[] { 0, 1, ... whatever }. And then "read /3" would give complete object as TLV and binary encoded values, right? If it is so, then wakaama does it right.

sbernard31 commented 9 years ago

In the way I understand the spec it's right, except for the /3/0/6 resource which should be encoded as TLV (as it is a multiple instance resource).

ghost commented 9 years ago

That is also my understanding, the content of the multiple resource is TLV-encoded, i.e. binary.