pkg/tinydtls: allow to set buffer size from application again#19892
Merged
bors[bot] merged 3 commits intoRIOT-OS:masterfrom Aug 24, 2023
Merged
pkg/tinydtls: allow to set buffer size from application again#19892bors[bot] merged 3 commits intoRIOT-OS:masterfrom
bors[bot] merged 3 commits intoRIOT-OS:masterfrom
Conversation
leandrolanzieri
added
Type: bug
benpicco
reviewed
Aug 21, 2023
Contributor
There was a problem hiding this comment.
This feels a bit hacky.
If applications currently don't (can't) set DTLS_MAX_BUF directly (and we didn't document that they should, so how would they even find out) we could instead add a variable:
ifneq (,$(filter gcoap,$(USEMODULE)))
DTLS_MAX_BUF ?= $(CONFIG_GCOAP_PDU_BUF_SIZE) + 36)
else
DTLS_MAX_BUF ?= 200
endif
CFLAGS += "-DDTLS_MAX_BUF=$(DTLS_MAX_BUF)"and document that applications can just set the DTLS_MAX_BUF variable (or maybe chose a different name to avoid confusion with the CFLAG).
P.S.: I think this should be set to CONFIG_NANOCOAP_BLOCKSIZE_DEFAULT when nanocoap_dtls is used.
leandrolanzieri
force-pushed
the
pr/pkg/tinydtls_buffer_cflag
branch
from
5e95c9b to
0e8ae62
Compare
leandrolanzieri
requested review from
PeterKietzmann,
cgundogan,
haukepetersen and
miri64
as code owners
github-actions
bot
added
Area: network
benpicco
reviewed
Aug 21, 2023
benpicco
reviewed
Aug 21, 2023
leandrolanzieri
force-pushed
the
pr/pkg/tinydtls_buffer_cflag
branch
from
0e8ae62 to
d721630
Compare
benpicco
reviewed
Aug 21, 2023
benpicco
reviewed
Aug 21, 2023
benpicco
added
the
CI: ready for build
leandrolanzieri
force-pushed
the
pr/pkg/tinydtls_buffer_cflag
branch
from
5d58657 to
e65f1c4
Compare
Contributor
Author
|
bors merge |
bors bot
added a commit
that referenced
this pull request
Aug 22, 2023
19892: pkg/tinydtls: allow to set buffer size from application again r=leandrolanzieri a=leandrolanzieri ### Contribution description Currently the buffer size on tinydtls is set in its Makefile whenever `gcoap` module is present. This limits the ability of the user to override the value. This adds a pre-check of the `CFLAGS` to see if it was set before. ### Testing procedure Try setting `CFLAGS += -DDTLS_MAX_BUF=<some_value>` on `examples/gcoap_dtls`, you should be able to override the default value without errors. ### Issues/PRs references Reported in #19838 Co-authored-by: Leandro Lanzieri <[email protected]>
Contributor
|
Build failed: |
Contributor
|
bors merge |
bors bot
added a commit
that referenced
this pull request
Aug 23, 2023
19886: cpu/efm32: fix DAC configuration r=benpicco a=gschorcht ### Contribution description The EFM32 MCU allows the reference voltage to be configured per DAC device, not per DAC channel. Also, the DAC reference voltage was defined in the configuration but not used anywhere. At the moment we have only defined one board (`stwstk6220a`) that uses the DAC, so changing the configuration interface shouldn't be critical. ### Testing procedure `tests/periph/dac` should still work for the `stwstk6220a` ``` BOARD=slwstk6220a make -j8 -C tests/periph/dac flash ``` I don't have a `stwstk6220a` board (EFM32 Series 0) so that I can't test it. I could only test it for the `sltb009a` board (EFM32 Series 1) with the change for VDAC in PR #19887. ### Issues/PRs references 19892: pkg/tinydtls: allow to set buffer size from application again r=benpicco a=leandrolanzieri ### Contribution description Currently the buffer size on tinydtls is set in its Makefile whenever `gcoap` module is present. This limits the ability of the user to override the value. This adds a pre-check of the `CFLAGS` to see if it was set before. ### Testing procedure Try setting `CFLAGS += -DDTLS_MAX_BUF=<some_value>` on `examples/gcoap_dtls`, you should be able to override the default value without errors. ### Issues/PRs references Reported in #19838 Co-authored-by: Gunar Schorcht <[email protected]> Co-authored-by: Leandro Lanzieri <[email protected]>
Contributor
|
Build failed (retrying...): |
bors bot
added a commit
that referenced
this pull request
Aug 23, 2023
19892: pkg/tinydtls: allow to set buffer size from application again r=benpicco a=leandrolanzieri ### Contribution description Currently the buffer size on tinydtls is set in its Makefile whenever `gcoap` module is present. This limits the ability of the user to override the value. This adds a pre-check of the `CFLAGS` to see if it was set before. ### Testing procedure Try setting `CFLAGS += -DDTLS_MAX_BUF=<some_value>` on `examples/gcoap_dtls`, you should be able to override the default value without errors. ### Issues/PRs references Reported in #19838 Co-authored-by: Leandro Lanzieri <[email protected]>
Contributor
|
Build failed: |
Contributor
|
bors merge |
Contributor
|
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
Contributor
|
arg, this breaks |
benpicco
reviewed
Feb 26, 2024
| @@ -0,0 +1,4 @@ | |||
| ifneq (,$(filter nanocoap_dtls,$(USEMODULE))) | |||
| CONFIG_NANOCOAP_BLOCKSIZE_DEFAULT := $(or $(CONFIG_NANOCOAP_BLOCKSIZE_DEFAULT),2) | |||
| DTLS_MAX_BUF ?= ((1 << ($(CONFIG_NANOCOAP_BLOCKSIZE_DEFAULT) + 3)) + 36) | |||
Contributor
There was a problem hiding this comment.
The handshake buffer size does not depend on the block size
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Contribution description
Currently the buffer size on tinydtls is set in its Makefile whenever
gcoapmodule is present. This limits the ability of the user to override the value. This adds a pre-check of theCFLAGSto see if it was set before.Testing procedure
Try setting
CFLAGS += -DDTLS_MAX_BUF=<some_value>DTLS_MAX_BUF=<some_value>onexamples/gcoap_dtls, you should be able to override the default value without errors.Issues/PRs references
Reported in #19838