Skip to content

gnrc_ipv6_netif: prepare for router discovery#3745

Merged
cgundogan merged 1 commit intoRIOT-OS:masterfrom
miri64:gnrc_ipv6_netif/enh/rtr-disc-prep
Sep 1, 2015
Merged

gnrc_ipv6_netif: prepare for router discovery#3745
cgundogan merged 1 commit intoRIOT-OS:masterfrom
miri64:gnrc_ipv6_netif/enh/rtr-disc-prep

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Aug 31, 2015

Adds some new flags and fields to the IPv6 interfaces required for router discovery (and also makes some functions private, that turned out to be a little bit more complicated there).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

s/GNRC_NDP_NETIF_MAX_ADV_INT_DEFAULT/GNRC_IPV6_NETIF_DEFAULT_MAX_ADV_INT/

@miri64 miri64 force-pushed the gnrc_ipv6_netif/enh/rtr-disc-prep branch from 1b6bc03 to 4b60189 Compare August 31, 2015 19:46
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Aug 31, 2015

Addressed comments.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added so I don't have to document the dummy macros (and so that the functions show up in the documentation)

@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Aug 31, 2015

Addressed comments

@cgundogan
Copy link
Copy Markdown
Member

please squash

@cgundogan cgundogan added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 1, 2015
@miri64 miri64 force-pushed the gnrc_ipv6_netif/enh/rtr-disc-prep branch from e4dbe8a to ccf1e4d Compare September 1, 2015 09:53
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 1, 2015

Squashed

@miri64 miri64 force-pushed the gnrc_ipv6_netif/enh/rtr-disc-prep branch from ccf1e4d to cd2208a Compare September 1, 2015 10:47
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 1, 2015

Rebased to current master

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Sep 1, 2015

@cgundogan, do you ACK?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you briefly explain why these dummy defines are needed? I don't have the full picture in mind and couldn't find any references in the current RIOT code.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wouldn't it be much cleaner to separate those functions out of the current header file and include this file only if MODULE_XYZ is defined?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Updated

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Wouldn't it be much cleaner to separate those functions out of the current header file and include this file only if MODULE_XYZ is defined?

They are. These functions call these functions in the other modules (gnrc_ndp_router and gnrc_sixlowpan_router_nd respectively) if the modules exist, depending if the interface is a 6LoWPAN interface or not.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So, why are these functions called for non-routers in the first place?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, so the choice was an ifdef in the header against multiple in the C files?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, got it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would've preferred the #ifdef in the C-File. This way it wouldn't be necessary to add an entry in the riot.doxyfile. But you know what they say.. de gustibus non est disputandum.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Plus it looks weird here, but @cgundogan is right - and in the worst case we can change this later on.

@cgundogan
Copy link
Copy Markdown
Member

ACK, squash and GO

@miri64 miri64 force-pushed the gnrc_ipv6_netif/enh/rtr-disc-prep branch from 739ac00 to b81a4dd Compare September 1, 2015 16:50
cgundogan added a commit that referenced this pull request Sep 1, 2015
…sc-prep

gnrc_ipv6_netif: prepare for router discovery
@cgundogan cgundogan merged commit 4a9d08e into RIOT-OS:master Sep 1, 2015
@miri64 miri64 deleted the gnrc_ipv6_netif/enh/rtr-disc-prep branch September 1, 2015 17:48
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 Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants