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.

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

Jump to bottom

gnrc_ndp: add adapters for NIB-based implementation #6380

Closed
wants to merge 2 commits into from
Closed

gnrc_ndp: add adapters for NIB-based implementation #6380

wants to merge 2 commits into from

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jan 16, 2017

With the NIB implementation GNRC's NDP module will have building and sending NDP packets left as its only task. Since this will wastly simplify their structure there will be no need for an internal submodule.
To simplify refactoring efforts later on this PR provides some adapters to emulate the later state of this module (they are just inline functions that wrap their gnrc_ndp_internal_ counter-parts).

@miri64 miri64 added GNRC Area: network Area: Networking labels Jan 16, 2017
@miri64 miri64 added this to the Release 2017.01 milestone Jan 16, 2017
@cgundogan
Copy link
Member

I have no strong opinion on this, but I am not quite seeing the point in having wrapper functions when there is no new implementation for them. Wouldn't this just clutter the docs and confuse developers to see two functions (e.g. gnrc_ndp_send_nbr_sol vs gnrc_ndp_internal_send_nbr_sol) which basically do exactly the same?

@miri64
Copy link
Member Author

miri64 commented Jan 18, 2017

The idea is to replace them soonish. NIB will take quite a lot of PRs to have a stress-free review (#6325 is just the beginning and expected to be the largest part except for the packet handling). I certainly can do without this PR, but as soon as I start refactoring (after NIB was merged completely) I probably would need to replace lots of lines in the NIB code.

@cgundogan
Copy link
Member

cgundogan commented Jan 18, 2017 •
edited
Loading

I am just saying it's my personal preference to not have wrapper functions in our current master that introduce more ambiguity. You will most probably build upon / use this commit for your upcoming work, so I see no hindrance here .. and I don't think this specific commit makes the review of upcoming PRs simpler. But as I accentuated before, my opinion is not that strong (: @OlegHahm what about yours?

@PeterKietzmann PeterKietzmann modified the milestones: Release 2017.01, Release 2017.04 Jan 26, 2017
@miri64
gnrc_nib: add adapters for NIB-based implementation
778388f 
With the NIB implementation GNRC's NDP module will have building and
sending NDP packets left as its only task. Since this will wastly
simplify their structure there will be no need for an internal
submodule.
To simplify refactoring efforts later on this PR provides some adapters
to emulate the later state of this module.
@miri64 miri64 force-pushed the gnrc_ndp/enh/add-nib-adapter branch from 7b5ec6b to 778388f Compare March 20, 2017 10:27
@miri64
Copy link
Member Author

miri64 commented Mar 20, 2017

Ping? Rebased to current master.

@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 Mar 20, 2017
@OlegHahm OlegHahm 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 Mar 21, 2017
@kaspar030 kaspar030 modified the milestone: Release 2017.04 Apr 21, 2017
@miri64 miri64 changed the title gnrc_nib: add adapters for NIB-based implementation gnrc_ndp: add adapters for NIB-based implementation Apr 25, 2017
@miri64 miri64 removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 25, 2017
miri64 added a commit to miri64/RIOT that referenced this pull request May 5, 2017
@miri64
Merge branch 'gnrc_ndp/enh/add-nib-adapter'i (RIOT-OS#6380) into HEAD
b9da3e2
@miri64 miri64 mentioned this pull request May 5, 2017
Merged
2 tasks
@miri64 miri64 added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label May 5, 2017
@miri64 miri64 mentioned this pull request May 16, 2017
Merged
@miri64
Copy link
Member Author

miri64 commented May 16, 2017

Superseded by #7064.

@miri64 miri64 closed this May 16, 2017
@miri64 miri64 deleted the gnrc_ndp/enh/add-nib-adapter branch May 16, 2017 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@cgundogan cgundogan

Labels
Area: network Area: Networking CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@miri64 @cgundogan @OlegHahm @kaspar030 @PeterKietzmann