doc: add `serialization` group to Doxygen by jia200x · Pull Request #9840 · RIOT-OS/RIOT

jia200x · 2018-08-24T15:46:17Z

Contribution description

This PR is the first of a series of PR for improving the documentation.

Here I group several serialization/deserialization format in a Serialization group, so they are easier to find for the end user.

A sys_serialization group was defined under sys group.

The following modules where touched and added to the corresponding groups:

cbor: sys_serialization
base64: sys_serialization
ubjson: sys_serialization
cayenne_lpp: sys_serialization, pkg
cn-cbor: sys_serialization, pkg
jsnm: sys_serialization, pkg
minmea: sys_serialization, pkg (I might add a Parsers section later, if applies)

Testing procedure

make doc. Go to System>Serialization in the generated documentation and see how these components are grouped.

Issues/PRs references

#8479

aabadie · 2018-08-27T08:37:19Z

@jia200x, this looks good in general but please rebase and fix the doxygen warning raised by the static tests.

smlng · 2018-09-20T14:49:44Z

sys/include/base64.h

 */

 /**
- * @defgroup    sys_base64 base64 encoder decoder


Why these changes, IIRC I had this discussion before and we concluded to use doc.txt for group definition only when there is no proper header file. However, here we have these header files, so no need to introduce doc.txt files, too.

I mean for sys_base64, sys_ubjson and the like

hmmmm, I could revert this change. I was adding doc.txt files because it's easier to write per-module documentation in a doc file rather than headers. Shall I revert?

ping @smlng

as said I had this discussion in one of my own PRs and the consensus was to use header files where possible and limit the usage of doc.txt to cases where no header file is available for the module or group.

TL;DR: in this case I would suggest to revert, yes.

@miri64 IIRC we had this discussion, any thought/comments here?

I agree that adding doc.txt when not actually needed is not the way to go. The documentation stays with the code this way and isn't "hidden" in some file nobody ever really looks at. ;-)

ok, I will revert them :)

smlng · 2018-10-05T10:03:09Z

btw. please rebase and resolve conflicts!

jia200x · 2018-10-05T12:05:07Z

@smlng done!

miri64 · 2018-10-05T12:59:37Z

sys/include/ubjson.h

- * @brief       A library to read and write UBJSON serialized data.
+ * @defgroup sys_ubjson  Universal Binary JSON deserializer
+ * @ingroup  sys_serialization
+ * @brief    Provides a BSON deserializer library to RIOT


To clarify my question after a little bit of research: I don't think BSON and UBJSON are the same thing. According to Wikipedia UBJSON is aimed to be the successor of BSON, but that still wouldn't be the same thing.

ok, I will differentiate them

jia200x · 2018-10-05T13:06:47Z

@miri64 done!

miri64 · 2018-10-05T13:28:42Z

sys/include/ubjson.h

- * @brief       A library to read and write UBJSON serialized data.
+ * @defgroup sys_ubjson  Universal Binary JSON deserializer
+ * @ingroup  sys_serialization
+ * @brief    Provides a UBJSON deserializer library to RIOT


Still weird, that the original doc said, the lib is able to read and write UBJSON, but your new doc states its only able to deserialize (i.e. read)...

You are right. Sorry

jia200x · 2018-10-05T13:34:01Z

@miri64 ok, fixed the @defgroup and @brief. Now it should be OK.

miri64

Sorry for the drippy non-review. Every time I look at this PR I see something new 🙄. But this should be my last comment now.

miri64 · 2018-10-05T13:35:37Z

doc/doxygen/src/getting-started.md


 * `APPLICATION`: should contain the (unique) name of your application
- * `BOARD`: specifies the platform the application should be build for by
+ * `BOARD`: specifies the platform the application should be built for by


This was also fixed in #10113 so you might not have rebased properly.

jia200x · 2018-10-05T13:38:43Z

@miri64 oh yes, there seemed to be a problem with the last push. Now it should be fixed

miri64

As promised, now I'm fine with this PR as well 😁 👍

jia200x · 2018-10-05T13:41:34Z

yay 🎉

miri64 · 2018-10-05T13:42:54Z

Wait -.- as far as I can see, lora-serilization is missing (#9863 was merged after this PR was opened). But I guess we can do that as a follow-up.

jia200x · 2018-10-05T13:43:46Z

Wait -.- as far as I can see, lora-serilization is missing (#9863 was merged after this PR was opened). But I guess we can do that as a follow-up.

Sure, I can open the follow up

miri64 · 2018-10-05T15:34:23Z

xtimer_now64_continuity caused false negatives again :-/

jia200x · 2018-10-08T08:16:41Z

xtimer_now64_continuity caused false negatives again :-/

Now it passed, can we merge then? :)

miri64 · 2018-10-08T09:33:59Z

Yupp let's do it. Even though the build is already a few days old we have to consider that this is just a doc update, so I think we don't need to retest ;-).

jia200x requested review from aabadie and miri64 August 24, 2018 15:46

jia200x added Area: doc Area: Documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 24, 2018

jia200x mentioned this pull request Aug 24, 2018

documentation: better end-user documentation and learning resources #8479

Open

20 tasks

jia200x force-pushed the pr/doc_serializer branch from 3cc23eb to cbe30af Compare August 28, 2018 16:31

smlng suggested changes Sep 20, 2018

View reviewed changes

miri64 removed their request for review September 25, 2018 19:14

jia200x force-pushed the pr/doc_serializer branch 2 times, most recently from dc6b645 to 8622dc9 Compare October 5, 2018 12:03

smlng approved these changes Oct 5, 2018

View reviewed changes

miri64 reviewed Oct 5, 2018

View reviewed changes

jia200x force-pushed the pr/doc_serializer branch from 8622dc9 to 7df54ce Compare October 5, 2018 13:06

miri64 reviewed Oct 5, 2018

View reviewed changes

jia200x force-pushed the pr/doc_serializer branch from 7df54ce to 560ee54 Compare October 5, 2018 13:33

jia200x added 7 commits October 5, 2018 15:34

sys/serialization: add sys_serialization doxygen group

dbd3183

sys/base64: change ingroup directive

8511508

sys/ubjson: change ingroup directive

dac54db

pkg/cayenne_lpp: change ingroup directive

93b4fe5

pkg/cn-cbor: change ingroup directive

a30fbbc

pkg/jsmn: change ingroup directive

5946344

pkg/minmea: change ingroup directive

470cec8

miri64 reviewed Oct 5, 2018

View reviewed changes

jia200x force-pushed the pr/doc_serializer branch from 560ee54 to 470cec8 Compare October 5, 2018 13:38

miri64 approved these changes Oct 5, 2018

View reviewed changes

miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 5, 2018

miri64 merged commit bbae45c into RIOT-OS:master Oct 8, 2018

jia200x mentioned this pull request Oct 10, 2018

pkg/lora_serialization: add doc to serialization group #10140

Merged

Conversation

jia200x commented Aug 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

aabadie commented Aug 27, 2018

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!

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!

smlng commented Oct 5, 2018

Uh oh!

jia200x commented Oct 5, 2018

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!

jia200x commented Oct 5, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jia200x commented Oct 5, 2018

Uh oh!

miri64 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jia200x commented Oct 5, 2018

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

jia200x commented Oct 5, 2018

Uh oh!

miri64 commented Oct 5, 2018

Uh oh!

jia200x commented Oct 5, 2018

Uh oh!

miri64 commented Oct 5, 2018

Uh oh!

jia200x commented Oct 8, 2018

Uh oh!

miri64 commented Oct 8, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jia200x commented Aug 24, 2018 •

edited

Loading

miri64 left a comment •

edited

Loading