Skip to content

sys/net/nanocoap: add missing dependency to sock_udp #22047

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.

Sign up for GitHub

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

Merged
miri64 merged 1 commit into from
Feb 9, 2026
+9 −0
Merged

sys/net/nanocoap: add missing dependency to sock_udp #22047

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions sys/Makefile.dep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,15 @@ ifneq (,$(filter nanocoap_%,$(USEMODULE)))
USEMODULE += nanocoap
endif

# bare nanocoap is supposed to be concerned only with parsing and building of
# CoAP message and should not care about any networking, but it sadly contains
# code such as coap_handle_request() that very much is intermixed with
# networking. Until that get's cleaned up, we have to model the dependency
# that is there in the code.
ifneq (,$(filter nanocoap,$(USEMODULE)))
USEMODULE += sock_udp
endif

Comment on lines +584 to +587
Copy link
Contributor

@mguetschow mguetschow Feb 5, 2026

Choose a reason for hiding this comment

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

Oh wait, but this looks wrong: why would nanocoap being the parser/generator part of the coap api require sock_udp?

In fact, the error message you provide shows an include of nanocoap_sock.h, which should probably not be in sys/net/nanocoap from my understanding.

Copy link
Member Author

@maribu maribu Feb 5, 2026

Choose a reason for hiding this comment

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

Well, in theory nanocoap should only be the parser. But in reality nanocoap is a hot mess. sys/net/application_layer/nanocoap/nanocoap.c includes nanocao_sock.h and therefore depends on sock_udp.h.

Copy link
Contributor

@mguetschow mguetschow Feb 5, 2026

Choose a reason for hiding this comment

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

Ah fun, just digged where this is coming from, a commit co-authored by me :P

feeb684

Don't we have a way to fix this in nanocoap_sock, e.g. by doing the observe stuff before calling coap_handle_req? And thereby fixing the dependencies in nanocoap.c

Copy link
Member Author

@maribu maribu Feb 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

To be honest, coap_handle_req() is already out of scope of the CoAP message parsing and message building that pure nanocoap is supposed to be.

Copy link
Contributor

@mguetschow mguetschow Feb 5, 2026

Choose a reason for hiding this comment

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

I see. If you think moving these three lines out of coap_handle_req is too cumbersome, I would at least add comments both on those lines and in the Makefile.dep pointing out that this is not the proper dependency/place to have it.

Just imagining myself looking at this in some months from now and being completely confused about it again.

# include unicoap dependencies
ifneq (,$(filter unicoap%,$(USEMODULE)))
include $(RIOTBASE)/sys/net/application_layer/unicoap/Makefile.dep
Expand Down