net/gcoap: add/use Packet API Block implementation by kb2ma · Pull Request #11056 · RIOT-OS/RIOT

kb2ma · 2019-02-24T13:51:28Z

Contribution description

#11002 and #11024 completed the Blockwise implementation for the Buffer API, traditionally used with nanocoap sock's message send/receive functions. This PR completes the implementation for the Packet API, used by gcoap. It includes two pairs of functions:

coap_opt_add_blockN() to send a blockwise payload (descriptive use)
coap_opt_add_blockN_control() to request/ack a blockwise payload (control use)

N is 1 when acting on a blockwise payload in a PUT/POST request, and 2 when acting on a blockwise payload in a GET response.

This PR also adds module documentation to describe typical use of these functions to send/receive blockwise content. Finally, this PR adds generic client side control use in the gcoap example to request blockwise content from a server. For example, this use allows gcoap to retrieve the full response from the nanocoap_server example for the /.well-known/core resource.

Testing procedure

The gcoap module doc references an example for each use:

gcoap-block-server app for server side descriptive and control use, like the nanocoap_server example
gcoap-block-client app for client side descriptive use
the gcoap example for client side control use, as mentioned above

The gcoap example caches the request path to retrieve remaining blocks. If the path is too long, the example displays Path too long; can't complete blockwise. To test this against nanocoap_server as described above, decrease _LAST_REQ_PATH_MAX in the example to 16.

FWIW, I use these examples in my automated functional tests.

Issues/PRs references

Partially implements #10732.

kb2ma · 2019-07-25T13:01:18Z

Rebased

kb2ma · 2019-08-03T13:15:11Z

Rebased and added a couple of minor fixups.

fjmolinas

Code looks good. For now I only tested the CLI example which is working nicely. I'll try to make time soon to reproduce your test suit.

Some minor comments, mostly regarding documentation and corner cases.

examples/gcoap/Makefile

examples/gcoap/gcoap_cli.c

fjmolinas · 2019-08-08T09:19:15Z

sys/include/net/gcoap.h

 * the Observe option value set to 1. The server does not support cancellation
 * via a reset (RST) response to a non-confirmable notification.
 *
+ * ## Block Operation ##


Doxygen is generating this as a big blob, ### are treated as comments and don't show up. Since this isn't a README
maybe consider using * ===== or * ------- do mark section limits.

I don't follow you. When I generate the documentation, the web page output looks as I would expect. Can you be more specific? See the screenshot below.
.........

That is indeed strange, when I generate it locally I get this layout:

This is when calling make doc locally and opening the generated html. How are you opening/generating your file?

I see that in http://riot-os.org/api is the same you are showing. This looks like an issue with my doxygen version, I have 1.8.16 which should support markdown. Anyway if your version is rendering correctly then I trust the issue is on my side.

I am using doxygen 1.8.13 from Ubuntu 19.04. I built the 1.8.16 release from source and regenerated the doc, with no change to the output. I display the doc from a local Apache web server. If I display from a file: URL, the fonts are different, but the structure is the same. So, I agree it seems that there is some difference in your setup.

I did notice that the level 3 headings for the block documentation in this PR do not include a closing ### at the end of the line. The doxygen doc shows that the absence of the closing hashes should not matter. However, I will add them to be consistent with the rest of the module documentation.

kb2ma · 2019-08-17T12:30:34Z

All comments addressed.

kb2ma · 2019-09-11T10:36:12Z

@fjmolinas, I reran my functional tests, and all looks good. I am happy to work through any issues you are experiencing.

benpicco · 2019-09-23T13:31:20Z

The documentation is generated properly for me too. (I checked group__net__gcoap.html with doxygen 1.8.13)

All other comments have been addressed and @fjmolinas provided testing.
I'd say squash so we can get this in before the deadline for the 2019.10 release.

fjmolinas · 2019-09-24T15:37:22Z

Running part functional tests (block related ones), all looking good.

sudo -E pytest block1_client_test.py

=============================================================================================================================== test session starts ===============================================================================================================================
platform linux -- Python 3.6.8, pytest-5.1.3, py-1.8.0, pluggy-0.12.0
rootdir: /home/francisco/workspace/riot-coap-pytest, inifile: pytest.ini
collected 1 item                                                                                                                                                                                                                                                                  

block1_client_test.py .                                                                                                                                                                                                                                                     [100%]

================================================================================================================================ 1 passed in 0.67s ================================================================================================================================

sudo -E pytest block2_client_test.py

========================================================= test session starts ==========================================================
platform linux -- Python 3.6.8, pytest-5.1.3, py-1.8.0, pluggy-0.12.0
rootdir: /home/francisco/workspace/riot-coap-pytest, inifile: pytest.ini
collected 3 items                                                                                                                      

block2_client_test.py ...                                                                                                        [100%]

========================================================== 3 passed in 2.31s ===========================================================

sudo -E pytest block1_server_test.py

========================================================= test session starts ==========================================================
platform linux -- Python 3.6.8, pytest-5.1.3, py-1.8.0, pluggy-0.12.0
rootdir: /home/francisco/workspace/riot-coap-pytest, inifile: pytest.ini
collected 6 items                                                                                                                      

block1_server_test.py ......                                                                                                     [100%]

========================================================== 6 passed in 24.78s ==========================================================

sudo -E pytest block2_server_test.py

========================================================= test session starts ==========================================================
platform linux -- Python 3.6.8, pytest-5.1.3, py-1.8.0, pluggy-0.12.0
rootdir: /home/francisco/workspace/riot-coap-pytest, inifile: pytest.ini
collected 4 items                                                                                                                      

block2_server_test.py ....                                                                                                       [100%]

========================================================== 4 passed in 16.55s ==========================================================

I did have an issue with the regex matching in the first test, but it was a test issue and not functional issue.

fjmolinas

My comments have been all addressed, and I tested basic application as well as @kb2ma functional tests for block coap. ACK, please squash @kb2ma!

kb2ma · 2019-09-24T19:38:17Z

Squashed and builds are happy.

kb2ma added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: CoAP Area: Constrained Application Protocol implementations labels Feb 24, 2019

kb2ma requested review from bergzand, haukepetersen and kaspar030 February 24, 2019 13:51

This was referenced Feb 24, 2019

net/coap: Block optimizations #11057

Merged

Completing CoAP Block implementation #10732

Closed

kb2ma added this to the Release 2019.04 milestone Feb 24, 2019

pokgak mentioned this pull request Apr 4, 2019

net: add CoRE RD lookup client implementation #10222

Merged

miri64 modified the milestones: Release 2019.04, Release 2019.07 May 7, 2019

MrKevinWeiss modified the milestones: Release 2019.07, Release 2019.10 Jul 16, 2019

kb2ma force-pushed the coap/pkt_api_block_write branch from ca69b47 to 96719c1 Compare July 25, 2019 12:57

net/nanocoap: add Packet API block write functions

513ddfd

kb2ma force-pushed the coap/pkt_api_block_write branch from 96719c1 to 257dade Compare August 3, 2019 09:39

fjmolinas reviewed Aug 8, 2019

View reviewed changes

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 12, 2019

benpicco requested a review from fjmolinas September 23, 2019 13:03

fjmolinas approved these changes Sep 24, 2019

View reviewed changes

fjmolinas 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 labels Sep 24, 2019

kb2ma added 2 commits September 24, 2019 14:03

examples/gcoap: handle block2 response

1f7fbb2

net/gcoap: add module documentation for block use

68ccf4b

kb2ma force-pushed the coap/pkt_api_block_write branch from b4cdc30 to 68ccf4b Compare September 24, 2019 18:40

benpicco merged commit e942f86 into RIOT-OS:master Sep 24, 2019

Conversation

kb2ma commented Feb 24, 2019 • 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!

kb2ma commented Jul 25, 2019

Uh oh!

kb2ma commented Aug 3, 2019

Uh oh!

fjmolinas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fjmolinas Aug 8, 2019

Choose a reason for hiding this comment

Uh oh!

kb2ma Aug 14, 2019

Choose a reason for hiding this comment

Uh oh!

fjmolinas Aug 14, 2019

Choose a reason for hiding this comment

Uh oh!

kb2ma Aug 15, 2019

Choose a reason for hiding this comment

Uh oh!

kb2ma commented Aug 17, 2019

Uh oh!

kb2ma commented Sep 11, 2019

Uh oh!

benpicco commented Sep 23, 2019

Uh oh!

fjmolinas commented Sep 24, 2019

Uh oh!

fjmolinas left a comment

Choose a reason for hiding this comment

Uh oh!

kb2ma commented Sep 24, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kb2ma commented Feb 24, 2019 •

edited

Loading