Skip to content
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

pkg/tinydtls: only build debug log when tinydtls_debug is enabled #18907

Closed
wants to merge 2 commits into from

Conversation

benpicco
Copy link
Contributor

Contribution description

tinydtls allows to switch the log level at run-time, which means all debug strings have to be present in ROM.
This is pretty wasteful, so define a tinydtls_debug module without which those debug facilities are dropped, saving ~20k ROM on Cortex-M0.

This also hooks the tinydtls log into RIOT's log facility, so even with tinydtls_debug enabled messages below the log level set in RIOT are dropped.

Dynamic log level switching does no longer work, but we don't have this in RIOT so we don't need it for this module.

Testing procedure

Build examples/dtls-sock

master

   text	   data	    bss	    dec	    hex	filename
 120412	    376	  23356	 144144	  23310	examples/dtls-sock/bin/samr21-xpro/dtls_sock.elf

this PR

   text	   data	    bss	    dec	    hex	filename
  98088	    272	  23236	 121596	  1dafc	examples/dtls-sock/bin/samr21-xpro/dtls_sock.elf

Issues/PRs references

Upstream PR: eclipse/tinydtls#177

@github-actions github-actions bot added the Area: pkg Area: External package ports label Nov 15, 2022
@@ -11,6 +11,11 @@ INCLUDES += -I$(PKG_BUILDDIR)
# Mandatory for tinyDTLS
CFLAGS += -DDTLSv12 -DWITH_SHA256

PSEUDOMODULES += tinydtls_debug
ifeq (,$(filter tinydtls_debug, $(USEMODULE)))
CFLAGS += -DNDEBUG=1
Copy link
Member

Choose a reason for hiding this comment

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

Won't this pollute the name space of all compilation units with an NDEBUG symbol? Maybe call it TINYDTLS_NDEBUG instead, to avoid name clashes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The symbol is already used by tinyDTLS.
Isn't the Makefile.include restricted to the module itself?
Or was it Makefile - I always mix that up.

@benpicco
Copy link
Contributor Author

Changes have been merged upstream.
Looks like NDEBUG gets set automatically with DEVELHELP=0.

@benpicco benpicco closed this Jan 21, 2023
@benpicco benpicco deleted the pkg/tinydtls-slim branch January 21, 2023 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: pkg Area: External package ports
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants