Skip to content

gnrc_mac: fix header includes#10505

Merged
bergzand merged 1 commit intoRIOT-OS:masterfrom
miri64:gnrc_mac/fix/header-includes
Nov 29, 2018
Merged

gnrc_mac: fix header includes#10505
bergzand merged 1 commit intoRIOT-OS:masterfrom
miri64:gnrc_mac/fix/header-includes

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Nov 29, 2018

Contribution description

The inclusion of net/gnrc.h in net/gnrc/mac/types.h header makes it impossible to include the net/gnrc/netif.h header within net/gnrc/netif/hdr.h (which we want to do in #10500), due to net/gnrc/mac/types.h being included with net/gnrc/netif/mac.h (which is included in net/gnrc/netif.h)

Testing procedure

examples/gnrc_networking_mac should still be able to run with default configuration (GoMacH) and LWMAC configuration:

# Use GoMacH as the MAC layer protocol
USEMODULE += gnrc_gomach
# In case of using LWMAC MAC protocol instead of GoMacH, uncomment the following line and comment the above line
# USEMODULE += gnrc_lwmac

tests/unittests (tests-gnrc_mac_internal) should still compilable and being able to run.

Issues/PRs references

Preparation step for #10500

@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 29, 2018
@miri64 miri64 force-pushed the gnrc_mac/fix/header-includes branch from 3f574c1 to c05e97d Compare November 29, 2018 09:40
@smlng smlng removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 29, 2018
@bergzand bergzand added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 29, 2018
bergzand
bergzand previously approved these changes Nov 29, 2018
Copy link
Copy Markdown
Member

@bergzand bergzand left a comment

Choose a reason for hiding this comment

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

Ack

The inclusion of `net/gnrc.h` in `net/gnrc/mac/types.h` header makes it
impossible to include the `net/gnrc/netif.h` header within
`net/gnrc/netif/hdr.h`, due to `net/gnrc/mac/types.h` being included
with `net/gnrc/netif/mac.h` (which is included in `net/gnrc/netif.h`)
@miri64 miri64 force-pushed the gnrc_mac/fix/header-includes branch from c05e97d to 9d6a32b Compare November 29, 2018 11:39
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 29, 2018

Fixed and squashed another unnecessary broad header inclusion in gnrc_gomach

@miri64 miri64 dismissed bergzand’s stale review November 29, 2018 11:40

Please re-ACK

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 29, 2018
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 29, 2018

Ping @bergzand?

@bergzand
Copy link
Copy Markdown
Member

Sorry, give me a sec to verify again

Copy link
Copy Markdown
Member

@bergzand bergzand left a comment

Choose a reason for hiding this comment

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

(re-)ACK

@bergzand bergzand merged commit b59bfd5 into RIOT-OS:master Nov 29, 2018
@miri64 miri64 deleted the gnrc_mac/fix/header-includes branch November 29, 2018 21:03
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Nov 29, 2018

Thank you! :-)

@aabadie aabadie added this to the Release 2019.01 milestone Dec 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants