SenML-CBOR encoder in leshan-core with unit tests #682

cavenaghi9 · 2019-04-12T07:42:30Z

SenML-CBOR encoder in leshan-core with unit tests

Signed-off-by: Boya Zhang <zhangboya@gmail.com>

sbernard31

I ask myself, if it makes sense to put cbor serialization code in SenMLPackSerDes class instead of a separated class?

sbernard31 · 2019-04-15T10:11:02Z

leshan-core/src/main/java/org/eclipse/leshan/senml/SenMLPackSerDes.java

+            for (SenMLRecord record : pack.getRecords()) {
+                generator.writeStartObject();
+
+                if (record.getBaseName() != null && record.getBaseName().length() > 0) {


Why not using !record.getBaseName().isEmpty() ?

Not all records have base name within same pack, so that only generate base name element when record contains base name and length great than 0.

I meant replacing record.getBaseName().length() > 0 by !record.getBaseName().isEmpty()
(keep the record.getBaseName() != null)

sbernard31 · 2019-04-15T10:18:19Z

leshan-core/src/main/java/org/eclipse/leshan/senml/SenMLPackSerDes.java

+            generator.writeEndArray();
+            generator.close();
+        } catch (Exception e) {
+            throw new CodecException(e.getLocalizedMessage(), e);


Instead of e.getLocalizedMessage() which is already available in e parameter. Should we add a more contextual exception message, like : "Unable to serialize to CBOR." ?

In addition, do you think CodecException can be used here, because of CodecException is thrown when a problem occurs during LWM2M node encoding or decoding. Or to extend a new Exception class for SenML-CBOR, like TlvException and LwM2MJsonException.

Yep maybe this is even better. You can create a SenMLCborException.

sbernard31 · 2019-04-15T10:22:37Z

leshan-core/src/test/java/org/eclipse/leshan/senml/SenMLSerializerTest.java

+                + "fbf613063362f306132fa3f800000ffbf613063362f316132fa40a00000ffbf613063372f306132fa456d8000ffbf6"
+                + "13063372f316132fa459c4000ffbf613063382f306132fa42fa0000ffbf613063382f316132fa44610000ffbf61306"
+                + "1396132fa42c80000ffbf61306231306132fa41700000ffbf61306431312f306132fa00000000ffbf61306231336132"
+                + "fa4ea30485ffbf61306231346133662b30323a3030ffbf613062313561336155ffff";


This doesn't look like to the CBOR payload example from the specification ? Does it should ?

Good point, example in TS is {-2: "/3/0/", 0: "0", 3: "Open Mobile Alliance"}...., however generated CBOR notation in test case is {"-2": "/3/0/", 0: "0", 3: "Open Mobile Alliance"}. Field name shall be number type, instead of string type. I will check with Jackson CBOR library, to correct it.

cavenaghi9 · 2019-04-16T06:52:31Z

I ask myself, if it makes sense to put cbor serialization code in SenMLPackSerDes class instead of a separated class?

If to give two seperated classes, one is named SenMLCborPackSerDes.java, another is named SenMLJsonPackSerDes.java, will it make sense?

sbernard31 · 2019-04-16T08:22:42Z

Yep, something like this.

1. Using !record.getBaseName().isEmpty(), instead of record.getBaseName().length() > 0 2. Add SenMLCborException for encoder. 3. Separate SenMLJson and SenMLCbor 4. Update unit test with LwM2M 1.1 TS example. Other updates: 1. Upgrade jackson-dataformat-cbor to 2.9.9 which support number type field. 2. Rename method 'toJsonSenML' to 'toSenMLJson'. Signed-off-by: Boya Zhang <zhangboya@gmail.com>

sbernard31

Sorry for the long delay...
Never so closed to merge it ;)

sbernard31 · 2019-06-14T16:29:00Z

leshan-core/src/test/java/org/eclipse/leshan/senml/SenMLCborSerializerTest.java

+
+public class SenMLCborSerializerTest extends AbstractSenMLTest {
+
+    private Logger LOG = LoggerFactory.getLogger(getClass());


Any reason to not use a static attribute for logger ?

After reviewing code, I found that LOGGER can be removed completely.

sbernard31 · 2019-06-14T16:34:12Z

leshan-core/src/test/java/org/eclipse/leshan/senml/SenMLCborSerializerTest.java

+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SenMLCborSerializerTest extends AbstractSenMLTest {


Generally, in this project we use "delegation" instead of "inheritance" for reuse code in tests.
But that's not a big deal, if we let it like this :)

I will take care about it.

Signed-off-by: Boya Zhang <zhangboya@gmail.com>

cavenaghi9 · 2019-06-19T09:36:04Z

Sorry for the long delay...
Never so closed to merge it ;)

Not so long time. I was taking break as well in past days. It is time to continue contribution.

Signed-off-by: Boya Zhang <zhangboya@gmail.com>

sbernard31 · 2019-06-19T15:26:18Z

Integrated in 2.0.x branch (all commits squashed in 876c1a2)

Thx @cavenaghi9 👍

Signed-off-by: Boya Zhang <zhangboya@gmail.com>

SenML-CBOR encoder in leshan-core with unit tests.

aa907ee

Signed-off-by: Boya Zhang <zhangboya@gmail.com>

sbernard31 reviewed Apr 15, 2019

View reviewed changes

sbernard31 mentioned this pull request Jun 3, 2019

why IntegerResource value didn't use Integer as data type but Long? As result,when use diagnostic tools to verify, the type is not correct. #694

Closed

sbernard31 reviewed Jun 14, 2019

View reviewed changes

Remove useless logger in unit test.

7ec52a6

Signed-off-by: Boya Zhang <zhangboya@gmail.com>

sbernard31 pushed a commit that referenced this pull request Jun 19, 2019

#682: Add SenML-CBOR encoder in leshan-core + unit tests.

876c1a2

Signed-off-by: Boya Zhang <zhangboya@gmail.com>

sbernard31 closed this Jun 19, 2019

cavenaghi9 added a commit to cavenaghi9/leshan that referenced this pull request Jun 26, 2019

eclipse-leshan#682: Add SenML-CBOR encoder in leshan-core + unit tests.

85eeb21

Signed-off-by: Boya Zhang <zhangboya@gmail.com>

cavenaghi9 mentioned this pull request Jul 1, 2019

LwM2mNodeSenMLDecoder for SenML-JSON with unit tests. #681

Closed

sbernard31 pushed a commit that referenced this pull request Jul 16, 2020

#682: Add SenML-CBOR encoder in leshan-core + unit tests.

44625ad

Signed-off-by: Boya Zhang <zhangboya@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SenML-CBOR encoder in leshan-core with unit tests #682

SenML-CBOR encoder in leshan-core with unit tests #682

cavenaghi9 commented Apr 12, 2019

sbernard31 left a comment

sbernard31 Apr 15, 2019

cavenaghi9 Apr 17, 2019

sbernard31 Apr 17, 2019

sbernard31 Apr 15, 2019 •

edited

Loading

cavenaghi9 Apr 17, 2019

sbernard31 Apr 17, 2019

sbernard31 Apr 15, 2019

cavenaghi9 Apr 17, 2019

cavenaghi9 commented Apr 16, 2019

sbernard31 commented Apr 16, 2019

sbernard31 left a comment

sbernard31 Jun 14, 2019

cavenaghi9 Jun 19, 2019

sbernard31 Jun 14, 2019 •

edited

Loading

cavenaghi9 Jun 19, 2019

cavenaghi9 commented Jun 19, 2019

sbernard31 commented Jun 19, 2019


		public class SenMLCborSerializerTest extends AbstractSenMLTest {

		private Logger LOG = LoggerFactory.getLogger(getClass());

SenML-CBOR encoder in leshan-core with unit tests #682

SenML-CBOR encoder in leshan-core with unit tests #682

Conversation

cavenaghi9 commented Apr 12, 2019

sbernard31 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbernard31 Apr 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cavenaghi9 commented Apr 16, 2019

sbernard31 commented Apr 16, 2019

sbernard31 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbernard31 Jun 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cavenaghi9 commented Jun 19, 2019

sbernard31 commented Jun 19, 2019

sbernard31 Apr 15, 2019 •

edited

Loading

sbernard31 Jun 14, 2019 •

edited

Loading