Skip to content

nhdp: use conn instead of socket_base#3876

Merged
miri64 merged 1 commit intoRIOT-OS:masterfrom
miri64:nhdp/enh/use-conn
Sep 23, 2015
Merged

nhdp: use conn instead of socket_base#3876
miri64 merged 1 commit intoRIOT-OS:masterfrom
miri64:nhdp/enh/use-conn

Conversation

@miri64
Copy link
Copy Markdown
Member

@miri64 miri64 commented Sep 17, 2015

Rework NHDP to use conn instead of the deprecated socket_base.

Depends on #3615

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking labels Sep 17, 2015
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 17, 2015

Would it be correct to add this to the release @OlegHahm?

@miri64 miri64 added the State: waiting for other PR State: The PR requires another PR to be merged first label Sep 17, 2015
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 17, 2015

Okay, oonf_api is using socket_base, too so we need to replace it there to, to get rid of sys/net/include and its containing modules.

@kaspar030 kaspar030 mentioned this pull request Sep 17, 2015
Closed
12 tasks
@miri64 miri64 mentioned this pull request Sep 17, 2015
Merged
@miri64 miri64 added this to the Release 2015.09 milestone Sep 17, 2015
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 21, 2015

Rebased to current #3615 .

@miri64 miri64 force-pushed the nhdp/enh/use-conn branch 2 times, most recently from c5d0c7e to e5b16f4 Compare September 21, 2015 19:32
@miri64 miri64 removed the State: waiting for other PR State: The PR requires another PR to be merged first label Sep 21, 2015
@miri64
Copy link
Copy Markdown
Member Author

miri64 commented Sep 21, 2015

Rebased to current master. (No longer waiting for another PR)

DipSwitch
DipSwitch reviewed Sep 21, 2015
View reviewed changes
sys/net/routing/nhdp/nhdp.c
Copy link
Copy Markdown
Member

@DipSwitch DipSwitch Sep 21, 2015

Choose a reason for hiding this comment

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

Can't you make this module depend on GNGC_CONN_UDP via the Makefile.dep?

Copy link
Copy Markdown
Member Author

@miri64 miri64 Sep 21, 2015

Choose a reason for hiding this comment

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

That would defeat conn's purpose of being an interface agnostic to a stack implementation (similar to sockets just light weight). The pseudo-module conn_udp is pulled in by any module called %_conn_udp, with % being a wildcard. If this wildcard is gnrc or openwsn or lwip or @kaspar030's infamous nano-stack doesn't matter, since that is exactly the point of that interface. Since conn_udp however doesn't supply any implementation on itself a user trying to compile this module without any conn interface would get more or less cryptic error messages and assume it is broken. So a friendly error message seemed to me to be the better solution :)

@OlegHahm OlegHahm added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 23, 2015
@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Sep 23, 2015

ACK

miri64 added a commit that referenced this pull request Sep 23, 2015
@miri64
Merge pull request #3876 from authmillenon/nhdp/enh/use-conn
320aa47 
nhdp: use conn instead of socket_base
@miri64 miri64 merged commit 320aa47 into RIOT-OS:master Sep 23, 2015
@miri64 miri64 deleted the nhdp/enh/use-conn branch September 23, 2015 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@OlegHahm OlegHahm

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: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

Release 2015.09

Development

Successfully merging this pull request may close these issues.

3 participants

@miri64 @OlegHahm @DipSwitch