RFC 9177: Add in Q-Block support #611
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mrdeep1
force-pushed
the
q-block
branch
4 times, most recently
from
5f3dcdd
to
4c8724a
Compare
mrdeep1
force-pushed
the
q-block
branch
4 times, most recently
from
ccea6a3
to
c1b26e6
Compare
mrdeep1
force-pushed
the
q-block
branch
2 times, most recently
from
a299b31
to
9a107d4
Compare
mrdeep1
force-pushed
the
q-block
branch
3 times, most recently
from
cc3fba8
to
94c0c0d
Compare
mrdeep1
force-pushed
the
q-block
branch
2 times, most recently
from
dfdf33f
to
6c36084
Compare
mrdeep1
force-pushed
the
q-block
branch
5 times, most recently
from
9cb00b2
to
b5d02c1
Compare
mrdeep1
force-pushed
the
q-block
branch
8 times, most recently
from
3eaf1bd
to
de8bd4d
Compare
mrdeep1
force-pushed
the
q-block
branch
3 times, most recently
from
daec8e9
to
cd1e06f
Compare
mrdeep1
force-pushed
the
q-block
branch
2 times, most recently
from
adebcb7
to
edb5501
Compare
mrdeep1
force-pushed
the
q-block
branch
2 times, most recently
from
628cba0
to
8bdd77e
Compare
mrdeep1
force-pushed
the
q-block
branch
2 times, most recently
from
5b4b413
to
d098bcf
Compare
mrdeep1
force-pushed
the
q-block
branch
4 times, most recently
from
dfa5550
to
1b59514
Compare
obgm
reviewed
Jun 30, 2023
There was a problem hiding this comment.
I did a first quick review, and this looks good to me. There is a bit of garbled whitespace that could be cleaned up later. Also, there are some changes that do not directly relate to this PR but I think it would be too difficult to strip them out at this stage.
obgm
reviewed
Jul 5, 2023
src/block.c
Outdated
| coap_delete_pdu(session->saved_pdu); | ||
| session->saved_pdu = actual; | ||
| return coap_send_internal(session, pdu); | ||
| if ((mid = coap_send_internal(session, pdu)) == COAP_INVALID_MID) |
There was a problem hiding this comment.
This looks much cleaner now. I suggest the following to make the code a bit more readable (as done in other parts):
mid = coap_send_internal(....);
if (mid != COAP_INVALID_MID) {
session->remote_test_mid = mid;
}
return mid;There was a problem hiding this comment.
Updated with suggestion
There was a problem hiding this comment.
Better. But now it reads:
if (x)
return x;
do_y();
return x;Which looks a bit weird. But feel free to keep it this way since it is neither wrong nor overly complex.
examples/coap-client.c: examples/coap-server.c: Add in Q-Block intelligence. include/coap3/block.h include/coap3/coap_block_internal.h include/coap3/coap_event.h Add in Q-Block references. Add in new functions coap_block_check_q_block1_xmit() coap_block_check_q_block2_xmit() coap_send_q_blocks() coap_send_q_block1() coap_send_q_block2() coap_block_drop_resp_q_block_xmit() coap_block_drop_resp_q_block2_crcv() coap_q_block_is_supported() include/coap3/coap_session.h: Add in new Congestion Control defaults and ability to configure them. include/coap3/pdu.h: Add in the new options. libcoap-3.map: libcoap-3.sym: Expose external Congestion Control functions. man/coap-client.txt.in: man/coap-server.txt.in: man/coap_block.txt.in: man/coap_handler.txt.in: man/coap_recovery.txt.in: man/coap.txt.in: Add in Q-Block information. src/block.c: Add in the new functions to support Q-Block. src/coap_debug.c: Output the new options as appropriate. src/coap_io.c: Check for Q-Block transmission timeouts. src/coap_session.c: Add in the Congestion Control support. src/net.c: src/resource.c: Add in Q-Block support. configure.ac CMakeLists.txt Add in support for enabling/disabling Q-Block support (enabled by default).
|
Thanks for doing the review. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
examples/client.c:
examples/coap-server.c:
Add in Q-Block intelligence.
include/coap2/block.h
include/coap2/coap_block_internal.h
include/coap2/coap_event.h
Add in Q-Block references.
Add in new functions
coap_block_check_q_block1_xmit()
coap_block_check_q_block2_xmit()
coap_send_q_blocks()
coap_send_q_block1()
coap_send_q_block2()
coap_block_drop_resp_q_block_xmit()
coap_block_drop_resp_q_block2_crcv()
include/coap2/coap_session.h:
Add in new Congestion Control defaults and ability to configure them.
include/coap2/coap_subscribe_internal.h:
include/coap2/resource.h:
Support Q-Block2 as well for subscriptions.
include/coap2/pdu.h:
Add in the new options.
libcoap-2.map:
libcoap-2.sym:
Expose external Congestion Control functions.
man/coap-client.txt.in:
man/coap-server.txt.in:
man/coap_block.txt.in:
man/coap_handler.txt.in:
Add in Q-Block information.
src/block.c:
Add in the new functions to support Q-Block.
src/coap_debug.c:
Output the new options as appropriate.
src/coap_io.c:
Check for Q-Block transmission timeouts.
src/coap_session.c:
Add in the Congestion Control support.
src/net.c:
src/pdu.c:
src/resource.c:
Add in Q-Block support.