sock_async: fix cyclic include problem

[miri64]

Contribution description

Typically a stack needs to add the callback for a sock as a member of its respective sock type so sock_types.h needs to include net/sock/async.h at the moment. As those however include
net/sock/<prot>.h, which in turn include sock_types.h, we create a cyclic dependency.

This fix resolves this cyclic dependency, by putting the callback definitions in its own header that then in turn can be also included by sock_types.h.

Testing procedure

tests/gnrc_sock_async should still work and run

make -C tests/gnrc_sock_async flash test

Issues/PRs references

Required for #12601, #12602, and #12907 to work.

[miri64]

@kaspar030 can you have a look please. This somewhat breaks the "one header for everything" idea we had in #11723, but I don't really see another way to resolve the cyclic dependencies.

[miri64]

LLVM doesn't like re-definitions of types :-(. Any idea how to solve this, or should we just deactivate that warning?

[fjmolinas]

LLVM doesn't like re-definitions of types :-(. Any idea how to solve this, or should we just deactivate that warning?

I do not get the warning when compiling RIOT_CI_BUILD=1 BUILD_IN_DOCKER=1 TOOLCHAIN=llvm make -C tests/gnrc_sock_async clean all, but a solution could be something like:

#if defined (__clang__)
  #pragma clang diagnostic push
  #pragma clang diagnostic ignored "-Wno-typedef-redefinition"
#endif
....
....
#if defined (__clang__)
  #pragma clang diagnostic pop
#endif

for the sections with the redefinitions?

[miri64]

I do not get the warning when compiling RIOT_CI_BUILD=1 BUILD_IN_DOCKER=1 TOOLCHAIN=llvm make -C tests/gnrc_sock_async clean all, but a solution could be something like:

If you tried the most current version of this PR, this does not surprise me... see my changes to makefiles/toolchain/llvm.inc.mk. The pragma definition you proposed might be a less global approach. Will try soon.

[miri64]

@fjmolinas you have to revert the last commit to get the effect I was describing.

[fjmolinas]

@fjmolinas you have to revert the last commit to get the effect I was describing.

I had removed CFLAGS += -Wno-typedef-redefinition, i.e. c4e0bcf, and was not getting warnings.

[fjmolinas]

@kaspar030 can you have a look please. This somewhat breaks the "one header for everything" idea we had in #11723, but I don't really see another way to resolve the cyclic dependencies.

@miri64 I tried to look into the discussion in #11723 and to me it seems the idea was to keep the changes restricted and not mix into existing headers. To me adding the async/type.h doesn't violate this. If there are could you summarize the other arguments for the single header approach?

[miri64]

@miri64 I tried to look into the discussion in #11723 and to me it seems the idea was to keep the changes restricted and not mix into existing headers. To me adding the async/type.h doesn't violate this. If there are could you summarize the other arguments for the single header approach?

I think @kaspar030 and I discussed this mostly offline, so its not surprising to not have it there. Basically we wanted to isolate sock_async as much as possible as it is still experimental (in an earlier version the new types and functions were in the respective net/sock/*.h header). That's why we put it all in one header. But you are right, having a separate types header does not contradict this.

[miri64]

I do not get the warning when compiling RIOT_CI_BUILD=1 BUILD_IN_DOCKER=1 TOOLCHAIN=llvm make -C tests/gnrc_sock_async clean all, but a solution could be something like:

If you tried the most current version of this PR, this does not surprise me... see my changes to makefiles/toolchain/llvm.inc.mk. The pragma definition you proposed might be a less global approach. Will try soon.

BTW you have to build for a board (e.g. for iotlab-m3).

[miri64]

Used pragmas now instead of globally deactivating the redefinition warning.

[fjmolinas]

BTW you have to build for a board (e.g. for iotlab-m3).

AHA! that was it, thanks! Test is still passing.

RIOT_CI_BUILD=1 BUILD_IN_DOCKER=1 TOOLCHAIN=llvm BOARD=iotlab-m3 make -C

tests/gnrc_sock_async test --no-print-directory
r
/home/francisco/workspace/RIOT2/dist/tools/pyterm/pyterm -p "/dev/riot/tty-iotlab-m3" -b "500000" --noprefix --no-repeat-command-on-empty-line
Connect to serial port /dev/riot/tty-iotlab-m3
Welcome to pyterm!
Type '/exit' to exit.
�Help: Press s to start test, r to print it is ready
READY
s
START
main(): This is RIOT! (Version: buildtest)
UDP event triggered: 0020
UDP message successfully sent
PKTDUMP: data to send:
~~ SNIP  0 - size:  40 byte, type: NETTYPE_UNDEF (0)
00000000  60  00  00  00  90  00  11  00  00  00  00  00  00  00  00  00
00000010  00  00  00  00  00  00  00  00  FE  80  00  00  00  00  00  00
00000020  00  00  00  00  00  00  00  02
~~ SNIP  1 - size:   8 byte, type: NETTYPE_TEST (2)
00000000  01  23  45  67  89  AB  CD  EF
~~ PKT    -  2 snips, total size:  48 byte
IP event triggered: 0020
IP message successfully sent
UDP event triggered: 0010
Received UDP packet from [fe80::2]:38663:
00000000  01  23  45  67  89  AB  CD  EF
IP event triggered: 0010
Received IP packet from [fe80::2]:
00000000  01  23  45  67  89  AB  CD  EF

[fjmolinas]

Test is still working, usage has been explained. Ignoring typdef redefinition has been limited to the async files and the sock files. I don't think there is another way around this issue.

AFAIKT the single file approach to contain async code is still respected.

@kaspar030 might want to take another look, but otherwise ACK on my side. feel free to squash.

[miri64]

May I squash?

[fjmolinas]

@kaspar030 might want to take another look, but otherwise ACK on my side. feel free to squash.

May I squash?

Yes.

[miri64]

Done.

[fjmolinas]

GO!