-
Notifications
You must be signed in to change notification settings - Fork 2.1k
net/unicoap: Unified and Modular CoAP stack: Parser and Message APIs (pt 1) #21390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
net/unicoap: Unified and Modular CoAP stack: Parser and Message APIs (pt 1) #21390
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this.
It is good for getting an impression for what the goal of a PR series is if the complete code is already available. But for the actual in-depth reviewing this is way too large.
Could you split out the basic message and option parsing plus unit testing into a single PR? It would be sufficient to start with just URI-Path and maybe some numeric Option (Max-Age?) in that PR. That way a quick and thorough review of one of the basic building blocks can be done and parts of this can get upstream quickly.
/** | ||
* @brief Marks the boundary between header and payload | ||
*/ | ||
#define UNICOAP_PAYLOAD_MARKER (0xFF) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defines of these constants do not contain any design decisions and can be shared between different CoAP implementations regardless of how different their design goals and use cases are. We already have sys/include/net/coap.h
for such a knowledge base of constants. Please just use that and extend it as needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do prefer having everything in one place, especially if eventually, someday we're going to deprecate GCoAP, nanoCoAP, nanoCoAP Sock. Plus, having this here keeps documentation in one place. After all, what we want is a single, unified CoAP suite that does not build on other, prior CoAP modules, like GCoAP does. Referencing nanoCoAP again does not contribute -- IMO -- to that single-documentation, single-module experience I am aiming for.
@carl-tud hey, the images in the performance analysis are 404 to me. I also took the audacity to refactor your PR description a bit, to increase its accessibility. |
I think they are still in your private repository. |
@miri64 @Teufelchen1 Thanks, they should be visible now. |
While I like this idea, I am not sure this would make things better. The bulk of this PR is documentation, which would still be part of the basic message PR. If we go for a split, I would rather go for an opaque (byte sequence) options to keep in this PR, as they should be the easiest ones to create and parse. |
879d85f
to
170fefb
Compare
How long can you / do you intend to provide maintenance for this code? |
Definitely over the summer until October. After that, I think I am going to need some help (1–2 people) maintaining it. Nevertheless, if someone was willing to help from the get-go, then that would also be great. |
Do you happen to have a size (RAM/ROM) comparison with either nanocoap or gcoap? |
1a8a3dc
to
189abc4
Compare
ready |
I ran
|
@carl-tud when you add the |
Okay, will do, but tomorrow |
189abc4
to
5e353a3
Compare
@crasbe thanks. done. |
Congrats! 🎉 |
This PR is the first in a series to introduce
unicoap
, a unified and modular CoAP implementation for RIOT. An overview of all PRs related tounicoap
is presented in #21389, including reasons whyunicoap
is needed.What does this PR include?
unicoap
, including config/Kconfig supportunicoap
, including parsing/serializing support for RFC 7252unicoap
The new API aims to reduce the need for in-depth protocol knowledge and minimizes necessary boilerplate. CoAP options can now be inserted in any order, which was not possible before. For example,
More in the documentation.
Lines of code (C code in sources and headers, Makefiles): ≈ 3500
plus, ≈ 5000 lines of SVG, ≈ 700 in Markdown files, ≈ 3700 lines of comments
Performance Analysis
The following figures compare
unicoap
with nanoCoAP in terms of execution time. Even thoughunicoap
is more flexible, it is not necessarily slower.Click to expand the analysis:
In this experiment, we measured the time to execute get, add/insert, and remove, given a message with a specified number of options. The results show the average time over multiple runs using the same parameter set relative to the number of existing options.
For each of the operations get, add/insert, and remove, we differentiate between a trivial case (i.e., the option is located at the end of the options buffer) and a complex case (i.e., the option is located in between other options).
The step effects occurring in (a) – (d) are artifacts of the 1 microsecond visualization resolution.
Getter
The average time needed to retrieve an option value grows linearly with the number of options present in the buffer, see (a) and (b). Both implementations require slightly less time when the option is present in the middle, see (b), because of the linear search, i.e., the algorithm finds an option in the middle earlier than at the end. The growth rate of
unicoap
is marginally higher in both cases because of sanity checks inunicoap
.Setter
In nanoCoAP (
coap_opt_add_opaque
method), when adding options, the timing is independent of the number of options already present. Inunicoap
, the timing scales linearly, see (c). The reason for this behavior is the following. nanoCoAP requires options to be inserted in-order and thus does not need to check whether there are adjacent options.unicoap
, however, does not introduce such a requirement and, therefore, must first iterate over the list of options until the correct insertion slot is found. Moreover,unicoap
implements safeguard checks to prevent adding options without sufficient buffer capacity.Figure (d) shows the results when trailing options change and must be moved. These results do not contain data for nanoCoAP because inserting options out of order is not supported.
Remove
When removing CoAP options from message buffers,
unicoap
outperforms nanoCoAP, see Figures (e) and (f). On average,unicoap
is faster by more than 45 microseconds. The cause of the outliers visible in Figure (f) has not been identified yet.Parser
unicoap
performs slightly better than nanoCoAP, in particular in cases of multiple options, see Figure (g).