Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

net/gcoap: add config macros to config doc group #10676

Merged
merged 3 commits into from
Jan 3, 2019

Conversation

kb2ma
Copy link
Member

@kb2ma kb2ma commented Dec 29, 2018

Contribution description

Adds a documentation group for the user configurable macros in gcoap, per #10566.

Also retitles the gcoap module in the source documentation from "CoAP" to "Gcoap" for clarity. We also have a Nanocoap module as well as a CoAP defines module. I plan to add a similar documentation group for the configurable macros in these other modules as well, which really triggered me to retitle here to avoid further confusion.

Testing procedure

Compile the documentation. You should see:

  • a new configuration group titled, "Gcoap compile configurations"
  • the main gcoap module move from "CoAP" to "Gcoap"
  • a new module within Gcoap, also titled "Gcoap compile configurations"

Issues/PRs references

Partially implements #10566

@kb2ma kb2ma added Area: doc Area: Documentation Area: CoAP Area: Constrained Application Protocol implementations labels Dec 29, 2018
@kb2ma kb2ma requested review from miri64 and jia200x December 29, 2018 16:08
@kb2ma
Copy link
Member Author

kb2ma commented Dec 30, 2018

Pushed fixups and new commit for retitle of documentation group. Looking for a response on comment about ordering of configurable macros.

Avoids confusion with generic CoAP and capitalization retains order
in documentation.
@kb2ma kb2ma force-pushed the gcoap/doc_config_group branch from 233fd61 to beae37c Compare December 30, 2018 15:46
@kb2ma kb2ma added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 30, 2018
@kb2ma
Copy link
Member Author

kb2ma commented Dec 30, 2018

Squashed and running CI build.

Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Still found something, otherwise I'm fine now with the PR as is.

@@ -270,7 +277,8 @@ extern "C" {
#define GCOAP_HEADER_MAXLEN (sizeof(coap_hdr_t) + GCOAP_TOKENLEN_MAX)

/**
* @brief Length in bytes for a token; use 2 if not defined
* @ingroup net_gcoap_conf
* @brief Length in bytes for a token
Copy link
Member

Choose a reason for hiding this comment

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

Is this supposed to be in any relation to GCOAP_TOKENLEN_MAX? If yes please document.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. Documented in 03d4230.

@jia200x jia200x added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Jan 3, 2019
@jia200x
Copy link
Member

jia200x commented Jan 3, 2019

documentation is generated accordingly and all GCOAP macros from #10566 are included here

@jia200x jia200x added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Jan 3, 2019
Copy link
Member

@jia200x jia200x left a comment

Choose a reason for hiding this comment

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

ACK. @miri64 are you ok with this PR?

Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Yep.

@miri64
Copy link
Member

miri64 commented Jan 3, 2019

Let's go! :-)

@miri64 miri64 merged commit 019c1ab into RIOT-OS:master Jan 3, 2019
@aabadie aabadie added this to the Release 2019.01 milestone Jan 4, 2019
@kb2ma kb2ma deleted the gcoap/doc_config_group branch February 4, 2019 11:36
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area: CoAP Area: Constrained Application Protocol implementations Area: doc Area: Documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants