Skip to content

net/nanocoap: group functions in module doc#10876

Merged
kaspar030 merged 5 commits intoRIOT-OS:masterfrom
kb2ma:nanocoap/doc_function_groups
Apr 30, 2019
Merged

net/nanocoap: group functions in module doc#10876
kaspar030 merged 5 commits intoRIOT-OS:masterfrom
kb2ma:nanocoap/doc_function_groups

Conversation

@kb2ma
Copy link
Copy Markdown
Member

@kb2ma kb2ma commented Jan 26, 2019

net/nanocoap: reorganize module doc

Contribution description

It is difficult to use the nanocoap function documentation. There are now 52 public functions, not in alphabetic order. This PR organizes these functions into groups, sorts them within each group, and updates the module documentation to reference them. See the list below.

The groups also use a new name for the two APIs for writing a message: Buffer API and Packet API. Previously they were called the minimal API and struct-based API. I have struggled with these names for a while, but I think the new ones are simple and clear.

The next sequence of PRs implement the remainder of Block, including for the Packet API. We plan to complete the documentation for Block with those PRs. So, to make it easier to read the module documentation, the current PR moves the guide to using the Buffer API and the nanocoap_sock functions to that header. Otherwise, the nanocoap page will become overloaded and harder to follow.

Function groups:

  • Header Read/Write
  • Options Read
  • Options for Block
  • Options Write Packet API
  • Options Write Buffer API
  • Messaging
  • gcoap specific

Testing procedure

make doc

See both the Nanocoap and Nanocoap Sock sections.

Issues/PRs references

Depends on #10931 to order the open PRs

@kb2ma kb2ma added Area: doc Area: Documentation Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: CoAP Area: Constrained Application Protocol implementations labels Jan 26, 2019
@kb2ma kb2ma requested review from kaspar030, miri64 and smlng January 26, 2019 11:44
@kb2ma kb2ma force-pushed the nanocoap/doc_function_groups branch from badbc95 to 5c0f331 Compare February 11, 2019 13:10
@kb2ma
Copy link
Copy Markdown
Member Author

kb2ma commented Feb 11, 2019

I just revised this PR to include splitting documentation for the Buffer API to a new module section, Nanocoap Sock. This section of the documentation already included use of nanocoap_sock functions like nanocoap_server() and nanocoap_request(), so it seemed like organic growth.

The reorganization of the function documentation means it touches work in currently open PRs, as well as PRs for work I am doing to complete Block for #10732. So, I have imposed an ordering on the PRs, with this one after #10931, the last in the currently open sequence. I'm not looking forward to rebasing, but this approach makes it easier to keep track of the changes.

@kaspar030
Copy link
Copy Markdown
Contributor

please rebase!

@kaspar030
Copy link
Copy Markdown
Contributor

I agree with the function grouping. Makes much more sense.

IMO we should make the nanocoap sock documentation a subgroup of the main nanocoap group, and call it the "high-level API". I think it would make sense to steer any users towards that, as most will probably be looking for "how to make a client request" or "how to start serving a coap resource".

@kaspar030
Copy link
Copy Markdown
Contributor

IMO we should ...

Is that in scope or should we go with the re-ordering first?

@kb2ma
Copy link
Copy Markdown
Member Author

kb2ma commented Apr 4, 2019

With this PR, the documentation is divided into three groups:

  1. nanocoap sock -- high level guide, procedural
  2. gcoap -- high level guide, procedural
  3. nanocoap -- low level, grouped, reference

These sections are siblings beneath Modules > Networking. The nanocoap and nanocoap sock docs are beside each other, so they should be easy to find. In addition both the high level and low level docs have links to each other.

I'm not sure exactly what you mean by making the nanocoap sock documentation a 'subgroup' of nanocoap, but I think the present arrangement provides a logical separation. I would defer any further shuffling until later.

Next step is to rebase. Thanks for reviewing.

@kb2ma kb2ma force-pushed the nanocoap/doc_function_groups branch from f4f0043 to 50aeaa9 Compare April 5, 2019 01:16
@kb2ma
Copy link
Copy Markdown
Member Author

kb2ma commented Apr 5, 2019

Rebased.

@kaspar030
Copy link
Copy Markdown
Contributor

I would defer any further shuffling until later.

Ok!

@kaspar030 kaspar030 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 12, 2019
@kb2ma kb2ma force-pushed the nanocoap/doc_function_groups branch from 50aeaa9 to 6b58039 Compare April 19, 2019 02:08
@kb2ma
Copy link
Copy Markdown
Member Author

kb2ma commented Apr 19, 2019

Rebased to incorporate changes from #11386. This PR is prone to conflicts, let's get it merged!

* path, specifically the ASCII encoding of the path characters (digit and
* capital precede lower case). nanocoap provides the
* COAP_WELL_KNOWN_CORE_DEFAULT_HANDLER entry for `/.well-known/core`.
*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The path matching section is missing here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch, Leandro!

I left the path matching section in the base nanocoap doc and added references in the nanocoap sock and gcoap docs. There was too much text in that section to duplicate.

Copy link
Copy Markdown
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

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

Just two minor comments, otherwise looks good. I like the grouping!

*
* See the nanocoap_server example, which is built on the nanocoap_server()
* function. A server must define an array of coap_resource_t resources for
* which it responds. See the declarations of `coap_resources` and
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it be possible to add a link to coap_resources and coap_resources_numof?

@@ -46,7 +46,8 @@
* structs) ordered by the resource path, specifically the ASCII encoding of
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know this did not change in this PR, but we are touching the doc already so: Where it says "Internally, gcoap depends on the nanocoap package for base level structs and functionality" a link to nanocoap could be handy, as it is the first reference to it in the document.

Also, maybe change 'package' to module? as package usually refers to external packages in RIOT.

@leandrolanzieri leandrolanzieri added Reviewed: 1-fundamentals The fundamentals of 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 Apr 23, 2019
@kaspar030
Copy link
Copy Markdown
Contributor

@leandrolanzieri would you mind postponing your change requests for a follow-up PR? This is a dependency of a couple of other coap PR's I'd like to get on with...

@leandrolanzieri leandrolanzieri dismissed their stale review April 25, 2019 07:15

Changes postponed for a follow-up PR

@kb2ma
Copy link
Copy Markdown
Member Author

kb2ma commented Apr 25, 2019

Thanks, @leandrolanzieri. I'll squash the fix, and then we just need a green check mark from someone. ;-)

@kb2ma kb2ma force-pushed the nanocoap/doc_function_groups branch from bb7b63d to 5fc86c9 Compare April 25, 2019 16:35
@kb2ma
Copy link
Copy Markdown
Member Author

kb2ma commented Apr 25, 2019

Squashed!

Copy link
Copy Markdown
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

ACK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants