sys: add Universal Binary JSON module (UBJSON) #1659

Kijewski · 2014-09-12T16:42:27Z

→ http://ubjson.org/

Right now I ~~only~~ implemented reading and write, ~~but this~~ and it should work.
It is an SAX/expat-style parser, i.e. you provide a read function and a callback function.
The example application is a pretty printer.

I implemented the dialect that's described on their homepage, which is quite different to the Wikipedia article.
The advantage of UBJSON is that it is

more easy to parse than JSON, while it's equally easy to read,
more concise than BSON,
less of an unfathomable monster than Apache Thrift or Google Protobuf
maybe less nice than MessagePack, but UBJSON has far less special cases.

I don't know CBOR.

UBJSON is -- like JSON -- schemaless.

kaspar030 · 2014-09-12T23:03:43Z

Nice!

mehlis · 2014-09-13T16:33:09Z

nice ! I think the message pack lib is that small, that we can integrate it as well...

LudwigKnuepfer · 2014-09-15T13:11:43Z

tests/ubjson_read/main.c

@@ -0,0 +1,252 @@
+#include <stdio.h>


license missing

Kijewski · 2014-09-23T05:00:50Z

Reading and writing is implemented. The example application was moved to examples. Licenses added for every file. All functions have doxygen. A few unittests are implemented.

Should be ready to merge.

LudwigKnuepfer · 2014-09-23T06:40:40Z

We want less examples, not more.

OlegHahm · 2014-09-23T06:47:59Z

Although I've probably stated exactly that recently, it's not entirely true from my perspective: I would say we do not want to increase the number of examples, but demonstrate more features.

I'll open an issue for that.

cgundogan · 2014-09-23T07:12:11Z

sys/ubjson/ubjson-read.c

+            *content = (marker == UBJSON_MARKER_TRUE);
+            break;
+
+        if (false) { case UBJSON_MARKER_INT8:  *content = UBJSON_INT32_INT8;  }


Is your attention here to comment out these case statements until a later phase of implementation, or did you just forget to delete them?

This shall reduce duplicated code.
Labels inside an if (false) block are still valid.

E.g. the lines

if (false) { case UBJSON_MARKER_INT8: *content = UBJSON_INT32_INT8; } if (false) { case UBJSON_MARKER_INT16: *content = UBJSON_INT32_INT16; } abc;

do the same as:

case UBJSON_MARKER_INT8: case UBJSON_MARKER_INT16: switch (marker) { case UBJSON_MARKER_INT8: *content = UBJSON_INT32_INT8; break; case UBJSON_MARKER_INT16: *content = UBJSON_INT32_INT16; break; } abc;

Indeed, I wasn't aware of this neat trick. Thanks for the hint (:

I would still vote for the more readable version - including newlines.

LudwigKnuepfer · 2014-09-23T08:33:44Z

@OlegHahm so you are in favor of having this example?

Kijewski · 2014-09-24T06:49:11Z

Depends on

OlegHahm · 2014-09-24T12:16:01Z

@LudwigOrtmann, not as part of the main RIOT repository, I would say.

Kijewski · 2014-09-24T12:55:06Z

Well, the apps repo is gone.

LudwigKnuepfer · 2014-09-24T13:09:27Z

No, it just lives in hiding.

LudwigKnuepfer · 2014-09-24T13:10:27Z

Well, living isn't the best term, but it exists.

Kijewski · 2014-09-25T17:10:38Z

@LudwigOrtmann, well, were is it hiding then? :)

LudwigKnuepfer · 2014-09-25T23:37:23Z

https://github.com/des-testbed/RIOT-projects

LudwigKnuepfer · 2014-09-25T23:37:43Z

But I assure you, it's as good as dead.

OlegHahm · 2014-11-10T23:22:56Z

Rebase.

Kijewski · 2014-11-11T09:47:52Z

The unittests need #1990 to compile.

miri64 · 2014-11-11T12:02:15Z

#1990 is merged

OlegHahm · 2014-12-03T14:45:05Z

sys/ubjson/ubjson-read.c

+     /* case UBJSON_MARKER_HP_NUMBER:
+      *     ...
+      *     break;
+      */


I'd like to keep the comments for high-precision numbers. Just so I know where to add code if I need to implement them later.

Same here: please add this as a comment to the code (the NOTE tag is nice for stuff like this)

OlegHahm · 2014-12-03T16:06:17Z

From my point that's an ACK. @authmillenon, @cgundogan, wanna add something?

miri64 · 2014-12-03T16:10:55Z

Nope. Please SQUASH

Kijewski · 2014-12-03T16:16:21Z

Rebased, squashed, 3092aed unpicked.

cgundogan · 2014-12-03T17:25:09Z

sys/ubjson/ubjson-internal.h

+}
+#endif
+
+#endif


add the guard name to the closing #endif: #endif /* ifndef UBJSON__INTERNAL_H__ */
I know,this is sugar.., but I like how you used this notation in your other files.

cgundogan · 2014-12-03T17:26:46Z

Looks all fine to me, but Travis complains about: ERROR: The following new files generate doxygen warnings: sys/include/ubjson.h

https://travis-ci.org/RIOT-OS/RIOT/builds/42878709

OlegHahm · 2014-12-03T17:52:12Z

First strik of the doxygen checker. :-)

RIOT/sys/include/ubjson.h:218: warning: Member ubjson_cookie_t (typedef) of group sys_ubjson is not documented.
RIOT/sys/include/ubjson.h:218: warning: Member ubjson_cookie_t (typedef) of group sys_ubjson is not documented.
RIOT/sys/include/ubjson.h:248: warning: Member rw (variable) of class ubjson_cookie is not documented.
RIOT/sys/include/ubjson.h:252: warning: Member callback (variable) of class ubjson_cookie is not documented.
RIOT/sys/include/ubjson.h:254: warning: Member marker (variable) of class ubjson_cookie is not documented.

Kijewski · 2014-12-03T18:54:08Z

$ ./dist/tools/doccheck/check.sh master || exit
ERROR: The following new files generate doxygen warnings:
sys/include/ubjson.h

Cannot reproduce the error at home.

LudwigKnuepfer · 2014-12-03T19:00:57Z

So, we want to use our own version of doxygen in addition to our own build of cppcheck... sheesh.

Kijewski · 2014-12-03T19:04:07Z

The problem was my master != origin/master. :)

LudwigKnuepfer · 2014-12-03T19:13:00Z

Sure? I checked out your branch and rebased on an up-to-date master (at least I think I did).

LudwigKnuepfer · 2014-12-03T19:13:55Z

Ah - the fix was already included.

LudwigKnuepfer · 2014-12-03T19:15:56Z

sys/include/ubjson.h

+/**
+ * @brief         A cookie passed between the read and write functions.
+ * @details       You probably want to wrap the cookie in some other data structure,
+ *                which you retreive with container_of() in the callback.


s/retreive/retrieve

LudwigKnuepfer · 2014-12-03T19:36:47Z

That's all the spelling errors my checker found =)

Kijewski · 2014-12-03T19:43:17Z

Squashed

OlegHahm · 2014-12-03T21:24:59Z

ACK and go.

sys: add Universal Binary JSON module (UBJSON)

Kijewski · 2014-12-03T21:37:40Z

Yay

Kijewski added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Sep 12, 2014

LudwigKnuepfer reviewed Sep 15, 2014
View reviewed changes

tests/ubjson_read/main.c

@@ -0,0 +1,252 @@

#include <stdio.h>

Copy link

Member

LudwigKnuepfer Sep 15, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

license missing

Kijewski added Type: new feature The issue requests / The PR implemements a new feature for RIOT and removed State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Sep 23, 2014

OlegHahm assigned cgundogan Sep 23, 2014

cgundogan reviewed Sep 23, 2014
View reviewed changes

OlegHahm reviewed Dec 3, 2014
View reviewed changes

OlegHahm added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Dec 3, 2014

cgundogan reviewed Dec 3, 2014
View reviewed changes

OlegHahm removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Dec 3, 2014

LudwigKnuepfer added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Dec 3, 2014

LudwigKnuepfer reviewed Dec 3, 2014
View reviewed changes

Kijewski added 2 commits December 3, 2014 20:42

Add UBJSON library

118dfd8

tests: add UBJSON unittests

6b559fa

Kijewski removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Dec 3, 2014

OlegHahm added a commit that referenced this pull request Dec 3, 2014

Merge pull request #1659 from Kijewski/ubjson

f386de6

sys: add Universal Binary JSON module (UBJSON)

OlegHahm merged commit f386de6 into RIOT-OS:master Dec 3, 2014

Kijewski deleted the ubjson branch December 3, 2014 21:37

+              }
+              #endif
+              #endif

sys: add Universal Binary JSON module (UBJSON) #1659

sys: add Universal Binary JSON module (UBJSON) #1659

Uh oh!

Conversation

Kijewski commented Sep 12, 2014

→ http://ubjson.org/

Uh oh!

kaspar030 commented Sep 12, 2014

Uh oh!

mehlis commented Sep 13, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kijewski commented Sep 23, 2014

Uh oh!

LudwigKnuepfer commented Sep 23, 2014

Uh oh!

OlegHahm commented Sep 23, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LudwigKnuepfer commented Sep 23, 2014

Uh oh!

Kijewski commented Sep 24, 2014

Uh oh!

OlegHahm commented Sep 24, 2014

Uh oh!

Kijewski commented Sep 24, 2014

Uh oh!

LudwigKnuepfer commented Sep 24, 2014

Uh oh!

LudwigKnuepfer commented Sep 24, 2014

Uh oh!

Kijewski commented Sep 25, 2014

Uh oh!

LudwigKnuepfer commented Sep 25, 2014

Uh oh!

LudwigKnuepfer commented Sep 25, 2014

Uh oh!

OlegHahm commented Nov 10, 2014

Uh oh!

Kijewski commented Nov 11, 2014

Uh oh!

miri64 commented Nov 11, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OlegHahm commented Dec 3, 2014

Uh oh!

miri64 commented Dec 3, 2014

Uh oh!

Kijewski commented Dec 3, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgundogan commented Dec 3, 2014

Uh oh!

OlegHahm commented Dec 3, 2014

Uh oh!

Kijewski commented Dec 3, 2014

Uh oh!

LudwigKnuepfer commented Dec 3, 2014

Uh oh!

Kijewski commented Dec 3, 2014

Uh oh!

LudwigKnuepfer commented Dec 3, 2014

Uh oh!

LudwigKnuepfer commented Dec 3, 2014